[PATCH v18 0/3] rust: leds: add led classdev abstractions

[PATCH v18 0/3] rust: leds: add led classdev abstractions

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

This series depends on [2].

[1] https://lore.kernel.org/rust-for-linux/DDDS2V0V2NVJ.16ZKXCKUA1HUV@kernel.org/
[2] https://lore.kernel.org/rust-for-linux/20260525202921.124698-1-dakr@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 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 (3):
      rust: leds: add basic led classdev abstractions
      rust: leds: add Mode trait
      rust: leds: add multicolor classdev abstractions

 MAINTAINERS                     |   8 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/led.rs              | 339 +++++++++++++++++++++++++++++++++
 rust/kernel/led/multicolor.rs   | 405 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/led/normal.rs       | 237 +++++++++++++++++++++++
 rust/kernel/lib.rs              |   1 +
 6 files changed, 991 insertions(+)
---
base-commit: 9e171fc1d7d7ab847a750c03571c87ac3c17bd84
change-id: 20251114-rust_leds-a959f7c2f7f9
prerequisite-message-id: 20260505152400.3905096-1-dakr@kernel.org
prerequisite-patch-id: d2aebf69b153c039bbed1d0ed26906708fd22534
prerequisite-patch-id: 84b28da2f5de20fc1785095c647b2ffc35d969a5
prerequisite-patch-id: 67318671a5eed5fb4ad23a450f1cf0e442bf8ca2
prerequisite-message-id: 20260525202921.124698-1-dakr@kernel.org
prerequisite-patch-id: b84db329d4372a175cb8d49e4e88c3eecf7eb228
prerequisite-patch-id: 2c30303f409cc8288cc87e241920219f5ddd8390
prerequisite-patch-id: 4e4f0ad370d763ad00b0f75b91fa216f2cc95953
prerequisite-patch-id: 5bcd6b37f3498feebda275dfef78136eba34004e
prerequisite-patch-id: 872b0982f3e5e7d1698d9df3b325e4cd27b27789
prerequisite-patch-id: 3a3c7749e017d9335f58497404d1350e96caf471
prerequisite-patch-id: 3526c9154f581497a11465b936d83ef61a875454
prerequisite-patch-id: 65d8c757b52475c2acc7d22ddc92cd3f0152b55d
prerequisite-patch-id: 4bd31f1414d5248dc080884caadf5f21684a8427
prerequisite-patch-id: 7beadbb0da3e589ed86d12f512d1c83427dd82b4
prerequisite-patch-id: 12cd0f67ffd27347f90c065db491945908206b7f
prerequisite-patch-id: 4642e31f66331f6c3b579377111ea733dbb3a11c
prerequisite-patch-id: 52d67b40b4396c741e2222d6a5bc7927abcb77aa
prerequisite-patch-id: 74ca82ff26cf9c7a993757c87db8be62006e820f
prerequisite-patch-id: 466fb9fa7febbffd8ef51b311c7d9893c11fc0f0
prerequisite-patch-id: e515cd98b06e26721cbbe6a4fbacd251d0073b63
prerequisite-patch-id: 8dc8e75d9f6499a554ef7e474bbacdbf3660a9f2
prerequisite-patch-id: 5fdb9f71dca2f44dd293760a60db125b770f1f55
prerequisite-patch-id: c766a24c2d5064f5ec09daada0b8e8fba862d3aa
prerequisite-patch-id: b768f6456d35fa7a80c015e34bbdba6082dbd593
prerequisite-patch-id: 6a8b17234f12f7084e6e2ce843a7031b0a891ce4
prerequisite-patch-id: 98b2deb9e60c1f28f90c5ee34fd608aaa9fd9420
prerequisite-patch-id: 774b29be66e641ee50cedb4704cf49d8b9fabf50
prerequisite-patch-id: cf95dc936cfc4b3a7a363435a51a48d9009645b3

[PATCH v18 1/3] 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>
---
 MAINTAINERS               |   8 ++
 rust/kernel/led.rs        | 288 ++++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/led/normal.rs | 229 ++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs        |   1 +
 4 files changed, 526 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4e118f704699..0bc8f7ca9ff1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14533,6 +14533,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
diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
new file mode 100644
index 000000000000..c92d99d68497
--- /dev/null
+++ b/rust/kernel/led.rs
@@ -0,0 +1,288 @@
+// 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, //
+    },
+    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<'a, S> {
+    fwnode: Option<ARef<FwNode>>,
+    name: Option<&'a CStr>,
+    devicename: Option<&'a 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<'a> DeviceBuilder<'a, 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: &'a 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<'a, S: DeviceBuilderState> DeviceBuilder<'a, 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<'a> DeviceBuilder<'a, 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: &'a CStr) -> Self {
+        Self {
+            name: Some(name),
+            ..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<'bound>(
+///         &self,
+///         _dev: &'bound platform::Device<device::Bound>,
+///         _classdev: &led::Device<'bound, Self>,
+///         _brightness: u32
+///     ) -> Result<()> {
+///         // Set the brightness for the led here
+///         Ok(())
+///     }
+/// }
+/// ```
+/// Led drivers must implement this trait in order to register and handle a [`Device`].
+#[vtable]
+pub trait LedOps: Send + Sync + Sized {
+    /// The bus device required by the implementation.
+    #[allow(private_bounds)]
+    type Bus: AsBusDevice<Bound>;
+
+    /// If set true, [`LedOps::brightness_set`] and [`LedOps::blink_set`] must perform the
+    /// operation immediately. If set false, they must not sleep.
+    const BLOCKING: bool;
+    /// The max brightness level.
+    const MAX_BRIGHTNESS: u32;
+
+    /// Sets the brightness level.
+    ///
+    /// See also [`LedOps::BLOCKING`].
+    fn brightness_set<'bound>(
+        &self,
+        dev: &'bound Self::Bus,
+        classdev: &Device<'bound, Self>,
+        brightness: u32,
+    ) -> Result<()>;
+
+    /// Gets the current brightness level.
+    fn brightness_get<'bound>(
+        &self,
+        dev: &'bound Self::Bus,
+        classdev: &Device<'bound, Self>,
+    ) -> Result<u32> {
+        let _ = (dev, classdev);
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Activates hardware accelerated blinking.
+    ///
+    /// delays are in milliseconds. If both are zero, a sensible default should be chosen.
+    /// The caller should adjust the timings in that case and if it can't match the values
+    /// specified exactly. Setting the brightness to 0 will disable the hardware accelerated
+    /// blinking.
+    ///
+    /// See also [`LedOps::BLOCKING`].
+    fn blink_set<'bound>(
+        &self,
+        dev: &'bound Self::Bus,
+        classdev: &Device<'bound, Self>,
+        delay_on: &mut usize,
+        delay_off: &mut usize,
+    ) -> Result<()> {
+        let _ = (dev, classdev, delay_on, delay_off);
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+}
+
+/// 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..09244133f14c
--- /dev/null
+++ b/rust/kernel/led/normal.rs
@@ -0,0 +1,229 @@
+// 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 class device representation.
+///
+/// This structure represents the Rust abstraction for a led class device.
+#[pin_data(PinnedDrop)]
+pub struct Device<'bound, T: LedOps + 'bound> {
+    #[pin]
+    ops: T,
+    #[pin]
+    classdev: Opaque<bindings::led_classdev>,
+    _p: PhantomData<&'bound ()>,
+}
+
+impl<'a, S: DeviceBuilderState> DeviceBuilder<'a, S> {
+    /// Registers a new [`Device`].
+    pub fn build<'bound: 'a, T: LedOps + 'bound>(
+        self,
+        parent: &'bound T::Bus,
+        ops: impl PinInit<T, Error> + 'a,
+    ) -> impl PinInit<Device<'bound, T>, Error> + 'a {
+        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_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,
+                        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: LedOps + 'bound> Device<'bound, T> {
+    /// # 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) }
+    }
+
+    #[inline]
+    fn parent(&self) -> &'bound 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<'bound, T: LedOps + '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: LedOps + '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) };
+        // 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()) };
+
+        // CAST: Resulting value will be casted back to i32 in the led subsystem.
+        from_result(|| {
+            classdev
+                .ops
+                .brightness_get(parent, classdev)
+                .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) };
+            // 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<'bound, T: LedOps + '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::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) };
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index b72b2fbe046d..7a18c738c934 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -89,6 +89,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.53.0

[PATCH v18 2/3] 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        | 27 +++++++++++++++++++++++----
 rust/kernel/led/normal.rs | 22 +++++++++++++++-------
 2 files changed, 38 insertions(+), 11 deletions(-)

diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
index c92d99d68497..6ee337008db7 100644
--- a/rust/kernel/led.rs
+++ b/rust/kernel/led.rs
@@ -32,7 +32,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 {}
@@ -161,6 +164,7 @@ pub fn name(self, name: &'a CStr) -> 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;
 ///
@@ -182,6 +186,11 @@ pub trait LedOps: Send + Sync + Sized {
     #[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;
@@ -194,7 +203,7 @@ pub trait LedOps: Send + Sync + Sized {
     fn brightness_set<'bound>(
         &self,
         dev: &'bound Self::Bus,
-        classdev: &Device<'bound, Self>,
+        classdev: &<Self::Mode as Mode>::Device<'bound, Self>,
         brightness: u32,
     ) -> Result<()>;
 
@@ -202,7 +211,7 @@ fn brightness_set<'bound>(
     fn brightness_get<'bound>(
         &self,
         dev: &'bound Self::Bus,
-        classdev: &Device<'bound, Self>,
+        classdev: &<Self::Mode as Mode>::Device<'bound, Self>,
     ) -> Result<u32> {
         let _ = (dev, classdev);
         build_error!(VTABLE_DEFAULT_ERROR)
@@ -219,7 +228,7 @@ fn brightness_get<'bound>(
     fn blink_set<'bound>(
         &self,
         dev: &'bound Self::Bus,
-        classdev: &Device<'bound, Self>,
+        classdev: &<Self::Mode as Mode>::Device<'bound, Self>,
         delay_on: &mut usize,
         delay_off: &mut usize,
     ) -> Result<()> {
@@ -283,6 +292,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>;
+}
+
 mod private {
     pub trait Sealed {}
 }
diff --git a/rust/kernel/led/normal.rs b/rust/kernel/led/normal.rs
index 09244133f14c..2ffe65bb9dc2 100644
--- a/rust/kernel/led/normal.rs
+++ b/rust/kernel/led/normal.rs
@@ -6,11 +6,19 @@
 
 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.
 #[pin_data(PinnedDrop)]
-pub struct Device<'bound, T: LedOps + 'bound> {
+pub struct Device<'bound, T: LedOps<Mode = Normal> + 'bound> {
     #[pin]
     ops: T,
     #[pin]
@@ -20,7 +28,7 @@ pub struct Device<'bound, T: LedOps + 'bound> {
 
 impl<'a, S: DeviceBuilderState> DeviceBuilder<'a, S> {
     /// Registers a new [`Device`].
-    pub fn build<'bound: 'a, T: LedOps + 'bound>(
+    pub fn build<'bound: 'a, T: LedOps<Mode = Normal> + 'bound>(
         self,
         parent: &'bound T::Bus,
         ops: impl PinInit<T, Error> + 'a,
@@ -87,7 +95,7 @@ pub fn build<'bound: 'a, T: LedOps + 'bound>(
     }
 }
 
-impl<'bound, T: LedOps + 'bound> Device<'bound, T> {
+impl<'bound, T: LedOps<Mode = Normal> + 'bound> Device<'bound, T> {
     /// # Safety
     /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
     /// `led::Device`.
@@ -106,17 +114,17 @@ fn parent(&self) -> &'bound device::Device<Bound> {
 }
 
 // SAFETY: A `led::Device` can be unregistered from any thread.
-unsafe impl<'bound, T: LedOps + 'bound + Send> Send for Device<'bound, T> {}
+unsafe impl<'bound, T: LedOps<Mode = Normal> + '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: LedOps + 'bound + Sync> Sync for Device<'bound, T> {}
+unsafe impl<'bound, T: LedOps<Mode = Normal> + 'bound + Sync> Sync for Device<'bound, T> {}
 
 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`.
@@ -209,7 +217,7 @@ impl<T: LedOps> Adapter<T> {
 }
 
 #[pinned_drop]
-impl<'bound, T: LedOps + 'bound> PinnedDrop for Device<'bound, T> {
+impl<'bound, T: LedOps<Mode = Normal> + '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

-- 
2.53.0

[PATCH v18 3/3] 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   | 405 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 439 insertions(+), 1 deletion(-)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 446dbeaf0866..17b5461453e0 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -67,6 +67,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 6ee337008db7..4540c4e5c9d7 100644
--- a/rust/kernel/led.rs
+++ b/rust/kernel/led.rs
@@ -30,8 +30,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, //
@@ -255,7 +263,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,
@@ -296,7 +321,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>;
diff --git a/rust/kernel/led/multicolor.rs b/rust/kernel/led/multicolor.rs
new file mode 100644
index 000000000000..18c9d36221f1
--- /dev/null
+++ b/rust/kernel/led/multicolor.rs
@@ -0,0 +1,405 @@
+// 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,
+    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(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.
+    #[inline]
+    pub const fn new(color: Color) -> Self {
+        Self {
+            color,
+            brightness: 0,
+            intensity: 0,
+            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 = 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<'bound, T: LedOps<Mode = MultiColor> + 'bound> {
+    #[pin]
+    ops: T,
+    #[pin]
+    classdev: Opaque<bindings::led_classdev_mc>,
+    _p: PhantomData<&'bound ()>,
+}
+
+impl<'a, S: DeviceBuilderState> DeviceBuilder<'a, S> {
+    /// Registers a new [`MulticolorDevice`].
+    pub fn build_multicolor<'bound: 'a, T: LedOps<Mode = MultiColor> + 'bound>(
+        self,
+        parent: &'bound T::Bus,
+        ops: impl PinInit<T, Error> + 'a,
+        subleds: &'a [MultiColorSubLed],
+    ) -> impl PinInit<MultiColorDevice<'bound, T>, Error> + 'a {
+        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| {
+                for (index_a, subled_a) in subleds.iter().enumerate() {
+                    for (index_b, subled_b) in subleds.iter().enumerate() {
+                        if index_a != index_b && subled_a.color == subled_b.color {
+                            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);
+
+                let subled_guard = ScopeGuard::new(|| {
+                    // 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) });
+                });
+
+                // 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(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 = 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: LedOps<Mode = MultiColor> + 'bound> MultiColorDevice<'bound, T> {
+    /// # 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) }
+    }
+
+    #[inline]
+    fn parent(&self) -> &'bound 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`].
+    #[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,
+            )
+        }
+    }
+}
+
+// SAFETY: A `led::MultiColorDevice` can be unregistered from any thread.
+unsafe impl<'bound, T: LedOps<Mode = MultiColor> + '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: LedOps<Mode = MultiColor> + '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.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()) };
+
+        // CAST: Resulting value will be casted back to i32 in the led subsystem.
+        from_result(|| {
+            classdev
+                .ops
+                .brightness_get(parent, classdev)
+                .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) };
+            // 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<'bound, T: LedOps<Mode = MultiColor> + '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::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.53.0

Re: [PATCH v18 1/3] rust: leds: add basic led classdev abstractions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `Device` abstraction drops the `'a` lifetime of the `name` reference, leading to a potential use-after-free of the LED name.
--

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

rust: leds: add basic led classdev abstractions

Implement the core abstractions needed for led class devices, including
LedOps, DeviceBuilder, and Device.

> diff --git a/rust/kernel/led/normal.rs b/rust/kernel/led/normal.rs
> new file mode 100644
> index 0000000000000..09244133f14c8
> --- /dev/null
> +++ b/rust/kernel/led/normal.rs

[ ... ]

> +impl<'a, S: DeviceBuilderState> DeviceBuilder<'a, S> {
> +    /// Registers a new [`Device`].
> +    pub fn build<'bound: 'a, T: LedOps + 'bound>(
> +        self,
> +        parent: &'bound T::Bus,
> +        ops: impl PinInit<T, Error> + 'a,
> +    ) -> impl PinInit<Device<'bound, T>, Error> + 'a {
> +        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),

[Severity: High]
Does this erase the lifetime of the name string and risk a use-after-free?

In DeviceBuilder::build, the builder takes a string reference tied to the 'a
lifetime, but the returned Device struct does not capture this lifetime. The
raw pointer from the name is then written directly into the long-lived
led_classdev struct.

If the name string is dropped immediately after the Device is initialized
(for example, if a dynamically allocated CString was used in the driver probe
function), the led_classdev struct registered with the C subsystem will
contain a dangling pointer.

Could this lead to memory corruption if C code inspects the global leds_list
and dereferences the dangling name pointer?

> +                        ..bindings::led_classdev::default()
> +                    })
> +                };

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260531-rust_leds-v18-0-d07102ba5170@posteo.de?part=1

Re: [PATCH v18 2/3] rust: leds: add Mode trait

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Flawed fwnode reference count management in `led::Device` leads to refcount underflow or memory leaks.
- [High] Inverted lifetime bound `'bound: 'a` in `DeviceBuilder::build` allows the `Device` to outlive its name string, causing a Use-After-Free.
--

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

rust: leds: add Mode trait
    
Add the led::Mode trait to allow for other types of led class devices
in led::LedOps.

> diff --git a/rust/kernel/led/normal.rs b/rust/kernel/led/normal.rs
> index 09244133f14c8..2ffe65bb9dc2c 100644
> --- a/rust/kernel/led/normal.rs
> +++ b/rust/kernel/led/normal.rs

[ ... ]

> @@ -20,7 +28,7 @@ pub struct Device<'bound, T: LedOps + 'bound> {
>  
>  impl<'a, S: DeviceBuilderState> DeviceBuilder<'a, S> {
>      /// Registers a new [`Device`].
> -    pub fn build<'bound: 'a, T: LedOps + 'bound>(
> +    pub fn build<'bound: 'a, T: LedOps<Mode = Normal> + 'bound>(

[Severity: High]
Does this inverted lifetime bound ('bound: 'a) allow the device to outlive its
name string?

It appears that a caller can safely initialize a long-lived device (e.g.
'static) using a short-lived local string, because the returned PinInit does
not capture 'a.

Since the short-lived name pointer is assigned to the persistent C struct:

    name: self.name.map_or(core::ptr::null(), CStrExt::as_char_ptr),

Does dropping the local string leave a dangling pointer in the C struct,
causing a use-after-free when the LED subsystem accesses led_cdev->name?

[Severity: High]
Can the fwnode reference count management here lead to underflows or memory
leaks?

In DeviceBuilder::build(), if self.fwnode is Some, its ARef is leaked:

    core::mem::forget(self.fwnode); // keep the reference count incremented

But if it was None, no reference is leaked.

>          self,
>          parent: &'bound T::Bus,
>          ops: impl PinInit<T, Error> + 'a,

[ ... ]

> @@ -209,7 +217,7 @@ impl<T: LedOps> Adapter<T> {
>  }
>  
>  #[pinned_drop]
> -impl<'bound, T: LedOps + 'bound> PinnedDrop for Device<'bound, T> {
> +impl<'bound, T: LedOps<Mode = Normal> + 'bound> PinnedDrop for Device<'bound, T> {

[Severity: High]
When <Device as PinnedDrop>::drop attempts to recover the fwnode reference from
the mutable C field dev->fwnode:

    let _fwnode = dev
        .fwnode()
        .map(|fwnode| unsafe { ARef::from_raw(NonNull::from(fwnode)) });

If self.fwnode was None, but the C core populates dev->fwnode (e.g. device_add
sets it to parent->fwnode), will PinnedDrop steal and drop a reference it does
not own, causing a refcount underflow?

Conversely, if self.fwnode was Some, but dev->fwnode is later cleared or
overwritten by C code, will the ARef never be dropped, causing a memory leak?

>      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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260531-rust_leds-v18-0-d07102ba5170@posteo.de?part=2

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Data race and Undefined Behavior due to `&[MultiColorSubLed]` exposing concurrently mutated plain `u32` fields.
- [High] Reconstructing KVec with a hardcoded capacity violates the `KVec::from_raw_parts` safety contract.
- [High] FwNode reference count leak or Use-After-Free when `name` is explicitly set.
- [Low] Dead documentation link for `MultiColorSubLed::brightness`.
- [Low] Missing `#[inline]` attribute on the `from_raw` internal helper function.
--

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

rust: leds: add multicolor classdev abstractions
    
Implement the abstractions needed for multicolor led class devices,
including `led::MultiColor` and `MultiColorDevice`.

> diff --git a/rust/kernel/led/multicolor.rs b/rust/kernel/led/multicolor.rs
> new file mode 100644
> index 0000000000000..18c9d36221f1e
> --- /dev/null
> +++ b/rust/kernel/led/multicolor.rs

[ ... ]

> +#[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`.

[Severity: Low]
Is the reference to MultiColor::pre_brightness_set correct?  This function
does not appear to exist in the code, and MultiColor is a marker enum.

> +    pub brightness: u32,
> +    /// the intensity of the sub led.
> +    pub intensity: u32,
> +    /// arbitrary data for the driver to store.
> +    pub channel: u32,
> +}

[Severity: High]
Does this expose concurrently mutated fields as plain integers?

When the C kernel LED core modifies the subled values (for instance, via
led_mc_calc_color_components modifying brightness during sysfs writes),
these fields are mutated without Rust-visible synchronization.

Since MultiColorDevice::subleds() hands out a shared, immutable slice
&[MultiColorSubLed] to the driver, does using plain u32 instead of
interior mutability primitives like Opaque or UnsafeCell violate strict
aliasing invariants and risk undefined behavior?

[ ... ]

> +                let subled_guard = ScopeGuard::new(|| {
> +                    // 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) });
> +                });

[Severity: High]
Is it safe to hardcode the capacity to num_colors when reconstructing
this KVec?

The original KVec was dynamically allocated using extend_from_slice(subleds)
which could potentially over-allocate.  Since the exact allocated capacity 
is verified but then discarded, reconstructing the KVec here (and during 
PinnedDrop) with num_colors as the capacity might violate 
KVec::from_raw_parts safety contract.

Should the exact capacity be stored, or perhaps a boxed slice (KBox<[T]>)
used instead to ensure the allocator frees the correct size?

[ ... ]

> +                // 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

[Severity: High]
Could this lead to a fwnode reference count leak or a use-after-free if
self.name is configured?

If self.name is set, a null pointer is passed to register the device instead
of &mut init_data, meaning the C device does not inherit the provided fwnode.
However, self.fwnode is unconditionally forgotten, which leaves its reference
count incremented.

Later, during device drop, dev.fwnode() is queried to decrement the refcount.
If the device has no fwnode, it returns None, permanently leaking the original
fwnode. If the device inherited a fwnode from its parent, dev.fwnode() would
return the parent's fwnode, incorrectly decrementing it and potentially causing
a use-after-free of the parent's fwnode.

[ ... ]

> +impl<'bound, T: LedOps<Mode = MultiColor> + 'bound> MultiColorDevice<'bound, T> {
> +    /// # 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 {

[Severity: Low]
this isn't a bug, but should this internal helper function have an #[inline]
attribute?

According to the Rust subsystem guidelines, small internal helper functions
within abstractions should be annotated with #[inline]. Since this method
consists exclusively of unsafe pointer arithmetic (container_of!), it seems
it should be inlined.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260531-rust_leds-v18-0-d07102ba5170@posteo.de?part=3

Re: [PATCH v18 1/3] rust: leds: add basic led classdev abstractions

[Markus Probst]

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

On Sat, 2026-05-30 at 23:05 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The `Device` abstraction drops the `'a` lifetime of the `name` reference, leading to a potential use-after-free of the LED name.
> --
> 
> commit bc77118736b867c88c1854d7a5c4eb57818535fd
> Author: Markus Probst <markus.probst@posteo.de>
> 
> rust: leds: add basic led classdev abstractions
> 
> Implement the core abstractions needed for led class devices, including
> LedOps, DeviceBuilder, and Device.
> 
> > diff --git a/rust/kernel/led/normal.rs b/rust/kernel/led/normal.rs
> > new file mode 100644
> > index 0000000000000..09244133f14c8
> > --- /dev/null
> > +++ b/rust/kernel/led/normal.rs
> 
> [ ... ]
> 
> > +impl<'a, S: DeviceBuilderState> DeviceBuilder<'a, S> {
> > +    /// Registers a new [`Device`].
> > +    pub fn build<'bound: 'a, T: LedOps + 'bound>(
> > +        self,
> > +        parent: &'bound T::Bus,
> > +        ops: impl PinInit<T, Error> + 'a,
> > +    ) -> impl PinInit<Device<'bound, T>, Error> + 'a {
> > +        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),
> 
> [Severity: High]
> Does this erase the lifetime of the name string and risk a use-after-free?
> 
> In DeviceBuilder::build, the builder takes a string reference tied to the 'a
> lifetime, but the returned Device struct does not capture this lifetime. The
> raw pointer from the name is then written directly into the long-lived
> led_classdev struct.
> 
> If the name string is dropped immediately after the Device is initialized
> (for example, if a dynamically allocated CString was used in the driver probe
> function), the led_classdev struct registered with the C subsystem will
> contain a dangling pointer.
> 
> Could this lead to memory corruption if C code inspects the global leds_list
> and dereferences the dangling name pointer?
false positive. The string gets copied in registration and doesn't have
to outlive the device.

> 
> > +                        ..bindings::led_classdev::default()
> > +                    })
> > +                };
> 
> [ ... ]

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

Re: [PATCH v18 2/3] rust: leds: add Mode trait

[Markus Probst]

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

On Sat, 2026-05-30 at 23:17 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Flawed fwnode reference count management in `led::Device` leads to refcount underflow or memory leaks.
> - [High] Inverted lifetime bound `'bound: 'a` in `DeviceBuilder::build` allows the `Device` to outlive its name string, causing a Use-After-Free.
> --
> 
> commit 79f921a7fb31b3f89538463530e40a222e664817
> Author: Markus Probst <markus.probst@posteo.de>
> 
> rust: leds: add Mode trait
>     
> Add the led::Mode trait to allow for other types of led class devices
> in led::LedOps.
> 
> > diff --git a/rust/kernel/led/normal.rs b/rust/kernel/led/normal.rs
> > index 09244133f14c8..2ffe65bb9dc2c 100644
> > --- a/rust/kernel/led/normal.rs
> > +++ b/rust/kernel/led/normal.rs
> 
> [ ... ]
> 
> > @@ -20,7 +28,7 @@ pub struct Device<'bound, T: LedOps + 'bound> {
> >  
> >  impl<'a, S: DeviceBuilderState> DeviceBuilder<'a, S> {
> >      /// Registers a new [`Device`].
> > -    pub fn build<'bound: 'a, T: LedOps + 'bound>(
> > +    pub fn build<'bound: 'a, T: LedOps<Mode = Normal> + 'bound>(
> 
> [Severity: High]
> Does this inverted lifetime bound ('bound: 'a) allow the device to outlive its
> name string?
> 
> It appears that a caller can safely initialize a long-lived device (e.g.
> 'static) using a short-lived local string, because the returned PinInit does
> not capture 'a.
> 
> Since the short-lived name pointer is assigned to the persistent C struct:
> 
>     name: self.name.map_or(core::ptr::null(), CStrExt::as_char_ptr),
> 
> Does dropping the local string leave a dangling pointer in the C struct,
> causing a use-after-free when the LED subsystem accesses led_cdev->name?
> 
> [Severity: High]
> Can the fwnode reference count management here lead to underflows or memory
> leaks?
> 
> In DeviceBuilder::build(), if self.fwnode is Some, its ARef is leaked:
> 
>     core::mem::forget(self.fwnode); // keep the reference count incremented
> 
> But if it was None, no reference is leaked.
> 
> >          self,
> >          parent: &'bound T::Bus,
> >          ops: impl PinInit<T, Error> + 'a,
> 
> [ ... ]
> 
> > @@ -209,7 +217,7 @@ impl<T: LedOps> Adapter<T> {
> >  }
> >  
> >  #[pinned_drop]
> > -impl<'bound, T: LedOps + 'bound> PinnedDrop for Device<'bound, T> {
> > +impl<'bound, T: LedOps<Mode = Normal> + 'bound> PinnedDrop for Device<'bound, T> {
> 
> [Severity: High]
> When <Device as PinnedDrop>::drop attempts to recover the fwnode reference from
> the mutable C field dev->fwnode:
> 
>     let _fwnode = dev
>         .fwnode()
>         .map(|fwnode| unsafe { ARef::from_raw(NonNull::from(fwnode)) });
> 
> If self.fwnode was None, but the C core populates dev->fwnode (e.g. device_add
> sets it to parent->fwnode), will PinnedDrop steal and drop a reference it does
> not own, causing a refcount underflow?
> 
> Conversely, if self.fwnode was Some, but dev->fwnode is later cleared or
> overwritten by C code, will the ARef never be dropped, causing a memory leak?
The fwnode shouldn't change, but we could make this safer via devres.
> 
> >      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

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

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

[Markus Probst]

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

On Sat, 2026-05-30 at 23:25 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [High] Data race and Undefined Behavior due to `&[MultiColorSubLed]` exposing concurrently mutated plain `u32` fields.
> - [High] Reconstructing KVec with a hardcoded capacity violates the `KVec::from_raw_parts` safety contract.
> - [High] FwNode reference count leak or Use-After-Free when `name` is explicitly set.
> - [Low] Dead documentation link for `MultiColorSubLed::brightness`.
> - [Low] Missing `#[inline]` attribute on the `from_raw` internal helper function.
> --
> 
> commit 06862c24665d65b7a33c6a01c9dd3881267810e1
> Author: Markus Probst <markus.probst@posteo.de>
> 
> rust: leds: add multicolor classdev abstractions
>     
> Implement the abstractions needed for multicolor led class devices,
> including `led::MultiColor` and `MultiColorDevice`.
> 
> > diff --git a/rust/kernel/led/multicolor.rs b/rust/kernel/led/multicolor.rs
> > new file mode 100644
> > index 0000000000000..18c9d36221f1e
> > --- /dev/null
> > +++ b/rust/kernel/led/multicolor.rs
> 
> [ ... ]
> 
> > +#[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`.
> 
> [Severity: Low]
> Is the reference to MultiColor::pre_brightness_set correct?  This function
> does not appear to exist in the code, and MultiColor is a marker enum.
Thats a typo.

> 
> > +    pub brightness: u32,
> > +    /// the intensity of the sub led.
> > +    pub intensity: u32,
> > +    /// arbitrary data for the driver to store.
> > +    pub channel: u32,
> > +}
> 
> [Severity: High]
> Does this expose concurrently mutated fields as plain integers?
> 
> When the C kernel LED core modifies the subled values (for instance, via
> led_mc_calc_color_components modifying brightness during sysfs writes),
> these fields are mutated without Rust-visible synchronization.
> 
> Since MultiColorDevice::subleds() hands out a shared, immutable slice
> &[MultiColorSubLed] to the driver, does using plain u32 instead of
> interior mutability primitives like Opaque or UnsafeCell violate strict
> aliasing invariants and risk undefined behavior?
I don't think there is a way to guarantee this safety.

> 
> [ ... ]
> 
> > +                let subled_guard = ScopeGuard::new(|| {
> > +                    // 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) });
> > +                });
> 
> [Severity: High]
> Is it safe to hardcode the capacity to num_colors when reconstructing
> this KVec?
> 
> The original KVec was dynamically allocated using extend_from_slice(subleds)
> which could potentially over-allocate.  Since the exact allocated capacity 
> is verified but then discarded, reconstructing the KVec here (and during 
> PinnedDrop) with num_colors as the capacity might violate 
> KVec::from_raw_parts safety contract.
> 
> Should the exact capacity be stored, or perhaps a boxed slice (KBox<[T]>)
> used instead to ensure the allocator frees the correct size?
I am not aware that its possible to convert it into an KBox<[T]>, if so
I would prefer this solution. But currently it is safe, as the capacity
is defined by max(new_len, current_capacity * 2) on allication. This
will always result in new_len on the first allocation.

> 
> [ ... ]
> 
> > +                // 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
> 
> [Severity: High]
> Could this lead to a fwnode reference count leak or a use-after-free if
> self.name is configured?
> 
> If self.name is set, a null pointer is passed to register the device instead
> of &mut init_data, meaning the C device does not inherit the provided fwnode.
> However, self.fwnode is unconditionally forgotten, which leaves its reference
> count incremented.
> 
> Later, during device drop, dev.fwnode() is queried to decrement the refcount.
> If the device has no fwnode, it returns None, permanently leaking the original
> fwnode. If the device inherited a fwnode from its parent, dev.fwnode() would
> return the parent's fwnode, incorrectly decrementing it and potentially causing
> a use-after-free of the parent's fwnode.
It only needs to outlive the registration, as its copied there.
> 
> [ ... ]
> 
> > +impl<'bound, T: LedOps<Mode = MultiColor> + 'bound> MultiColorDevice<'bound, T> {
> > +    /// # 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 {
> 
> [Severity: Low]
> this isn't a bug, but should this internal helper function have an #[inline]
> attribute?
> 
> According to the Rust subsystem guidelines, small internal helper functions
> within abstractions should be annotated with #[inline]. Since this method
> consists exclusively of unsafe pointer arithmetic (container_of!), it seems
> it should be inlined.
I will add a #[inline] there.

> 
> [ ... ]

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

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

[Onur Özkan]

On Sat, 30 May 2026 22:49:58 +0000
Markus Probst <markus.probst@posteo.de> wrote:

> 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   | 405 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 439 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 446dbeaf0866..17b5461453e0 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -67,6 +67,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 6ee337008db7..4540c4e5c9d7 100644
> --- a/rust/kernel/led.rs
> +++ b/rust/kernel/led.rs
> @@ -30,8 +30,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, //
> @@ -255,7 +263,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,
> @@ -296,7 +321,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>;
> diff --git a/rust/kernel/led/multicolor.rs b/rust/kernel/led/multicolor.rs
> new file mode 100644
> index 000000000000..18c9d36221f1
> --- /dev/null
> +++ b/rust/kernel/led/multicolor.rs
> @@ -0,0 +1,405 @@
> +// 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,
> +    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(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.
> +    #[inline]
> +    pub const fn new(color: Color) -> Self {
> +        Self {
> +            color,
> +            brightness: 0,
> +            intensity: 0,
> +            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 = 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<'bound, T: LedOps<Mode = MultiColor> + 'bound> {
> +    #[pin]
> +    ops: T,
> +    #[pin]
> +    classdev: Opaque<bindings::led_classdev_mc>,
> +    _p: PhantomData<&'bound ()>,
> +}
> +
> +impl<'a, S: DeviceBuilderState> DeviceBuilder<'a, S> {
> +    /// Registers a new [`MulticolorDevice`].
> +    pub fn build_multicolor<'bound: 'a, T: LedOps<Mode = MultiColor> + 'bound>(
> +        self,
> +        parent: &'bound T::Bus,
> +        ops: impl PinInit<T, Error> + 'a,
> +        subleds: &'a [MultiColorSubLed],
> +    ) -> impl PinInit<MultiColorDevice<'bound, T>, Error> + 'a {
> +        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| {
> +                for (index_a, subled_a) in subleds.iter().enumerate() {
> +                    for (index_b, subled_b) in subleds.iter().enumerate() {
> +                        if index_a != index_b && subled_a.color == subled_b.color {
> +                            dev_err!(parent.as_ref(), "duplicate color in multicolor led\n");
> +                            return Err(EINVAL);

If we are failing on the first duplicate color then we can easily avoid O(n2)
here.

Instead of doing that, what I would do is:

	let mut list = [false; LED_COLOR_ID_MAX as usize];
	for subled in subleds {
		if list[subled.color as usize] {
			// duplication found, fail here.
		}

		list[subled.color as usize] = true;
	}

I know that we only have 15 colors today (I don't know if it that list ever
grows) but I think we should always avoid landing that kind of *potentially*
resource exhausting flows to kernel no matter how large/small the input is.
I think it's just not a good practice.

The latter should also be easier to read/follow at a glance.

Thanks,
Onur

> +                        }
> +                    }
> +                }
> +                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);
> +
> +                let subled_guard = ScopeGuard::new(|| {
> +                    // 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) });
> +                });
> +
> +                // 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(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 = 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: LedOps<Mode = MultiColor> + 'bound> MultiColorDevice<'bound, T> {
> +    /// # 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) }
> +    }
> +
> +    #[inline]
> +    fn parent(&self) -> &'bound 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`].
> +    #[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,
> +            )
> +        }
> +    }
> +}
> +
> +// SAFETY: A `led::MultiColorDevice` can be unregistered from any thread.
> +unsafe impl<'bound, T: LedOps<Mode = MultiColor> + '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: LedOps<Mode = MultiColor> + '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.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()) };
> +
> +        // CAST: Resulting value will be casted back to i32 in the led subsystem.
> +        from_result(|| {
> +            classdev
> +                .ops
> +                .brightness_get(parent, classdev)
> +                .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) };
> +            // 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<'bound, T: LedOps<Mode = MultiColor> + '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::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.53.0
>