Allow Rust drivers to access children of a fwnode either by name or by
iterating over all of them.

In C, there is the function `fwnode_get_next_child_node` for iteration
and the macro `fwnode_for_each_child_node` that helps with handling the
pointers. Instead of a macro, a native iterator is used in Rust such
that regular for-loops can be used.

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

diff --git a/rust/kernel/property.rs b/rust/kernel/property.rs
index 0d4ea3168..d9fb773b6 100644
--- a/rust/kernel/property.rs
+++ b/rust/kernel/property.rs
@@ -13,7 +13,7 @@
     error::{to_result, Result},
     prelude::*,
     str::{BStr, CStr, CString},
-    types::Opaque,
+    types::{ARef, Opaque},
 };
 
 impl Device {
@@ -68,6 +68,16 @@ pub fn property_read<'fwnode, 'name, T: Property>(
     ) -> 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>> {
+        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.
@@ -89,6 +99,22 @@ pub fn property_read<'fwnode, 'name, T: Property>(
 pub struct FwNode(Opaque<bindings::fwnode_handle>);
 
 impl FwNode {
+    /// # Safety
+    ///
+    /// Callers must ensure that the reference count was incremented at least
+    /// once, and that they are properly relinquishing one increment. That is,
+    /// if there is only one increment, callers must not use the underlying
+    /// object anymore -- it is only safe to do so via the newly created
+    /// [`ARef`].
+    unsafe fn from_raw(raw: *mut bindings::fwnode_handle) -> ARef<Self> {
+        // SAFETY: As per the safety requirement, raw has an incremented
+        // refcount which won't be decremented elsewhere. It also it not null.
+        // It is safe to cast from a `*mut fwnode_handle` to `*mut FwNode`,
+        // because `FwNode` is  defined as a `#[repr(transparent)]` wrapper
+        // around `fwnode_handle`.
+        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()
@@ -243,6 +269,53 @@ fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
 
         FwNodeDisplayPath(self)
     }
+
+    /// 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()
+                }
+            };
+            // SAFETY: `self.as_raw()` is valid. `prev_ptr` may be null,
+            // which is allowed and corresponds to getting the first child.
+            // Otherwise, `prev_ptr` is valid, as it is the stored return
+            // value from the previous invocation. `prev_ptr` has its refount
+            // incremented and it won't be decremented anymore on the Rust
+            // side. `fwnode_get_next_child_node` decrements the refcount of
+            // `prev_ptr`, so the refcount is handled correctly.
+            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.
-- 
2.49.0

On Mon, Apr 14, 2025 at 05:26:28PM +0200, Remo Senekowitsch wrote:
>  impl Device {
> @@ -68,6 +68,16 @@ pub fn property_read<'fwnode, 'name, T: Property>(
>      ) -> 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>> {
> +        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()
> +    }
>  }

Since those functions are within the impl Device block, please move them to
device.rs.

>  
>  /// A reference-counted fwnode_handle.
> @@ -89,6 +99,22 @@ pub fn property_read<'fwnode, 'name, T: Property>(
>  pub struct FwNode(Opaque<bindings::fwnode_handle>);
>  
>  impl FwNode {
> +    /// # Safety
> +    ///
> +    /// Callers must ensure that the reference count was incremented at least
> +    /// once, and that they are properly relinquishing one increment. That is,
> +    /// if there is only one increment, callers must not use the underlying
> +    /// object anymore -- it is only safe to do so via the newly created
> +    /// [`ARef`].

Please compile multiple safety requirements into a list.

> +    unsafe fn from_raw(raw: *mut bindings::fwnode_handle) -> ARef<Self> {
> +        // SAFETY: As per the safety requirement, raw has an incremented
> +        // refcount which won't be decremented elsewhere. It also it not null.

Same here.

> +        // It is safe to cast from a `*mut fwnode_handle` to `*mut FwNode`,
> +        // because `FwNode` is  defined as a `#[repr(transparent)]` wrapper
> +        // around `fwnode_handle`.

This should be CAST: instead.

> +        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()
> @@ -243,6 +269,53 @@ fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
>  
>          FwNodeDisplayPath(self)
>      }
> +
> +    /// 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.

I'd say that `&self` is valid by its type invariant.

> +        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.

Please cover all safety requirements from from_raw().

> +        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()
> +                }
> +            };
> +            // SAFETY: `self.as_raw()` is valid. `prev_ptr` may be null,
> +            // which is allowed and corresponds to getting the first child.
> +            // Otherwise, `prev_ptr` is valid, as it is the stored return
> +            // value from the previous invocation. `prev_ptr` has its refount
> +            // incremented and it won't be decremented anymore on the Rust
> +            // side. `fwnode_get_next_child_node` decrements the refcount of
> +            // `prev_ptr`, so the refcount is handled correctly.

Please also compile this into a list.

> +            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.

You rather need to justify why the next you pass into from_raw() is valid.

> +            let next = unsafe { FwNode::from_raw(next) };
> +            prev = Some(next.clone());
> +            Some(next)
> +        })
> +    }
>  }
>  
>  // SAFETY: Instances of `FwNode` are always reference-counted.
> -- 
> 2.49.0
>