[PATCH 09/10] rust: property: Add PropertyGuard

Remo Senekowitsch
2025-03-26T17:15:23Z

Forcing users to specify whether a property is supposed to be required
or not allows us to move error logging of missing required properties
into core, preventing a lot of boilerplate in drivers.

Signed-off-by: Remo Senekowitsch <remo@buenzli.dev>
---
rust/kernel/property.rs | 146 ++++++++++++++++++++++++++++++++++------
1 file changed, 127 insertions(+), 19 deletions(-)

diff --git a/rust/kernel/property.rs b/rust/kernel/property.rs
index f1d0a33ba..e4f0e5f97 100644
--- a/rust/kernel/property.rs
+++ b/rust/kernel/property.rs
@@ -42,7 +42,11 @@ pub fn property_match_string(&self, name: &CStr, match_str: &CStr) -> Result<usi
}

/// Returns firmware property `name` integer array values in a KVec
- pub fn property_read_array_vec<T: Integer>(&self, name: &CStr, len: usize) -> Result<KVec<T>> {
+ pub fn property_read_array_vec<'fwnode, 'name, T: Integer>(
+ &'fwnode self,
+ name: &'name CStr,
+ len: usize,
+ ) -> Result<PropertyGuard<'fwnode, 'name, KVec<T>>> {
self.fwnode().property_read_array_vec(name, len)
}

@@ -52,12 +56,18 @@ pub fn property_count_elem<T: Integer>(&self, name: &CStr) -> Result<usize> {
}

/// Returns firmware property `name` integer scalar value
- pub fn property_read<T: Property>(&self, name: &CStr) -> Result<T> {
+ pub fn property_read<'fwnode, 'name, T: Property>(
+ &'fwnode self,
+ name: &'name CStr,
+ ) -> PropertyGuard<'fwnode, 'name, T> {
self.fwnode().property_read(name)
}

/// Returns first matching named child node handle.
- pub fn get_child_by_name(&self, name: &CStr) -> Option<ARef<FwNode>> {
+ pub fn get_child_by_name<'fwnode, 'name>(
+ &'fwnode self,
+ name: &'name CStr,
+ ) -> PropertyGuard<'fwnode, 'name, ARef<FwNode>> {
self.fwnode().get_child_by_name(name)
}

@@ -132,12 +142,16 @@ pub fn property_match_string(&self, name: &CStr, match_str: &CStr) -> Result<usi
}

/// Returns firmware property `name` integer array values in a KVec
- pub fn property_read_array_vec<T: Integer>(&self, name: &CStr, len: usize) -> Result<KVec<T>> {
+ pub fn property_read_array_vec<'fwnode, 'name, T: Integer>(
+ &'fwnode self,
+ name: &'name CStr,
+ len: usize,
+ ) -> Result<PropertyGuard<'fwnode, 'name, KVec<T>>> {
let mut val: KVec<T> = KVec::with_capacity(len, GFP_KERNEL)?;

// SAFETY: `name` is non-null and null-terminated. `self.as_raw` is valid
// because `self` is valid. `val.as_ptr` is valid because `val` is valid.
- to_result(unsafe {
+ let err = unsafe {
bindings::fwnode_property_read_int_array(
self.as_raw(),
name.as_char_ptr(),
@@ -145,11 +159,19 @@ pub fn property_read_array_vec<T: Integer>(&self, name: &CStr, len: usize) -> Re
val.as_ptr() as *mut c_void,
len,
)
- })?;
-
- // SAFETY: fwnode_property_read_int_array() writes exactly `len` entries on success
- unsafe { val.set_len(len) }
- Ok(val)
+ };
+ let res = if err < 0 {
+ Err(Error::from_errno(err))
+ } else {
+ // SAFETY: fwnode_property_read_int_array() writes exactly `len` entries on success
+ unsafe { val.set_len(len) }
+ Ok(val)
+ };
+ Ok(PropertyGuard {
+ inner: res,
+ fwnode: self,
+ name,
+ })
}

/// Returns integer array length for firmware property `name`
@@ -194,24 +216,42 @@ pub fn property_count_elem<T: Integer>(&self, name: &CStr) -> Result<usize> {
/// # use crate::{device::Device, types::CString};
/// fn examples(dev: &Device) -> Result {
/// let fwnode = dev.fwnode();
- /// let b: bool = fwnode.property_read("some-bool")?;
- /// let s: CString = fwnode.property_read("some-str")?;
+ /// let b: bool = fwnode.property_read("some-bool").required()?;
+ /// if let Some(s) = fwnode.property_read::<CString>("some-str").optional() {
+ /// // ...
+ /// }
/// }
/// ```
- pub fn property_read<T: Property>(&self, name: &CStr) -> Result<T> {
- T::read(&self, name)
+ pub fn property_read<'fwnode, 'name, T: Property>(
+ &'fwnode self,
+ name: &'name CStr,
+ ) -> PropertyGuard<'fwnode, 'name, T> {
+ PropertyGuard {
+ inner: T::read(&self, name),
+ fwnode: self,
+ name,
+ }
}

/// Returns first matching named child node handle.
- pub fn get_child_by_name(&self, name: &CStr) -> Option<ARef<Self>> {
+ pub fn get_child_by_name<'fwnode, 'name>(
+ &'fwnode self,
+ name: &'name CStr,
+ ) -> PropertyGuard<'fwnode, 'name, ARef<Self>> {
// SAFETY: `self` and `name` are valid.
let child =
unsafe { bindings::fwnode_get_named_child_node(self.as_raw(), name.as_char_ptr()) };
- if child.is_null() {
- return None;
+ let res = if child.is_null() {
+ Err(ENOENT)
+ } else {
+ // SAFETY: `fwnode_get_named_child_node` returns a pointer with refcount incremented.
+ Ok(unsafe { Self::from_raw(child) })
+ };
+ PropertyGuard {
+ inner: res,
+ fwnode: self,
+ name,
}
- // SAFETY: `fwnode_get_named_child_node` returns a pointer with refcount incremented.
- Some(unsafe { Self::from_raw(child) })
}

/// Returns an iterator over a node's children.
@@ -365,3 +405,71 @@ pub enum NArgs<'a> {
/// The known number of arguments.
N(u32),
}
+
+/// A helper for reading device properties.
+///
+/// Use [Self::required] if a missing property is considered a bug and
+/// [Self::optional] otherwise.
+///
+/// For convenience, [Self::or] and [Self::or_default] are provided.
+pub struct PropertyGuard<'fwnode, 'name, T> {
+ /// The result of reading the property.
+ inner: Result<T>,
+ /// The fwnode of the property, used for logging in the "required" case.
+ fwnode: &'fwnode FwNode,
+ /// The name of the property, used for logging in the "required" case.
+ name: &'name CStr,
+}
+
+impl<T> PropertyGuard<'_, '_, T> {
+ /// Access the property, indicating it is required.
+ ///
+ /// If the property is not present, the error is automatically logged. If a
+ /// missing property is not an error, use [Self::optional] instead.
+ pub fn required(self) -> Result<T> {
+ if self.inner.is_err() {
+ // Get the device associated with the fwnode for device-associated
+ // logging.
+ // TODO: Are we allowed to do this? The field `fwnode_handle.dev`
+ // has a somewhat vague comment, which could mean we're not
+ // supposed to access it:
+ // https://elixir.bootlin.com/linux/v6.13.6/source/include/linux/fwnode.h#L51
+ // SAFETY: According to the invariant of FwNode, it is valid.
+ let dev = unsafe { (*self.fwnode.as_raw()).dev };
+
+ if dev.is_null() {
+ pr_err!("property '{}' is missing\n", self.name);
+ } else {
+ // SAFETY: If dev is not null, it points to a valid device.
+ let dev: &Device = unsafe { &*dev.cast() };
+ dev_err!(dev, "property '{}' is missing\n", self.name);
+ };
+ }
+ self.inner
+ }
+
+ /// Access the property, indicating it is optional.
+ ///
+ /// In contrast to [Self::required], no error message is logged if the
+ /// property is not present.
+ pub fn optional(self) -> Option<T> {
+ self.inner.ok()
+ }
+
+ /// Access the property or the specified default value.
+ ///
+ /// Do not pass a sentinel value as default to detect a missing property.
+ /// Use [Self::required] or [Self::optional] instead.
+ pub fn or(self, default: T) -> T {
+ self.inner.unwrap_or(default)
+ }
+}
+
+impl<T: Default> PropertyGuard<'_, '_, T> {
+ /// Access the property or a default value.
+ ///
+ /// Use [Self::or] to specify a custom default value.
+ pub fn or_default(self) -> T {
+ self.inner.unwrap_or_default()
+ }
+}
--
2.49.0