diff mbox series

[04/10] rust: Add bindings for reading device properties

Message ID 20250326171411.590681-5-remo@buenzli.dev (mailing list archive)
State New
Headers show
Series More Rust bindings for device property reads | expand

Commit Message

Remo Senekowitsch March 26, 2025, 5:13 p.m. UTC
The device property API is a firmware agnostic API for reading
properties from firmware (DT/ACPI) devices nodes and swnodes.

While the C API takes a pointer to a caller allocated variable/buffer,
the rust API is designed to return a value and can be used in struct
initialization. Rust generics are also utilized to support different
sizes of properties (e.g. u8, u16, u32).

Co-developed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Remo Senekowitsch <remo@buenzli.dev>
---
 rust/kernel/property.rs | 153 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 151 insertions(+), 2 deletions(-)

Comments

Rob Herring (Arm) March 26, 2025, 9:27 p.m. UTC | #1
On Wed, Mar 26, 2025 at 06:13:43PM +0100, Remo Senekowitsch wrote:
> The device property API is a firmware agnostic API for reading
> properties from firmware (DT/ACPI) devices nodes and swnodes.
> 
> While the C API takes a pointer to a caller allocated variable/buffer,
> the rust API is designed to return a value and can be used in struct
> initialization. Rust generics are also utilized to support different
> sizes of properties (e.g. u8, u16, u32).
> 
> Co-developed-by: Rob Herring (Arm) <robh@kernel.org>
> Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> Signed-off-by: Remo Senekowitsch <remo@buenzli.dev>
> ---
>  rust/kernel/property.rs | 153 +++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 151 insertions(+), 2 deletions(-)
> 
> diff --git a/rust/kernel/property.rs b/rust/kernel/property.rs
> index b0a4bb63a..4756ea766 100644
> --- a/rust/kernel/property.rs
> +++ b/rust/kernel/property.rs
> @@ -4,9 +4,17 @@
>  //!
>  //! C header: [`include/linux/property.h`](srctree/include/linux/property.h)
>  
> -use core::ptr;
> +use core::{ffi::c_void, mem::MaybeUninit, ptr};
>  
> -use crate::{bindings, device::Device, str::CStr, types::Opaque};
> +use crate::{
> +    alloc::KVec,
> +    bindings,
> +    device::Device,
> +    error::{to_result, Result},
> +    prelude::*,
> +    str::{CStr, CString},
> +    types::{Integer, Opaque},
> +};
>  
>  impl Device {
>      /// Obtain the fwnode corresponding to the device.
> @@ -26,6 +34,41 @@ fn fwnode(&self) -> &FwNode {
>      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`

Comment doesn't match the function.

> +    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)
> +    }
> +
> +    /// Returns integer array length for firmware property `name`
> +    pub fn property_count_elem<T: Integer>(&self, name: &CStr) -> Result<usize> {
> +        self.fwnode().property_count_elem::<T>(name)
> +    }
>  }
>  
>  /// A reference-counted fwnode_handle.
> @@ -57,6 +100,112 @@ 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()) }
>      }
> +
> +    /// 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`

Same comment copy-n-paste mismatch.

> +    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()?)
> +    }

There's a problem with the C version of this function that I'd like to 
not repeat in Rust especially since ownership is clear. 

The issue is that we never know when the returned string is no longer 
needed. For DT overlays, we need to be able free the string when/if an 
overlay is removed. Though overlays are somewhat orthogonal to this. 
It's really just when the property's node refcount goes to 0 that the 
property value could be freed.

So this function should probably return a copy of the string that the 
caller owns.

Rob
diff mbox series

Patch

diff --git a/rust/kernel/property.rs b/rust/kernel/property.rs
index b0a4bb63a..4756ea766 100644
--- a/rust/kernel/property.rs
+++ b/rust/kernel/property.rs
@@ -4,9 +4,17 @@ 
 //!
 //! C header: [`include/linux/property.h`](srctree/include/linux/property.h)
 
-use core::ptr;
+use core::{ffi::c_void, mem::MaybeUninit, ptr};
 
-use crate::{bindings, device::Device, str::CStr, types::Opaque};
+use crate::{
+    alloc::KVec,
+    bindings,
+    device::Device,
+    error::{to_result, Result},
+    prelude::*,
+    str::{CStr, CString},
+    types::{Integer, Opaque},
+};
 
 impl Device {
     /// Obtain the fwnode corresponding to the device.
@@ -26,6 +34,41 @@  fn fwnode(&self) -> &FwNode {
     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)
+    }
+
+    /// Returns integer array length for firmware property `name`
+    pub fn property_count_elem<T: Integer>(&self, name: &CStr) -> Result<usize> {
+        self.fwnode().property_count_elem::<T>(name)
+    }
 }
 
 /// A reference-counted fwnode_handle.
@@ -57,6 +100,112 @@  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()) }
     }
+
+    /// 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
+        // valid because `self` is valid.
+        let ret = unsafe {
+            bindings::fwnode_property_match_string(
+                self.as_raw(),
+                name.as_char_ptr(),
+                match_str.as_char_ptr(),
+            )
+        };
+        to_result(ret)?;
+        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)?;
+
+        // 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 {
+            bindings::fwnode_property_read_int_array(
+                self.as_raw(),
+                name.as_char_ptr(),
+                T::SIZE as u32,
+                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)
+    }
+
+    /// Returns integer array length for firmware property `name`
+    pub fn property_count_elem<T: Integer>(&self, name: &CStr) -> Result<usize> {
+        // SAFETY: `name` is non-null and null-terminated. `self.as_raw` is valid
+        // because `self` is valid. Passing null pointer buffer is valid to obtain
+        // the number of elements in the property array.
+        let ret = unsafe {
+            bindings::fwnode_property_read_int_array(
+                self.as_raw(),
+                name.as_char_ptr(),
+                T::SIZE as u32,
+                ptr::null_mut(),
+                0,
+            )
+        };
+        to_result(ret)?;
+        Ok(ret as usize)
+    }
 }
 
 // SAFETY: Instances of `FwNode` are always reference-counted.