Re: [PATCH v24 1/4] rust: leds: add basic led classdev abstractions

[Gary Guo]

On Thu Sep 3, 2026 at 12:01 AM BST, 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::DeviceBuilder` - the builder for the led class device
>
> * `led::Device` - a safe wrapper around `led_classdev`
>
> Signed-off-by: Markus Probst <markus.probst@xxxxxxxxx>
> ---
>  rust/kernel/led.rs        | 288 ++++++++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/led/normal.rs | 230 ++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs        |   1 +
>  3 files changed, 519 insertions(+)
>
> [snip]
>
> +/// Trait defining the operations for a LED driver.
> +///
> +/// # Examples
> +/// ```
> +/// use kernel::{
> +///      device,
> +///      devres::Devres,
> +///      led,
> +///      macros::vtable,
> +///      platform,
> +///      prelude::*, //
> +///  };
> +///
> +/// struct MyLedOps;
> +///
> +///
> +/// #[vtable]
> +/// impl led::LedOps for MyLedOps {
> +///     type Bus = platform::Device<device::Bound>;
> +///     const BLOCKING: bool = false;
> +///     const MAX_BRIGHTNESS: u32 = 255;
> +///
> +///     fn brightness_set<'bound>(
> +///         &self,
> +///         _dev: &'bound platform::Device<device::Bound>,
> +///         _classdev: &led::Device<'bound, Self>,
> +///         _brightness: u32
> +///     ) -> Result<()> {
> +///         // Set the brightness for the led here
> +///         Ok(())
> +///     }
> +/// }
> +/// ```
> +/// Led drivers must implement this trait in order to register and handle a [`Device`].
> +#[vtable]
> +pub trait LedOps: Send + Sync + Sized {
> +    /// The bus device required by the implementation.
> +    #[allow(private_bounds)]
> +    type Bus: AsBusDevice<Bound>;

Does LED class device has no private data that driver can use? This can be
either a private pointer or extra allocation living at the end of the classdev
struct.

It's usually a antipattern to get the bus device directly, especially that in
Rust we do not allow anything other than callbacks to access data on bus
devices.

Instead, the class device registration should provide a data initializer, and
the callbacks would receive a pointer to the data instead. In cases that a
device resource has to be referenced, it should be kept inside the private data
by the driver themselves.

Best,
Gary

> +
> +    /// If set true, [`LedOps::brightness_set`] and [`LedOps::blink_set`] must perform the
> +    /// operation immediately. If set false, they must not sleep.
> +    const BLOCKING: bool;
> +    /// The max brightness level.
> +    const MAX_BRIGHTNESS: u32;
> +
> +    /// Sets the brightness level.
> +    ///
> +    /// See also [`LedOps::BLOCKING`].
> +    fn brightness_set<'bound>(
> +        &self,
> +        dev: &'bound Self::Bus,
> +        classdev: &Device<'bound, Self>,
> +        brightness: u32,
> +    ) -> Result<()>;
> +
> +    /// Gets the current brightness level.
> +    fn brightness_get<'bound>(
> +        &self,
> +        dev: &'bound Self::Bus,
> +        classdev: &Device<'bound, Self>,
> +    ) -> Result<u32> {
> +        let _ = (dev, classdev);
> +        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<'bound>(
> +        &self,
> +        dev: &'bound Self::Bus,
> +        classdev: &Device<'bound, Self>,
> +        delay_on: &mut usize,
> +        delay_off: &mut usize,
> +    ) -> Result<()> {
> +        let _ = (dev, classdev, delay_on, delay_off);
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }
> +}