[PATCH v8 0/9] More Rust bindings for device property reads

Remo Senekowitsch posted 9 patches 4 months ago
MAINTAINERS                                  |   1 +
drivers/cpufreq/rcpufreq_dt.rs               |   3 +-
drivers/of/unittest-data/tests-platform.dtsi |   3 +
rust/helpers/helpers.c                       |   1 +
rust/helpers/property.c                      |   8 +
rust/kernel/device.rs                        |  20 +-
rust/kernel/device/property.rs               | 589 +++++++++++++++++++
samples/rust/rust_driver_platform.rs         |  60 +-
8 files changed, 678 insertions(+), 7 deletions(-)
create mode 100644 rust/helpers/property.c
create mode 100644 rust/kernel/device/property.rs
[PATCH v8 0/9] More Rust bindings for device property reads
Posted by Remo Senekowitsch 4 months ago
changes in v8:
* Use a custom enum instead of `Either`, which is scheduled for removal.
* Add Tested-by tags from Dirk Behme (thanks!)
* Implement `Display` directly on `FwNode`, which prints the full path
  by default. Printing only the name is still possible with the method
  `display_name`.
* Rebase onto 6.16-rc1.
* Remove `property_present` from `Device` and update users.

Best regards,
Remo

Remo Senekowitsch (9):
  rust: device: Create FwNode abstraction for accessing device
    properties
  rust: device: Enable accessing the FwNode of a Device
  rust: device: Move property_present() to FwNode
  rust: device: Enable printing fwnode name and path
  rust: device: Introduce PropertyGuard
  rust: device: Implement accessors for firmware properties
  rust: device: Add child accessor and iterator
  rust: device: Add property_get_reference_args
  samples: rust: platform: Add property read examples

 MAINTAINERS                                  |   1 +
 drivers/cpufreq/rcpufreq_dt.rs               |   3 +-
 drivers/of/unittest-data/tests-platform.dtsi |   3 +
 rust/helpers/helpers.c                       |   1 +
 rust/helpers/property.c                      |   8 +
 rust/kernel/device.rs                        |  20 +-
 rust/kernel/device/property.rs               | 589 +++++++++++++++++++
 samples/rust/rust_driver_platform.rs         |  60 +-
 8 files changed, 678 insertions(+), 7 deletions(-)
 create mode 100644 rust/helpers/property.c
 create mode 100644 rust/kernel/device/property.rs

-- 
2.49.0
Re: [PATCH v8 0/9] More Rust bindings for device property reads
Posted by Danilo Krummrich 4 months ago
On Wed, Jun 11, 2025 at 12:28:59PM +0200, Remo Senekowitsch wrote:
> Remo Senekowitsch (9):
>   rust: device: Create FwNode abstraction for accessing device
>     properties
>   rust: device: Enable accessing the FwNode of a Device
>   rust: device: Move property_present() to FwNode
>   rust: device: Enable printing fwnode name and path
>   rust: device: Introduce PropertyGuard
>   rust: device: Implement accessors for firmware properties
>   samples: rust: platform: Add property read examples

Applied to driver-core-testing, thanks!

Once the patches passed 0-day testing they will be merged into driver-core-next.

>   rust: device: Add child accessor and iterator
>   rust: device: Add property_get_reference_args

I did drop those two patches for now, because:

  (1) They're basically dead code.

  (2) It seems that FwNode::property_get_reference_args() leaks a struct
      fwnode_handle reference. property_get_reference_args() explicitly says
      that the caller is responsible "for calling fwnode_handle_put() on the
      returned @args->fwnode pointer", which doesn't seem to happen.

Can you please fix the leak and resend those two patches with an additional one
that adds a sample usage of the introduced methods to the platform sample
driver?
Re: [PATCH v8 0/9] More Rust bindings for device property reads
Posted by Remo Senekowitsch 3 months, 4 weeks ago
On Fri Jun 13, 2025 at 1:31 AM CEST, Danilo Krummrich wrote:
> On Wed, Jun 11, 2025 at 12:28:59PM +0200, Remo Senekowitsch wrote:
>> Remo Senekowitsch (9):
>>   rust: device: Create FwNode abstraction for accessing device
>>     properties
>>   rust: device: Enable accessing the FwNode of a Device
>>   rust: device: Move property_present() to FwNode
>>   rust: device: Enable printing fwnode name and path
>>   rust: device: Introduce PropertyGuard
>>   rust: device: Implement accessors for firmware properties
>>   samples: rust: platform: Add property read examples
>
> Applied to driver-core-testing, thanks!
>
> Once the patches passed 0-day testing they will be merged into driver-core-next.
>
>>   rust: device: Add child accessor and iterator
>>   rust: device: Add property_get_reference_args
>
> I did drop those two patches for now, because:
>
>   (1) They're basically dead code.
>
>   (2) It seems that FwNode::property_get_reference_args() leaks a struct
>       fwnode_handle reference. property_get_reference_args() explicitly says
>       that the caller is responsible "for calling fwnode_handle_put() on the
>       returned @args->fwnode pointer", which doesn't seem to happen.
>
> Can you please fix the leak and resend those two patches with an additional one
> that adds a sample usage of the introduced methods to the platform sample
> driver?

Alright, thanks. I suppose that should be a new patch series, with a new
title and starting at v1 ..?
Re: [PATCH v8 0/9] More Rust bindings for device property reads
Posted by Danilo Krummrich 3 months, 4 weeks ago
On Fri, Jun 13, 2025 at 09:45:47AM +0200, Remo Senekowitsch wrote:
> On Fri Jun 13, 2025 at 1:31 AM CEST, Danilo Krummrich wrote:
> > On Wed, Jun 11, 2025 at 12:28:59PM +0200, Remo Senekowitsch wrote:
> >> Remo Senekowitsch (9):
> >>   rust: device: Create FwNode abstraction for accessing device
> >>     properties
> >>   rust: device: Enable accessing the FwNode of a Device
> >>   rust: device: Move property_present() to FwNode
> >>   rust: device: Enable printing fwnode name and path
> >>   rust: device: Introduce PropertyGuard
> >>   rust: device: Implement accessors for firmware properties
> >>   samples: rust: platform: Add property read examples
> >
> > Applied to driver-core-testing, thanks!
> >
> > Once the patches passed 0-day testing they will be merged into driver-core-next.
> >
> >>   rust: device: Add child accessor and iterator
> >>   rust: device: Add property_get_reference_args
> >
> > I did drop those two patches for now, because:
> >
> >   (1) They're basically dead code.
> >
> >   (2) It seems that FwNode::property_get_reference_args() leaks a struct
> >       fwnode_handle reference. property_get_reference_args() explicitly says
> >       that the caller is responsible "for calling fwnode_handle_put() on the
> >       returned @args->fwnode pointer", which doesn't seem to happen.
> >
> > Can you please fix the leak and resend those two patches with an additional one
> > that adds a sample usage of the introduced methods to the platform sample
> > driver?
> 
> Alright, thanks. I suppose that should be a new patch series, with a new
> title and starting at v1 ..?

Yes, that's fine. Please make sure to mention that it's a follow-up of this
series (ideally with a link).
Re: [PATCH v8 0/9] More Rust bindings for device property reads
Posted by Danilo Krummrich 4 months ago
Hi Remo,

On Wed, Jun 11, 2025 at 12:28:59PM +0200, Remo Senekowitsch wrote:

I went through the single patches and found the following issues.

> Remo Senekowitsch (9):
>   rust: device: Create FwNode abstraction for accessing device
>     properties

This patch creates a warning for FwNode::from_raw() to be unused, and needs this
fix:

diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs
index 03850b7bb808..82ef54b54f18 100644
--- a/rust/kernel/device/property.rs
+++ b/rust/kernel/device/property.rs
@@ -37,6 +37,7 @@ impl FwNode {
     /// - They relinquish that increment. That is, if there is only one
     ///   increment, callers must not use the underlying object anymore -- it is
     ///   only safe to do so via the newly created `ARef<FwNode>`.
+    #[expect(dead_code)]
     unsafe fn from_raw(raw: *mut bindings::fwnode_handle) -> ARef<Self> {
         // SAFETY: As per the safety requirements of this function:
         // - `NonNull::new_unchecked`:

>   rust: device: Enable accessing the FwNode of a Device
>   rust: device: Move property_present() to FwNode
>   rust: device: Enable printing fwnode name and path

This patch starts using FwNode::from_raw() and hence needs:

diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs
index a5ebadd878d6..4cac335bad78 100644
--- a/rust/kernel/device/property.rs
+++ b/rust/kernel/device/property.rs
@@ -38,7 +38,6 @@ impl FwNode {
     /// - They relinquish that increment. That is, if there is only one
     ///   increment, callers must not use the underlying object anymore -- it is
     ///   only safe to do so via the newly created `ARef<FwNode>`.
-    #[expect(dead_code)]
     unsafe fn from_raw(raw: *mut bindings::fwnode_handle) -> ARef<Self> {
         // SAFETY: As per the safety requirements of this function:
         // - `NonNull::new_unchecked`:

>   rust: device: Introduce PropertyGuard

This patch fails to compile (print macros are not included) and needs the
following change.

diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs
index a10033d310e6..1286669a9a40 100644
--- a/rust/kernel/device/property.rs
+++ b/rust/kernel/device/property.rs
@@ -8,6 +8,7 @@
 
 use crate::{
     bindings,
+    prelude::*,
     str::CStr,
     types::{ARef, Opaque},
 };

>   rust: device: Implement accessors for firmware properties

This patch also fails to compile, throws a warning about transmute() and hence
needs the following change.

diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs
index 75621abe8117..fe0c5469a278 100644
--- a/rust/kernel/device/property.rs
+++ b/rust/kernel/device/property.rs
@@ -6,12 +6,12 @@

 use core::{mem::MaybeUninit, ptr};

+use super::private::Sealed;
 use crate::{
     alloc::KVec,
     bindings,
     error::{to_result, Result},
     prelude::*,
-    private::Sealed,
     str::{CStr, CString},
     types::{ARef, Opaque},
 };
@@ -303,7 +303,7 @@ fn read_array_from_fwnode_property<'a>(
                 // `&'a mut [Self]` is sound, because the previous call to a
                 // `fwnode_property_read_*_array` function (which didn't fail)
                 // fully initialized the slice.
-                Ok(unsafe { core::mem::transmute(out) })
+                Ok(unsafe { core::mem::transmute::<&mut [MaybeUninit<Self>], &mut [Self]>(out) })
             }

             fn read_array_len_from_fwnode_property(fwnode: &FwNode, name: &CStr) -> Result<usize> {

>   rust: device: Add child accessor and iterator
>   rust: device: Add property_get_reference_args
>   samples: rust: platform: Add property read examples

In general, please make sure to compile test every patch of a series
individually, not only the entire series.

Otherwise the series looks good. Given that the required fixes are minor, there
is no need for you to send a new version, I can apply those changes why I apply
the patch series.

- Danilo