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

["Danilo Krummrich" <dakr@kernel.org>]

On Mon Oct 13, 2025 at 8:34 PM CEST, Alice Ryhl wrote:
> 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).

Indeed!

@Markus: Note that this guarantee is given by the LED device registration being
lifetime controlled by devres, while led_classdev_unregister() is synchronized
against those callbacks.

For the latter, please double check that this is actually the case -- I'm not
familiar with the LED subsystem, I'm reviewing from driver-core perspective. But
from a quick look it should be the case. :)