Re: [PATCH v4 2/2] rust: leds: add basic led classdev abstractions

[Alice Ryhl <aliceryhl@google.com>]

On Sun, Oct 12, 2025 at 02:52:39PM +0000, Markus Probst wrote:
> Implement the core abstractions needed for led class devices, including:
> 
> * `led::LedOps` - the trait for handling leds, including
>   `brightness_set`, `brightness_get` and `blink_set`
> 
> * `led::InitData` - data set for the led class device
> 
> * `led::Device` - a safe wrapper around `led_classdev`
> 
> Signed-off-by: Markus Probst <markus.probst@posteo.de>

> +pub trait LedOps: Send + 'static + Sized {
> +    /// If set true, [`LedOps::brightness_set`] and [`LedOps::blink_set`] must not sleep
> +    /// and perform the operation immediately.
> +    const BLOCKING: bool;
> +    /// The max brightness level
> +    const MAX_BRIGHTNESS: u32;
> +
> +    /// Sets the brightness level.
> +    ///
> +    /// See also [`LedOps::BLOCKING`]
> +    fn brightness_set(&self, brightness: u32) -> Result<()>;
> +
> +    /// Gets the current brightness level.
> +    fn brightness_get(&self) -> u32 {
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }
> +
> +    /// Activates hardware accelerated blinking.
> +    ///
> +    /// delays are in milliseconds. If both are zero, a sensible default should be chosen.
> +    /// The caller should adjust the timings in that case and if it can't match the values
> +    /// specified exactly. Setting the brightness to 0 will disable the hardware accelerated
> +    /// blinking.
> +    ///
> +    /// See also [`LedOps::BLOCKING`]
> +    fn blink_set(&self, _delay_on: &mut usize, _delay_off: &mut usize) -> Result<()> {
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }

These functions should probably take a &Device<Bound> argument so that
they can use methods that require a bound device (such as IO).

> +impl<T: LedOps> Device<T> {
> +    /// Registers a new led classdev.
> +    ///
> +    /// The [`Device`] will be unregistered on drop.
> +    pub fn new<'a>(
> +        parent: &'a device::Device<Bound>,
> +        init_data: InitData<'a>,
> +        ops: T,
> +    ) -> impl PinInit<Devres<Self>, Error> + 'a {
> +        Devres::new(
> +            parent,
> +            try_pin_init!(Self {
> +                ops,
> +                classdev <- Opaque::try_ffi_init(|ptr: *mut bindings::led_classdev| {
> +                    // SAFETY: `try_ffi_init` guarantees that `ptr` is valid for write.
> +                    // `led_classdev` gets fully initialized in-place by
> +                    // `led_classdev_register_ext` including `mutex` and `list_head`.
> +                    unsafe { ptr.write(bindings::led_classdev {
> +                        max_brightness: T::MAX_BRIGHTNESS,
> +                        brightness_set: T::BLOCKING
> +                            .then_some(Adapter::<T>::brightness_set_callback),
> +                        brightness_set_blocking: (!T::BLOCKING)
> +                            .then_some(Adapter::<T>::brightness_set_blocking_callback),
> +                        brightness_get: T::HAS_BRIGHTNESS_GET
> +                            .then_some(Adapter::<T>::brightness_get_callback),
> +                        blink_set: T::HAS_BLINK_SET
> +                            .then_some(Adapter::<T>::blink_set_callback),
> +                        .. bindings::led_classdev::default()
> +                    }) };

This doesn't look like something rustfmt would output? Could you run
rustfmt if you haven't already. If it doesn't do anything, then please
format it outside of the macro and move it back in.

Alice