Re: [PATCH] rust: of: Add basic DeviceNode abstractions

[Markus Probst <markus.probst@posteo.de>]

I justed noticed a bug in the code, in which the `DeviceNodeIterator`
skipped the first child in the device node. Weirdly this bug hasn't
been spotted in testing, but it seems it should have been avoidable, so
I apologize. I will submit a second version of this patch.

On Mon, 2025-09-29 at 13:29 +0000, Markus Probst wrote:
> Add a safe wrapper arround `struct device_node`, which is capable of:
> 
> * reading string, u32 and bool properties
> 
> * iterating over children
> 
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
>  rust/bindings/bindings_helper.h |   1 +
>  rust/helpers/of.c               |  10 +++
>  rust/kernel/of.rs               | 150
> ++++++++++++++++++++++++++++++++
>  3 files changed, 161 insertions(+)
> 
> diff --git a/rust/bindings/bindings_helper.h
> b/rust/bindings/bindings_helper.h
> index 81796d5e16e8..e670b8e42787 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -59,6 +59,7 @@
>  #include <linux/jump_label.h>
>  #include <linux/mdio.h>
>  #include <linux/miscdevice.h>
> +#include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/pci.h>
>  #include <linux/phy.h>
> diff --git a/rust/helpers/of.c b/rust/helpers/of.c
> index 86b51167c913..293cc43452aa 100644
> --- a/rust/helpers/of.c
> +++ b/rust/helpers/of.c
> @@ -6,3 +6,13 @@ bool rust_helper_is_of_node(const struct
> fwnode_handle
> *fwnode)
>  {
>  	return is_of_node(fwnode);
>  }
> +
> +struct device_node *rust_helper_of_node_get(struct device_node
> *node)
> +{
> +	return of_node_get(node);
> +}
> +
> +void rust_helper_of_node_put(struct device_node *node)
> +{
> +	of_node_put(node);
> +}
> diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
> index b76b35265df2..61a470d9ecd7 100644
> --- a/rust/kernel/of.rs
> +++ b/rust/kernel/of.rs
> @@ -1,12 +1,16 @@
>  // SPDX-License-Identifier: GPL-2.0
> 
>  //! Device Tree / Open Firmware abstractions.
> +//!
> +//! C header: [`include/linux/of.h`](srctree/include/linux/of.h)
> 
>  use crate::{
>      bindings,
>      device_id::{RawDeviceId, RawDeviceIdIndex},
>      prelude::*,
> +    types::{ARef, Opaque},
>  };
> +use core::ptr::NonNull;
> 
>  /// IdTable type for OF drivers.
>  pub type IdTable<T> = &'static dyn
> kernel::device_id::IdTable<DeviceId, T>;
> @@ -16,6 +20,20 @@
>  #[derive(Clone, Copy)]
>  pub struct DeviceId(bindings::of_device_id);
> 
> +/// The device node representation.
> +///
> +/// This structure represents the Rust abstraction for a C `struct
> device_node`. The implementation
> +/// abstracts the usage of an already existing C `struct
> device_node`
> within Rust code that we get
> +/// passed from the C side.
> +///
> +/// # Invariants
> +///
> +/// A [`DeviceNode`] instance represents a valid `struct
> device_node`
> created by the C portion of the kernel.
> +#[repr(transparent)]
> +pub struct DeviceNode(Opaque<bindings::device_node>);
> +
> +struct DeviceNodeIterator<'a>(&'a DeviceNode,
> Option<NonNull<bindings::device_node>>);
> +
>  // SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct
> of_device_id` and
>  // does not add additional invariants, so it's safe to transmute to
> `RawType`.
>  unsafe impl RawDeviceId for DeviceId {
> @@ -63,3 +81,135 @@ macro_rules! of_device_table {
>          $crate::module_device_table!("of", $module_table_name,
> $table_name);
>      };
>  }
> +
> +impl DeviceNode {
> +    const fn as_raw(&self) -> *mut bindings::device_node {
> +        self.0.get()
> +    }
> +
> +    /// Returns the device tree populated by the bootloader.
> +    pub fn root() -> Option<&'static DeviceNode> {
> +        // SAFETY: `of_root` is guaranteed to be a pointer to a
> valid
> `struct device_node` or a null-pointer.
> +        NonNull::new(unsafe { bindings::of_root })
> +            // CAST: `DeviceNode` is a transparent wrapper of
> `Opaque<bindings::device_node>`.
> +            // SAFETY: `ptr` is guaranteed to be a pointer to a
> valid
> `struct device_node`.
> +            .map(|ptr| unsafe { ptr.cast().as_ref() })
> +    }
> +
> +    /// Returns an iterator over the children of this device node.
> +    pub fn children(&self) -> impl Iterator<Item = ARef<DeviceNode>>
> +
> use<'_> {
> +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a
> valid `struct device_node`.
> +        let initial = unsafe {
> bindings::of_get_next_child(self.as_raw(), core::ptr::null_mut()) };
> +        DeviceNodeIterator(self, NonNull::new(initial.cast()))
> +    }
> +
> +    /// Returns the name of the device node.
> +    pub fn name(&self) -> Option<&CStr> {
> +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a
> valid `struct device_node`.
> +        let name = unsafe { (*self.as_raw()).name };
> +        if name.is_null() {
> +            None
> +        } else {
> +            // SAFETY: `name` is valid by the safety requirements.
> +            Some(unsafe { CStr::from_char_ptr(name) })
> +        }
> +    }
> +
> +    /// Returns the full name (name including the full_name of the
> parent) of the device node.
> +    pub fn full_name(&self) -> Option<&CStr> {
> +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a
> valid `struct device_node`.
> +        let full_name = unsafe { (*self.as_raw()).full_name };
> +        if full_name.is_null() {
> +            None
> +        } else {
> +            // SAFETY: `full_name` is valid by the safety
> requirements.
> +            Some(unsafe { CStr::from_char_ptr(full_name) })
> +        }
> +    }
> +
> +    /// Find and read a u32 from a multi-value property.
> +    pub fn property_read_u32_index(&self, propname: &CStr, index:
> u32)
> -> Result<u32> {
> +        let mut value = 0;
> +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a
> valid `struct device_node`.
> +        let ret = unsafe {
> +            bindings::of_property_read_u32_index(
> +                self.as_raw(),
> +                propname.as_char_ptr(),
> +                index,
> +                &mut value,
> +            )
> +        };
> +        if ret != 0 {
> +            return Err(Error::from_errno(ret));
> +        }
> +        Ok(value)
> +    }
> +
> +    /// Find and read a string from a property.
> +    pub fn property_read_string(&self, propname: &CStr) ->
> Result<&CStr> {
> +        let mut value = core::ptr::null();
> +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a
> valid `struct device_node`.
> +        let ret = unsafe {
> +            bindings::of_property_read_string(self.as_raw(),
> propname.as_char_ptr(), &mut value)
> +        };
> +        if ret != 0 {
> +            return Err(Error::from_errno(ret));
> +        }
> +        // SAFETY: `value` is guaranteed to be a valid C string
> pointer.
> +        Ok(unsafe { CStr::from_char_ptr(value) })
> +    }
> +
> +    /// Find a property.
> +    ///
> +    /// Returns true if the property exists false otherwise.
> +    pub fn property_read_bool(&self, propname: &CStr) -> bool {
> +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a
> valid `struct device_node`.
> +        unsafe { bindings::of_property_read_bool(self.as_raw(),
> propname.as_char_ptr()) }
> +    }
> +
> +    /// Find the child node by name for this device node.
> +    pub fn child_by_name(&self, name: &CStr) ->
> Option<ARef<DeviceNode>> {
> +        // SAFETY: `self.as_raw` is guaranteed to be a pointer to a
> valid `struct device_node`.
> +        let node = unsafe {
> bindings::of_get_child_by_name(self.as_raw(), name.as_char_ptr()) };
> +        // SAFETY: `node` is guaranteed to be a pointer to a valid
> `struct device_node` or a null-pointer.
> +        Some(unsafe { ARef::from_raw(NonNull::new(node)?.cast()) })
> +    }
> +}
> +
> +// SAFETY: A `DeviceNode` is always reference-counted and can be
> released from any thread.
> +unsafe impl Send for DeviceNode {}
> +
> +// SAFETY: `DeviceNode` can be shared among threads because all
> methods of `DeviceNode` are thread safe.
> +unsafe impl Sync for DeviceNode {}
> +
> +// SAFETY: Instances of `DeviceNode` are always reference-counted.
> +unsafe impl kernel::types::AlwaysRefCounted for DeviceNode {
> +    fn inc_ref(&self) {
> +        // SAFETY: The existence of a shared reference guarantees
> that
> the refcount is non-zero.
> +        unsafe { bindings::of_node_get(self.as_raw()) };
> +    }
> +
> +    unsafe fn dec_ref(obj: NonNull<Self>) {
> +        // SAFETY: The safety requirements guarantee that the
> refcount
> is non-zero.
> +        unsafe { bindings::of_node_put(obj.cast().as_ptr()) }
> +    }
> +}
> +
> +impl<'a> Iterator for DeviceNodeIterator<'a> {
> +    type Item = ARef<DeviceNode>;
> +
> +    fn next(&mut self) -> Option<Self::Item> {
> +        let prev = self.1.take()?;
> +
> +        // SAFETY:
> +        // - `self.0.as_raw` is guaranteed to be a pointer to a
> valid
> `struct device_node`.
> +        // - `prev` is guaranteed to be a pointer to a valid `struct
> device_node`.
> +        self.1 =
> +            NonNull::new(unsafe {
> bindings::of_get_next_child(self.0.as_raw(), prev.as_ptr()) });
> +        self.1
> +            .as_ref()
> +            // CAST: `DeviceNode` is a transparent wrapper of
> `Opaque<bindings::device_node>`.
> +            // SAFETY: `ptr` is guaranteed to be a pointer to a
> valid
> `struct device_node`.
> +            .map(|ptr| ARef::from(unsafe {
> ptr.cast::<DeviceNode>().as_ref() }))
> +    }
> +}
> --
> 2.49.1