public inbox for linux-leds@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v11 0/3] rust: leds: add led classdev abstractions
@ 2026-02-02 13:52 Markus Probst
  2026-02-02 13:52 ` [PATCH v11 1/3] rust: leds: add basic " Markus Probst
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Markus Probst @ 2026-02-02 13:52 UTC (permalink / raw)
  To: Lee Jones, Pavel Machek, Greg Kroah-Hartman, Dave Ertman,
	Ira Weiny, Leon Romanovsky, Miguel Ojeda, Alex Gaynor, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, Rafael J. Wysocki,
	Bjorn Helgaas, Krzysztof Wilczyński
  Cc: rust-for-linux, linux-leds, linux-kernel, linux-pci,
	Markus Probst

This patch series has previously been contained in
https://lore.kernel.org/rust-for-linux/20251008181027.662616-1-markus.probst@posteo.de/T/#t
which added a rust written led driver for a microcontroller via i2c.

As the reading and writing to the i2c client via the register!
macro has not been implemented yet [1], the patch series will only
contain the additional abstractions required.

[1] https://lore.kernel.org/rust-for-linux/DDDS2V0V2NVJ.16ZKXCKUA1HUV@kernel.org/

The following changes were made:
* add basic led classdev abstractions to register and unregister leds

* add basic led classdev abstractions to register and unregister
  multicolor leds

Changes since v10:
* allow in-place initialization of `LedOps`
* run rustfmt for code inside `try_pin_init!`

Changes since v9:
* add missing periods in documentation
* duplicate `led::Device` and `led::Adapter` instead of using a complex
  trait
* fix imports not using prelude
* adapt to CStr change
* documented `led::Color::Multi` and `led::Color::Rgb`

Changes since v8:
* accept `Option<ARef<Fwnode>>` in `led::InitData::fwnode()`
* make functions in `MultiColorSubLed` const
* drop the "rust: Add trait to convert a device reference to a bus
  device reference" patch, as it has been picked into driver-core

Changes since v7:
* adjusted import style
* added classdev parameter to callback functions in `LedOps`
* implement `led::Color`
* extend `led::InitData` with
  - initial_brightness
  - default_trigger
  - default_color
* split generic and normal led classdev abstractions up (see patch 3/4)
* add multicolor led class device abstractions (see patch 4/4)
* added MAINTAINERS entry

Changes since v6:
* fixed typos
* improved documentation

Changes since v5:
* rename `IntoBusDevice` trait into `AsBusDevice`
* fix documentation about `LedOps::BLOCKING`
* removed dependency on i2c bindings
* added `AsBusDevice` implementation for `platform::Device`
* removed `device::Device` fallback implementation
* document that `AsBusDevice` must not be used by drivers and is
  intended for bus and class device abstractions only.

Changes since v4:
* add abstraction to convert a device reference to a bus device
  reference
* require the bus device as parent device and provide it in class device
  callbacks
* remove Pin<Vec<_>> abstraction (as not relevant for the led
  abstractions)
* fixed formatting in `led::Device::new`
* fixed `LedOps::BLOCKING` did the inverse effect

Changes since v3:
* fixed kunit tests failing because of example in documentation

Changes since v2:
* return `Devres` on `led::Device` creation
* replace KBox<T> with T in struct definition
* increment and decrement reference-count of fwnode
* make a device parent mandatory for led classdev creation
* rename `led::Handler` to `led::LedOps`
* add optional `brightness_get` function to `led::LedOps`
* use `#[vtable]` instead of `const BLINK: bool`
* use `Opaque::cast_from` instead of casting a pointer
* improve documentation
* improve support for older rust versions
* use `&Device<Bound>` for parent

Changes since v1:
* fixed typos noticed by Onur Özkan

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
Markus Probst (3):
      rust: leds: add basic led classdev abstractions
      rust: leds: split generic and normal led classdev abstractions up
      rust: leds: add multicolor classdev abstractions

 MAINTAINERS                     |   8 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/led.rs              | 300 +++++++++++++++++++++++++++++++
 rust/kernel/led/multicolor.rs   | 382 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/led/normal.rs       | 226 ++++++++++++++++++++++++
 rust/kernel/lib.rs              |   1 +
 6 files changed, 918 insertions(+)
---
base-commit: 18f7fcd5e69a04df57b563360b88be72471d6b62
change-id: 20251114-rust_leds-a959f7c2f7f9


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v11 1/3] rust: leds: add basic led classdev abstractions
  2026-02-02 13:52 [PATCH v11 0/3] rust: leds: add led classdev abstractions Markus Probst
@ 2026-02-02 13:52 ` Markus Probst
  2026-02-02 15:41   ` Gary Guo
  2026-02-02 13:52 ` [PATCH v11 2/3] rust: leds: split generic and normal led classdev abstractions up Markus Probst
  2026-02-02 13:52 ` [PATCH v11 3/3] rust: leds: add multicolor classdev abstractions Markus Probst
  2 siblings, 1 reply; 7+ messages in thread
From: Markus Probst @ 2026-02-02 13:52 UTC (permalink / raw)
  To: Lee Jones, Pavel Machek, Greg Kroah-Hartman, Dave Ertman,
	Ira Weiny, Leon Romanovsky, Miguel Ojeda, Alex Gaynor, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, Rafael J. Wysocki,
	Bjorn Helgaas, Krzysztof Wilczyński
  Cc: rust-for-linux, linux-leds, linux-kernel, linux-pci,
	Markus Probst

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>
---
 MAINTAINERS        |   7 +
 rust/kernel/led.rs | 453 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs |   1 +
 3 files changed, 461 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0efa8cc6775b..26765fecb9a9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14279,6 +14279,13 @@ F:	drivers/leds/
 F:	include/dt-bindings/leds/
 F:	include/linux/leds.h
 
+LED SUBSYSTEM [RUST]
+M:	Markus Probst <markus.probst@posteo.de>
+L:	linux-leds@vger.kernel.org
+L:	rust-for-linux@vger.kernel.org
+S:	Maintained
+F:	rust/kernel/led.rs
+
 LEGO MINDSTORMS EV3
 R:	David Lechner <david@lechnology.com>
 S:	Maintained
diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
new file mode 100644
index 000000000000..9acb6946f3da
--- /dev/null
+++ b/rust/kernel/led.rs
@@ -0,0 +1,453 @@
+// 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,
+    ptr::NonNull, //
+};
+
+use crate::{
+    container_of,
+    device::{
+        self,
+        property::FwNode,
+        AsBusDevice,
+        Bound, //
+    },
+    devres::Devres,
+    error::{
+        from_result,
+        to_result,
+        VTABLE_DEFAULT_ERROR, //
+    },
+    macros::vtable,
+    prelude::*,
+    str::CStrExt,
+    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> {
+    #[pin]
+    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<ARef<FwNode>>,
+    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: Option<ARef<FwNode>>) -> Self {
+        Self { fwnode, ..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::{
+///      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(
+///         &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(),
+///         Ok(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: impl PinInit<T, Error> + 'a,
+    ) -> impl PinInit<Devres<Self>, Error> + 'a {
+        Devres::new(
+            parent.as_ref(),
+            try_pin_init!(Self {
+                ops <- 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(), CStrExt::as_char_ptr),
+                            color: init_data.color as u32,
+                            ..bindings::led_classdev::default()
+                        })
+                    };
+
+                    let mut init_data_raw = bindings::led_init_data {
+                        fwnode: init_data
+                            .fwnode
+                            .as_ref()
+                            .map_or(core::ptr::null_mut(), |fwnode| fwnode.as_raw()),
+                        default_label: core::ptr::null(),
+                        devicename: init_data
+                            .devicename
+                            .map_or(core::ptr::null(), CStrExt::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,
+                            &raw mut init_data_raw,
+                        )
+                    })?;
+
+                    core::mem::forget(init_data.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 f812cf120042..231818e0614b 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -108,6 +108,7 @@
 pub mod jump_label;
 #[cfg(CONFIG_KUNIT)]
 pub mod kunit;
+pub mod led;
 pub mod list;
 pub mod maple_tree;
 pub mod miscdevice;

-- 
2.52.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v11 2/3] rust: leds: split generic and normal led classdev abstractions up
  2026-02-02 13:52 [PATCH v11 0/3] rust: leds: add led classdev abstractions Markus Probst
  2026-02-02 13:52 ` [PATCH v11 1/3] rust: leds: add basic " Markus Probst
@ 2026-02-02 13:52 ` Markus Probst
  2026-02-02 15:43   ` Gary Guo
  2026-02-02 13:52 ` [PATCH v11 3/3] rust: leds: add multicolor classdev abstractions Markus Probst
  2 siblings, 1 reply; 7+ messages in thread
From: Markus Probst @ 2026-02-02 13:52 UTC (permalink / raw)
  To: Lee Jones, Pavel Machek, Greg Kroah-Hartman, Dave Ertman,
	Ira Weiny, Leon Romanovsky, Miguel Ojeda, Alex Gaynor, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, Rafael J. Wysocki,
	Bjorn Helgaas, Krzysztof Wilczyński
  Cc: rust-for-linux, linux-leds, linux-kernel, linux-pci,
	Markus Probst

Move code specific to normal led class devices into a separate file and
introduce the `led::Mode` trait to allow for other types of led class
devices in `led::LedOps`.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 MAINTAINERS               |   1 +
 rust/kernel/led.rs        | 245 ++++++----------------------------------------
 rust/kernel/led/normal.rs | 226 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 259 insertions(+), 213 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 26765fecb9a9..5f8b59678785 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14285,6 +14285,7 @@ L:	linux-leds@vger.kernel.org
 L:	rust-for-linux@vger.kernel.org
 S:	Maintained
 F:	rust/kernel/led.rs
+F:	rust/kernel/led/
 
 LEGO MINDSTORMS EV3
 R:	David Lechner <david@lechnology.com>
diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
index 9acb6946f3da..a32473d61c61 100644
--- a/rust/kernel/led.rs
+++ b/rust/kernel/led.rs
@@ -33,16 +33,9 @@
     }, //
 };
 
-/// 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> {
-    #[pin]
-    ops: T,
-    #[pin]
-    classdev: Opaque<bindings::led_classdev>,
-}
+mod normal;
+
+pub use normal::{Device, Normal};
 
 /// The led init data representation.
 ///
@@ -135,6 +128,7 @@ pub fn color(self, color: Color) -> Self {
 /// #[vtable]
 /// impl led::LedOps for MyLedOps {
 ///     type Bus = platform::Device<device::Bound>;
+///     type Mode = led::Normal;
 ///     const BLOCKING: bool = false;
 ///     const MAX_BRIGHTNESS: u32 = 255;
 ///
@@ -165,6 +159,12 @@ pub trait LedOps: Send + 'static + Sized {
     /// The bus device required by the implementation.
     #[allow(private_bounds)]
     type Bus: AsBusDevice<Bound>;
+
+    /// The led mode to use.
+    ///
+    /// See [`Mode`].
+    type Mode: Mode;
+
     /// 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;
@@ -177,12 +177,17 @@ pub trait LedOps: Send + 'static + Sized {
     fn brightness_set(
         &self,
         dev: &Self::Bus,
-        classdev: &Device<Self>,
+        classdev: &<Self::Mode as Mode>::Device<Self>,
         brightness: u32,
     ) -> Result<()>;
 
     /// Gets the current brightness level.
-    fn brightness_get(&self, _dev: &Self::Bus, _classdev: &Device<Self>) -> u32 {
+    fn brightness_get(
+        &self,
+        dev: &Self::Bus,
+        classdev: &<Self::Mode as Mode>::Device<Self>,
+    ) -> u32 {
+        let _ = (dev, classdev);
         build_error!(VTABLE_DEFAULT_ERROR)
     }
 
@@ -196,11 +201,12 @@ fn brightness_get(&self, _dev: &Self::Bus, _classdev: &Device<Self>) -> u32 {
     /// See also [`LedOps::BLOCKING`].
     fn blink_set(
         &self,
-        _dev: &Self::Bus,
-        _classdev: &Device<Self>,
-        _delay_on: &mut usize,
-        _delay_off: &mut usize,
+        dev: &Self::Bus,
+        classdev: &<Self::Mode as Mode>::Device<Self>,
+        delay_on: &mut usize,
+        delay_off: &mut usize,
     ) -> Result<()> {
+        let _ = (dev, classdev, delay_on, delay_off);
         build_error!(VTABLE_DEFAULT_ERROR)
     }
 }
@@ -251,203 +257,16 @@ fn try_from(value: u32) -> core::result::Result<Self, Self::Error> {
     }
 }
 
-// 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: impl PinInit<T, Error> + 'a,
-    ) -> impl PinInit<Devres<Self>, Error> + 'a {
-        Devres::new(
-            parent.as_ref(),
-            try_pin_init!(Self {
-                ops <- 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(), CStrExt::as_char_ptr),
-                            color: init_data.color as u32,
-                            ..bindings::led_classdev::default()
-                        })
-                    };
-
-                    let mut init_data_raw = bindings::led_init_data {
-                        fwnode: init_data
-                            .fwnode
-                            .as_ref()
-                            .map_or(core::ptr::null_mut(), |fwnode| fwnode.as_raw()),
-                        default_label: core::ptr::null(),
-                        devicename: init_data
-                            .devicename
-                            .map_or(core::ptr::null(), CStrExt::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,
-                            &raw mut init_data_raw,
-                        )
-                    })?;
-
-                    core::mem::forget(init_data.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)
-        })
-    }
+/// The led mode.
+///
+/// Each led mode has its own led class device type with different capabilities.
+///
+/// See [`Normal`].
+pub trait Mode: private::Sealed {
+    /// The class device for the led mode.
+    type Device<T: LedOps<Mode = Self>>;
 }
 
-#[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()) };
-    }
+mod private {
+    pub trait Sealed {}
 }
diff --git a/rust/kernel/led/normal.rs b/rust/kernel/led/normal.rs
new file mode 100644
index 000000000000..1a17bb2a6d97
--- /dev/null
+++ b/rust/kernel/led/normal.rs
@@ -0,0 +1,226 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Led mode for the `struct led_classdev`.
+//!
+//! C header: [`include/linux/leds.h`](srctree/include/linux/leds.h)
+
+use super::*;
+
+/// The led mode for the `struct led_classdev`. Leds with this mode can only have a fixed color.
+pub enum Normal {}
+
+impl Mode for Normal {
+    type Device<T: LedOps<Mode = Self>> = Device<T>;
+}
+impl private::Sealed for Normal {}
+
+/// The led class device representation.
+///
+/// This structure represents the Rust abstraction for a led class device.
+#[pin_data(PinnedDrop)]
+pub struct Device<T: LedOps<Mode = Normal>> {
+    #[pin]
+    ops: T,
+    #[pin]
+    classdev: Opaque<bindings::led_classdev>,
+}
+
+impl<T: LedOps<Mode = Normal>> 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: impl PinInit<T, Error> + 'a,
+    ) -> impl PinInit<Devres<Self>, Error> + 'a {
+        Devres::new(
+            parent.as_ref(),
+            try_pin_init!(Self {
+                ops <- 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(), CStrExt::as_char_ptr),
+                            color: init_data.color as u32,
+                            ..bindings::led_classdev::default()
+                        })
+                    };
+
+                    let mut init_data_raw = bindings::led_init_data {
+                        fwnode: init_data
+                            .fwnode
+                            .as_ref()
+                            .map_or(core::ptr::null_mut(), |fwnode| fwnode.as_raw()),
+                        default_label: core::ptr::null(),
+                        devicename: init_data
+                            .devicename
+                            .map_or(core::ptr::null(), CStrExt::as_char_ptr),
+                        devname_mandatory: init_data.devname_mandatory,
+                    };
+
+                    // SAFETY:
+                    // - `parent.as_ref().as_raw()` is guaranteed to be a pointer to a valid
+                    //    `device`.
+                    // - `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,
+                            &raw mut init_data_raw,
+                        )
+                    })?;
+
+                    core::mem::forget(init_data.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) }
+    }
+}
+
+// SAFETY: A `led::Device` can be unregistered from any thread.
+unsafe impl<T: LedOps<Mode = Normal> + 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<Mode = Normal> + Sync> Sync for Device<T> {}
+
+struct Adapter<T: LedOps<Mode = Normal>> {
+    _p: PhantomData<T>,
+}
+
+impl<T: LedOps<Mode = Normal>> 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<Mode = Normal>> 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 `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(raw) };
+    }
+}

-- 
2.52.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v11 3/3] rust: leds: add multicolor classdev abstractions
  2026-02-02 13:52 [PATCH v11 0/3] rust: leds: add led classdev abstractions Markus Probst
  2026-02-02 13:52 ` [PATCH v11 1/3] rust: leds: add basic " Markus Probst
  2026-02-02 13:52 ` [PATCH v11 2/3] rust: leds: split generic and normal led classdev abstractions up Markus Probst
@ 2026-02-02 13:52 ` Markus Probst
  2 siblings, 0 replies; 7+ messages in thread
From: Markus Probst @ 2026-02-02 13:52 UTC (permalink / raw)
  To: Lee Jones, Pavel Machek, Greg Kroah-Hartman, Dave Ertman,
	Ira Weiny, Leon Romanovsky, Miguel Ojeda, Alex Gaynor, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
	Alice Ryhl, Trevor Gross, Danilo Krummrich, Rafael J. Wysocki,
	Bjorn Helgaas, Krzysztof Wilczyński
  Cc: rust-for-linux, linux-leds, linux-kernel, linux-pci,
	Markus Probst

Implement the abstractions needed for multicolor led class devices,
including:

* `led::MultiColor` - the led mode implementation

* `MultiColorSubLed` - a safe wrapper arround `mc_subled`

* `led::MultiColorDevice` - a safe wrapper around `led_classdev_mc`

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/led.rs              |  30 +++-
 rust/kernel/led/multicolor.rs   | 382 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 412 insertions(+), 1 deletion(-)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index a067038b4b42..765a4198b85d 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -62,6 +62,7 @@
 #include <linux/ioport.h>
 #include <linux/jiffies.h>
 #include <linux/jump_label.h>
+#include <linux/led-class-multicolor.h>
 #include <linux/mdio.h>
 #include <linux/mm.h>
 #include <linux/miscdevice.h>
diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
index a32473d61c61..464e100ab3db 100644
--- a/rust/kernel/led.rs
+++ b/rust/kernel/led.rs
@@ -33,8 +33,12 @@
     }, //
 };
 
+#[cfg(CONFIG_LEDS_CLASS_MULTICOLOR)]
+mod multicolor;
 mod normal;
 
+#[cfg(CONFIG_LEDS_CLASS_MULTICOLOR)]
+pub use multicolor::{MultiColor, MultiColorDevice, MultiColorSubLed};
 pub use normal::{Device, Normal};
 
 /// The led init data representation.
@@ -229,7 +233,24 @@ pub enum Color {
     Violet = bindings::LED_COLOR_ID_VIOLET,
     Yellow = bindings::LED_COLOR_ID_YELLOW,
     Ir = bindings::LED_COLOR_ID_IR,
+    #[cfg_attr(
+        CONFIG_LEDS_CLASS_MULTICOLOR,
+        doc = "Use this color for a [`MultiColor`] led."
+    )]
+    #[cfg_attr(
+        not(CONFIG_LEDS_CLASS_MULTICOLOR),
+        doc = "Use this color for a `MultiColor` led."
+    )]
+    /// If the led supports RGB, use [`Color::Rgb`] instead.
     Multi = bindings::LED_COLOR_ID_MULTI,
+    #[cfg_attr(
+        CONFIG_LEDS_CLASS_MULTICOLOR,
+        doc = "Use this color for a [`MultiColor`] led with rgb support."
+    )]
+    #[cfg_attr(
+        not(CONFIG_LEDS_CLASS_MULTICOLOR),
+        doc = "Use this color for a `MultiColor` led with rgb support."
+    )]
     Rgb = bindings::LED_COLOR_ID_RGB,
     Purple = bindings::LED_COLOR_ID_PURPLE,
     Orange = bindings::LED_COLOR_ID_ORANGE,
@@ -261,7 +282,14 @@ fn try_from(value: u32) -> core::result::Result<Self, Self::Error> {
 ///
 /// Each led mode has its own led class device type with different capabilities.
 ///
-/// See [`Normal`].
+#[cfg_attr(
+    CONFIG_LEDS_CLASS_MULTICOLOR,
+    doc = "See [`Normal`] and [`MultiColor`]."
+)]
+#[cfg_attr(
+    not(CONFIG_LEDS_CLASS_MULTICOLOR),
+    doc = "See [`Normal`] and `MultiColor`."
+)]
 pub trait Mode: private::Sealed {
     /// The class device for the led mode.
     type Device<T: LedOps<Mode = Self>>;
diff --git a/rust/kernel/led/multicolor.rs b/rust/kernel/led/multicolor.rs
new file mode 100644
index 000000000000..7bd143d0646e
--- /dev/null
+++ b/rust/kernel/led/multicolor.rs
@@ -0,0 +1,382 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Led mode for the `struct led_classdev_mc`.
+//!
+//! C header: [`include/linux/led-class-multicolor.h`](srctree/include/linux/led-class-multicolor.h)
+
+use crate::alloc::KVec;
+
+use super::*;
+
+/// The led mode for the `struct led_classdev_mc`. Leds with this mode can have multiple colors.
+pub enum MultiColor {}
+impl Mode for MultiColor {
+    type Device<T: LedOps<Mode = Self>> = MultiColorDevice<T>;
+}
+impl private::Sealed for MultiColor {}
+
+/// The multicolor sub led info representation.
+///
+/// This structure represents the Rust abstraction for a C `struct mc_subled`.
+#[repr(C)]
+#[derive(Copy, Clone, Debug)]
+#[non_exhaustive]
+pub struct MultiColorSubLed {
+    /// the color of the sub led
+    pub color: Color,
+    /// the brightness of the sub led.
+    ///
+    /// The value will be automatically calculated.
+    /// See `MultiColor::pre_brightness_set`.
+    pub brightness: u32,
+    /// the intensity of the sub led.
+    pub intensity: u32,
+    /// arbitrary data for the driver to store.
+    pub channel: u32,
+}
+
+// We directly pass a reference to the `subled_info` field in `led_classdev_mc` to the driver via
+// `Device::subleds()`.
+// We need safeguards to ensure `MultiColorSubLed` and `mc_subled` stay identical.
+const _: () = {
+    use core::mem::offset_of;
+
+    const fn assert_same_type<T>(_: &T, _: &T) {}
+
+    let rust_zeroed = MultiColorSubLed {
+        color: Color::White,
+        brightness: 0,
+        intensity: 0,
+        channel: 0,
+    };
+    let c_zeroed = bindings::mc_subled {
+        color_index: 0,
+        brightness: 0,
+        intensity: 0,
+        channel: 0,
+    };
+
+    assert!(offset_of!(MultiColorSubLed, color) == offset_of!(bindings::mc_subled, color_index));
+    assert_same_type(&0u32, &c_zeroed.color_index);
+
+    assert!(
+        offset_of!(MultiColorSubLed, brightness) == offset_of!(bindings::mc_subled, brightness)
+    );
+    assert_same_type(&rust_zeroed.brightness, &c_zeroed.brightness);
+
+    assert!(offset_of!(MultiColorSubLed, intensity) == offset_of!(bindings::mc_subled, intensity));
+    assert_same_type(&rust_zeroed.intensity, &c_zeroed.intensity);
+
+    assert!(offset_of!(MultiColorSubLed, channel) == offset_of!(bindings::mc_subled, channel));
+    assert_same_type(&rust_zeroed.channel, &c_zeroed.channel);
+
+    assert!(size_of::<MultiColorSubLed>() == size_of::<bindings::mc_subled>());
+};
+
+impl MultiColorSubLed {
+    /// Create a new multicolor sub led info.
+    pub const fn new(color: Color) -> Self {
+        Self {
+            color,
+            brightness: 0,
+            intensity: 0,
+            channel: 0,
+        }
+    }
+
+    /// Set arbitrary data for the driver.
+    pub const fn channel(mut self, channel: u32) -> Self {
+        self.channel = channel;
+        self
+    }
+
+    /// Set the initial intensity of the subled.
+    pub const fn initial_intensity(mut self, intensity: u32) -> Self {
+        self.intensity = intensity;
+        self
+    }
+}
+
+/// The multicolor led class device representation.
+///
+/// This structure represents the Rust abstraction for a multicolor led class device.
+#[pin_data(PinnedDrop)]
+pub struct MultiColorDevice<T: LedOps<Mode = MultiColor>> {
+    #[pin]
+    ops: T,
+    #[pin]
+    classdev: Opaque<bindings::led_classdev_mc>,
+}
+
+impl<T: LedOps<Mode = MultiColor>> MultiColorDevice<T> {
+    /// Registers a new led classdev.
+    ///
+    /// The [`MultiColorDevice`] will be unregistered on drop.
+    pub fn new<'a>(
+        parent: &'a T::Bus,
+        init_data: InitData<'a>,
+        ops: impl PinInit<T, Error> + 'a,
+        subleds: &'a [MultiColorSubLed],
+    ) -> impl PinInit<Devres<Self>, Error> + 'a {
+        Devres::new(
+            parent.as_ref(),
+            try_pin_init!(Self {
+                ops <- ops,
+                classdev <- Opaque::try_ffi_init(|ptr: *mut bindings::led_classdev_mc| {
+                    let mut used = 0;
+                    if subleds.iter().any(|subled| {
+                        let bit = 1 << (subled.color as u32);
+                        if (used & bit) != 0 {
+                            true
+                        } else {
+                            used |= bit;
+                            false
+                        }
+                    }) {
+                        dev_err!(parent.as_ref(), "duplicate color in multicolor led\n");
+                        return Err(EINVAL);
+                    }
+                    let mut subleds_vec = KVec::new();
+                    subleds_vec.extend_from_slice(subleds, GFP_KERNEL)?;
+                    let (subled_info, num_colors, capacity) = subleds_vec.into_raw_parts();
+                    debug_assert_eq!(num_colors, capacity);
+
+                    // SAFETY: `try_ffi_init` guarantees that `ptr` is valid for write.
+                    // `led_classdev_mc` gets fully initialized in-place by
+                    // `led_classdev_multicolor_register_ext` including `mutex` and `list_head`.
+                    unsafe {
+                        ptr.write(bindings::led_classdev_mc {
+                            led_cdev: 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(), CStrExt::as_char_ptr),
+                                color: init_data.color as u32,
+                                ..bindings::led_classdev::default()
+                            },
+                            num_colors: u32::try_from(num_colors)?,
+                            // CAST: The safeguards in the const block ensure that
+                            // `MultiColorSubLed` has an identical layout to `mc_subled`.
+                            subled_info: subled_info.cast::<bindings::mc_subled>(),
+                        })
+                    };
+
+                    let mut init_data_raw = bindings::led_init_data {
+                        fwnode: init_data
+                            .fwnode
+                            .as_ref()
+                            .map_or(core::ptr::null_mut(), |fwnode| fwnode.as_raw()),
+                        default_label: core::ptr::null(),
+                        devicename: init_data
+                            .devicename
+                            .map_or(core::ptr::null(), CStrExt::as_char_ptr),
+                        devname_mandatory: init_data.devname_mandatory,
+                    };
+
+                    // SAFETY:
+                    // - `parent.as_ref().as_raw()` is guaranteed to be a pointer to a valid
+                    //    `device`.
+                    // - `ptr` is guaranteed to be a pointer to an initialized `led_classdev_mc`.
+                    to_result(unsafe {
+                        bindings::led_classdev_multicolor_register_ext(
+                            parent.as_ref().as_raw(),
+                            ptr,
+                            &raw mut init_data_raw,
+                        )
+                    })
+                    .inspect_err(|_err| {
+                        // SAFETY: `subled_info` is guaranteed to be a valid array pointer to
+                        // `mc_subled` with the length and capacity of `num_colors`.
+                        drop(unsafe { KVec::from_raw_parts(subled_info, num_colors, num_colors) });
+                    })?;
+
+                    core::mem::forget(init_data.fwnode); // keep the reference count incremented
+
+                    Ok::<_, Error>(())
+                }),
+            }),
+        )
+    }
+
+    /// # Safety
+    /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+    /// `led::MultiColorDevice`.
+    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::MultiColorDevice`. `container_of!` can therefore
+        // safely calculate the address of the containing struct.
+        let led_mc_cdev = unsafe { container_of!(led_cdev, bindings::led_classdev_mc, led_cdev) };
+
+        // SAFETY: It is guaranteed that `led_mc_cdev` points to a `led_classdev_mc`
+        // field embedded within a valid `led::MultiColorDevice`. `container_of!` can therefore
+        // safely calculate the address of the containing struct.
+        unsafe { &*container_of!(Opaque::cast_from(led_mc_cdev), Self, classdev) }
+    }
+
+    fn parent(&self) -> &device::Device<Bound> {
+        // SAFETY: `self.classdev.get()` is guaranteed to be a valid pointer to `led_classdev_mc`.
+        unsafe { device::Device::from_raw((*(*self.classdev.get()).led_cdev.dev).parent) }
+    }
+
+    /// Returns the subleds passed to [`Device::new_multicolor`].
+    pub fn subleds(&self) -> &[MultiColorSubLed] {
+        // SAFETY: The existence of `self` guarantees that `self.classdev.get()` is a pointer to a
+        // valid `led_classdev_mc`.
+        let raw = unsafe { &*self.classdev.get() };
+        // SAFETY: `raw.subled_info` is a valid pointer to `mc_subled[num_colors]`.
+        // CAST: The safeguards in the const block ensure that `MultiColorSubLed` has an identical
+        // layout to `mc_subled`.
+        unsafe {
+            core::slice::from_raw_parts(
+                raw.subled_info.cast::<MultiColorSubLed>(),
+                raw.num_colors as usize,
+            )
+        }
+    }
+}
+
+// SAFETY: A `led::MultiColorDevice` can be unregistered from any thread.
+unsafe impl<T: LedOps<Mode = MultiColor> + Send> Send for MultiColorDevice<T> {}
+
+// SAFETY: `led::MultiColorDevice` can be shared among threads because all methods of `led::Device`
+// are thread safe.
+unsafe impl<T: LedOps<Mode = MultiColor> + Sync> Sync for MultiColorDevice<T> {}
+
+struct Adapter<T: LedOps<Mode = MultiColor>> {
+    _p: PhantomData<T>,
+}
+
+impl<T: LedOps<Mode = MultiColor>> Adapter<T> {
+    /// # Safety
+    /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+    /// `led::MultiColorDevice`.
+    /// 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::MultiColorDevice`.
+        let classdev = unsafe { MultiColorDevice::<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()) };
+
+        // SAFETY: `classdev.classdev.get()` is guaranteed to be a pointer to a valid
+        // `led_classdev_mc`.
+        unsafe { bindings::led_mc_calc_color_components(classdev.classdev.get(), brightness) };
+
+        let _ = classdev.ops.brightness_set(parent, classdev, brightness);
+    }
+
+    /// # Safety
+    /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+    /// `led::MultiColorDevice`.
+    /// 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::MultiColorDevice`.
+            let classdev = unsafe { MultiColorDevice::<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()) };
+
+            // SAFETY: `classdev.classdev.get()` is guaranteed to be a pointer to a valid
+            // `led_classdev_mc`.
+            unsafe { bindings::led_mc_calc_color_components(classdev.classdev.get(), brightness) };
+
+            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::MultiColorDevice`.
+    /// 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::MultiColorDevice`.
+        let classdev = unsafe { MultiColorDevice::<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::MultiColorDevice`.
+    /// `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::MultiColorDevice`.
+            let classdev = unsafe { MultiColorDevice::<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<Mode = MultiColor>> PinnedDrop for MultiColorDevice<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 `led_classdev_mc`.
+        let dev: &device::Device = unsafe { device::Device::from_raw((*raw).led_cdev.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_multicolor_register_ext`.
+        unsafe { bindings::led_classdev_multicolor_unregister(raw) };
+
+        // SAFETY: `raw` is guaranteed to be a valid pointer to `led_classdev_mc`.
+        let led_cdev = unsafe { &*raw };
+
+        // SAFETY: `subled_info` is guaranteed to be a valid array pointer to `mc_subled` with the
+        // length and capacity of `led_cdev.num_colors`. See `led::MulticolorDevice::new`.
+        drop(unsafe {
+            KVec::from_raw_parts(
+                led_cdev.subled_info,
+                led_cdev.num_colors as usize,
+                led_cdev.num_colors as usize,
+            )
+        });
+    }
+}

-- 
2.52.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v11 1/3] rust: leds: add basic led classdev abstractions
  2026-02-02 13:52 ` [PATCH v11 1/3] rust: leds: add basic " Markus Probst
@ 2026-02-02 15:41   ` Gary Guo
  2026-02-02 16:53     ` Markus Probst
  0 siblings, 1 reply; 7+ messages in thread
From: Gary Guo @ 2026-02-02 15:41 UTC (permalink / raw)
  To: Markus Probst, Lee Jones, Pavel Machek, Greg Kroah-Hartman,
	Dave Ertman, Ira Weiny, Leon Romanovsky, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Rafael J. Wysocki, Bjorn Helgaas,
	Krzysztof Wilczyński
  Cc: rust-for-linux, linux-leds, linux-kernel, linux-pci

On Mon Feb 2, 2026 at 1:52 PM GMT, 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>
> ---
>  MAINTAINERS        |   7 +
>  rust/kernel/led.rs | 453 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs |   1 +
>  3 files changed, 461 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0efa8cc6775b..26765fecb9a9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -14279,6 +14279,13 @@ F:	drivers/leds/
>  F:	include/dt-bindings/leds/
>  F:	include/linux/leds.h
>  
> +LED SUBSYSTEM [RUST]
> +M:	Markus Probst <markus.probst@posteo.de>
> +L:	linux-leds@vger.kernel.org
> +L:	rust-for-linux@vger.kernel.org
> +S:	Maintained
> +F:	rust/kernel/led.rs
> +
>  LEGO MINDSTORMS EV3
>  R:	David Lechner <david@lechnology.com>
>  S:	Maintained
> diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
> new file mode 100644
> index 000000000000..9acb6946f3da
> --- /dev/null
> +++ b/rust/kernel/led.rs
> @@ -0,0 +1,453 @@
> +// 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,
> +    ptr::NonNull, //
> +};
> +
> +use crate::{
> +    container_of,
> +    device::{
> +        self,
> +        property::FwNode,
> +        AsBusDevice,
> +        Bound, //
> +    },
> +    devres::Devres,
> +    error::{
> +        from_result,
> +        to_result,
> +        VTABLE_DEFAULT_ERROR, //
> +    },
> +    macros::vtable,
> +    prelude::*,
> +    str::CStrExt,
> +    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> {
> +    #[pin]
> +    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<ARef<FwNode>>,
> +    devicename: Option<&'a CStr>,
> +    devname_mandatory: bool,
> +    initial_brightness: u32,
> +    default_trigger: Option<&'a CStr>,
> +    color: Color,
> +}

It appears to me that while this reflects on the C API, on the Rust side this is
more commonly known as the builder pattern.

I think this should properly be name `led::DeviceBuilder`, as it does more than
what `led_init_data` does on the C side (e.g. initial_brightness).

Perhaps the device creation can be part of this too, e.g.

    LedDeviceBuilder::new()
        .fwnode(...)
        .devicename(...)
        .initial_brightness(...)
        .build(parent, ops)

?


> +
> +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: Option<ARef<FwNode>>) -> Self {
> +        Self { fwnode, ..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::{
> +///      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(
> +///         &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(),
> +///         Ok(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);
> +        };

`static_assert!()` and move this out from the impl.

> +        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: impl PinInit<T, Error> + 'a,
> +    ) -> impl PinInit<Devres<Self>, Error> + 'a {
> +        Devres::new(
> +            parent.as_ref(),
> +            try_pin_init!(Self {
> +                ops <- 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(), CStrExt::as_char_ptr),
> +                            color: init_data.color as u32,
> +                            ..bindings::led_classdev::default()
> +                        })
> +                    };
> +
> +                    let mut init_data_raw = bindings::led_init_data {
> +                        fwnode: init_data
> +                            .fwnode
> +                            .as_ref()
> +                            .map_or(core::ptr::null_mut(), |fwnode| fwnode.as_raw()),

This should be `fwnode.into_raw()` which directly takes the ownership for
`ARef`, rather than `as_raw()` and forget the `ARef` later.

Best,
Gary

> +                        default_label: core::ptr::null(),
> +                        devicename: init_data
> +                            .devicename
> +                            .map_or(core::ptr::null(), CStrExt::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,
> +                            &raw mut init_data_raw,
> +                        )
> +                    })?;
> +
> +                    core::mem::forget(init_data.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) }
> +    }
> +}


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v11 2/3] rust: leds: split generic and normal led classdev abstractions up
  2026-02-02 13:52 ` [PATCH v11 2/3] rust: leds: split generic and normal led classdev abstractions up Markus Probst
@ 2026-02-02 15:43   ` Gary Guo
  0 siblings, 0 replies; 7+ messages in thread
From: Gary Guo @ 2026-02-02 15:43 UTC (permalink / raw)
  To: Markus Probst, Lee Jones, Pavel Machek, Greg Kroah-Hartman,
	Dave Ertman, Ira Weiny, Leon Romanovsky, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Rafael J. Wysocki, Bjorn Helgaas,
	Krzysztof Wilczyński
  Cc: rust-for-linux, linux-leds, linux-kernel, linux-pci

On Mon Feb 2, 2026 at 1:52 PM GMT, Markus Probst wrote:
> Move code specific to normal led class devices into a separate file and
> introduce the `led::Mode` trait to allow for other types of led class
> devices in `led::LedOps`.
>
> Signed-off-by: Markus Probst <markus.probst@posteo.de>

This patch deleted a lot of code that's added in the previous one.

Could you structure it in a way without doing this?

Best,
Gary

> ---
>  MAINTAINERS               |   1 +
>  rust/kernel/led.rs        | 245 ++++++----------------------------------------
>  rust/kernel/led/normal.rs | 226 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 259 insertions(+), 213 deletions(-)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v11 1/3] rust: leds: add basic led classdev abstractions
  2026-02-02 15:41   ` Gary Guo
@ 2026-02-02 16:53     ` Markus Probst
  0 siblings, 0 replies; 7+ messages in thread
From: Markus Probst @ 2026-02-02 16:53 UTC (permalink / raw)
  To: Gary Guo, Lee Jones, Pavel Machek, Greg Kroah-Hartman,
	Dave Ertman, Ira Weiny, Leon Romanovsky, Miguel Ojeda,
	Alex Gaynor, Boqun Feng, Björn Roy Baron, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
	Rafael J. Wysocki, Bjorn Helgaas, Krzysztof Wilczyński
  Cc: rust-for-linux, linux-leds, linux-kernel, linux-pci

[-- Attachment #1: Type: text/plain, Size: 15457 bytes --]

On Mon, 2026-02-02 at 15:41 +0000, Gary Guo wrote:
> On Mon Feb 2, 2026 at 1:52 PM GMT, 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>
> > ---
> >  MAINTAINERS        |   7 +
> >  rust/kernel/led.rs | 453 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  rust/kernel/lib.rs |   1 +
> >  3 files changed, 461 insertions(+)
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 0efa8cc6775b..26765fecb9a9 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -14279,6 +14279,13 @@ F:	drivers/leds/
> >  F:	include/dt-bindings/leds/
> >  F:	include/linux/leds.h
> >  
> > +LED SUBSYSTEM [RUST]
> > +M:	Markus Probst <markus.probst@posteo.de>
> > +L:	linux-leds@vger.kernel.org
> > +L:	rust-for-linux@vger.kernel.org
> > +S:	Maintained
> > +F:	rust/kernel/led.rs
> > +
> >  LEGO MINDSTORMS EV3
> >  R:	David Lechner <david@lechnology.com>
> >  S:	Maintained
> > diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
> > new file mode 100644
> > index 000000000000..9acb6946f3da
> > --- /dev/null
> > +++ b/rust/kernel/led.rs
> > @@ -0,0 +1,453 @@
> > +// 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,
> > +    ptr::NonNull, //
> > +};
> > +
> > +use crate::{
> > +    container_of,
> > +    device::{
> > +        self,
> > +        property::FwNode,
> > +        AsBusDevice,
> > +        Bound, //
> > +    },
> > +    devres::Devres,
> > +    error::{
> > +        from_result,
> > +        to_result,
> > +        VTABLE_DEFAULT_ERROR, //
> > +    },
> > +    macros::vtable,
> > +    prelude::*,
> > +    str::CStrExt,
> > +    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> {
> > +    #[pin]
> > +    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<ARef<FwNode>>,
> > +    devicename: Option<&'a CStr>,
> > +    devname_mandatory: bool,
> > +    initial_brightness: u32,
> > +    default_trigger: Option<&'a CStr>,
> > +    color: Color,
> > +}
> 
> It appears to me that while this reflects on the C API, on the Rust side this is
> more commonly known as the builder pattern.
> 
> I think this should properly be name `led::DeviceBuilder`, as it does more than
> what `led_init_data` does on the C side (e.g. initial_brightness).
> 
> Perhaps the device creation can be part of this too, e.g.
> 
>     LedDeviceBuilder::new()
>         .fwnode(...)
>         .devicename(...)
>         .initial_brightness(...)
>         .build(parent, ops)
> 
> ?
> 
> 
> > +
> > +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: Option<ARef<FwNode>>) -> Self {
> > +        Self { fwnode, ..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::{
> > +///      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(
> > +///         &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(),
> > +///         Ok(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);
> > +        };
> 
> `static_assert!()` and move this out from the impl.
> 
> > +        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: impl PinInit<T, Error> + 'a,
> > +    ) -> impl PinInit<Devres<Self>, Error> + 'a {
> > +        Devres::new(
> > +            parent.as_ref(),
> > +            try_pin_init!(Self {
> > +                ops <- 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(), CStrExt::as_char_ptr),
> > +                            color: init_data.color as u32,
> > +                            ..bindings::led_classdev::default()
> > +                        })
> > +                    };
> > +
> > +                    let mut init_data_raw = bindings::led_init_data {
> > +                        fwnode: init_data
> > +                            .fwnode
> > +                            .as_ref()
> > +                            .map_or(core::ptr::null_mut(), |fwnode| fwnode.as_raw()),
> 
> This should be `fwnode.into_raw()` which directly takes the ownership for
> `ARef`, rather than `as_raw()` and forget the `ARef` later.
With into_raw(), the reference count of fwnode would not decrement if
the registration fails.

Thanks
- Markus Probst

> 
> Best,
> Gary
> 
> > +                        default_label: core::ptr::null(),
> > +                        devicename: init_data
> > +                            .devicename
> > +                            .map_or(core::ptr::null(), CStrExt::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,
> > +                            &raw mut init_data_raw,
> > +                        )
> > +                    })?;
> > +
> > +                    core::mem::forget(init_data.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) }
> > +    }
> > +}

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-02-02 16:53 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-02 13:52 [PATCH v11 0/3] rust: leds: add led classdev abstractions Markus Probst
2026-02-02 13:52 ` [PATCH v11 1/3] rust: leds: add basic " Markus Probst
2026-02-02 15:41   ` Gary Guo
2026-02-02 16:53     ` Markus Probst
2026-02-02 13:52 ` [PATCH v11 2/3] rust: leds: split generic and normal led classdev abstractions up Markus Probst
2026-02-02 15:43   ` Gary Guo
2026-02-02 13:52 ` [PATCH v11 3/3] rust: leds: add multicolor classdev abstractions Markus Probst

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox