[06/10] rust: property: Add child accessor and iterator

Commit Message

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

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

Comments

Andrew Ballance March 26, 2025, 9:04 p.m. UTC | #1

On Wed, Mar 26, 2025 at 12:13 PM Remo Senekowitsch Wrote: 
>  impl FwNode {
> +    // SAFETY: `raw` must have its refcount incremented.
> +    unsafe fn from_raw(raw: *mut bindings::fwnode_handle) -> ARef<Self> {
> +        unsafe { ARef::from_raw(ptr::NonNull::new_unchecked(raw.cast())) }
> +    }

this safety comment should say why it is ok to cast from a
struct fwnode_handle* to a *const FwNode

> +                    // We will pass `prev` to `fwnode_get_next_child_node`,
> +                    // which decrements its refcount, so we use
> +                    // `ARef::into_raw` to avoid decrementing the refcount
> +                    // twice.
> +                    let prev = ARef::into_raw(prev);
> +                    prev.as_ptr().cast()
> +                }
> +            };
> +            let next = unsafe { bindings::fwnode_get_next_child_node(self.as_raw(), prev_ptr) };

this is missing a safety comment

> +            if next.is_null() {
> +                return None;
> +            }

Andrew

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

On Wed, Mar 26, 2025 at 06:13:45PM +0100, Remo Senekowitsch wrote:
> Signed-off-by: Remo Senekowitsch <[e-mail]>

Needs a commit msg.

> ---
>  rust/kernel/property.rs | 57 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 56 insertions(+), 1 deletion(-)

Patch

diff --git a/rust/kernel/property.rs b/rust/kernel/property.rs
index 4a03008ce..dc927ad93 100644
--- a/rust/kernel/property.rs
+++ b/rust/kernel/property.rs
@@ -13,7 +13,7 @@ 
     error::{to_result, Result},
     prelude::*,
     str::{CStr, CString},
-    types::{Integer, Opaque},
+    types::{ARef, Integer, Opaque},
 };
 
 impl Device {
@@ -54,6 +54,16 @@  pub fn property_count_elem<T: Integer>(&self, name: &CStr) -> Result<usize> {
     pub fn property_read<T: Property>(&self, name: &CStr) -> Result<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>> {
+        self.fwnode().get_child_by_name(name)
+    }
+
+    /// Returns an iterator over a node's children.
+    pub fn children<'a>(&'a self) -> impl Iterator<Item = ARef<FwNode>> + 'a {
+        self.fwnode().children()
+    }
 }
 
 /// A reference-counted fwnode_handle.
@@ -75,6 +85,11 @@  pub fn property_read<T: Property>(&self, name: &CStr) -> Result<T> {
 pub struct FwNode(Opaque<bindings::fwnode_handle>);
 
 impl FwNode {
+    // SAFETY: `raw` must have its refcount incremented.
+    unsafe fn from_raw(raw: *mut bindings::fwnode_handle) -> ARef<Self> {
+        unsafe { ARef::from_raw(ptr::NonNull::new_unchecked(raw.cast())) }
+    }
+
     /// Obtain the raw `struct fwnode_handle *`.
     pub(crate) fn as_raw(&self) -> *mut bindings::fwnode_handle {
         self.0.get()
@@ -171,6 +186,46 @@  pub fn property_count_elem<T: Integer>(&self, name: &CStr) -> Result<usize> {
     pub fn property_read<T: Property>(&self, name: &CStr) -> Result<T> {
         T::read(&self, name)
     }
+
+    /// Returns first matching named child node handle.
+    pub fn get_child_by_name(&self, name: &CStr) -> Option<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;
+        }
+        // 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.
+    pub fn children<'a>(&'a self) -> impl Iterator<Item = ARef<FwNode>> + 'a {
+        let mut prev: Option<ARef<FwNode>> = None;
+
+        core::iter::from_fn(move || {
+            let prev_ptr = match prev.take() {
+                None => ptr::null_mut(),
+                Some(prev) => {
+                    // We will pass `prev` to `fwnode_get_next_child_node`,
+                    // which decrements its refcount, so we use
+                    // `ARef::into_raw` to avoid decrementing the refcount
+                    // twice.
+                    let prev = ARef::into_raw(prev);
+                    prev.as_ptr().cast()
+                }
+            };
+            let next = unsafe { bindings::fwnode_get_next_child_node(self.as_raw(), prev_ptr) };
+            if next.is_null() {
+                return None;
+            }
+            // SAFETY: `fwnode_get_next_child_node` returns a pointer with
+            // refcount incremented.
+            let next = unsafe { FwNode::from_raw(next) };
+            prev = Some(next.clone());
+            Some(next)
+        })
+    }
 }
 
 // SAFETY: Instances of `FwNode` are always reference-counted.