On Wed, Mar 26, 2025 at 06:13:40PM +0100, Remo Senekowitsch wrote: > Not all property-related APIs can be exposed directly on a device. > For example, iterating over child nodes of a device will yield > fwnode_handle. Thus, in order to access properties on these child nodes, > the APIs has to be duplicated on a fwnode as they are in C. s/has/have/ > > A related discussion can be found on the R4L Zulip[1]. > > [1] https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/DS90UB954.20driver.20done.2C.20ready.20to.20upstream.3F/near/505415697 Useful below the '---', but I don't think we want to keep this link forever. And who knows how long it will be valid? The commit msg needs to stand on its own, and I think it does. > > Signed-off-by: Remo Senekowitsch <remo@xxxxxxxxxxx> > --- > rust/helpers/helpers.c | 1 + > rust/helpers/property.c | 13 ++++++++ > rust/kernel/device.rs | 7 ---- > rust/kernel/lib.rs | 1 + > rust/kernel/property.rs | 73 +++++++++++++++++++++++++++++++++++++++++ > 5 files changed, 88 insertions(+), 7 deletions(-) > create mode 100644 rust/helpers/property.c > create mode 100644 rust/kernel/property.rs > > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c > index 0640b7e11..b4eec5bf2 100644 > --- a/rust/helpers/helpers.c > +++ b/rust/helpers/helpers.c > @@ -23,6 +23,7 @@ > #include "platform.c" > #include "pci.c" > #include "pid_namespace.c" > +#include "property.c" > #include "rbtree.c" > #include "rcu.c" > #include "refcount.c" > diff --git a/rust/helpers/property.c b/rust/helpers/property.c > new file mode 100644 > index 000000000..c37c74488 > --- /dev/null > +++ b/rust/helpers/property.c > @@ -0,0 +1,13 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <linux/property.h> > + > +struct fwnode_handle *rust_helper_dev_fwnode(struct device *dev) > +{ > + return dev_fwnode(dev); > +} > + > +void rust_helper_fwnode_handle_put(struct fwnode_handle *fwnode) > +{ > + fwnode_handle_put(fwnode); > +} > diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs > index db2d9658b..d5e6a19ff 100644 > --- a/rust/kernel/device.rs > +++ b/rust/kernel/device.rs > @@ -6,7 +6,6 @@ > > use crate::{ > bindings, > - str::CStr, > types::{ARef, Opaque}, > }; > use core::{fmt, ptr}; > @@ -181,12 +180,6 @@ unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) { > ) > }; > } > - > - /// Checks if property is present or not. > - pub fn property_present(&self, name: &CStr) -> bool { > - // SAFETY: By the invariant of `CStr`, `name` is null-terminated. > - unsafe { bindings::device_property_present(self.as_raw().cast_const(), name.as_char_ptr()) } > - } > } > > // SAFETY: Instances of `Device` are always reference-counted. > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs > index 496ed32b0..ca233fd20 100644 > --- a/rust/kernel/lib.rs > +++ b/rust/kernel/lib.rs > @@ -67,6 +67,7 @@ > pub mod platform; > pub mod prelude; > pub mod print; > +pub mod property; > pub mod rbtree; > pub mod revocable; > pub mod security; > diff --git a/rust/kernel/property.rs b/rust/kernel/property.rs > new file mode 100644 > index 000000000..b0a4bb63a > --- /dev/null > +++ b/rust/kernel/property.rs > @@ -0,0 +1,73 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Unified device property interface. > +//! > +//! C header: [`include/linux/property.h`](srctree/include/linux/property.h) > + > +use core::ptr; > + > +use crate::{bindings, device::Device, str::CStr, types::Opaque}; > + > +impl Device { > + /// Obtain the fwnode corresponding to the device. > + fn fwnode(&self) -> &FwNode { > + // SAFETY: `self` is valid. > + let fwnode_handle = unsafe { bindings::dev_fwnode(self.as_raw()) }; > + if fwnode_handle.is_null() { > + panic!("fwnode_handle cannot be null"); It's usually not a good idea to panic the kernel especially with something a driver calls as that's probably recoverable. Users/drivers testing fwnode_handle/of_node for NULL is pretty common. Though often that's a legacy code path, so maybe not allowing NULL is fine for now. > + } > + // SAFETY: `fwnode_handle` is valid. Its lifetime is tied to `&self`. We > + // return a reference instead of an `ARef<FwNode>` because `dev_fwnode()` > + // doesn't increment the refcount. > + unsafe { &*fwnode_handle.cast() } > + } > + > + /// Checks if property is present or not. > + pub fn property_present(&self, name: &CStr) -> bool { > + self.fwnode().property_present(name) > + } > +} The C developer in me wants to put this after the FwNode stuff since this uses it. > + > +/// A reference-counted fwnode_handle. > +/// > +/// This structure represents the Rust abstraction for a > +/// C `struct fwnode_handle`. This implementation abstracts the usage of an > +/// already existing C `struct fwnode_handle` within Rust code that we get > +/// passed from the C side. > +/// > +/// # Invariants > +/// > +/// A `FwNode` instance represents a valid `struct fwnode_handle` created by the > +/// C portion of the kernel. > +/// > +/// Instances of this type are always reference-counted, that is, a call to > +/// `fwnode_handle_get` ensures that the allocation remains valid at least until > +/// the matching call to `fwnode_handle_put`. > +#[repr(transparent)] > +pub struct FwNode(Opaque<bindings::fwnode_handle>); > + > +impl FwNode { > + /// Obtain the raw `struct fwnode_handle *`. > + pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle { > + self.0.get() > + } > + > + /// Checks if property is present or not. > + pub fn property_present(&self, name: &CStr) -> bool { > + // SAFETY: By the invariant of `CStr`, `name` is null-terminated. > + unsafe { bindings::fwnode_property_present(self.as_raw().cast_const(), name.as_char_ptr()) } > + } > +} > + > +// SAFETY: Instances of `FwNode` are always reference-counted. > +unsafe impl crate::types::AlwaysRefCounted for FwNode { > + fn inc_ref(&self) { > + // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero. > + unsafe { bindings::fwnode_handle_get(self.as_raw()) }; > + } > + > + unsafe fn dec_ref(obj: ptr::NonNull<Self>) { > + // SAFETY: The safety requirements guarantee that the refcount is non-zero. > + unsafe { bindings::fwnode_handle_put(obj.cast().as_ptr()) } > + } > +} > -- > 2.49.0 >
