Message-ID: <9efcbdf770b41eea9a38f4d10b1c9859b05152a6.camel@posteo.de>
Date: Sat, 15 Nov 2025 20:48:56 +0000
From: Markus Probst <markus.probst@...teo.de>
To: Lee Jones <lee@...nel.org>, Pavel Machek <pavel@...nel.org>, Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>, Dave Ertman
 <david.m.ertman@...el.com>, Ira Weiny	 <ira.weiny@...el.com>, Leon
 Romanovsky <leon@...nel.org>, Miguel Ojeda	 <ojeda@...nel.org>, Alex Gaynor
 <alex.gaynor@...il.com>, Boqun Feng	 <boqun.feng@...il.com>, Gary Guo
 <gary@...yguo.net>,  Björn Roy Baron	
 <bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>, Andreas
 Hindborg	 <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>,
 Trevor Gross	 <tmgross@...ch.edu>, Danilo Krummrich <dakr@...nel.org>,
 "Rafael J. Wysocki"	 <rafael@...nel.org>, Bjorn Helgaas
 <bhelgaas@...gle.com>, Krzysztof Wilczyński	
 <kwilczynski@...nel.org>
Cc: rust-for-linux@...r.kernel.org, linux-leds@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: [PATCH v8 2/4] rust: leds: add basic led classdev abstractions

On Sat, 2025-11-15 at 17:26 +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@...teo.de>
> ---
>  MAINTAINERS        |   7 +
>  rust/kernel/led.rs | 477 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs |   1 +
>  3 files changed, 485 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ddecf1ef3bed..2d66db23a63b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14104,6 +14104,13 @@ F:	drivers/leds/
>  F:	include/dt-bindings/leds/
>  F:	include/linux/leds.h
>  
> +LED SUBSYSTEM [RUST]
> +M:	Markus Probst <markus.probst@...teo.de>
> +L:	linux-leds@...r.kernel.org
> +L:	rust-for-linux@...r.kernel.org
> +S:	Maintained
> +F:	rust/kernel/led.rs
> +
>  LEGO MINDSTORMS EV3
>  R:	David Lechner <david@...hnology.com>
>  S:	Maintained
> diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
> new file mode 100644
> index 000000000000..4c1b72c998e1
> --- /dev/null
> +++ b/rust/kernel/led.rs
> @@ -0,0 +1,477 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Abstractions for the leds driver model.
> +//!
> +//! C header: [`include/linux/leds.h`](srctree/include/linux/leds.h)
> +
> +use core::{
> +    marker::PhantomData,
> +    mem::transmute,
> +    pin::Pin,
> +    ptr::NonNull, //
> +};
> +
> +use pin_init::{
> +    pin_data,
> +    pinned_drop,
> +    PinInit, //
> +};
> +
> +use crate::{
> +    build_error,
> +    container_of,
> +    device::{
> +        self,
> +        property::FwNode,
> +        AsBusDevice,
> +        Bound, //
> +    },
> +    devres::Devres,
> +    error::{
> +        code::EINVAL,
> +        from_result,
> +        to_result,
> +        Error,
> +        Result,
> +        VTABLE_DEFAULT_ERROR, //
> +    },
> +    macros::vtable,
> +    str::CStr,
> +    try_pin_init,
> +    types::{
> +        ARef,
> +        Opaque, //
> +    }, //
> +};
> +
> +/// The led class device representation.
> +///
> +/// This structure represents the Rust abstraction for a C `struct led_classdev`.
> +#[pin_data(PinnedDrop)]
> +pub struct Device<T: LedOps> {
> +    ops: T,
> +    #[pin]
> +    classdev: Opaque<bindings::led_classdev>,
> +}
> +
> +/// The led init data representation.
> +///
> +/// This structure represents the Rust abstraction for a C `struct led_init_data` with additional
> +/// fields from `struct led_classdev`.
> +#[derive(Default)]
> +pub struct InitData<'a> {
> +    fwnode: Option<&'a FwNode>,
> +    default_label: Option<&'a CStr>,
> +    devicename: Option<&'a CStr>,
> +    devname_mandatory: bool,
> +    initial_brightness: u32,
> +    default_trigger: Option<&'a CStr>,
> +    color: Color,
> +}
> +
> +impl InitData<'static> {
> +    /// Creates a new [`InitData`]
> +    pub fn new() -> Self {
> +        Self::default()
> +    }
> +}
> +
> +impl<'a> InitData<'a> {
> +    /// Sets the firmware node
> +    pub fn fwnode(self, fwnode: &'a FwNode) -> Self {
> +        InitData {
> +            fwnode: Some(fwnode),
> +            ..self
> +        }
> +    }
`led::InitData::fwnode()` will accept a `Option<ARef<Fwnode>>` as
parameter instead in the next patch iteration. This is the same type
returned by `Fwnode::get_child_by_name()`.

Thanks
- Markus Probst
> +
> +    /// Sets a default label
> +    pub fn default_label(self, label: &'a CStr) -> Self {
> +        InitData {
> +            default_label: Some(label),
> +            ..self
> +        }
> +    }
> +
> +    /// Sets the device name
> +    pub fn devicename(self, devicename: &'a CStr) -> Self {
> +        Self {
> +            devicename: Some(devicename),
> +            ..self
> +        }
> +    }
> +
> +    /// Sets if a device name is mandatory
> +    pub fn devicename_mandatory(self, mandatory: bool) -> Self {
> +        Self {
> +            devname_mandatory: mandatory,
> +            ..self
> +        }
> +    }
> +
> +    /// Sets the initial brightness value for the led
> +    ///
> +    /// The default brightness is 0.
> +    /// If [`LedOps::brightness_get`] is implemented, this value will be ignored.
> +    pub fn initial_brightness(self, brightness: u32) -> Self {
> +        Self {
> +            initial_brightness: brightness,
> +            ..self
> +        }
> +    }
> +
> +    /// Set the default led trigger
> +    ///
> +    /// This value can be overwritten by the "linux,default-trigger" fwnode property.
> +    pub fn default_trigger(self, trigger: &'a CStr) -> Self {
> +        Self {
> +            default_trigger: Some(trigger),
> +            ..self
> +        }
> +    }
> +
> +    /// Sets the color of the led
> +    ///
> +    /// This value can be overwritten by the "color" fwnode property.
> +    pub fn color(self, color: Color) -> Self {
> +        Self { color, ..self }
> +    }
> +}
> +
> +/// Trait defining the operations for a LED driver.
> +///
> +/// # Examples
> +///
> +///```
> +/// # use kernel::{
> +/// #     c_str, device, devres::Devres,
> +/// #     error::Result, led,
> +/// #     macros::vtable, platform, prelude::*,
> +/// # };
> +/// # use core::pin::Pin;
> +///
> +/// 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(
> +///         &self,
> +///         _dev: &platform::Device<device::Bound>,
> +///         _classdev: &led::Device<Self>,
> +///         _brightness: u32
> +///     ) -> Result<()> {
> +///         // Set the brightness for the led here
> +///         Ok(())
> +///     }
> +/// }
> +///
> +/// fn register_my_led(
> +///     parent: &platform::Device<device::Bound>,
> +/// ) -> Result<Pin<KBox<Devres<led::Device<MyLedOps>>>>> {
> +///     KBox::pin_init(led::Device::new(
> +///         parent,
> +///         led::InitData::new()
> +///             .default_label(c_str!("my_led")),
> +///         MyLedOps,
> +///     ), GFP_KERNEL)
> +/// }
> +///```
> +/// Led drivers must implement this trait in order to register and handle a [`Device`].
> +#[vtable]
> +pub trait LedOps: Send + 'static + Sized {
> +    /// The bus device required by the implementation.
> +    #[allow(private_bounds)]
> +    type Bus: AsBusDevice<Bound>;
> +    /// 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(
> +        &self,
> +        dev: &Self::Bus,
> +        classdev: &Device<Self>,
> +        brightness: u32,
> +    ) -> Result<()>;
> +
> +    /// Gets the current brightness level.
> +    fn brightness_get(&self, _dev: &Self::Bus, _classdev: &Device<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,
> +        _dev: &Self::Bus,
> +        _classdev: &Device<Self>,
> +        _delay_on: &mut usize,
> +        _delay_off: &mut usize,
> +    ) -> Result<()> {
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }
> +}
> +
> +/// Led colors.
> +#[derive(Copy, Clone, Debug, Default)]
> +#[repr(u32)]
> +#[non_exhaustive]
> +#[expect(
> +    missing_docs,
> +    reason = "it shouldn't be necessary to document each color"
> +)]
> +pub enum Color {
> +    #[default]
> +    White = bindings::LED_COLOR_ID_WHITE,
> +    Red = bindings::LED_COLOR_ID_RED,
> +    Green = bindings::LED_COLOR_ID_GREEN,
> +    Blue = bindings::LED_COLOR_ID_BLUE,
> +    Amber = bindings::LED_COLOR_ID_AMBER,
> +    Violet = bindings::LED_COLOR_ID_VIOLET,
> +    Yellow = bindings::LED_COLOR_ID_YELLOW,
> +    Ir = bindings::LED_COLOR_ID_IR,
> +    Multi = bindings::LED_COLOR_ID_MULTI,
> +    Rgb = bindings::LED_COLOR_ID_RGB,
> +    Purple = bindings::LED_COLOR_ID_PURPLE,
> +    Orange = bindings::LED_COLOR_ID_ORANGE,
> +    Pink = bindings::LED_COLOR_ID_PINK,
> +    Cyan = bindings::LED_COLOR_ID_CYAN,
> +    Lime = bindings::LED_COLOR_ID_LIME,
> +}
> +
> +impl TryFrom<u32> for Color {
> +    type Error = Error;
> +
> +    fn try_from(value: u32) -> core::result::Result<Self, Self::Error> {
> +        const _: () = {
> +            assert!(bindings::LED_COLOR_ID_MAX == 15);
> +        };
> +        if value < bindings::LED_COLOR_ID_MAX {
> +            // SAFETY:
> +            // - `Color` is represented as `u32`
> +            // - the const block above guarantees that no additional color has been added
> +            // - `value` is guaranteed to be in the color id range
> +            Ok(unsafe { transmute::<u32, Color>(value) })
> +        } else {
> +            Err(EINVAL)
> +        }
> +    }
> +}
> +
> +// SAFETY: A `led::Device` can be unregistered from any thread.
> +unsafe impl<T: LedOps + Send> Send for Device<T> {}
> +
> +// SAFETY: `led::Device` can be shared among threads because all methods of `led::Device`
> +// are thread safe.
> +unsafe impl<T: LedOps + Sync> Sync for Device<T> {}
> +
> +impl<T: LedOps> Device<T> {
> +    /// Registers a new led classdev.
> +    ///
> +    /// The [`Device`] will be unregistered on drop.
> +    pub fn new<'a>(
> +        parent: &'a T::Bus,
> +        init_data: InitData<'a>,
> +        ops: T,
> +    ) -> impl PinInit<Devres<Self>, Error> + 'a {
> +        Devres::new(
> +            parent.as_ref(),
> +            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 {
> +                            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),
> +                            max_brightness: T::MAX_BRIGHTNESS,
> +                            brightness: init_data.initial_brightness,
> +                            default_trigger: init_data.default_trigger
> +                                .map_or(core::ptr::null(), CStr::as_char_ptr),
> +                            color: init_data.color as u32,
> +                            ..bindings::led_classdev::default()
> +                        })
> +                    };
> +
> +                    let fwnode = init_data.fwnode.map(ARef::from);
> +
> +                    let mut init_data = bindings::led_init_data {
> +                        fwnode: fwnode
> +                            .as_ref()
> +                            .map_or(core::ptr::null_mut(), |fwnode| fwnode.as_raw()),
> +                        default_label: init_data
> +                            .default_label
> +                            .map_or(core::ptr::null(), CStr::as_char_ptr),
> +                        devicename: init_data
> +                            .devicename
> +                            .map_or(core::ptr::null(), CStr::as_char_ptr),
> +                        devname_mandatory: init_data.devname_mandatory,
> +                    };
> +
> +                    // SAFETY:
> +                    // - `parent.as_raw()` is guaranteed to be a pointer to a valid `device`
> +                    //    or a null pointer.
> +                    // - `ptr` is guaranteed to be a pointer to an initialized `led_classdev`.
> +                    to_result(unsafe {
> +                        bindings::led_classdev_register_ext(
> +                            parent.as_ref().as_raw(),
> +                            ptr,
> +                            &mut init_data,
> +                        )
> +                    })?;
> +
> +                    core::mem::forget(fwnode); // keep the reference count incremented
> +
> +                    Ok::<_, Error>(())
> +                }),
> +            }),
> +        )
> +    }
> +
> +    /// # Safety
> +    /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
> +    /// `led::Device`.
> +    unsafe fn from_raw<'a>(led_cdev: *mut bindings::led_classdev) -> &'a Self {
> +        // SAFETY: The function's contract guarantees that `led_cdev` points to a `led_classdev`
> +        // field embedded within a valid `led::Device`. `container_of!` can therefore
> +        // safely calculate the address of the containing struct.
> +        unsafe { &*container_of!(Opaque::cast_from(led_cdev), Self, classdev) }
> +    }
> +
> +    fn parent(&self) -> &device::Device<Bound> {
> +        // SAFETY:
> +        // - `self.classdev.get()` is guaranteed to be a valid pointer to `led_classdev`.
> +        unsafe { device::Device::from_raw((*(*self.classdev.get()).dev).parent) }
> +    }
> +}
> +
> +struct Adapter<T: LedOps> {
> +    _p: PhantomData<T>,
> +}
> +
> +impl<T: LedOps> Adapter<T> {
> +    /// # Safety
> +    /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
> +    /// `led::Device`.
> +    /// This function is called on setting the brightness of a led.
> +    unsafe extern "C" fn brightness_set_callback(
> +        led_cdev: *mut bindings::led_classdev,
> +        brightness: u32,
> +    ) {
> +        // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
> +        // `led_classdev` embedded within a `led::Device`.
> +        let classdev = unsafe { Device::<T>::from_raw(led_cdev) };
> +        // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
> +        let parent = unsafe { T::Bus::from_device(classdev.parent()) };
> +
> +        let _ = classdev.ops.brightness_set(parent, classdev, brightness);
> +    }
> +
> +    /// # Safety
> +    /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
> +    /// `led::Device`.
> +    /// This function is called on setting the brightness of a led immediately.
> +    unsafe extern "C" fn brightness_set_blocking_callback(
> +        led_cdev: *mut bindings::led_classdev,
> +        brightness: u32,
> +    ) -> i32 {
> +        from_result(|| {
> +            // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
> +            // `led_classdev` embedded within a `led::Device`.
> +            let classdev = unsafe { Device::<T>::from_raw(led_cdev) };
> +            // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
> +            let parent = unsafe { T::Bus::from_device(classdev.parent()) };
> +
> +            classdev.ops.brightness_set(parent, classdev, brightness)?;
> +            Ok(0)
> +        })
> +    }
> +
> +    /// # Safety
> +    /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
> +    /// `led::Device`.
> +    /// This function is called on getting the brightness of a led.
> +    unsafe extern "C" fn brightness_get_callback(led_cdev: *mut bindings::led_classdev) -> u32 {
> +        // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
> +        // `led_classdev` embedded within a `led::Device`.
> +        let classdev = unsafe { Device::<T>::from_raw(led_cdev) };
> +        // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
> +        let parent = unsafe { T::Bus::from_device(classdev.parent()) };
> +
> +        classdev.ops.brightness_get(parent, classdev)
> +    }
> +
> +    /// # Safety
> +    /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
> +    /// `led::Device`.
> +    /// `delay_on` and `delay_off` must be valid pointers to `usize` and have
> +    /// exclusive access for the period of this function.
> +    /// This function is called on enabling hardware accelerated blinking.
> +    unsafe extern "C" fn blink_set_callback(
> +        led_cdev: *mut bindings::led_classdev,
> +        delay_on: *mut usize,
> +        delay_off: *mut usize,
> +    ) -> i32 {
> +        from_result(|| {
> +            // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
> +            // `led_classdev` embedded within a `led::Device`.
> +            let classdev = unsafe { Device::<T>::from_raw(led_cdev) };
> +            // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
> +            let parent = unsafe { T::Bus::from_device(classdev.parent()) };
> +
> +            classdev.ops.blink_set(
> +                parent,
> +                classdev,
> +                // SAFETY: The function's contract guarantees that `delay_on` points to a `usize`
> +                // and is exclusive for the period of this function.
> +                unsafe { &mut *delay_on },
> +                // SAFETY: The function's contract guarantees that `delay_off` points to a `usize`
> +                // and is exclusive for the period of this function.
> +                unsafe { &mut *delay_off },
> +            )?;
> +            Ok(0)
> +        })
> +    }
> +}
> +
> +#[pinned_drop]
> +impl<T: LedOps> PinnedDrop for Device<T> {
> +    fn drop(self: Pin<&mut Self>) {
> +        let raw = self.classdev.get();
> +        // SAFETY: The existence of `self` guarantees that `self.classdev.get()` is a pointer to a
> +        // valid `struct led_classdev`.
> +        let dev: &device::Device = unsafe { device::Device::from_raw((*raw).dev) };
> +
> +        let _fwnode = dev
> +            .fwnode()
> +            // SAFETY: the reference count of `fwnode` has previously been
> +            // incremented in `led::Device::new`.
> +            .map(|fwnode| unsafe { ARef::from_raw(NonNull::from(fwnode)) });
> +
> +        // SAFETY: The existence of `self` guarantees that `self.classdev` has previously been
> +        // successfully registered with `led_classdev_register_ext`.
> +        unsafe { bindings::led_classdev_unregister(self.classdev.get()) };
> +    }
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 3dd7bebe7888..c03b7b242a2b 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -103,6 +103,7 @@
>  pub mod jump_label;
>  #[cfg(CONFIG_KUNIT)]
>  pub mod kunit;
> +pub mod led;
>  pub mod list;
>  pub mod maple_tree;
>  pub mod miscdevice;

Powered by blists - more mailing lists