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

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

Signed-off-by: Markus Probst <markus.probst@...teo.de>
---
 MAINTAINERS               |   1 +
 rust/kernel/device.rs     |   2 +-
 rust/kernel/led.rs        | 128 +++++++++++++++++++++++++++++++++++-----------
 rust/kernel/led/normal.rs |  39 ++++++++++++++
 4 files changed, 139 insertions(+), 31 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2d66db23a63b..03081d2ac9f8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14110,6 +14110,7 @@ L:	linux-leds@...r.kernel.org
 L:	rust-for-linux@...r.kernel.org
 S:	Maintained
 F:	rust/kernel/led.rs
+F:	rust/kernel/led/
 
 LEGO MINDSTORMS EV3
 R:	David Lechner <david@...hnology.com>
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index bf09fd0b0199..1d5d8e7581f2 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -250,7 +250,7 @@ pub(crate) fn as_raw(&self) -> *mut bindings::device {
     }
 
     /// Returns a reference to the parent device, if any.
-    #[cfg_attr(not(CONFIG_AUXILIARY_BUS), expect(dead_code))]
+    #[cfg_attr(not(any(CONFIG_AUXILIARY_BUS, CONFIG_LEDS_CLASS)), expect(dead_code))]
     pub(crate) fn parent(&self) -> Option<&Device> {
         // SAFETY:
         // - By the type invariant `self.as_raw()` is always valid.
diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
index 4c1b72c998e1..c55f9852d378 100644
--- a/rust/kernel/led.rs
+++ b/rust/kernel/led.rs
@@ -44,14 +44,18 @@
     }, //
 };
 
+mod normal;
+
+pub use normal::Normal;
+
 /// The led class device representation.
 ///
-/// This structure represents the Rust abstraction for a C `struct led_classdev`.
+/// This structure represents the Rust abstraction for a led class device.
 #[pin_data(PinnedDrop)]
 pub struct Device<T: LedOps> {
     ops: T,
     #[pin]
-    classdev: Opaque<bindings::led_classdev>,
+    classdev: Opaque<<T::Mode as private::Mode>::Type>,
 }
 
 /// The led init data representation.
@@ -156,6 +160,7 @@ pub fn color(self, color: Color) -> Self {
 /// #[vtable]
 /// impl led::LedOps for MyLedOps {
 ///     type Bus = platform::Device<device::Bound>;
+///     type Mode = led::Normal;
 ///     const BLOCKING: bool = false;
 ///     const MAX_BRIGHTNESS: u32 = 255;
 ///
@@ -187,6 +192,12 @@ pub trait LedOps: Send + 'static + Sized {
     /// The bus device required by the implementation.
     #[allow(private_bounds)]
     type Bus: AsBusDevice<Bound>;
+
+    /// The led mode to use.
+    ///
+    /// See [`Mode`].
+    type Mode: Mode;
+
     /// If set true, [`LedOps::brightness_set`] and [`LedOps::blink_set`] must perform the
     /// operation immediately. If set false, they must not sleep.
     const BLOCKING: bool;
@@ -273,6 +284,42 @@ 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::Mode {}
+
+impl<T: private::Mode> Mode for T {}
+
+type RegisterFunc<T> =
+    unsafe extern "C" fn(*mut bindings::device, *mut T, *mut bindings::led_init_data) -> i32;
+
+type UnregisterFunc<T> = unsafe extern "C" fn(*mut T);
+
+mod private {
+    pub trait Mode {
+        type Type;
+        const REGISTER: super::RegisterFunc<Self::Type>;
+        const UNREGISTER: super::UnregisterFunc<Self::Type>;
+
+        /// # Safety
+        /// `raw` must be a valid pointer to [`Self::Type`].
+        unsafe fn device<'a>(raw: *mut Self::Type) -> &'a crate::device::Device;
+
+        /// # Safety
+        /// `led_cdev` must be a valid pointer to `led_classdev` embedded within [`Self::Type`].
+        unsafe fn from_classdev(led_cdev: *mut bindings::led_classdev) -> *mut Self::Type;
+
+        /// # Safety
+        /// `raw` must be a valid pointer to [`Self::Type`].
+        unsafe fn pre_brightness_set(_raw: *mut Self::Type, _brightness: u32) {}
+
+        fn release(_led_cdev: &mut Self::Type) {}
+    }
+}
+
 // SAFETY: A `led::Device` can be unregistered from any thread.
 unsafe impl<T: LedOps + Send> Send for Device<T> {}
 
@@ -281,24 +328,22 @@ unsafe impl<T: LedOps + Send> Send for Device<T> {}
 unsafe impl<T: LedOps + Sync> Sync for Device<T> {}
 
 impl<T: LedOps> Device<T> {
-    /// Registers a new led classdev.
-    ///
-    /// The [`Device`] will be unregistered on drop.
-    pub fn new<'a>(
+    fn __new<'a>(
         parent: &'a T::Bus,
         init_data: InitData<'a>,
         ops: T,
+        func: impl FnOnce(bindings::led_classdev) -> Result<<T::Mode as private::Mode>::Type> + 'a,
     ) -> impl PinInit<Devres<Self>, Error> + 'a {
         Devres::new(
             parent.as_ref(),
             try_pin_init!(Self {
                 ops,
-                classdev <- Opaque::try_ffi_init(|ptr: *mut bindings::led_classdev| {
+                classdev <- Opaque::try_ffi_init(|ptr: *mut <T::Mode as private::Mode>::Type| {
                     // 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`.
+                    // `T::Mode::Type` (and the embedded led_classdev) gets fully initialized
+                    // in-place by `T::Mode::REGISTER` including `mutex` and `list_head`.
                     unsafe {
-                        ptr.write(bindings::led_classdev {
+                        ptr.write((func)(bindings::led_classdev {
                             brightness_set: (!T::BLOCKING)
                                 .then_some(Adapter::<T>::brightness_set_callback),
                             brightness_set_blocking: T::BLOCKING
@@ -312,7 +357,7 @@ pub fn new<'a>(
                                 .map_or(core::ptr::null(), CStr::as_char_ptr),
                             color: init_data.color as u32,
                             ..bindings::led_classdev::default()
-                        })
+                        })?)
                     };
 
                     let fwnode = init_data.fwnode.map(ARef::from);
@@ -331,11 +376,11 @@ pub fn new<'a>(
                     };
 
                     // SAFETY:
-                    // - `parent.as_raw()` is guaranteed to be a pointer to a valid `device`
-                    //    or a null pointer.
-                    // - `ptr` is guaranteed to be a pointer to an initialized `led_classdev`.
+                    // - `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 `T::Mode::Type`.
                     to_result(unsafe {
-                        bindings::led_classdev_register_ext(
+                        (<T::Mode as private::Mode>::REGISTER)(
                             parent.as_ref().as_raw(),
                             ptr,
                             &mut init_data,
@@ -355,15 +400,22 @@ pub fn new<'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) }
+        // embedded within a `led::Device` and thus is embedded within `T::Mode::Type`.
+        let raw = unsafe { <T::Mode as private::Mode>::from_classdev(led_cdev) };
+
+        // SAFETY: The function's contract guarantees that `raw` 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(raw), Self, classdev) }
     }
 
     fn parent(&self) -> &device::Device<Bound> {
-        // SAFETY:
-        // - `self.classdev.get()` is guaranteed to be a valid pointer to `led_classdev`.
-        unsafe { device::Device::from_raw((*(*self.classdev.get()).dev).parent) }
+        // SAFETY: `self.classdev.get()` is guaranteed to be a valid pointer to `T::Mode::Type`.
+        let device = unsafe { <T::Mode as private::Mode>::device(self.classdev.get()) };
+        // SAFETY: `led::Device::__new` doesn't allow to register a class device without an parent.
+        let parent = unsafe { device.parent().unwrap_unchecked() };
+        // SAFETY: the existence of `self` guarantees that `parent` is bound to a driver.
+        unsafe { parent.as_bound() }
     }
 }
 
@@ -381,11 +433,17 @@ impl<T: LedOps> Adapter<T> {
         brightness: u32,
     ) {
         // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
-        // `led_classdev` embedded within a `led::Device`.
+        // `T::Mode::Type` 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()) };
 
+        // SAFETY: `classdev.classdev.get()` is guaranteed to be a valid pointer to a
+        // `T::Mode::Type`.
+        unsafe {
+            <T::Mode as private::Mode>::pre_brightness_set(classdev.classdev.get(), brightness);
+        }
+
         let _ = classdev.ops.brightness_set(parent, classdev, brightness);
     }
 
@@ -399,11 +457,17 @@ impl<T: LedOps> Adapter<T> {
     ) -> 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`.
+            // `T::Mode::Type` 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()) };
 
+            // SAFETY: `classdev.classdev.get()` is guaranteed to be a valid pointer to a
+            // `T::Mode::Type`.
+            unsafe {
+                <T::Mode as private::Mode>::pre_brightness_set(classdev.classdev.get(), brightness);
+            }
+
             classdev.ops.brightness_set(parent, classdev, brightness)?;
             Ok(0)
         })
@@ -415,7 +479,7 @@ impl<T: LedOps> Adapter<T> {
     /// 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`.
+        // `T::Mode::Type` 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()) };
@@ -436,7 +500,7 @@ impl<T: LedOps> Adapter<T> {
     ) -> 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`.
+            // `T::Mode::Type` 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()) };
@@ -461,17 +525,21 @@ impl<T: LedOps> PinnedDrop for Device<T> {
     fn drop(self: Pin<&mut Self>) {
         let raw = self.classdev.get();
         // SAFETY: The existence of `self` guarantees that `self.classdev.get()` is a pointer to a
-        // valid `struct led_classdev`.
-        let dev: &device::Device = unsafe { device::Device::from_raw((*raw).dev) };
+        // valid `T::Mode::Type`.
+        let dev: &device::Device = unsafe { <T::Mode as private::Mode>::device(raw) };
 
         let _fwnode = dev
             .fwnode()
             // SAFETY: the reference count of `fwnode` has previously been
-            // incremented in `led::Device::new`.
+            // incremented in `led::Device::__new`.
             .map(|fwnode| unsafe { ARef::from_raw(NonNull::from(fwnode)) });
 
         // SAFETY: The existence of `self` guarantees that `self.classdev` has previously been
-        // successfully registered with `led_classdev_register_ext`.
-        unsafe { bindings::led_classdev_unregister(self.classdev.get()) };
+        // successfully registered with `T::Mode::REGISTER`.
+        unsafe { (<T::Mode as private::Mode>::UNREGISTER)(raw) };
+
+        // SAFETY: The existence of `self` guarantees that `self.classdev.get()` is a pointer to a
+        // valid `T::Mode::Type`.
+        <T::Mode as private::Mode>::release(unsafe { &mut *raw });
     }
 }
diff --git a/rust/kernel/led/normal.rs b/rust/kernel/led/normal.rs
new file mode 100644
index 000000000000..13feeb3256f3
--- /dev/null
+++ b/rust/kernel/led/normal.rs
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Led mode for the `struct led_classdev`.
+//!
+//! C header: [`include/linux/leds.h`](srctree/include/linux/leds.h)
+
+use super::*;
+
+/// The led mode for the `struct led_classdev`. Leds with this mode can only have a fixed color.
+pub enum Normal {}
+
+impl private::Mode for Normal {
+    type Type = bindings::led_classdev;
+    const REGISTER: RegisterFunc<Self::Type> = bindings::led_classdev_register_ext;
+    const UNREGISTER: UnregisterFunc<Self::Type> = bindings::led_classdev_unregister;
+
+    unsafe fn device<'a>(raw: *mut Self::Type) -> &'a device::Device {
+        // SAFETY:
+        // - The function's contract guarantees that `raw` is a valid pointer to `led_classdev`.
+        unsafe { device::Device::from_raw((*raw).dev) }
+    }
+
+    unsafe fn from_classdev(led_cdev: *mut bindings::led_classdev) -> *mut Self::Type {
+        led_cdev
+    }
+}
+
+impl<T: LedOps<Mode = Normal>> Device<T> {
+    /// Registers a new led classdev.
+    ///
+    /// The [`Device`] will be unregistered on drop.
+    pub fn new<'a>(
+        parent: &'a T::Bus,
+        init_data: InitData<'a>,
+        ops: T,
+    ) -> impl PinInit<Devres<Self>, Error> + 'a {
+        Self::__new(parent, init_data, ops, Ok)
+    }
+}

-- 
2.51.0

Powered by blists - more mailing lists