[05/10] rust: Read properties via single generic method

Commit Message

Remo Senekowitsch March 26, 2025, 5:13 p.m. UTC

This can probably be squashed into its parent. I haven't done that
yet in case there is something wrong with the generic approach and the
previous one is preferred.

Signed-off-by: Remo Senekowitsch <[e-mail]>
---
 rust/kernel/property.rs | 171 +++++++++++++++++++++++-----------------
 1 file changed, 99 insertions(+), 72 deletions(-)

Comments

Rob Herring (Arm) March 26, 2025, 9:33 p.m. UTC | #1

On Wed, Mar 26, 2025 at 06:13:44PM +0100, Remo Senekowitsch wrote:
> This can probably be squashed into its parent. I haven't done that
> yet in case there is something wrong with the generic approach and the
> previous one is preferred.

Looks good to me other than the returning of strings issue. And yes it 
should be squashed. (Really, it should have been in this series so 
reviewers aren't reviewing code you delete in the next patch).

> 
> Signed-off-by: Remo Senekowitsch <[e-mail]>
> ---
>  rust/kernel/property.rs | 171 +++++++++++++++++++++++-----------------
>  1 file changed, 99 insertions(+), 72 deletions(-)

Patch

diff --git a/rust/kernel/property.rs b/rust/kernel/property.rs
index 4756ea766..4a03008ce 100644
--- a/rust/kernel/property.rs
+++ b/rust/kernel/property.rs
@@ -35,31 +35,11 @@  pub fn property_present(&self, name: &CStr) -> bool {
         self.fwnode().property_present(name)
     }
 
-    /// Returns if a firmware property `name` is true or false
-    pub fn property_read_bool(&self, name: &CStr) -> bool {
-        self.fwnode().property_read_bool(name)
-    }
-
-    /// Returns the index of matching string `match_str` for firmware string property `name`
-    pub fn property_read_string(&self, name: &CStr) -> Result<CString> {
-        self.fwnode().property_read_string(name)
-    }
-
     /// Returns the index of matching string `match_str` for firmware string property `name`
     pub fn property_match_string(&self, name: &CStr, match_str: &CStr) -> Result<usize> {
         self.fwnode().property_match_string(name, match_str)
     }
 
-    /// Returns firmware property `name` integer scalar value
-    pub fn property_read<T: Integer>(&self, name: &CStr) -> Result<T> {
-        self.fwnode().property_read(name)
-    }
-
-    /// Returns firmware property `name` integer array values
-    pub fn property_read_array<T: Integer, const N: usize>(&self, name: &CStr) -> Result<[T; N]> {
-        self.fwnode().property_read_array(name)
-    }
-
     /// 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>> {
         self.fwnode().property_read_array_vec(name, len)
@@ -69,6 +49,11 @@  pub fn property_read_array_vec<T: Integer>(&self, name: &CStr, len: usize) -> Re
     pub fn property_count_elem<T: Integer>(&self, name: &CStr) -> Result<usize> {
         self.fwnode().property_count_elem::<T>(name)
     }
+
+    /// Returns firmware property `name` integer scalar value
+    pub fn property_read<T: Property>(&self, name: &CStr) -> Result<T> {
+        self.fwnode().property_read(name)
+    }
 }
 
 /// A reference-counted fwnode_handle.
@@ -101,30 +86,6 @@  pub fn property_present(&self, name: &CStr) -> bool {
         unsafe { bindings::fwnode_property_present(self.as_raw().cast_const(), name.as_char_ptr()) }
     }
 
-    /// Returns if a firmware property `name` is true or false
-    pub fn property_read_bool(&self, name: &CStr) -> bool {
-        // SAFETY: `name` is non-null and null-terminated. `self.as_raw` is valid
-        // because `self` is valid.
-        unsafe { bindings::fwnode_property_read_bool(self.as_raw(), name.as_char_ptr()) }
-    }
-
-    /// Returns the index of matching string `match_str` for firmware string property `name`
-    pub fn property_read_string(&self, name: &CStr) -> Result<CString> {
-        let mut str = core::ptr::null_mut();
-        let pstr: *mut _ = &mut str;
-
-        // SAFETY: `name` is non-null and null-terminated. `self.as_raw` is
-        // valid because `self` is valid.
-        let ret = unsafe {
-            bindings::fwnode_property_read_string(self.as_raw(), name.as_char_ptr(), pstr as _)
-        };
-        to_result(ret)?;
-
-        // SAFETY: `pstr` contains a non-null ptr on success
-        let str = unsafe { CStr::from_char_ptr(*pstr) };
-        Ok(str.try_into()?)
-    }
-
     /// Returns the index of matching string `match_str` for firmware string property `name`
     pub fn property_match_string(&self, name: &CStr, match_str: &CStr) -> Result<usize> {
         // SAFETY: `name` and `match_str` are non-null and null-terminated. `self.as_raw` is
@@ -140,34 +101,6 @@  pub fn property_match_string(&self, name: &CStr, match_str: &CStr) -> Result<usi
         Ok(ret as usize)
     }
 
-    /// Returns firmware property `name` integer scalar value
-    pub fn property_read<T: Integer>(&self, name: &CStr) -> Result<T> {
-        let val: [_; 1] = Self::property_read_array(self, name)?;
-        Ok(val[0])
-    }
-
-    /// Returns firmware property `name` integer array values
-    pub fn property_read_array<T: Integer, const N: usize>(&self, name: &CStr) -> Result<[T; N]> {
-        let val: [MaybeUninit<T>; N] = [const { MaybeUninit::uninit() }; N];
-
-        // 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.
-        let ret = unsafe {
-            bindings::fwnode_property_read_int_array(
-                self.as_raw(),
-                name.as_char_ptr(),
-                T::SIZE as u32,
-                val.as_ptr() as *mut c_void,
-                val.len(),
-            )
-        };
-        to_result(ret)?;
-
-        // SAFETY: `val` is always initialized when
-        // fwnode_property_read_int_array is successful.
-        Ok(val.map(|v| unsafe { v.assume_init() }))
-    }
-
     /// 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>> {
         let mut val: KVec<T> = KVec::with_capacity(len, GFP_KERNEL)?;
@@ -206,6 +139,38 @@  pub fn property_count_elem<T: Integer>(&self, name: &CStr) -> Result<usize> {
         to_result(ret)?;
         Ok(ret as usize)
     }
+
+    /// Returns the value of firmware property `name`.
+    ///
+    /// This method is generic over the type of value to read. Informally,
+    /// the types that can be read are booleans, strings, integers and arrays
+    /// of integers.
+    ///
+    /// Reading a `KVec` of integers is done with the
+    /// separate method [Self::property_read_array_vec], because it takes an
+    /// additional `len` argument.
+    ///
+    /// When reading a boolean, this method never fails. A missing property
+    /// is interpreted as `false`, whereas a present property is interpreted
+    /// as `true`.
+    ///
+    /// For more precise documentation about what types can be read, see
+    /// the [implementors of Property][Property#implementors] and [its
+    /// implementations on foreign types][Property#foreign-impls].
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// # 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")?;
+    /// }
+    /// ```
+    pub fn property_read<T: Property>(&self, name: &CStr) -> Result<T> {
+        T::read(&self, name)
+    }
 }
 
 // SAFETY: Instances of `FwNode` are always reference-counted.
@@ -220,3 +185,65 @@  unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
         unsafe { bindings::fwnode_handle_put(obj.cast().as_ptr()) }
     }
 }
+
+/// Implemented for all types that can be read as properties.
+///
+/// This is used to make [FwNode::property_read] generic over the type of
+/// property being read.
+pub trait Property: Sized {
+    /// Used to make [FwNode::property_read] generic.
+    fn read(fwnode: &FwNode, name: &CStr) -> Result<Self>;
+}
+
+impl Property for bool {
+    fn read(fwnode: &FwNode, name: &CStr) -> Result<Self> {
+        // SAFETY: `name` is non-null and null-terminated. `fwnode.as_raw` is valid
+        // because `fwnode` is valid.
+        Ok(unsafe { bindings::fwnode_property_read_bool(fwnode.as_raw(), name.as_char_ptr()) })
+    }
+}
+impl Property for CString {
+    fn read(fwnode: &FwNode, name: &CStr) -> Result<Self> {
+        let mut str: *mut u8 = ptr::null_mut();
+        let pstr: *mut _ = &mut str;
+
+        // SAFETY: `name` is non-null and null-terminated. `fwnode.as_raw` is
+        // valid because `fwnode` is valid.
+        let ret = unsafe {
+            bindings::fwnode_property_read_string(fwnode.as_raw(), name.as_char_ptr(), pstr.cast())
+        };
+        to_result(ret)?;
+
+        // SAFETY: `pstr` contains a non-null ptr on success
+        let str = unsafe { CStr::from_char_ptr(*pstr) };
+        Ok(str.try_into()?)
+    }
+}
+impl<T: Integer, const N: usize> Property for [T; N] {
+    fn read(fwnode: &FwNode, name: &CStr) -> Result<Self> {
+        let mut val: [MaybeUninit<T>; N] = [const { MaybeUninit::uninit() }; N];
+
+        // SAFETY: `name` is non-null and null-terminated. `fwnode.as_raw` is valid
+        // because `fwnode` is valid. `val.as_ptr` is valid because `val` is valid.
+        let ret = unsafe {
+            bindings::fwnode_property_read_int_array(
+                fwnode.as_raw(),
+                name.as_char_ptr(),
+                T::SIZE as u32,
+                val.as_mut_ptr().cast(),
+                val.len(),
+            )
+        };
+        to_result(ret)?;
+
+        // SAFETY: `val` is always initialized when
+        // fwnode_property_read_int_array is successful.
+        Ok(val.map(|v| unsafe { v.assume_init() }))
+    }
+}
+impl<T: Integer> Property for T {
+    fn read(fwnode: &FwNode, name: &CStr) -> Result<T> {
+        let val: [_; 1] = <[T; 1] as Property>::read(fwnode, name)?;
+        Ok(val[0])
+    }
+}