Re: [PATCH RFC 2/3] rust: Add bindings for device properties

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Oct 25, 2024 at 11:06 PM Rob Herring (Arm) <robh@xxxxxxxxxx> wrote:
>
> 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
> sizes of properties (e.g. u8, u16, u32).
>
> Signed-off-by: Rob Herring (Arm) <robh@xxxxxxxxxx>
> ---
> Not sure if we need the KVec variant, but I kept it as that was my first
> pass attempt. Most callers are filling in some value in a driver data
> struct. Sometimes the number of elements is not known, so the caller
> calls to get the array size, allocs the correct size buffer, and then
> reads the property again to fill in the buffer.
>
> I have not implemented a wrapper for device_property_read_string(_array)
> because that API is problematic for dynamic DT nodes. The API just
> returns pointer(s) into the raw DT data. We probably need to return a
> copy of the string(s) instead for rust.
>
> After property accessors, next up is child node accessors/iterators.
> ---
>  rust/bindings/bindings_helper.h |   1 +
>  rust/kernel/device.rs           | 145 +++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 145 insertions(+), 1 deletion(-)
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 217c776615b9..65717cc20a23 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -19,6 +19,7 @@
>  #include <linux/pci.h>
>  #include <linux/phy.h>
>  #include <linux/platform_device.h>
> +#include <linux/property.h>
>  #include <linux/refcount.h>
>  #include <linux/sched.h>
>  #include <linux/slab.h>
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 0c28b1e6b004..bb66a28df890 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -5,10 +5,14 @@
>  //! C header: [`include/linux/device.h`](srctree/include/linux/device.h)
>
>  use crate::{
> +    alloc::KVec,
>      bindings,
> +    error::{to_result, Result},
> +    prelude::*,
> +    str::CStr,
>      types::{ARef, Opaque},
>  };
> -use core::{fmt, ptr};
> +use core::{fmt, mem::size_of, ptr};
>
>  #[cfg(CONFIG_PRINTK)]
>  use crate::c_str;
> @@ -189,6 +193,145 @@ unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) {
>              )
>          };
>      }
> +
> +    /// Returns if a firmware property `name` is true or false
> +    pub fn property_read_bool(&self, name: &CStr) -> bool {
> +        unsafe { bindings::device_property_present(self.as_raw(), name.as_ptr() as *const i8) }
> +    }
> +
> +    /// Returns if a firmware string property `name` has match for `match_str`
> +    pub fn property_match_string(&self, name: &CStr, match_str: &CStr) -> Result<usize> {
> +        let ret = unsafe {
> +            bindings::device_property_match_string(
> +                self.as_raw(),
> +                name.as_ptr() as *const i8,
> +                match_str.as_ptr() as *const i8,
> +            )
> +        };
> +        to_result(ret)?;
> +        Ok(ret as usize)
> +    }
> +
> +    /// Returns firmware property `name` scalar value
> +    ///
> +    /// Valid types are i8, u8, i16, u16, i32, u32, i64, u64
> +    pub fn property_read<T: Copy>(&self, name: &CStr) -> Result<T> {
> +        let mut val: [T; 1] = unsafe { core::mem::zeroed() };
> +
> +        Self::_property_read_array(&self, name, &mut val)?;
> +        Ok(val[0])
> +    }
> +
> +    /// Returns firmware property `name` array values
> +    ///
> +    /// Valid types are i8, u8, i16, u16, i32, u32, i64, u64
> +    pub fn property_read_array<T, const N: usize>(&self, name: &CStr) -> Result<[T; N]> {
> +        let mut val: [T; N] = unsafe { core::mem::zeroed() };
> +
> +        Self::_property_read_array(self, name, &mut val)?;
> +        Ok(val)
> +    }
> +
> +    fn _property_read_array<T>(&self, name: &CStr, val: &mut [T]) -> Result {
> +        match size_of::<T>() {
> +            1 => to_result(unsafe {
> +                bindings::device_property_read_u8_array(
> +                    self.as_raw(),
> +                    name.as_ptr() as *const i8,
> +                    val.as_ptr() as *mut u8,
> +                    val.len(),
> +                )
> +            })?,
> +            2 => to_result(unsafe {
> +                bindings::device_property_read_u16_array(
> +                    self.as_raw(),
> +                    name.as_ptr() as *const i8,
> +                    val.as_ptr() as *mut u16,
> +                    val.len(),
> +                )
> +            })?,
> +            4 => to_result(unsafe {
> +                bindings::device_property_read_u32_array(
> +                    self.as_raw(),
> +                    name.as_ptr() as *const i8,
> +                    val.as_ptr() as *mut u32,
> +                    val.len(),
> +                )
> +            })?,
> +            8 => to_result(unsafe {
> +                bindings::device_property_read_u64_array(
> +                    self.as_raw(),
> +                    name.as_ptr() as *const i8,
> +                    val.as_ptr() as *mut u64,
> +                    val.len(),
> +                )
> +            })?,
> +            _ => return Err(EINVAL),
> +        }
> +        Ok(())
> +    }
> +
> +    pub fn property_read_array_vec<T>(&self, name: &CStr, len: usize) -> Result<KVec<T>> {
> +        let mut val: KVec<T> = KVec::with_capacity(len, GFP_KERNEL)?;
> +
> +        // SAFETY: len always matches capacity
> +        unsafe { val.set_len(len) }
> +        Self::_property_read_array::<T>(&self, name, val.as_mut_slice())?;
> +        Ok(val)
> +    }
> +
> +    /// Returns array length for firmware property `name`
> +    ///
> +    /// Valid types are i8, u8, i16, u16, i32, u32, i64, u64
> +    pub fn property_count_elem<T>(&self, name: &CStr) -> Result<usize> {

This always returns usize? I'm a bit confused ...

> +        match size_of::<T>() {
> +            1 => {
> +                ret = unsafe {
> +                    bindings::device_property_read_u8_array(
> +                        self.as_raw(),
> +                        name.as_ptr() as *const i8,
> +                        ptr::null_mut(),
> +                        0,
> +                    )
> +                }
> +            }
> +            2 => {
> +                ret = unsafe {
> +                    bindings::device_property_read_u16_array(
> +                        self.as_raw(),
> +                        name.as_ptr() as *const i8,
> +                        ptr::null_mut(),
> +                        0,
> +                    )
> +                }
> +            }
> +            4 => {
> +                ret = unsafe {
> +                    bindings::device_property_read_u32_array(
> +                        self.as_raw(),
> +                        name.as_ptr() as *const i8,
> +                        ptr::null_mut(),
> +                        0,
> +                    )
> +                }
> +            }
> +            8 => {
> +                ret = unsafe {
> +                    bindings::device_property_read_u64_array(
> +                        self.as_raw(),
> +                        name.as_ptr() as *const i8,
> +                        ptr::null_mut(),
> +                        0,
> +                    )
> +                }
> +            }
> +            _ => return Err(EINVAL),

You can use `kernel::build_error!` here to trigger a build failure if
the size is wrong.

Alice





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux