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
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
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?
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 ..?
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).
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
© 2016 - 2025 Red Hat, Inc.