diff mbox series

[08/10] rust: property: Add property_get_reference_args

Message ID 20250326171411.590681-9-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 
Signed-off-by: Remo Senekowitsch <remo@buenzli.dev>
---
 rust/kernel/property.rs | 63 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

Comments

Andrew Ballance March 26, 2025, 9:07 p.m. UTC | #1 
On Wed, Mar 26, 2025 at 12:13 PM Remo Senekowitsch Wrote: 
> +    pub fn property_get_reference_args(
> +        &self,
> +        prop: &CStr,
> +        nargs: NArgs<'_>,
> +        index: u32,
> +    ) -> Result<(
> +        ARef<Self>,
> +        ArrayVec<{ bindings::NR_FWNODE_REFERENCE_ARGS as usize }, u64>,
> +    )> {
> +        let mut out_args = bindings::fwnode_reference_args::default();
> +
> +        let (nargs_prop, nargs) = match nargs {
> +            NArgs::Prop(nargs_prop) => (nargs_prop.as_char_ptr(), 0),
> +            NArgs::N(nargs) => (ptr::null(), nargs),
> +        };
> +
> +        let ret = unsafe {
> +            bindings::fwnode_property_get_reference_args(
> +                self.0.get(),
> +                prop.as_char_ptr(),
> +                nargs_prop,
> +                nargs,
> +                index,
> +                &mut out_args,
> +            )
> +        };
> +        to_result(ret)?;
> +
> +        let node = unsafe { FwNode::from_raw(out_args.fwnode) };
> +        let mut args = ArrayVec::default();
> +
> +        for i in 0..out_args.nargs {
> +            args.push(out_args.args[i as usize]);
> +        }
> +
> +        Ok((node, args))
> +    }

This function is missing safety comments.

Andrew
Miguel Ojeda March 26, 2025, 9:25 p.m. UTC | #2 
On Wed, Mar 26, 2025 at 10:07 PM Andrew Ballance
<andrewjballance@gmail.com> wrote:
>
> This function is missing safety comments.

Yeah -- that should be spotted by Clippy.

Remo: did you build with `CLIPPY=1`?

Thanks!

Cheers,
Miguel
Remo Senekowitsch March 26, 2025, 9:45 p.m. UTC | #3 
On Wed Mar 26, 2025 at 10:25 PM CET, Miguel Ojeda wrote:
>
> Remo: did you build with `CLIPPY=1`?

I didn't! Usually I have rust-analyzer configured with clippy, but that doesn't seem to work here.
I added it to my build scripts and can see the warnings now, future patches should be free of clippy warnings.

Thanks for the tip!

Remo
Danilo Krummrich March 27, 2025, 2:32 p.m. UTC | #4 
On Wed, Mar 26, 2025 at 06:13:47PM +0100, Remo Senekowitsch wrote:
> Signed-off-by: Remo Senekowitsch <remo@buenzli.dev>
> ---
>  rust/kernel/property.rs | 63 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 63 insertions(+)
> 
> diff --git a/rust/kernel/property.rs b/rust/kernel/property.rs
> index dc927ad93..f1d0a33ba 100644
> --- a/rust/kernel/property.rs
> +++ b/rust/kernel/property.rs
> @@ -8,6 +8,7 @@
>  
>  use crate::{
>      alloc::KVec,
> +    arrayvec::ArrayVec,
>      bindings,
>      device::Device,
>      error::{to_result, Result},
> @@ -64,6 +65,20 @@ pub fn get_child_by_name(&self, name: &CStr) -> Option<ARef<FwNode>> {
>      pub fn children<'a>(&'a self) -> impl Iterator<Item = ARef<FwNode>> + 'a {
>          self.fwnode().children()
>      }
> +
> +    /// Finds a reference with arguments.
> +    pub fn property_get_reference_args(
> +        &self,
> +        prop: &CStr,
> +        nargs: NArgs<'_>,
> +        index: u32,
> +    ) -> Result<(
> +        ARef<FwNode>,
> +        ArrayVec<{ bindings::NR_FWNODE_REFERENCE_ARGS as usize }, u64>,
> +    )> {
> +        self.fwnode()
> +            .property_get_reference_args(prop, nargs, index)
> +    }
>  }
>  
>  /// A reference-counted fwnode_handle.
> @@ -226,6 +241,45 @@ pub fn children<'a>(&'a self) -> impl Iterator<Item = ARef<FwNode>> + 'a {
>              Some(next)
>          })
>      }
> +
> +    /// Finds a reference with arguments.
> +    pub fn property_get_reference_args(
> +        &self,
> +        prop: &CStr,
> +        nargs: NArgs<'_>,
> +        index: u32,
> +    ) -> Result<(
> +        ARef<Self>,
> +        ArrayVec<{ bindings::NR_FWNODE_REFERENCE_ARGS as usize }, u64>,
> +    )> {
> +        let mut out_args = bindings::fwnode_reference_args::default();
> +
> +        let (nargs_prop, nargs) = match nargs {
> +            NArgs::Prop(nargs_prop) => (nargs_prop.as_char_ptr(), 0),
> +            NArgs::N(nargs) => (ptr::null(), nargs),
> +        };
> +
> +        let ret = unsafe {
> +            bindings::fwnode_property_get_reference_args(
> +                self.0.get(),
> +                prop.as_char_ptr(),
> +                nargs_prop,
> +                nargs,
> +                index,
> +                &mut out_args,
> +            )
> +        };
> +        to_result(ret)?;
> +
> +        let node = unsafe { FwNode::from_raw(out_args.fwnode) };
> +        let mut args = ArrayVec::default();
> +
> +        for i in 0..out_args.nargs {
> +            args.push(out_args.args[i as usize]);
> +        }

Why the copy? Can't you just write an abstraction for struct
fwnode_reference_args and just return an instance of that?
diff mbox series

Patch

diff --git a/rust/kernel/property.rs b/rust/kernel/property.rs
index dc927ad93..f1d0a33ba 100644
--- a/rust/kernel/property.rs
+++ b/rust/kernel/property.rs
@@ -8,6 +8,7 @@ 
 
 use crate::{
     alloc::KVec,
+    arrayvec::ArrayVec,
     bindings,
     device::Device,
     error::{to_result, Result},
@@ -64,6 +65,20 @@  pub fn get_child_by_name(&self, name: &CStr) -> Option<ARef<FwNode>> {
     pub fn children<'a>(&'a self) -> impl Iterator<Item = ARef<FwNode>> + 'a {
         self.fwnode().children()
     }
+
+    /// Finds a reference with arguments.
+    pub fn property_get_reference_args(
+        &self,
+        prop: &CStr,
+        nargs: NArgs<'_>,
+        index: u32,
+    ) -> Result<(
+        ARef<FwNode>,
+        ArrayVec<{ bindings::NR_FWNODE_REFERENCE_ARGS as usize }, u64>,
+    )> {
+        self.fwnode()
+            .property_get_reference_args(prop, nargs, index)
+    }
 }
 
 /// A reference-counted fwnode_handle.
@@ -226,6 +241,45 @@  pub fn children<'a>(&'a self) -> impl Iterator<Item = ARef<FwNode>> + 'a {
             Some(next)
         })
     }
+
+    /// Finds a reference with arguments.
+    pub fn property_get_reference_args(
+        &self,
+        prop: &CStr,
+        nargs: NArgs<'_>,
+        index: u32,
+    ) -> Result<(
+        ARef<Self>,
+        ArrayVec<{ bindings::NR_FWNODE_REFERENCE_ARGS as usize }, u64>,
+    )> {
+        let mut out_args = bindings::fwnode_reference_args::default();
+
+        let (nargs_prop, nargs) = match nargs {
+            NArgs::Prop(nargs_prop) => (nargs_prop.as_char_ptr(), 0),
+            NArgs::N(nargs) => (ptr::null(), nargs),
+        };
+
+        let ret = unsafe {
+            bindings::fwnode_property_get_reference_args(
+                self.0.get(),
+                prop.as_char_ptr(),
+                nargs_prop,
+                nargs,
+                index,
+                &mut out_args,
+            )
+        };
+        to_result(ret)?;
+
+        let node = unsafe { FwNode::from_raw(out_args.fwnode) };
+        let mut args = ArrayVec::default();
+
+        for i in 0..out_args.nargs {
+            args.push(out_args.args[i as usize]);
+        }
+
+        Ok((node, args))
+    }
 }
 
 // SAFETY: Instances of `FwNode` are always reference-counted.
@@ -302,3 +356,12 @@  fn read(fwnode: &FwNode, name: &CStr) -> Result<T> {
         Ok(val[0])
     }
 }
+
+/// The number of arguments of a reference.
+pub enum NArgs<'a> {
+    /// The name of the property of the reference indicating the number of
+    /// arguments.
+    Prop(&'a CStr),
+    /// The known number of arguments.
+    N(u32),
+}