More Rust bindings for device property reads

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 property access methods must be implemented on the abstraction over fwnode_handle.

 rust/kernel/{device.rs => device/mod.rs} |  2 +
 rust/kernel/device/property.rs           | 47 ++++++++++++++++++++++++

The new FwNode abstraction will be used for accessing all device properties, so it must have the property_present method.

It's possible to duplicate the methods on the device itself, but since some of the methods on Device would have different type sigatures as the ones on FwNode, this would only lead to inconsistency and confusion. Hence, remove the method from Device.

There aren't any users to update yet.

Signed-off-by Remo Senekowitsch <[email protected]>

---
 rust/kernel/device/mod.rs      | 7 -------
 rust/kernel/device/property.rs | 8 +++++++-
 2 files changed, 7 insertions(+), 8 deletions(-)

rust/kernel/device/mod.rs +0 -7

index b4b7056eb80f8..15d89cd45e871 100644
@@ -6,7 +6,6 @@
 
 use crate::{
     bindings,
-    str::CStr,
     types::{ARef, Opaque},
 };
 use core::{fmt, marker::PhantomData, ptr};

@@ -200,12 +199,6 @@ pub fn fwnode(&self) -> Option<&property::FwNode> {
         // defined as a `#[repr(transparent)]` wrapper around `fwnode_handle`.
         Some(unsafe { &*fwnode_handle.cast() })
     }
-
-    /// 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: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic

rust/kernel/device/property.rs +7 -1

index e75d55f5856cf..70593343bd811 100644
@@ -6,7 +6,7 @@
 
 use core::ptr;
 
-use crate::{bindings, types::Opaque};
+use crate::{bindings, str::CStr, types::Opaque};
 
 /// A reference-counted fwnode_handle.
 ///

@@ -31,6 +31,12 @@ impl FwNode {
     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.

Signature · 1 line

2.49.0

Quoted 8 lines · Remo Senekowitsch

The new FwNode abstraction will be used for accessing all device properties, so it must have the property_present method.

It's possible to duplicate the methods on the device itself, but since some of the methods on Device would have different type sigatures as the ones on FwNode, this would only lead to inconsistency and confusion. Hence, remove the method from Device.

Quoted 1 line

There aren't any users to update yet.

But there is one going into 6.16 most likely with the cpufreq stuff[1] which complicates merging.

Rob

[1] https://lore.kernel.org/all/f7e96b7da77ac217be5ccb09b9309da28fd96c90.1745218976.git.viresh.kumar@linaro.org/

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

It will be used by upcoming methods for reading device properties.

Signed-off-by Remo Senekowitsch <[email protected]>

---
 rust/kernel/device/property.rs | 59 ++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

rust/kernel/device/property.rs +23 -0

index 6ccc7947f9c31..59c61e2493831 100644
@@ -123,3 +123,62 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
         unsafe { bindings::fwnode_handle_put(obj.cast().as_ptr()) }
     }
 }
+
+/// A helper for reading device properties.
+///
+/// Use [`Self::required_by`] 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. The
+    /// device is required to associate the log with it.
+    pub fn required_by(self, dev: &super::Device) -> Result<T> {

Quoted 51 lines · Remo Senekowitsch

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

It will be used by upcoming methods for reading device properties.

Signed-off-by Remo Senekowitsch <[email protected]>

---
 rust/kernel/device/property.rs | 59 ++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

rust/kernel/device/property.rs +31 -0

index 6ccc7947f9c31..59c61e2493831 100644
@@ -123,3 +123,62 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
         unsafe { bindings::fwnode_handle_put(obj.cast().as_ptr()) }
     }
 }
+
+/// A helper for reading device properties.
+///
+/// Use [`Self::required_by`] 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. The
+    /// device is required to associate the log with it.
+    pub fn required_by(self, dev: &super::Device) -> Result<T> {
+        if self.inner.is_err() {
+            dev_err!(
+                dev,
+                "{}: property '{}' is missing\n",
+                self.fwnode.display_path(),
+                self.name
+            );
+        }

rust/kernel/device/property.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 158

Quoted 2 lines

rust/kernel/device/property.rs +2 -0

+        self.inner
+    }

Thinking about the .required_by(dev) I wonder if there will be cases where we do *not* have a device? I.e. where we really have a fwnode, only. And therefore can't pass a device. If we have such cases do we need to be able to pass e.g. Option(dev) and switch back to pr_err() in case of None?

From the beginning of our discussion I think to remember that the C API has both the fwnode_property_*() and device_property_*() because there are use cases for the fwnode_property_*() API where is no device?

Quoted 52 lines · Dirk Behme

Quoted 51 lines · Remo Senekowitsch

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

It will be used by upcoming methods for reading device properties.

Signed-off-by Remo Senekowitsch <[email protected]>

---
 rust/kernel/device/property.rs | 59 ++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

rust/kernel/device/property.rs +31 -0

index 6ccc7947f9c31..59c61e2493831 100644
@@ -123,3 +123,62 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
         unsafe { bindings::fwnode_handle_put(obj.cast().as_ptr()) }
     }
 }
+
+/// A helper for reading device properties.
+///
+/// Use [`Self::required_by`] 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. The
+    /// device is required to associate the log with it.
+    pub fn required_by(self, dev: &super::Device) -> Result<T> {
+        if self.inner.is_err() {
+            dev_err!(
+                dev,
+                "{}: property '{}' is missing\n",
+                self.fwnode.display_path(),
+                self.name
+            );
+        }

Quoted 8 lines

rust/kernel/device/property.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 158

Quoted 2 lines

rust/kernel/device/property.rs +2 -0

+        self.inner
+    }

Thinking about the .required_by(dev) I wonder if there will be cases where we do *not* have a device? I.e. where we really have a fwnode, only. And therefore can't pass a device. If we have such cases do we need to be able to pass e.g. Option(dev) and switch back to pr_err() in case of None?

In that case, bringing back the previous .required() method seems reasonable to me. But only if we definitely know such cases exist.

Quoted 3 lines

From the beginning of our discussion I think to remember that the C API has both the fwnode_property_*() and device_property_*() because there are use cases for the fwnode_property_*() API where is no device?

I'm not sure what you're referring to, the closest thing I can think of is this comment by Rob [1] where he mentions the device_property_*() functions only exist in C for a minimal convenience gain and we may not want to keep that in Rust.

Quoted 63 lines · Remo Senekowitsch

Quoted 60 lines · Dirk Behme

Quoted 43 lines · Remo Senekowitsch

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

It will be used by upcoming methods for reading device properties.

Signed-off-by Remo Senekowitsch <[email protected]>

---
 rust/kernel/device/property.rs | 59 ++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

rust/kernel/device/property.rs +23 -0

index 6ccc7947f9c31..59c61e2493831 100644
@@ -123,3 +123,62 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
         unsafe { bindings::fwnode_handle_put(obj.cast().as_ptr()) }
     }
 }
+
+/// A helper for reading device properties.
+///
+/// Use [`Self::required_by`] 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. The
+    /// device is required to associate the log with it.
+    pub fn required_by(self, dev: &super::Device) -> Result<T> {

rust/kernel/device/property.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 158

Quoted 10 lines

rust/kernel/device/property.rs +10 -0

+        if self.inner.is_err() {
+            dev_err!(
+                dev,
+                "{}: property '{}' is missing\n",
+                self.fwnode.display_path(),
+                self.name
+            );
+        }
+        self.inner
+    }

Thinking about the .required_by(dev) I wonder if there will be cases where we do *not* have a device? I.e. where we really have a fwnode, only. And therefore can't pass a device. If we have such cases do we need to be able to pass e.g. Option(dev) and switch back to pr_err() in case of None?

Quoted 2 lines

In that case, bringing back the previous .required() method seems reasonable to me. But only if we definitely know such cases exist.

They definitely exist. Any property in a child node of the device's node when the child itself is not another device for example.

Quoted 3 lines

Quoted 3 lines

From the beginning of our discussion I think to remember that the C API has both the fwnode_property_*() and device_property_*() because there are use cases for the fwnode_property_*() API where is no device?

Correct.

Quoted 4 lines

I'm not sure what you're referring to, the closest thing I can think of is this comment by Rob [1] where he mentions the device_property_*() functions only exist in C for a minimal convenience gain and we may not want to keep that in Rust.

The point there was if there's not the same convenience with Rust, then we shouldn't keep the API.

I think this came up previously, but I don't think it matters whether we print the device name or fwnode path. If you have either one, you can figure out both the driver and node. Arguably the fwnode path is more useful because that's likely where the error is.

Rob

Quoted 2 lines · Rob Herring

They definitely exist. Any property in a child node of the device's node when the child itself is not another device for example.

I don't think that counts, because you do have a device in that situation. The log should be assicated with that. So callers are responsible to propagate a reference to the device to wherever the call to .required_by(dev) is happening.

Quoted 67 lines · Rob Herring

Quoted 65 lines · Remo Senekowitsch

Quoted 52 lines · Dirk Behme

Quoted 51 lines · Remo Senekowitsch

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

It will be used by upcoming methods for reading device properties.

Signed-off-by Remo Senekowitsch <[email protected]>

---
 rust/kernel/device/property.rs | 59 ++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

rust/kernel/device/property.rs +31 -0

index 6ccc7947f9c31..59c61e2493831 100644
@@ -123,3 +123,62 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
         unsafe { bindings::fwnode_handle_put(obj.cast().as_ptr()) }
     }
 }
+
+/// A helper for reading device properties.
+///
+/// Use [`Self::required_by`] 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. The
+    /// device is required to associate the log with it.
+    pub fn required_by(self, dev: &super::Device) -> Result<T> {
+        if self.inner.is_err() {
+            dev_err!(
+                dev,
+                "{}: property '{}' is missing\n",
+                self.fwnode.display_path(),
+                self.name
+            );
+        }

Quoted 8 lines

rust/kernel/device/property.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 158

Quoted 2 lines

rust/kernel/device/property.rs +2 -0

+        self.inner
+    }

Thinking about the .required_by(dev) I wonder if there will be cases where we do *not* have a device? I.e. where we really have a fwnode, only. And therefore can't pass a device. If we have such cases do we need to be able to pass e.g. Option(dev) and switch back to pr_err() in case of None?

In that case, bringing back the previous .required() method seems reasonable to me. But only if we definitely know such cases exist.

Quoted 2 lines

They definitely exist. Any property in a child node of the device's node when the child itself is not another device for example.

I don't think that counts, because you do have a device in that situation. The log should be assicated with that. So callers are responsible to propagate a reference to the device to wherever the call to .required_by(dev) is happening.

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

It will be used by upcoming methods for reading device properties.

Signed-off-by Remo Senekowitsch <[email protected]>

---
 rust/kernel/device/property.rs | 59 ++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

rust/kernel/device/property.rs +18 -0

index 6ccc7947f9c31..59c61e2493831 100644
@@ -123,3 +123,62 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
         unsafe { bindings::fwnode_handle_put(obj.cast().as_ptr()) }
     }
 }
+
+/// A helper for reading device properties.
+///
+/// Use [`Self::required_by`] 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.

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 types of properties where appropriate.

Co-developed-by Rob Herring (Arm) <[email protected]>

Signed-off-by Rob Herring (Arm) <[email protected]>

Signed-off-by Remo Senekowitsch <[email protected]>

---
 rust/kernel/device/property.rs | 232 ++++++++++++++++++++++++++++++++-
 1 file changed, 230 insertions(+), 2 deletions(-)

rust/kernel/device/property.rs +40 -2

index 59c61e2493831..413166e2d082e 100644
@@ -4,9 +4,16 @@
 //!
 //! C header: [`include/linux/property.h`](srctree/include/linux/property.h)
 
-use core::ptr;
+use core::{mem::MaybeUninit, ptr};
 
-use crate::{bindings, str::CStr, types::Opaque};
+use crate::{
+    alloc::KVec,
+    bindings,
+    error::{to_result, Result},
+    prelude::*,
+    str::{CStr, CString},
+    types::Opaque,
+};
 
 /// A reference-counted fwnode_handle.
 ///

@@ -109,6 +116,105 @@ 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 firmware property `name` boolean value
+    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_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 array values in a KVec
+    pub fn property_read_array_vec<'fwnode, 'name, T: PropertyInt>(
+        &'fwnode self,
+        name: &'name CStr,
+        len: usize,
+    ) -> Result<PropertyGuard<'fwnode, 'name, KVec<T>>> {
+        let mut val: KVec<T> = KVec::with_capacity(len, GFP_KERNEL)?;
+

rust/kernel/device/property.rs +14 -0

+            Ok(val)
+        };
+        Ok(PropertyGuard {
+            inner: res,
+            fwnode: self,
+            name,
+        })
+    }
+
+    /// Returns integer array length for firmware property `name`
+    pub fn property_count_elem<T: PropertyInt>(&self, name: &CStr) -> Result<usize> {
+        // SAFETY: `out_param` is allowed to be null because `len` is zero.
+        let ret = unsafe { read_array_out_param::<T>(self, name, ptr::null_mut(), 0) };
+        to_result(ret)?;

rust/kernel/device/property.rs +34 -0

+    /// 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 kernel::{c_str, device::{Device, property::FwNode}, str::CString};
+    /// fn examples(dev: &Device) -> Result {
+    ///     let fwnode = dev.fwnode().ok_or(ENOENT)?;
+    ///     let b: u32 = fwnode.property_read(c_str!("some-number")).required_by(dev)?;
+    ///     if let Some(s) = fwnode.property_read::<CString>(c_str!("some-str")).optional() {
+    ///         // ...
+    ///     }
+    ///     Ok(())
+    /// }
+    /// ```
+    pub fn property_read<'fwnode, 'name, T: Property>(
+        &'fwnode self,
+        name: &'name CStr,
+    ) -> PropertyGuard<'fwnode, 'name, T> {
+        PropertyGuard {
+            inner: T::read_from_fwnode_property(self, name),
+            fwnode: self,
+            name,
+        }
+    }
 }
 
 // SAFETY: Instances of `FwNode` are always reference-counted.

rust/kernel/device/property.rs +9 -0

+pub trait Property: Sized {
+    /// Used to make [`FwNode::property_read`] generic.
+    fn read_from_fwnode_property(fwnode: &FwNode, name: &CStr) -> Result<Self>;
+}
+
+impl Property for CString {
+    fn read_from_fwnode_property(fwnode: &FwNode, name: &CStr) -> Result<Self> {
+        let mut str: *mut u8 = ptr::null_mut();
+        let pstr: *mut _ = &mut str;

rust/kernel/device/property.rs +57 -0

+    unsafe fn read_array_from_fwnode_property(
+        fwnode: *const bindings::fwnode_handle,
+        propname: *const ffi::c_char,
+        val: *mut Self,
+        nval: usize,
+    ) -> ffi::c_int;
+}
+// This macro generates implementations of the traits `Property` and
+// `PropertyInt` for integers of various sizes. Its input is a list
+// of pairs separated by commas. The first element of the pair is the
+// type of the integer, the second one is the name of its corresponding
+// `fwnode_property_read_*_array` function.
+macro_rules! impl_property_for_int {
+    ($($int:ty: $f:ident),* $(,)?) => { $(
+        impl PropertyInt for $int {
+            unsafe fn read_array_from_fwnode_property(
+                fwnode: *const bindings::fwnode_handle,
+                propname: *const ffi::c_char,
+                val: *mut Self,
+                nval: usize,
+            ) -> ffi::c_int {
+                // SAFETY: The safety invariants on the trait require
+                // callers to uphold the invariants of the functions
+                // this macro is called with.
+                unsafe {
+                    bindings::$f(fwnode, propname, val.cast(), nval)
+                }
+            }
+        }
+    )* };
+}
+impl_property_for_int! {
+    u8: fwnode_property_read_u8_array,
+    u16: fwnode_property_read_u16_array,
+    u32: fwnode_property_read_u32_array,
+    u64: fwnode_property_read_u64_array,
+    i8: fwnode_property_read_u8_array,
+    i16: fwnode_property_read_u16_array,
+    i32: fwnode_property_read_u32_array,
+    i64: fwnode_property_read_u64_array,
+}
+/// # Safety
+///
+/// Callers must ensure that if `len` is non-zero, `out_param` must be
+/// valid and point to memory that has enough space to hold at least
+/// `len` number of elements.
+unsafe fn read_array_out_param<T: PropertyInt>(
+    fwnode: &FwNode,
+    name: &CStr,
+    out_param: *mut T,
+    len: usize,
+) -> ffi::c_int {
+    // SAFETY: `name` is non-null and null-terminated.
+    // `fwnode.as_raw` is valid because `fwnode` is valid.
+    // `out_param` is valid and has enough space for at least
+    // `len` number of elements as per the safety requirement.
+    unsafe {

rust/kernel/device/property.rs +15 -0

+        let ret = unsafe { read_array_out_param::<T>(fwnode, name, val.as_mut_ptr().cast(), N) };
+        to_result(ret)?;
+
+        // SAFETY: `val` is always initialized when
+        // fwnode_property_read_<T>_array is successful.
+        Ok(val.map(|v| unsafe { v.assume_init() }))
+    }
+}
+impl<T: PropertyInt> Property for T {
+    fn read_from_fwnode_property(fwnode: &FwNode, name: &CStr) -> Result<Self> {
+        let val: [_; 1] = <[T; 1] as Property>::read_from_fwnode_property(fwnode, name)?;
+        Ok(val[0])
+    }
+}
+
 /// A helper for reading device properties.
 ///
 /// Use [`Self::required_by`] if a missing property is considered a bug and

Signature · 1 line

2.49.0

Quoted 5 lines

rust/kernel/device/property.rs +5 -0

+/// Implemented for all integers that can be read as properties.
+///
+/// This helper trait is needed on top of the existing [`Property`]
+/// trait to associate the integer types of various sizes with their
+/// corresponding `fwnode_property_read_*_array` functions.

rust/kernel/device/property.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 272

Quoted 5 lines

rust/kernel/device/property.rs +5 -0

+pub trait PropertyInt: Copy {
+    /// # Safety
+    ///
+    /// Callers must uphold the same safety invariants as for the various
+    /// `fwnode_property_read_*_array` functions.

I think you have additional requirements on the fwnode, propname and val pointers as well as on nval, please document them as well.

Quoted 41 lines

rust/kernel/device/property.rs +41 -0

+    unsafe fn read_array_from_fwnode_property(
+        fwnode: *const bindings::fwnode_handle,
+        propname: *const ffi::c_char,
+        val: *mut Self,
+        nval: usize,
+    ) -> ffi::c_int;
+}
+// This macro generates implementations of the traits `Property` and
+// `PropertyInt` for integers of various sizes. Its input is a list
+// of pairs separated by commas. The first element of the pair is the
+// type of the integer, the second one is the name of its corresponding
+// `fwnode_property_read_*_array` function.
+macro_rules! impl_property_for_int {
+    ($($int:ty: $f:ident),* $(,)?) => { $(
+        impl PropertyInt for $int {
+            unsafe fn read_array_from_fwnode_property(
+                fwnode: *const bindings::fwnode_handle,
+                propname: *const ffi::c_char,
+                val: *mut Self,
+                nval: usize,
+            ) -> ffi::c_int {
+                // SAFETY: The safety invariants on the trait require
+                // callers to uphold the invariants of the functions
+                // this macro is called with.
+                unsafe {
+                    bindings::$f(fwnode, propname, val.cast(), nval)
+                }
+            }
+        }
+    )* };
+}
+impl_property_for_int! {
+    u8: fwnode_property_read_u8_array,
+    u16: fwnode_property_read_u16_array,
+    u32: fwnode_property_read_u32_array,
+    u64: fwnode_property_read_u64_array,
+    i8: fwnode_property_read_u8_array,
+    i16: fwnode_property_read_u16_array,
+    i32: fwnode_property_read_u32_array,
+    i64: fwnode_property_read_u64_array,
+}

Quoted 5 lines · Danilo Krummrich

Quoted 5 lines

rust/kernel/device/property.rs +5 -0

+/// Implemented for all integers that can be read as properties.
+///
+/// This helper trait is needed on top of the existing [`Property`]
+/// trait to associate the integer types of various sizes with their
+/// corresponding `fwnode_property_read_*_array` functions.

Quoted 8 lines

rust/kernel/device/property.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 272

Quoted 5 lines

rust/kernel/device/property.rs +5 -0

+pub trait PropertyInt: Copy {
+    /// # Safety
+    ///
+    /// Callers must uphold the same safety invariants as for the various
+    /// `fwnode_property_read_*_array` functions.

I think you have additional requirements on the fwnode, propname and val pointers as well as on nval, please document them as well.

What are the additional requirements? The implementation just calls the underlying fwnode_property_read_*_array with the exact same arguments, so I don't know what the additional requirements are.

Quoted 7 lines

Quoted 7 lines

rust/kernel/device/property.rs +7 -0

+    unsafe fn read_array_from_fwnode_property(
+        fwnode: *const bindings::fwnode_handle,
+        propname: *const ffi::c_char,
+        val: *mut Self,
+        nval: usize,
+    ) -> ffi::c_int;
+}

Quoted 15 lines · Remo Senekowitsch

Quoted 5 lines · Danilo Krummrich

Quoted 5 lines

rust/kernel/device/property.rs +5 -0

+/// Implemented for all integers that can be read as properties.
+///
+/// This helper trait is needed on top of the existing [`Property`]
+/// trait to associate the integer types of various sizes with their
+/// corresponding `fwnode_property_read_*_array` functions.

Quoted 8 lines

rust/kernel/device/property.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 272

Quoted 5 lines

rust/kernel/device/property.rs +5 -0

+pub trait PropertyInt: Copy {
+    /// # Safety
+    ///
+    /// Callers must uphold the same safety invariants as for the various
+    /// `fwnode_property_read_*_array` functions.

I think you have additional requirements on the fwnode, propname and val pointers as well as on nval, please document them as well.

Quoted 3 lines

What are the additional requirements? The implementation just calls the underlying fwnode_property_read_*_array with the exact same arguments, so I don't know what the additional requirements are.

First of all, I don't think you can refer to the safety requirements of the fwnode_property_read_*_array functions, since they don't have any documented safety requirements.

So, I think you have safety requirements regarding pointer validity of fwnode, propname and val.

Additionally, there's the requirement that val has to be an array of nval length.

Also, the PropertyInt trait itself has to be unsafe, given that it contains unsafe functions.

I also pinged Benno about it, he usually knows best how to cover such things properly. :)

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

rust/kernel/device/property.rs +7 -0

+    unsafe fn read_array_from_fwnode_property(
+        fwnode: *const bindings::fwnode_handle,
+        propname: *const ffi::c_char,
+        val: *mut Self,
+        nval: usize,
+    ) -> ffi::c_int;
+}

Quoted 16 lines · Danilo Krummrich

Quoted 15 lines · Remo Senekowitsch

Quoted 13 lines · Danilo Krummrich

rust/kernel/device/property.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 272

Quoted 10 lines

rust/kernel/device/property.rs +10 -0

+/// Implemented for all integers that can be read as properties.
+///
+/// This helper trait is needed on top of the existing [`Property`]
+/// trait to associate the integer types of various sizes with their
+/// corresponding `fwnode_property_read_*_array` functions.
+pub trait PropertyInt: Copy {
+    /// # Safety
+    ///
+    /// Callers must uphold the same safety invariants as for the various
+    /// `fwnode_property_read_*_array` functions.

I think you have additional requirements on the fwnode, propname and val pointers as well as on nval, please document them as well.

Quoted 7 lines

Quoted 3 lines

What are the additional requirements? The implementation just calls the underlying fwnode_property_read_*_array with the exact same arguments, so I don't know what the additional requirements are.

First of all, I don't think you can refer to the safety requirements of the fwnode_property_read_*_array functions, since they don't have any documented safety requirements.

Quoted 5 lines

So, I think you have safety requirements regarding pointer validity of fwnode, propname and val.

Additionally, there's the requirement that val has to be an array of nval length.

Quoted 2 lines

Also, the PropertyInt trait itself has to be unsafe, given that it contains unsafe functions.

I don't think a trait necessarily has to be marked unsafe just because it has unsafe methods. Marking a trait as unsafe means that implementors of the trait must uphold some invariants. This is not the case here IIUC. Here's a good explanation of my understanding: [1]

But I should anyway seal the two traits. They're not supposed to be implemented outside the kernel crate.

[1] https://users.rust-lang.org/t/safe-trait-with-an-unsafe-method/67993/3

Quoted 10 lines

I also pinged Benno about it, he usually knows best how to cover such things properly. :)

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

rust/kernel/device/property.rs +7 -0

+    unsafe fn read_array_from_fwnode_property(
+        fwnode: *const bindings::fwnode_handle,
+        propname: *const ffi::c_char,
+        val: *mut Self,
+        nval: usize,
+    ) -> ffi::c_int;
+}

Quoted 4 lines · Remo Senekowitsch

Quoted 2 lines · Danilo Krummrich

Also, the PropertyInt trait itself has to be unsafe, given that it contains unsafe functions.

Quoted 4 lines

I don't think a trait necessarily has to be marked unsafe just because it has unsafe methods. Marking a trait as unsafe means that implementors of the trait must uphold some invariants. This is not the case here IIUC. Here's a good explanation of my understanding: [1]

Yes this is correct, I don't think that the trait itself should be unsafe.

Quoted 10 lines · Benno Lossin

Quoted 8 lines · Remo Senekowitsch

Quoted 2 lines · Danilo Krummrich

Also, the PropertyInt trait itself has to be unsafe, given that it contains unsafe functions.

I don't think a trait necessarily has to be marked unsafe just because it has unsafe methods. Marking a trait as unsafe means that implementors of the trait must uphold some invariants. This is not the case here IIUC. Here's a good explanation of my understanding: [1]

Quoted 2 lines

Yes this is correct, I don't think that the trait itself should be unsafe.

Ahh, I understood now why Danilo suggested this: if the trait should guarantee that fwnode_property_read_*_array is called, then the trait would have to be unsafe.

But I don't think that's necessary, we don't have any other unsafe code that needs to rely on that.

rust/kernel/device/property.rs +6 -0

+/// Implemented for all integers that can be read as properties.
+///
+/// This helper trait is needed on top of the existing [`Property`]
+/// trait to associate the integer types of various sizes with their
+/// corresponding `fwnode_property_read_*_array` functions.
+pub trait PropertyInt: Copy {

rust/kernel/device/property.rs +44 -0

+}
+// This macro generates implementations of the traits `Property` and
+// `PropertyInt` for integers of various sizes. Its input is a list
+// of pairs separated by commas. The first element of the pair is the
+// type of the integer, the second one is the name of its corresponding
+// `fwnode_property_read_*_array` function.
+macro_rules! impl_property_for_int {
+    ($($int:ty: $f:ident),* $(,)?) => { $(
+        impl PropertyInt for $int {
+            unsafe fn read_array_from_fwnode_property(
+                fwnode: *const bindings::fwnode_handle,
+                propname: *const ffi::c_char,
+                val: *mut Self,
+                nval: usize,
+            ) -> ffi::c_int {
+                // SAFETY: The safety invariants on the trait require
+                // callers to uphold the invariants of the functions
+                // this macro is called with.
+                unsafe {
+                    bindings::$f(fwnode, propname, val.cast(), nval)
+                }
+            }
+        }
+    )* };
+}
+impl_property_for_int! {
+    u8: fwnode_property_read_u8_array,
+    u16: fwnode_property_read_u16_array,
+    u32: fwnode_property_read_u32_array,
+    u64: fwnode_property_read_u64_array,
+    i8: fwnode_property_read_u8_array,
+    i16: fwnode_property_read_u16_array,
+    i32: fwnode_property_read_u32_array,
+    i64: fwnode_property_read_u64_array,
+}
+/// # Safety
+///
+/// Callers must ensure that if `len` is non-zero, `out_param` must be
+/// valid and point to memory that has enough space to hold at least
+/// `len` number of elements.
+unsafe fn read_array_out_param<T: PropertyInt>(
+    fwnode: &FwNode,
+    name: &CStr,
+    out_param: *mut T,

Quoted 11 lines · Remo Senekowitsch

rust/kernel/device/property.rs +11 -0

+/// Implemented for all integers that can be read as properties.
+///
+/// This helper trait is needed on top of the existing [`Property`]
+/// trait to associate the integer types of various sizes with their
+/// corresponding `fwnode_property_read_*_array` functions.
+pub trait PropertyInt: Copy {
+    /// # Safety
+    ///
+    /// Callers must uphold the same safety invariants as for the various
+    /// `fwnode_property_read_*_array` functions.
+    unsafe fn read_array_from_fwnode_property(

rust/kernel/device/property.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 278

Quoted 5 lines

rust/kernel/device/property.rs +5 -0

+        fwnode: *const bindings::fwnode_handle,
+        propname: *const ffi::c_char,
+        val: *mut Self,
+        nval: usize,
+    ) -> ffi::c_int;

I really, really dislike that this trait has to have an unsafe function with all those raw pointer inputs.

Quoted 49 lines

rust/kernel/device/property.rs +49 -0

+}
+// This macro generates implementations of the traits `Property` and
+// `PropertyInt` for integers of various sizes. Its input is a list
+// of pairs separated by commas. The first element of the pair is the
+// type of the integer, the second one is the name of its corresponding
+// `fwnode_property_read_*_array` function.
+macro_rules! impl_property_for_int {
+    ($($int:ty: $f:ident),* $(,)?) => { $(
+        impl PropertyInt for $int {
+            unsafe fn read_array_from_fwnode_property(
+                fwnode: *const bindings::fwnode_handle,
+                propname: *const ffi::c_char,
+                val: *mut Self,
+                nval: usize,
+            ) -> ffi::c_int {
+                // SAFETY: The safety invariants on the trait require
+                // callers to uphold the invariants of the functions
+                // this macro is called with.
+                unsafe {
+                    bindings::$f(fwnode, propname, val.cast(), nval)
+                }
+            }
+        }
+    )* };
+}
+impl_property_for_int! {
+    u8: fwnode_property_read_u8_array,
+    u16: fwnode_property_read_u16_array,
+    u32: fwnode_property_read_u32_array,
+    u64: fwnode_property_read_u64_array,
+    i8: fwnode_property_read_u8_array,
+    i16: fwnode_property_read_u16_array,
+    i32: fwnode_property_read_u32_array,
+    i64: fwnode_property_read_u64_array,
+}
+/// # Safety
+///
+/// Callers must ensure that if `len` is non-zero, `out_param` must be
+/// valid and point to memory that has enough space to hold at least
+/// `len` number of elements.
+unsafe fn read_array_out_param<T: PropertyInt>(
+    fwnode: &FwNode,
+    name: &CStr,
+    out_param: *mut T,
+    len: usize,
+) -> ffi::c_int {
+    // SAFETY: `name` is non-null and null-terminated.
+    // `fwnode.as_raw` is valid because `fwnode` is valid.
+    // `out_param` is valid and has enough space for at least

rust/kernel/device/property.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 332

Quoted 5 lines

rust/kernel/device/property.rs +5 -0

+    // `len` number of elements as per the safety requirement.
+    unsafe {
+        T::read_array_from_fwnode_property(fwnode.as_raw(), name.as_char_ptr(), out_param, len)
+    }
+}

Why does this function exist? It doesn't do anything and just delegates the call to T::read_array_from_fwnode_property.

This feels like you're dragging the C interface through the lower layers of your Rust abstractions, which I wouldn't do. I also looked a bit at the C code and saw this comment in driver/base/property.c:324:

* It's recommended to call fwnode_property_count_u8() instead of calling
* this function with @val equals %NULL and @nval equals 0.

That probably holds also for the other functions, so maybe we should do that instead? (although fwnode_property_count_u8 just delegates and calls with fwnode_property_read_u8_array...)

How about we do it like this:

    pub trait PropertyInt: Copy + Sized + private::Sealed {
        /// ...
        ///
        /// Returns a reference to `out` containing all written elements.
        fn read<'a>(
            fwnode: &FwNode,
            name: &CStr,
            out: &'a mut [MaybeUninit<Self>],
        ) -> Result<&'a mut [Self]>;

        fn length(fwnode: &FwNode, name: &CStr) -> Result<usize>;
    }

And then have a macro to implement it on all the integers.

Quoted 12 lines · Benno Lossin

Quoted 11 lines · Remo Senekowitsch

rust/kernel/device/property.rs +11 -0

+/// Implemented for all integers that can be read as properties.
+///
+/// This helper trait is needed on top of the existing [`Property`]
+/// trait to associate the integer types of various sizes with their
+/// corresponding `fwnode_property_read_*_array` functions.
+pub trait PropertyInt: Copy {
+    /// # Safety
+    ///
+    /// Callers must uphold the same safety invariants as for the various
+    /// `fwnode_property_read_*_array` functions.
+    unsafe fn read_array_from_fwnode_property(

Quoted 8 lines

rust/kernel/device/property.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 278

Quoted 5 lines

rust/kernel/device/property.rs +5 -0

+        fwnode: *const bindings::fwnode_handle,
+        propname: *const ffi::c_char,
+        val: *mut Self,
+        nval: usize,
+    ) -> ffi::c_int;

I really, really dislike that this trait has to have an unsafe function with all those raw pointer inputs.

What is the concrete problem with that? Or is it just "not pretty"? (I'm fine with making it prettier, just making sure I understand correctly.)

Quoted 49 lines

Quoted 49 lines

rust/kernel/device/property.rs +49 -0

+}
+// This macro generates implementations of the traits `Property` and
+// `PropertyInt` for integers of various sizes. Its input is a list
+// of pairs separated by commas. The first element of the pair is the
+// type of the integer, the second one is the name of its corresponding
+// `fwnode_property_read_*_array` function.
+macro_rules! impl_property_for_int {
+    ($($int:ty: $f:ident),* $(,)?) => { $(
+        impl PropertyInt for $int {
+            unsafe fn read_array_from_fwnode_property(
+                fwnode: *const bindings::fwnode_handle,
+                propname: *const ffi::c_char,
+                val: *mut Self,
+                nval: usize,
+            ) -> ffi::c_int {
+                // SAFETY: The safety invariants on the trait require
+                // callers to uphold the invariants of the functions
+                // this macro is called with.
+                unsafe {
+                    bindings::$f(fwnode, propname, val.cast(), nval)
+                }
+            }
+        }
+    )* };
+}
+impl_property_for_int! {
+    u8: fwnode_property_read_u8_array,
+    u16: fwnode_property_read_u16_array,
+    u32: fwnode_property_read_u32_array,
+    u64: fwnode_property_read_u64_array,
+    i8: fwnode_property_read_u8_array,
+    i16: fwnode_property_read_u16_array,
+    i32: fwnode_property_read_u32_array,
+    i64: fwnode_property_read_u64_array,
+}
+/// # Safety
+///
+/// Callers must ensure that if `len` is non-zero, `out_param` must be
+/// valid and point to memory that has enough space to hold at least
+/// `len` number of elements.
+unsafe fn read_array_out_param<T: PropertyInt>(
+    fwnode: &FwNode,
+    name: &CStr,
+    out_param: *mut T,
+    len: usize,
+) -> ffi::c_int {
+    // SAFETY: `name` is non-null and null-terminated.
+    // `fwnode.as_raw` is valid because `fwnode` is valid.
+    // `out_param` is valid and has enough space for at least

Quoted 8 lines

rust/kernel/device/property.rs unsafe fn dec_ref(obj: ptr::NonNull<Self>) { · line 332

Quoted 5 lines

rust/kernel/device/property.rs +5 -0

+    // `len` number of elements as per the safety requirement.
+    unsafe {
+        T::read_array_from_fwnode_property(fwnode.as_raw(), name.as_char_ptr(), out_param, len)
+    }
+}

Why does this function exist? It doesn't do anything and just delegates the call to T::read_array_from_fwnode_property.

It's used in three places. Taking Rust references as input reduces the safety requirements the callers must uphold. But I guess it's a small benefit, I can remove it.

Quoted 7 lines

This feels like you're dragging the C interface through the lower layers of your Rust abstractions, which I wouldn't do. I also looked a bit at the C code and saw this comment in driver/base/property.c:324:

* It's recommended to call fwnode_property_count_u8() instead of calling
* this function with @val equals %NULL and @nval equals 0.

Quoted 3 lines

That probably holds also for the other functions, so maybe we should do that instead? (although fwnode_property_count_u8 just delegates and calls with fwnode_property_read_u8_array...)

Yeah, I saw that too. The original implementation is from Rob. I assume the recommendation is for users, maybe for clarity and readability of the code. These Rust abstractions are pretty low level, so I thought it's probably fine.

Quoted 15 lines

How about we do it like this:

    pub trait PropertyInt: Copy + Sized + private::Sealed {
        /// ...
        ///
        /// Returns a reference to `out` containing all written elements.
        fn read<'a>(
            fwnode: &FwNode,
            name: &CStr,
            out: &'a mut [MaybeUninit<Self>],
        ) -> Result<&'a mut [Self]>;

        fn length(fwnode: &FwNode, name: &CStr) -> Result<usize>;
    }

Quoted 1 line

And then have a macro to implement it on all the integers.

I think I tried to make the macro as small as possible and use regular generics on top, because I was concerned people would be put off by big complicated macros. I'm fine with moving the actual trait implementation into the macro body, seems reasonable.

But still, I'm not sure why we're trying to make the PropertyInt trait's interface pretty or rusty. It's not supposed to be used, implemented or in any way interacted with outside this module. It's just a low-level building block to make some functions generic, giving users type inference.

Add some example usage of the device property read methods for DT/ACPI/swnode properties.

Co-developed-by Rob Herring (Arm) <[email protected]>

Signed-off-by Rob Herring (Arm) <[email protected]>

Signed-off-by Remo Senekowitsch <[email protected]>

---
 drivers/of/unittest-data/tests-platform.dtsi |  3 +
 samples/rust/rust_driver_platform.rs         | 71 +++++++++++++++++++-
 2 files changed, 72 insertions(+), 2 deletions(-)

drivers/of/unittest-data/tests-platform.dtsi +3 -0

index 4171f43cf01cc..50a51f38afb60 100644
@@ -37,6 +37,9 @@ dev@100 {
 			test-device@2 {
 				compatible = "test,rust-device";
 				reg = <0x2>;
+
+				test,u32-prop = <0xdeadbeef>;
+				test,i16-array = /bits/ 16 <1 2 (-3) (-4)>;
 			};
 		};
 

samples/rust/rust_driver_platform.rs +1 -1

index 8b42b3cfb363a..a04ff4afb1325 100644
@@ -2,7 +2,7 @@
 
 //! Rust Platform driver sample.
 
-use kernel::{c_str, device::Core, of, platform, prelude::*, types::ARef};
+use kernel::{c_str, device::Core, of, platform, prelude::*, str::CString, types::ARef};
 
 struct SampleDriver {
     pdev: ARef<platform::Device>,

samples/rust/rust_driver_platform.rs +4 -0

+        let fwnode = dev.fwnode().ok_or(ENOENT)?;
+
+        if let Ok(idx) =
+            fwnode.property_match_string(c_str!("compatible"), c_str!("test,rust-device"))

samples/rust/rust_driver_platform.rs +28 -0

+        let _ = fwnode
+            .property_read::<u32>(c_str!("test,u32-required-prop"))
+            .required_by(dev);
+
+        let prop: u32 = fwnode
+            .property_read(c_str!("test,u32-prop"))
+            .required_by(dev)?;
+        dev_info!(dev, "'test,u32-prop' is {:#x}\n", prop);
+
+        let prop: [i16; 4] = fwnode
+            .property_read(c_str!("test,i16-array"))
+            .required_by(dev)?;
+        dev_info!(dev, "'test,i16-array' is {:?}\n", prop);
+        dev_info!(
+            dev,
+            "'test,i16-array' length is {}\n",
+            fwnode.property_count_elem::<u16>(c_str!("test,i16-array"))?,
+        );
+
+        let prop: KVec<i16> = fwnode
+            .property_read_array_vec(c_str!("test,i16-array"), 4)?
+            .required_by(dev)?;
+        dev_info!(dev, "'test,i16-array' is KVec {:?}\n", prop);
+
+        Ok(())
+    }
+}
+
 impl Drop for SampleDriver {
     fn drop(&mut self) {
         dev_dbg!(self.pdev.as_ref(), "Remove Rust Platform driver sample.\n");

Signature · 1 line

2.49.0

I haven't gotten any actionable feedback on this version. What's the next step?