[PATCH v25 0/4] rust: leds: add led classdev abstractions

[PATCH v25 0/4] rust: leds: add led classdev abstractions

[Markus Probst]

The abstraction is used by
https://lore.kernel.org/rust-for-linux/20260724-synology_microp_initial-v18-0-fb2f49f10e77@posteo.de/
.

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 v24:
* remove `LedOps::Bus` (Gary)
* support type-erased `Device` and `MultiColorDevice` types (Gary)
* use `Device` and `MultiColorDevice` as arbitrary self types in
  callbacks. Remove separate `classdev` argument (Gary)
* remove unnecessary imports in doc example

Changes since v23:
* add separate patch for MAINTAINERS file update

Changes since v22:
* readded CStrExt import, because it is imported with `as _` in prelude.
  A `# CONFIG_RUST is not set` sneaked into my .config while
  development, so the compile error was unnoticed.

Changes since v21:
* use 'init for lifetime that is only alive during initialization
* remove unnecessary CStrExt import

Changes since v20:
* resolve Sashiko regressions:
  * fix typo
  * fix fwnode refcount decremented too early

Changes since v19:
* rebase on v7.2-rc1:
  * Add `max_intensity` to `MultiColorSubLed`
* use safer `KBox::pin_slice` instead of `KVec`
  (len might not equal capacity)
* explicitly call `FwNode::dec_ref` instead of dropping a reconstructed
  `ARef<FwNode>`.
* remove direct access to `intensity` and `brightness` fields,
  which may get mutated concurrently by the C side
* fix safety comments pointing to functions from previous revisions

Changes since v18:
* add inlines
* fix invalid documentation
* improve led color duplicate checking

Changes since v17:
* use lifetimes instead of Devres

Changes since v16:
* use for loops for duplicate checking

Changes since v15:
* fix issues reported by Sashiko bot:
  * fix returning error not possible on `brightness_get` callback

Changes since v14:
* fix issues reported by Sashiko bot:
  * add missing inlines
  * add missing Sync trait bound
  * fix vertical import layout for public export of private types
  * fix potential memory leak, if a multicolor led device with over
    `u32::MAX` subleds is registered
* remove default_trigger option
* fix missing CAST doc

Changes since v13:
* rebased onto v7.1-rc1

Changes since v12:
* add `led::DeviceBuilder::name()` and `DeviceBuilderState'
* add `led::Color::as_c_str`

Changes since v11:
* use `led::DeviceBuilder` instead of `led::InitData`
* use static_assert instead of const { assert!(...) }
* restructured patches to avoid moving `led::Device` from
  rust/kernel/led.rs to rust/kernel/led/normal.rs in the 2. patch

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 (4):
      rust: leds: add basic led classdev abstractions
      rust: leds: add Mode trait
      rust: leds: add multicolor classdev abstractions
      MAINTAINERS: rust: leds: Add rust abstraction entry

 MAINTAINERS                     |   8 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/led.rs              | 312 ++++++++++++++++++++++++++++
 rust/kernel/led/multicolor.rs   | 445 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/led/normal.rs       | 230 +++++++++++++++++++++
 rust/kernel/lib.rs              |   1 +
 6 files changed, 997 insertions(+)
---
base-commit: e5e04726cdd043e309677071ab1b65a4b18f422b
change-id: 20251114-rust_leds-a959f7c2f7f9

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

[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::DeviceBuilder` - the builder for the led class device

* `led::Device` - a safe wrapper around `led_classdev`

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 rust/kernel/led.rs        | 265 ++++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/led/normal.rs | 222 ++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs        |   1 +
 3 files changed, 488 insertions(+)

diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
new file mode 100644
index 000000000000..deda8cc548a8
--- /dev/null
+++ b/rust/kernel/led.rs
@@ -0,0 +1,265 @@
+// 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,
+        Bound, //
+    },
+    error::{
+        from_result,
+        to_result,
+        VTABLE_DEFAULT_ERROR, //
+    },
+    macros::vtable,
+    prelude::*,
+    str::CStrExt,
+    sync::aref::ARef,
+    types::Opaque, //
+};
+
+mod normal;
+
+pub use normal::Device;
+
+/// The name of the led is determined by the driver.
+pub enum Named {}
+/// The name of the led is determined by its fwnode.
+pub enum Unnamed {}
+
+/// How the name of the led should be determined.
+pub trait DeviceBuilderState: private::Sealed {}
+
+impl DeviceBuilderState for Named {}
+impl private::Sealed for Named {}
+impl DeviceBuilderState for Unnamed {}
+impl private::Sealed for Unnamed {}
+
+/// The builder to register a led class device.
+///
+/// See [`LedOps`].
+pub struct DeviceBuilder<'init, S> {
+    fwnode: Option<ARef<FwNode>>,
+    name: Option<&'init CStr>,
+    devicename: Option<&'init CStr>,
+    devname_mandatory: bool,
+    initial_brightness: u32,
+    color: Color,
+    _p: PhantomData<S>,
+}
+
+impl<S: DeviceBuilderState> DeviceBuilder<'static, S> {
+    /// Creates a new [`DeviceBuilder`].
+    #[inline]
+    #[expect(
+        clippy::new_without_default,
+        reason = "no need and derive is prevented by S"
+    )]
+    pub fn new() -> Self {
+        Self {
+            fwnode: None,
+            name: None,
+            devicename: None,
+            devname_mandatory: false,
+            initial_brightness: 0,
+            color: Color::default(),
+            _p: PhantomData,
+        }
+    }
+}
+
+impl<'init> DeviceBuilder<'init, Unnamed> {
+    /// Sets the firmware node.
+    #[inline]
+    pub fn fwnode(self, fwnode: Option<ARef<FwNode>>) -> Self {
+        Self { fwnode, ..self }
+    }
+
+    /// Sets the device name.
+    #[inline]
+    pub fn devicename(self, devicename: &'init CStr) -> Self {
+        Self {
+            devicename: Some(devicename),
+            ..self
+        }
+    }
+
+    /// Sets if a device name is mandatory.
+    #[inline]
+    pub fn devicename_mandatory(self, mandatory: bool) -> Self {
+        Self {
+            devname_mandatory: mandatory,
+            ..self
+        }
+    }
+}
+
+impl<'init, S: DeviceBuilderState> DeviceBuilder<'init, S> {
+    /// Sets the initial brightness value for the led.
+    ///
+    /// The default brightness is 0.
+    /// If [`LedOps::brightness_get`] is implemented, this value will be ignored.
+    #[inline]
+    pub fn initial_brightness(self, brightness: u32) -> Self {
+        Self {
+            initial_brightness: brightness,
+            ..self
+        }
+    }
+
+    /// Sets the color of the led.
+    ///
+    /// This value can be overwritten by the "color" fwnode property.
+    #[inline]
+    pub fn color(self, color: Color) -> Self {
+        Self { color, ..self }
+    }
+}
+
+impl<'init> DeviceBuilder<'init, Named> {
+    /// Sets the name of the led.
+    ///
+    /// Setting this will prevent the fwnode from being used and prevents automatic name
+    /// composition.
+    #[inline]
+    pub fn name(self, name: &'init CStr) -> Self {
+        Self {
+            name: Some(name),
+            ..self
+        }
+    }
+}
+
+/// Trait defining the operations for a LED driver.
+///
+/// # Examples
+/// ```
+/// use kernel::{
+///      led,
+///      macros::vtable,
+///      prelude::*, //
+///  };
+///
+/// struct MyLedOps;
+///
+///
+/// #[vtable]
+/// impl led::LedOps for MyLedOps {
+///     const BLOCKING: bool = false;
+///     const MAX_BRIGHTNESS: u32 = 255;
+///
+///     fn brightness_set<'bound>(
+///         self: &led::Device<'bound, Self>,
+///         _brightness: u32
+///     ) -> Result<()> {
+///         // Set the brightness for the led here
+///         Ok(())
+///     }
+/// }
+/// ```
+/// Led drivers must implement this trait in order to register and handle a [`Device`].
+#[vtable]
+pub trait LedOps: Send + Sync + Sized {
+    /// If set true, [`LedOps::brightness_set`] and [`LedOps::blink_set`] must perform the
+    /// operation immediately. If set false, they must not sleep.
+    const BLOCKING: bool;
+    /// The max brightness level.
+    const MAX_BRIGHTNESS: u32;
+
+    /// Sets the brightness level.
+    ///
+    /// See also [`LedOps::BLOCKING`].
+    fn brightness_set<'bound>(self: &Device<'bound, Self>, brightness: u32) -> Result<()>;
+
+    /// Gets the current brightness level.
+    fn brightness_get<'bound>(self: &Device<'bound, Self>) -> Result<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<'bound>(
+        self: &Device<'bound, Self>,
+        delay_on: &mut usize,
+        delay_off: &mut usize,
+    ) -> Result<()> {
+        let _ = (delay_on, delay_off);
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+}
+
+/// Led colors.
+#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
+#[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,
+}
+static_assert!(bindings::LED_COLOR_ID_MAX == 15);
+
+impl Color {
+    /// Name of the color
+    #[inline]
+    pub fn as_c_str(self) -> &'static CStr {
+        // SAFETY:
+        // - `self as u8` is a valid led color id.
+        // - `led_get_color_name` always returns a valid C string pointer.
+        unsafe { CStr::from_char_ptr(bindings::led_get_color_name(self as u8)) }
+    }
+}
+
+impl TryFrom<u32> for Color {
+    type Error = Error;
+
+    fn try_from(value: u32) -> core::result::Result<Self, Self::Error> {
+        if value < bindings::LED_COLOR_ID_MAX {
+            // SAFETY:
+            // - `Color` is represented as `u32`
+            // - the static_assert 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)
+        }
+    }
+}
+
+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..a22c29cfd262
--- /dev/null
+++ b/rust/kernel/led/normal.rs
@@ -0,0 +1,222 @@
+// 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 core::ops::Deref;
+
+use super::*;
+
+/// The led class device representation.
+///
+/// This structure represents the Rust abstraction for a led class device.
+#[pin_data(PinnedDrop)]
+pub struct Device<'bound, T: 'bound = ()> {
+    #[pin]
+    ops: T,
+    #[pin]
+    classdev: Opaque<bindings::led_classdev>,
+    _p: PhantomData<&'bound ()>,
+}
+
+impl<'init, S: DeviceBuilderState> DeviceBuilder<'init, S> {
+    /// Registers a new [`Device`].
+    pub fn build<'bound: 'init, T: LedOps + 'bound>(
+        self,
+        parent: &'bound device::Device<Bound>,
+        ops: impl PinInit<T, Error> + 'init,
+    ) -> impl PinInit<Device<'bound, T>, Error> + 'init {
+        const_assert!(T::MAX_BRIGHTNESS <= i32::MAX.unsigned_abs() || !T::HAS_BRIGHTNESS_GET);
+
+        try_pin_init!(Device {
+            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: self.initial_brightness,
+                        color: self.color as u32,
+                        name: self.name.map_or(core::ptr::null(), CStrExt::as_char_ptr),
+                        ..bindings::led_classdev::default()
+                    })
+                };
+
+                let mut init_data = bindings::led_init_data {
+                    fwnode: self
+                        .fwnode
+                        .as_ref()
+                        .map_or(core::ptr::null_mut(), |fwnode| fwnode.as_raw()),
+                    default_label: core::ptr::null(),
+                    devicename: self
+                        .devicename
+                        .map_or(core::ptr::null(), CStrExt::as_char_ptr),
+                    devname_mandatory: self.devname_mandatory,
+                };
+
+                // SAFETY:
+                // - `parent.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_raw(),
+                        ptr,
+                        if self.name.is_none() {
+                            &raw mut init_data
+                        } else {
+                            core::ptr::null_mut()
+                        },
+                    )
+                })?;
+
+                core::mem::forget(self.fwnode); // keep the reference count incremented
+
+                Ok::<_, Error>(())
+            }),
+            _p: PhantomData,
+        })
+    }
+}
+
+impl<'bound, T: 'bound> Device<'bound, T> {
+    /// # Safety
+    /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+    /// `led::Device`.
+    #[inline]
+    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) }
+    }
+}
+
+impl<'bound, T: LedOps + 'bound> Deref for Device<'bound, T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        &self.ops
+    }
+}
+
+// SAFETY: A `led::Device` can be unregistered from any thread.
+unsafe impl<'bound, T: 'bound + Send> Send for Device<'bound, T> {}
+
+// SAFETY: `led::Device` can be shared among threads because all methods of `led::Device`
+// are thread safe.
+unsafe impl<'bound, T: 'bound + Sync> Sync for Device<'bound, T> {}
+
+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) };
+
+        let _ = classdev.brightness_set(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) };
+
+            classdev.brightness_set(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) };
+
+        // CAST: Resulting value will be casted back to i32 in the led subsystem.
+        from_result(|| {
+            classdev
+                .brightness_get()
+                .inspect(|val| debug_assert!(*val <= T::MAX_BRIGHTNESS))
+                .and_then(|val| Ok(i32::try_from(val)?))
+        }) as u32
+    }
+
+    /// # 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) };
+
+            classdev.blink_set(
+                // 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<'bound, T: 'bound> PinnedDrop for Device<'bound, 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::DeviceBuilder::build`.
+            .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) };
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 4d5c96ddc49c..748788ad131b 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -96,6 +96,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.55.0

[PATCH v25 2/4] rust: leds: add Mode trait

[Markus Probst]

Add 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>
---
 rust/kernel/led.rs        | 37 ++++++++++++++++++++++++++-----------
 rust/kernel/led/normal.rs | 12 ++++++++++--
 2 files changed, 36 insertions(+), 13 deletions(-)

diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
index deda8cc548a8..7cd8f0504649 100644
--- a/rust/kernel/led.rs
+++ b/rust/kernel/led.rs
@@ -4,11 +4,7 @@
 //!
 //! C header: [`include/linux/leds.h`](srctree/include/linux/leds.h)
 
-use core::{
-    marker::PhantomData,
-    mem::transmute,
-    ptr::NonNull, //
-};
+use core::{marker::PhantomData, mem::transmute, ops::Deref, ptr::NonNull};
 
 use crate::{
     container_of,
@@ -31,7 +27,10 @@
 
 mod normal;
 
-pub use normal::Device;
+pub use normal::{
+    Device,
+    Normal, //
+};
 
 /// The name of the led is determined by the driver.
 pub enum Named {}
@@ -156,6 +155,7 @@ pub fn name(self, name: &'init CStr) -> Self {
 ///
 /// #[vtable]
 /// impl led::LedOps for MyLedOps {
+///     type Mode = led::Normal;
 ///     const BLOCKING: bool = false;
 ///     const MAX_BRIGHTNESS: u32 = 255;
 ///
@@ -176,16 +176,21 @@ pub trait LedOps: Send + Sync + Sized {
     const BLOCKING: bool;
     /// The max brightness level.
     const MAX_BRIGHTNESS: u32;
-
     /// Sets the brightness level.
     ///
     /// See also [`LedOps::BLOCKING`].
-    fn brightness_set<'bound>(self: &Device<'bound, Self>, brightness: u32) -> Result<()>;
-
+    fn brightness_set<'bound>(
+        self: &<Self::Mode as Mode>::Device<'bound, Self>,
+        brightness: u32,
+    ) -> Result<()>;
     /// Gets the current brightness level.
-    fn brightness_get<'bound>(self: &Device<'bound, Self>) -> Result<u32> {
+    fn brightness_get<'bound>(self: &<Self::Mode as Mode>::Device<'bound, Self>) -> Result<u32> {
         build_error!(VTABLE_DEFAULT_ERROR)
     }
+    /// The led mode to use.
+    ///
+    /// See [`Mode`].
+    type Mode: Mode;
 
     /// Activates hardware accelerated blinking.
     ///
@@ -196,7 +201,7 @@ fn brightness_get<'bound>(self: &Device<'bound, Self>) -> Result<u32> {
     ///
     /// See also [`LedOps::BLOCKING`].
     fn blink_set<'bound>(
-        self: &Device<'bound, Self>,
+        self: &<Self::Mode as Mode>::Device<'bound, Self>,
         delay_on: &mut usize,
         delay_off: &mut usize,
     ) -> Result<()> {
@@ -260,6 +265,16 @@ fn try_from(value: u32) -> core::result::Result<Self, Self::Error> {
     }
 }
 
+/// 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<'bound, T: LedOps<Mode = Self> + 'bound>: Deref<Target = T>;
+}
+
 mod private {
     pub trait Sealed {}
 }
diff --git a/rust/kernel/led/normal.rs b/rust/kernel/led/normal.rs
index a22c29cfd262..992353319522 100644
--- a/rust/kernel/led/normal.rs
+++ b/rust/kernel/led/normal.rs
@@ -8,6 +8,14 @@
 
 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<'bound, T: LedOps<Mode = Self> + 'bound> = Device<'bound, T>;
+}
+impl private::Sealed for Normal {}
+
 /// The led class device representation.
 ///
 /// This structure represents the Rust abstraction for a led class device.
@@ -22,7 +30,7 @@ pub struct Device<'bound, T: 'bound = ()> {
 
 impl<'init, S: DeviceBuilderState> DeviceBuilder<'init, S> {
     /// Registers a new [`Device`].
-    pub fn build<'bound: 'init, T: LedOps + 'bound>(
+    pub fn build<'bound: 'init, T: LedOps<Mode = Normal> + 'bound>(
         self,
         parent: &'bound device::Device<Bound>,
         ops: impl PinInit<T, Error> + 'init,
@@ -120,7 +128,7 @@ struct Adapter<T: LedOps> {
     _p: PhantomData<T>,
 }
 
-impl<T: LedOps> Adapter<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`.

-- 
2.55.0

[PATCH v25 3/4] rust: leds: add multicolor classdev abstractions

[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`

* `led::DeviceBuilder::build_multicolor` - a function to register a new
  multicolor led class device

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

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 4b31aa7f432f..81a03985322a 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -69,6 +69,7 @@
 #include <linux/iosys-map.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 7cd8f0504649..b25a6f992a62 100644
--- a/rust/kernel/led.rs
+++ b/rust/kernel/led.rs
@@ -25,8 +25,16 @@
     types::Opaque, //
 };
 
+#[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, //
@@ -228,7 +236,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,
@@ -269,7 +294,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<'bound, T: LedOps<Mode = Self> + 'bound>: Deref<Target = T>;
diff --git a/rust/kernel/led/multicolor.rs b/rust/kernel/led/multicolor.rs
new file mode 100644
index 000000000000..309487bdf38a
--- /dev/null
+++ b/rust/kernel/led/multicolor.rs
@@ -0,0 +1,445 @@
+// 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 core::{
+    cell::UnsafeCell,
+    num::NonZero,
+    ptr, //
+};
+
+use crate::types::ScopeGuard;
+
+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<'bound, T: LedOps<Mode = Self> + 'bound> = MultiColorDevice<'bound, 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(Debug)]
+#[non_exhaustive]
+pub struct MultiColorSubLed {
+    /// The color of the sub led
+    pub color: Color,
+    brightness: UnsafeCell<u32>,
+    intensity: UnsafeCell<u32>,
+    /// The maximum supported intensity value.
+    ///
+    /// If None the maximum intensity equals to [`LedOps::MAX_BRIGHTNESS`].
+    pub max_intensity: Option<NonZero<u32>>,
+    /// Arbitrary data for the driver to store.
+    pub channel: u32,
+}
+
+// SAFETY: `MultiColorSubLed` can be shared among threads.
+unsafe impl Sync for MultiColorSubLed {}
+
+impl Clone for MultiColorSubLed {
+    fn clone(&self) -> Self {
+        Self {
+            color: self.color,
+            brightness: self.brightness().into(),
+            intensity: self.intensity().into(),
+            max_intensity: self.max_intensity,
+            channel: self.channel,
+        }
+    }
+}
+
+// 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: UnsafeCell::new(0),
+        intensity: UnsafeCell::new(0),
+        max_intensity: None,
+        channel: 0,
+    };
+    let c_zeroed = bindings::mc_subled {
+        color_index: 0,
+        brightness: 0,
+        intensity: 0,
+        max_intensity: 0,
+        channel: 0,
+    };
+
+    assert!(offset_of!(MultiColorSubLed, color) == offset_of!(bindings::mc_subled, color_index));
+    assert!(
+        offset_of!(MultiColorSubLed, brightness) == offset_of!(bindings::mc_subled, brightness)
+    );
+    assert!(offset_of!(MultiColorSubLed, intensity) == offset_of!(bindings::mc_subled, intensity));
+    assert!(
+        offset_of!(MultiColorSubLed, max_intensity)
+            == offset_of!(bindings::mc_subled, max_intensity)
+    );
+    assert!(offset_of!(MultiColorSubLed, channel) == offset_of!(bindings::mc_subled, channel));
+
+    assert_same_type(&0u32, &c_zeroed.color_index);
+    assert_same_type(&rust_zeroed.brightness.into_inner(), &c_zeroed.brightness);
+    assert_same_type(&rust_zeroed.intensity.into_inner(), &c_zeroed.intensity);
+    assert!(size_of_val(&rust_zeroed.max_intensity) == size_of_val(&c_zeroed.max_intensity));
+    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.
+    #[inline]
+    pub const fn new(color: Color) -> Self {
+        Self {
+            color,
+            brightness: UnsafeCell::new(0),
+            intensity: UnsafeCell::new(0),
+            max_intensity: None,
+            channel: 0,
+        }
+    }
+
+    /// Set arbitrary data for the driver.
+    #[inline]
+    pub const fn channel(mut self, channel: u32) -> Self {
+        self.channel = channel;
+        self
+    }
+
+    /// Set the initial intensity of the subled.
+    #[inline]
+    pub const fn initial_intensity(mut self, intensity: u32) -> Self {
+        self.intensity = UnsafeCell::new(intensity);
+        self
+    }
+
+    /// Set the maximum supported intensity of the subled.
+    #[inline]
+    pub const fn max_intensity(mut self, max_intensity: NonZero<u32>) -> Self {
+        self.max_intensity = Some(max_intensity);
+        self
+    }
+
+    /// The intensity of the sub led.
+    #[inline]
+    pub const fn intensity(&self) -> u32 {
+        // SAFETY:
+        // - `self.intensity.get()` is a valid pointer to `u32`.
+        // - We don't have exclusive or immutable access to `self.intensity`,
+        //   but the alignment should prevent "load tearing".
+        unsafe { *self.intensity.get() }
+    }
+
+    /// The brightness of the sub led.
+    #[inline]
+    pub const fn brightness(&self) -> u32 {
+        // SAFETY:
+        // - `self.brightness.get()` is a valid pointer to `u32`.
+        // - We don't have exclusive or immutable access to `self.brightness`,
+        //   but the alignment should prevent "load tearing".
+        unsafe { *self.brightness.get() }
+    }
+}
+
+/// The multicolor led class device representation.
+///
+/// This structure represents the Rust abstraction for a multicolor led class device.
+#[pin_data(PinnedDrop)]
+pub struct MultiColorDevice<'bound, T: 'bound = ()> {
+    #[pin]
+    ops: T,
+    #[pin]
+    classdev: Opaque<bindings::led_classdev_mc>,
+    _p: PhantomData<&'bound ()>,
+}
+
+impl<'init, S: DeviceBuilderState> DeviceBuilder<'init, S> {
+    /// Registers a new [`MulticolorDevice`].
+    pub fn build_multicolor<'bound: 'init, T: LedOps<Mode = MultiColor> + 'bound>(
+        self,
+        parent: &'bound device::Device<Bound>,
+        ops: impl PinInit<T, Error> + 'init,
+        subleds: &'init [MultiColorSubLed],
+    ) -> impl PinInit<MultiColorDevice<'bound, T>, Error> + 'init {
+        const_assert!(T::MAX_BRIGHTNESS <= i32::MAX.unsigned_abs() || !T::HAS_BRIGHTNESS_GET);
+
+        try_pin_init!(MultiColorDevice {
+            ops <- ops,
+            classdev <- Opaque::try_ffi_init(|ptr: *mut bindings::led_classdev_mc| {
+                let mut colors = [false; bindings::LED_COLOR_ID_MAX as usize];
+                for subled in subleds {
+                    if colors[subled.color as usize] {
+                        dev_err!(parent.as_ref(), "duplicate color in multicolor led\n");
+                        return Err(EINVAL);
+                    }
+                    colors[subled.color as usize] = true;
+                }
+                let subleds_box = KBox::pin_slice(
+                    |index| Ok::<_, Error>(subleds[index].clone()),
+                    subleds.len(),
+                    GFP_KERNEL,
+                )?;
+                let subleds_box_raw = KBox::into_raw(Pin::into_inner(subleds_box));
+
+                let subled_guard = ScopeGuard::new(|| {
+                    // SAFETY: `subleds_box_raw` is guaranteed to be a valid pointer to
+                    // `[MultiColorSubLed]`.
+                    drop(unsafe { <KBox<[MultiColorSubLed]>>::from_raw(subleds_box_raw) });
+                });
+
+                // 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: self.initial_brightness,
+                            color: self.color as u32,
+                            name: self.name.map_or(core::ptr::null(), CStrExt::as_char_ptr),
+                            ..bindings::led_classdev::default()
+                        },
+                        num_colors: u32::try_from(subleds_box_raw.len())?,
+                        // CAST: The safeguards in the const block ensure that
+                        // `MultiColorSubLed` has an identical layout to `mc_subled`.
+                        subled_info: subleds_box_raw.cast::<bindings::mc_subled>(),
+                    })
+                };
+
+                let mut init_data = bindings::led_init_data {
+                    fwnode: self
+                        .fwnode
+                        .as_ref()
+                        .map_or(core::ptr::null_mut(), |fwnode| fwnode.as_raw()),
+                    default_label: core::ptr::null(),
+                    devicename: self
+                        .devicename
+                        .map_or(core::ptr::null(), CStrExt::as_char_ptr),
+                    devname_mandatory: self.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,
+                        if self.name.is_none() {
+                            &raw mut init_data
+                        } else {
+                            core::ptr::null_mut()
+                        },
+                    )
+                })?;
+
+                subled_guard.dismiss();
+
+                core::mem::forget(self.fwnode); // keep the reference count incremented
+
+                Ok::<_, Error>(())
+            }),
+            _p: PhantomData,
+        })
+    }
+}
+
+impl<'bound, T: 'bound> MultiColorDevice<'bound, T> {
+    /// # Safety
+    /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+    /// `led::MultiColorDevice`.
+    #[inline]
+    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) }
+    }
+
+    /// Returns the subleds passed to [`Device::new_multicolor`].
+    #[inline]
+    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>(),
+                // CAST: It is guaranteed that `num_colors` fits into an `usize`.
+                raw.num_colors as usize,
+            )
+        }
+    }
+}
+
+impl<'bound, T: LedOps + 'bound> Deref for MultiColorDevice<'bound, T> {
+    type Target = T;
+
+    fn deref(&self) -> &Self::Target {
+        &self.ops
+    }
+}
+
+// SAFETY: A `led::MultiColorDevice` can be unregistered from any thread.
+unsafe impl<'bound, T: 'bound + Send> Send for MultiColorDevice<'bound, T> {}
+
+// SAFETY: `led::MultiColorDevice` can be shared among threads because all methods of `led::Device`
+// are thread safe.
+unsafe impl<'bound, T: 'bound + Sync> Sync for MultiColorDevice<'bound, 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.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.brightness_set(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.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.brightness_set(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) };
+
+        // CAST: Resulting value will be casted back to i32 in the led subsystem.
+        from_result(|| {
+            classdev
+                .brightness_get()
+                .inspect(|val| debug_assert!(*val <= T::MAX_BRIGHTNESS))
+                .and_then(|val| Ok(i32::try_from(val)?))
+        }) as u32
+    }
+
+    /// # 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) };
+
+            classdev.blink_set(
+                // 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<'bound, T: 'bound> PinnedDrop for MultiColorDevice<'bound, 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::DeviceBuilder::build_multicolor`.
+            .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: `led_cdev.subled_info` is guaranteed to be a valid pointer to
+        // `[MultiColorSubLed]` with length of `led_cdev.num_colors`.
+        // See `led::DeviceBuilder::build_multicolor`.
+        drop(unsafe {
+            KBox::from_raw(ptr::slice_from_raw_parts_mut(
+                led_cdev.subled_info.cast::<MultiColorSubLed>(),
+                // CAST: `num_colors` was originally converted from a `usize` in
+                // `DeviceBuilder::build_multicolor`. Thus it is guaranteed that
+                // `num_colors` fits into an usize.
+                led_cdev.num_colors as usize,
+            ))
+        });
+    }
+}

-- 
2.55.0

[PATCH v25 4/4] MAINTAINERS: rust: leds: Add rust abstraction entry

[Markus Probst]

Add self as maintainer.

Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3a19da74d00c..54931e850570 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14795,6 +14795,14 @@ 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
+F:	rust/kernel/led/
+
 LEGO MINDSTORMS EV3
 R:	David Lechner <david@lechnology.com>
 S:	Maintained

-- 
2.55.0

Re: [PATCH v25 2/4] rust: leds: add Mode trait

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The `core` import uses horizontal formatting instead of the required vertical format.
- [Low] The commit message description is not capitalized, violating the LED subsystem guidelines.
--

commit fc494e48581e40fa3f62db639a08746b74e239f2
Author: Markus Probst <markus.probst@posteo.de>

rust: leds: add Mode trait

This commit adds the Mode trait to allow for other types of led class
devices in LedOps.

> rust: leds: add Mode trait

[Severity: Low]
This isn't a bug, but the commit message description is not capitalized,
violating the LED subsystem guidelines.

The global review guidelines for the LED subsystem mandate that the
description following the commit message prefix must be capitalized.

Should the description start with a capital letter, as in "rust: leds: Add
Mode trait"?

[ ... ]

> diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
> index deda8cc548a8c..7cd8f0504649a 100644
> --- a/rust/kernel/led.rs
> +++ b/rust/kernel/led.rs
> @@ -4,11 +4,7 @@
>  //!
>  //! C header: [`include/linux/leds.h`](srctree/include/linux/leds.h)
>  
> -use core::{
> -    marker::PhantomData,
> -    mem::transmute,
> -    ptr::NonNull, //
> -};
> +use core::{marker::PhantomData, mem::transmute, ops::Deref, ptr::NonNull};

[Severity: Low]
This isn't a bug, but does this import for the core crate use horizontal
formatting instead of the required vertical format?

The kernel Rust coding guidelines explicitly require using the vertical
import style for non-vendored crates. The commit replaces a multi-line
vertical import block with a single-line horizontal block.

Should this be changed back to the vertical format?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913-rust_leds-v25-0-1a10371d78c3@posteo.de?part=2

Re: [PATCH v25 2/4] rust: leds: add Mode trait

[Markus Probst]

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

On Sun, 2026-09-13 at 16:15 +0000, Markus Probst wrote:
> Add 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>
> ---
>  rust/kernel/led.rs        | 37 ++++++++++++++++++++++++++-----------
>  rust/kernel/led/normal.rs | 12 ++++++++++--
>  2 files changed, 36 insertions(+), 13 deletions(-)
> 
> diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
> index deda8cc548a8..7cd8f0504649 100644
> --- a/rust/kernel/led.rs
> +++ b/rust/kernel/led.rs
> @@ -4,11 +4,7 @@
>  //!
>  //! C header: [`include/linux/leds.h`](srctree/include/linux/leds.h)
>  
> -use core::{
> -    marker::PhantomData,
> -    mem::transmute,
> -    ptr::NonNull, //
> -};
> +use core::{marker::PhantomData, mem::transmute, ops::Deref, ptr::NonNull};
supposed to be vertical.

Thanks
- Markus Probst

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] `blink_set_callback` creates exclusive `&mut usize` references from concurrently accessible pointers, causing Undefined Behavior.
- [High] Incorrect lifetime bounds and raw pointer coercion in `DeviceBuilder::build` allow the registered `Device` to outlive borrowed C strings, leading to a Use-After-Free.
- [High] Reconstructing `ARef<FwNode>` from the C `struct device` in `PinnedDrop` is unsound and causes reference count underflows if the fwnode is assigned by the C subsystem.
- [High] Using unmanaged `led_classdev_register_ext` combined with `PinnedDrop` breaks devres cleanup ordering, leading to a race condition and use-after-free on device removal.
- [Low] Commit message description is not capitalized.
- [Low] Small forwarding functions in the abstraction lack the `#[inline]` attribute, violating Rust subsystem guidelines.
--

commit 7ba1dd17b1d18204d9b691fb84bcc75aeb22eaca
Author: Markus Probst <markus.probst@posteo.de>

rust: leds: add basic led classdev abstractions

This commit implements the core abstractions needed for LED class devices in
Rust. It introduces the LedOps trait for handling LEDs, along with a
DeviceBuilder and a safe Device wrapper around led_classdev.

> rust: leds: add basic led classdev abstractions

[Severity: Low]
This isn't a bug, but does this violate the subsystem formatting rules?
The LED subsystem guidelines state that the description after the subsystem
prefix should always be capitalized for MFD, LED, and Backlight subsystems.
Should "add" be capitalized here?

> diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
> index 0000000000000..deda8cc548a8c
> --- /dev/null
> +++ b/rust/kernel/led.rs

[ ... ]

> @@ -249,6 +249,7 @@ impl TryFrom<u32> for Color {
>      type Error = Error;
>  
>      fn try_from(value: u32) -> core::result::Result<Self, Self::Error> {

[Severity: Low]
This isn't a bug, but could this small forwarding function be missing the
#[inline] attribute? According to the Rust subsystem guidelines, small
functions or those forwarding to C bindings in abstractions should be
annotated with #[inline].

> diff --git a/rust/kernel/led/normal.rs b/rust/kernel/led/normal.rs
> index 0000000000000..a22c29cfd2628
> --- /dev/null
> +++ b/rust/kernel/led/normal.rs

[ ... ]

> @@ -23,6 +23,7 @@ impl<'init, S: DeviceBuilderState> DeviceBuilder<'init, S> {
>      pub fn build<'bound: 'init, T: LedOps + 'bound>(
>          self,
>          parent: &'bound device::Device<Bound>,
>          ops: impl PinInit<T, Error> + 'init,
>      ) -> impl PinInit<Device<'bound, T>, Error> + 'init {

[Severity: High]
Does this inverted lifetime constraint '<'bound: 'init>' allow the returned
Device to outlive the borrowed C strings? The DeviceBuilder holds string
references tied to the 'init lifetime. This signature means 'bound can
outlive 'init.

[ ... ]

> @@ -43,6 +43,7 @@ impl<'init, S: DeviceBuilderState> DeviceBuilder<'init, S> {
>                          max_brightness: T::MAX_BRIGHTNESS,
>                          brightness: self.initial_brightness,
>                          color: self.color as u32,
>                          name: self.name.map_or(core::ptr::null(), CStrExt::as_char_ptr),
>                          ..bindings::led_classdev::default()

[Severity: High]
Could this raw pointer coercion lead to a use-after-free? This strips the
'init lifetime, storing a potentially short-lived pointer into the long-lived
C struct. A caller could pass a local CString, and since the returned
Device<'bound, T> lacks the 'init lifetime parameter, the C LED subsystem
might retain these raw pointers for sysfs and logging after the string is
dropped. Should the constraint be 'init: 'bound instead?

[ ... ]

> @@ -66,13 +66,9 @@ impl<'init, S: DeviceBuilderState> DeviceBuilder<'init, S> {
>                  // SAFETY:
>                  // - `parent.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_raw(),
>                          ptr,
>                          if self.name.is_none() {
>                              &raw mut init_data
>                          } else {
>                              core::ptr::null_mut()
>                          },
>                      )
>                  })?;

[Severity: High]
Does using the unmanaged led_classdev_register_ext combined with PinnedDrop
break devres cleanup ordering? In Rust, driver data structs embedding
led::Device are allocated early in probe() using devm. Subsequent devm
allocations register their cleanup actions after the struct's drop action.

Since this unmanaged C API relies entirely on the struct's PinnedDrop to call
led_classdev_unregister, devres will execute cleanup in reverse order on
device removal, freeing IRQs and I/O memory before PinnedDrop unregisters
the LED.

Can a concurrent write to brightness_set access already-freed I/O memory
during this window?

[ ... ]

> @@ -103,6 +103,7 @@ impl<'bound, T: LedOps + 'bound> Deref for Device<'bound, T> {
>      type Target = T;
>  
>      fn deref(&self) -> &Self::Target {
>          &self.ops
>      }

[Severity: Low]
This isn't a bug, but could this small forwarding function be missing the
#[inline] attribute? The Rust subsystem guidelines suggest adding it for
small functions in abstractions.

[ ... ]

> @@ -182,10 +182,10 @@ impl<T: LedOps> Adapter<T> {
>              let classdev = unsafe { Device::<T>::from_raw(led_cdev) };
>  
>              classdev.blink_set(
>                  // 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 },
>              )?;

[Severity: High]
Does unconditionally creating exclusive &mut usize references here trigger
Undefined Behavior?

The LED subsystem in C calls led_blink_set directly from sysfs store
handlers without holding locks. Multiple threads can concurrently write to
sysfs and invoke blink_set_callback with pointers to the shared
led_cdev->blink_delay_on and blink_delay_off fields.

Having multiple concurrent &mut references to the same memory location
violates Rust's aliasing rules.

[ ... ]

> @@ -203,9 +203,6 @@ impl<'bound, T: 'bound> PinnedDrop for Device<'bound, T> {
>          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::DeviceBuilder::build`.
>              .map(|fwnode| unsafe { ARef::from_raw(NonNull::from(fwnode)) });

[Severity: High]
Is recovering the ARef<FwNode> from the C struct device in PinnedDrop sound?

If self.fwnode in the builder was None, no reference was leaked. However,
during registration, the driver core or subsystem may automatically assign a
fwnode to the device.

In this case, dev.fwnode() would return Some, and the Rust drop logic would
blindly steal ownership of an unowned fwnode. Could this decrement its
reference count and cause an underflow and use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913-rust_leds-v25-0-1a10371d78c3@posteo.de?part=1

Re: [PATCH v25 3/4] rust: leds: add multicolor classdev abstractions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Safe methods `brightness()` and `intensity()` in `MultiColorSubLed` perform non-atomic plain reads that race with C subsystem writes, causing Undefined Behavior.
- [High] The `MultiColorDevice` struct fails to capture the `'init` lifetime constraint from `DeviceBuilder`, allowing the device name `CStr` to be dropped prematurely and leaving a dangling pointer in the C framework.
- [Medium] Logic in `build_multicolor` permanently leaks `fwnode` refcounts when a custom device name is provided.
--

commit 7485bc19e8ccc2afa9099cb7107096ea73b39fe0
Author: Markus Probst <markus.probst@posteo.de>

rust: leds: add multicolor classdev abstractions

This commit implements the abstractions needed for multicolor led class devices.
It introduces MultiColor, MultiColorSubLed, and MultiColorDevice, along with
a DeviceBuilder function to register a new multicolor led class device.

> diff --git a/rust/kernel/led/multicolor.rs b/rust/kernel/led/multicolor.rs
> new file mode 100644
> index 0000000000000..309487bdf38aa
> --- /dev/null
> +++ b/rust/kernel/led/multicolor.rs
[ ... ]
> +    /// The intensity of the sub led.
> +    #[inline]
> +    pub const fn intensity(&self) -> u32 {
> +        // SAFETY:
> +        // - `self.intensity.get()` is a valid pointer to `u32`.
> +        // - We don't have exclusive or immutable access to `self.intensity`,
> +        //   but the alignment should prevent "load tearing".
> +        unsafe { *self.intensity.get() }
> +    }
> +
> +    /// The brightness of the sub led.
> +    #[inline]
> +    pub const fn brightness(&self) -> u32 {
> +        // SAFETY:
> +        // - `self.brightness.get()` is a valid pointer to `u32`.
> +        // - We don't have exclusive or immutable access to `self.brightness`,
> +        //   but the alignment should prevent "load tearing".
> +        unsafe { *self.brightness.get() }
> +    }

[Severity: High]
Does this plain read trigger undefined behavior due to a data race?

Since MultiColorSubLed implements Sync, references can be shared safely
across threads. The underlying C subsystem mutates these exact fields
concurrently. For example, multi_intensity_store() in led-class-multicolor.c
modifies intensity from sysfs, and led_mc_calc_color_components() modifies
brightness in softirq context.

In the Rust memory model, a data race on a non-atomic memory access is strict
undefined behavior, regardless of architecture-level load tearing.

[ ... ]
> +#[pin_data(PinnedDrop)]
> +pub struct MultiColorDevice<'bound, T: 'bound = ()> {
> +    #[pin]
> +    ops: T,
> +    #[pin]
> +    classdev: Opaque<bindings::led_classdev_mc>,
> +    _p: PhantomData<&'bound ()>,
> +}
> +
> +impl<'init, S: DeviceBuilderState> DeviceBuilder<'init, S> {
> +    /// Registers a new [`MulticolorDevice`].
> +    pub fn build_multicolor<'bound: 'init, T: LedOps<Mode = MultiColor> + 'bound>(
> +        self,
> +        parent: &'bound device::Device<Bound>,
> +        ops: impl PinInit<T, Error> + 'init,
> +        subleds: &'init [MultiColorSubLed],
> +    ) -> impl PinInit<MultiColorDevice<'bound, T>, Error> + 'init {
[ ... ]
> +                unsafe {
> +                    ptr.write(bindings::led_classdev_mc {
> +                        led_cdev: bindings::led_classdev {
[ ... ]
> +                            name: self.name.map_or(core::ptr::null(), CStrExt::as_char_ptr),

[Severity: High]
Could this result in a dangling pointer in the C framework?

DeviceBuilder accepts an &'init CStr for the device name. In build_multicolor,
this bounded reference is converted to a raw pointer and written into the name
field of the C struct bindings::led_classdev_mc.

Because the instantiated MultiColorDevice type does not include the 'init
lifetime parameter, the lifetime is erased. Can this allow safe Rust code to
dynamically allocate a CString, pass a reference to name(), initialize the
device, and then safely drop the CString while the device remains alive and
the C struct still holds the name pointer?

[ ... ]
> +                to_result(unsafe {
> +                    bindings::led_classdev_multicolor_register_ext(
> +                        parent.as_ref().as_raw(),
> +                        ptr,
> +                        if self.name.is_none() {
> +                            &raw mut init_data
> +                        } else {
> +                            core::ptr::null_mut()
> +                        },
> +                    )
> +                })?;
> +
> +                subled_guard.dismiss();
> +
> +                core::mem::forget(self.fwnode); // keep the reference count incremented

[Severity: Medium]
Does this logic permanently leak the fwnode reference count when a custom
device name is provided?

When self.name is Some, core::ptr::null_mut() is passed instead of init_data.
This bypasses init_data, meaning the LED subsystem never associates the fwnode
with the created device (dev->fwnode remains unset).

Despite this, core::mem::forget(self.fwnode) is called unconditionally.
During teardown in PinnedDrop, dev.fwnode() will correctly return None since
the C core never set it. As a result, the ARef is never reconstructed and the
reference count is permanently leaked.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913-rust_leds-v25-0-1a10371d78c3@posteo.de?part=3