Re: [PATCH v4 6/9] rust: device: Add bindings for reading device properties

[Remo Senekowitsch]

On Mon May 12, 2025 at 3:36 PM CEST, Danilo Krummrich wrote:
>> +/// Implemented for all integers that can be read as properties.
>> +///
>> +/// This helper trait is needed on top of the existing [`Property`]
>> +/// trait to associate the integer types of various sizes with their
>> +/// corresponding `fwnode_property_read_*_array` functions.
>> +pub trait PropertyInt: Copy {
>> +    /// # Safety
>> +    ///
>> +    /// Callers must uphold the same safety invariants as for the various
>> +    /// `fwnode_property_read_*_array` functions.
>
> I think you have additional requirements on the fwnode, propname and val
> pointers as well as on nval, please document them as well.

What are the additional requirements? The implementation just calls the
underlying `fwnode_property_read_*_array` with the exact same arguments,
so I don't know what the additional requirements are.

>> +    unsafe fn read_array_from_fwnode_property(
>> +        fwnode: *const bindings::fwnode_handle,
>> +        propname: *const ffi::c_char,
>> +        val: *mut Self,
>> +        nval: usize,
>> +    ) -> ffi::c_int;
>> +}
>> +// This macro generates implementations of the traits `Property` and
>> +// `PropertyInt` for integers of various sizes. Its input is a list
>> +// of pairs separated by commas. The first element of the pair is the
>> +// type of the integer, the second one is the name of its corresponding
>> +// `fwnode_property_read_*_array` function.
>> +macro_rules! impl_property_for_int {
>> +    ($($int:ty: $f:ident),* $(,)?) => { $(
>> +        impl PropertyInt for $int {
>> +            unsafe fn read_array_from_fwnode_property(
>> +                fwnode: *const bindings::fwnode_handle,
>> +                propname: *const ffi::c_char,
>> +                val: *mut Self,
>> +                nval: usize,
>> +            ) -> ffi::c_int {
>> +                // SAFETY: The safety invariants on the trait require
>> +                // callers to uphold the invariants of the functions
>> +                // this macro is called with.
>> +                unsafe {
>> +                    bindings::$f(fwnode, propname, val.cast(), nval)
>> +                }
>> +            }
>> +        }
>> +    )* };
>> +}
>> +impl_property_for_int! {
>> +    u8: fwnode_property_read_u8_array,
>> +    u16: fwnode_property_read_u16_array,
>> +    u32: fwnode_property_read_u32_array,
>> +    u64: fwnode_property_read_u64_array,
>> +    i8: fwnode_property_read_u8_array,
>> +    i16: fwnode_property_read_u16_array,
>> +    i32: fwnode_property_read_u32_array,
>> +    i64: fwnode_property_read_u64_array,
>> +}