On Tue, Oct 29, 2024 at 6:58 PM Rob Herring [off-list ref] wrote:

On Tue, Oct 29, 2024 at 9:16 AM Alice Ryhl [off-list ref] wrote:

quoted

On Fri, Oct 25, 2024 at 11:06 PM Rob Herring (Arm) [off-list ref] wrote:

quoted

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

The C version returned an int so we could return an errno or positive
count. With Result, we don't need negative values and isn't usize
generally used for counts of things like size_t in C?

Ok, I think I misunderstood what this does. usize is fine.

quoted

quoted

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

I really want a build error if the type is wrong, then the _ case
would be unreachable. No way to do that?

One option is to define a trait for integers:

trait Integer: FromBytes + AsBytes + Copy {
    const SIZE: IntSize;
}

enum IntSize {
    S8,
    S16,
    S32,
    S64,
}

macro_rules! impl_int {
    ($($typ:ty),* $(,)?) => {$(
        impl Integer for $typ {
            const SIZE: IntSize = match size_of::<Self>() {
                1 => IntSize::S8,
                2 => IntSize::S16,
                4 => IntSize::S32,
                8 => IntSize::S64,
                _ => panic!("invalid size"),
            };
        }
    )*};
}

impl_int! {
    u8, u16, u32, u64, usize,
    i8, i16, i32, i64, isize,
}

Using the above trait, you can match on the IntSize.

pub fn property_count_elem<T: Integer>(&self, name: &CStr) -> Result<usize> {
match T::SIZE {
    IntSize::S8 => ...,
    IntSize::S16 => ...,
    IntSize::S32 => ...,
    IntSize::S64 => ...,
}

this leaves no catch-all case and calling it with non-integer types
will not compile.

Alice