[RFC PATCH 0/2] rust: LED abstractions

[RFC PATCH 0/2] rust: LED abstractions

[Fiona Behrens]

This RFC implements a basic LED abstraction to show how this would work with rust.

Currently this just implements a sample driver, to show how to use the abstraction, which just
prints the requested state, supporting a on/off LED and an led with brightness level up to 255 and
hardware blinking. I intend to write a hardware specific driver for submitting.

The abstractions is a generic struct that holds a generic driver data on which the vtable is
implemented. Because this struct also holds the c led_classdev (include/linux/leds.h) struct this
struct is pinned and is using pin_init to create and directly register the LED.
Dropping the struct unregisteres the LED. I plan to also add devm functions later, but as device
abstractions in rust are not yet that far I opted agains that for the first iteration of the LED
abstractions.

This is currently using core::time::Duration for the blinking interval, but will likely change that
to use the Delta time type from FUJITA Tomonori [1].

This is requiring the Opaque::try_ffi_init patch by Alice Ryhl[2] which just got merged into
char-misc-testing.

[1]: https://lore.kernel.org/rust-for-linux/20241005122531.20298-3-fujita.tomonori@gmail.com/
[2]: https://lore.kernel.org/rust-for-linux/20240926-b4-miscdevice-v1-1-7349c2b2837a@google.com/

Fiona Behrens (2):
  rust: LED abstraction
  samples: rust: led sample

 rust/kernel/leds.rs      | 399 +++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs       |   2 +
 samples/rust/Kconfig     |  10 +
 samples/rust/Makefile    |   1 +
 samples/rust/rust_led.rs | 103 ++++++++++
 5 files changed, 515 insertions(+)
 create mode 100644 rust/kernel/leds.rs
 create mode 100644 samples/rust/rust_led.rs

-- 
2.46.0

[RFC PATCH 1/2] rust: LED abstraction

[Fiona Behrens]

Add abstractions for a simple LED, supporting Single color LED and
hardware accelerated blinking.

This is implemented using a pinned Led struct which holds driver data
and the C led_classdev struct. The driver data then implements a
vtable which currently provides `brightness_set`, `brightness_get` and
`blink_set`. This LED is then registered with the LED core and
unregistered when it is dropped.

Signed-off-by: Fiona Behrens <me@kloenk.dev>
---
 rust/kernel/leds.rs | 399 ++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs  |   2 +
 2 files changed, 401 insertions(+)
 create mode 100644 rust/kernel/leds.rs

diff --git a/rust/kernel/leds.rs b/rust/kernel/leds.rs
new file mode 100644
index 000000000000..5348c1af5b31
--- /dev/null
+++ b/rust/kernel/leds.rs
@@ -0,0 +1,399 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! LED subsystem abstractions.
+//!
+//! C header: [`includes/linux/leds.h`](srctree/include/linux/leds.h)
+
+use core::ffi::c_ulong;
+use core::num::NonZeroU8;
+use core::ptr;
+use core::time::Duration;
+
+use crate::device::Device;
+use crate::{error::from_result, init::PinInit, prelude::*, types::Opaque};
+
+/// Color of an LED.
+#[derive(Copy, Clone)]
+pub enum Color {
+    /// White
+    White,
+    /// Red
+    Red,
+    /// Green
+    Green,
+    /// Blue
+    Blue,
+    /// Amber
+    Amber,
+    /// Violet
+    Violet,
+    /// Yellow
+    Yellow,
+    /// Purple
+    Purple,
+    /// Orange
+    Orange,
+    /// Pink
+    Pink,
+    /// Cyan
+    Cyan,
+    /// Lime
+    Lime,
+
+    /// Infrared
+    IR,
+    /// Multicolor LEDs
+    Multi,
+    /// Multicolor LEDs that can do arbitrary color,
+    /// so this would include `RGBW` and similar
+    RGB,
+}
+
+impl Color {
+    /// Get String representation of the Color.
+    pub fn name_cstr(&self) -> Option<&'static CStr> {
+        // SAFETY: C function call, enum is valid argument.
+        let name = unsafe { bindings::led_get_color_name(u32::from(self) as u8) };
+        if name.is_null() {
+            return None;
+        }
+        // SAFETY: pointer from above, valid for static lifetime.
+        Some(unsafe { CStr::from_char_ptr(name) })
+    }
+
+    /// Get String representation of the Color.
+    #[inline]
+    pub fn name(&self) -> Option<&'static str> {
+        // SAFETY: name from C name array which is valid UTF-8.
+        self.name_cstr()
+            .map(|name| unsafe { name.as_str_unchecked() })
+    }
+}
+
+impl core::fmt::Display for Color {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        f.write_str(self.name().unwrap_or("Invalid color"))
+    }
+}
+
+impl From<Color> for u32 {
+    fn from(color: Color) -> Self {
+        match color {
+            Color::White => bindings::LED_COLOR_ID_WHITE,
+            Color::Red => bindings::LED_COLOR_ID_RED,
+            Color::Green => bindings::LED_COLOR_ID_GREEN,
+            Color::Blue => bindings::LED_COLOR_ID_BLUE,
+            Color::Amber => bindings::LED_COLOR_ID_AMBER,
+            Color::Violet => bindings::LED_COLOR_ID_VIOLET,
+            Color::Yellow => bindings::LED_COLOR_ID_YELLOW,
+            Color::Purple => bindings::LED_COLOR_ID_PURPLE,
+            Color::Orange => bindings::LED_COLOR_ID_ORANGE,
+            Color::Pink => bindings::LED_COLOR_ID_PINK,
+            Color::Cyan => bindings::LED_COLOR_ID_CYAN,
+            Color::Lime => bindings::LED_COLOR_ID_LIME,
+            Color::IR => bindings::LED_COLOR_ID_IR,
+            Color::Multi => bindings::LED_COLOR_ID_MULTI,
+            Color::RGB => bindings::LED_COLOR_ID_RGB,
+        }
+    }
+}
+
+impl From<&Color> for u32 {
+    fn from(color: &Color) -> Self {
+        (*color).into()
+    }
+}
+
+impl TryFrom<u32> for Color {
+    type Error = Error;
+
+    fn try_from(value: u32) -> Result<Self, Self::Error> {
+        Ok(match value {
+            bindings::LED_COLOR_ID_WHITE => Color::White,
+            bindings::LED_COLOR_ID_RED => Color::Red,
+            bindings::LED_COLOR_ID_GREEN => Color::Green,
+            bindings::LED_COLOR_ID_BLUE => Color::Blue,
+            bindings::LED_COLOR_ID_AMBER => Color::Amber,
+            bindings::LED_COLOR_ID_VIOLET => Color::Violet,
+            bindings::LED_COLOR_ID_YELLOW => Color::Yellow,
+            bindings::LED_COLOR_ID_PURPLE => Color::Purple,
+            bindings::LED_COLOR_ID_ORANGE => Color::Orange,
+            bindings::LED_COLOR_ID_PINK => Color::Pink,
+            bindings::LED_COLOR_ID_CYAN => Color::Cyan,
+            bindings::LED_COLOR_ID_LIME => Color::Lime,
+            bindings::LED_COLOR_ID_IR => Color::IR,
+            bindings::LED_COLOR_ID_MULTI => Color::Multi,
+            bindings::LED_COLOR_ID_RGB => Color::RGB,
+            _ => return Err(EINVAL),
+        })
+    }
+}
+
+/// Config for the LED.
+///
+/// Some fields are optional and only used depending on how the led is registered.
+pub struct LedConfig {
+    /// Color of the LED.
+    pub color: Color,
+}
+
+/// A Led backed by a C `struct led_classdev`, additionally offering
+/// driver data storage.
+///
+/// The LED is unregistered once this LED struct is dropped.
+// TODO: add devm register variant
+#[pin_data(PinnedDrop)]
+pub struct Led<T> {
+    #[pin]
+    led: Opaque<bindings::led_classdev>,
+    /// Driver private data
+    pub data: T,
+}
+
+impl<T> Led<T> {
+    /// Return a raw reference to `Self` from a raw `struct led_classdev` C pointer.
+    ///
+    /// # Safety
+    ///
+    /// `ptr` must point to a [`bindings::led_classdev`] field in a struct of type `Self`.
+    unsafe fn led_container_of(ptr: *mut bindings::led_classdev) -> *mut Self {
+        let ptr = ptr.cast::<Opaque<bindings::led_classdev>>();
+
+        // SAFETY: By the safety requirement of this function ptr is embedded in self.
+        unsafe { crate::container_of!(ptr, Self, led).cast_mut() }
+    }
+}
+
+impl<'a, T> Led<T>
+where
+    T: Operations + 'a,
+{
+    /// Register a new LED with a predefine name.
+    pub fn register_with_name(
+        name: &'a CStr,
+        device: Option<&'a Device>,
+        config: &'a LedConfig,
+        data: T,
+    ) -> impl PinInit<Self, Error> + 'a {
+        try_pin_init!( Self {
+            led <- Opaque::try_ffi_init(move |place: *mut bindings::led_classdev| {
+            // SAFETY: `place` is a pointer to a live allocation, so erasing is valid.
+            unsafe { place.write_bytes(0, 1) };
+
+            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
+            unsafe { Self::build_with_name(place, name) };
+
+            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
+            unsafe { Self::build_config(place, config) };
+
+            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
+            unsafe { Self::build_vtable(place) };
+
+        let dev = device.map(|dev| dev.as_raw()).unwrap_or(ptr::null_mut());
+            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
+        crate::error::to_result(unsafe {
+            bindings::led_classdev_register_ext(dev, place, ptr::null_mut())
+        })
+            }),
+            data: data,
+        })
+    }
+
+    /// Add nameto the led_classdev.
+    ///
+    /// # Safety
+    ///
+    /// `ptr` has to be valid.
+    unsafe fn build_with_name(ptr: *mut bindings::led_classdev, name: &'a CStr) {
+        // SAFETY: `ptr` is pointing to a live allocation, so the deref is safe.
+        let name_ptr = unsafe { ptr::addr_of_mut!((*ptr).name) };
+        // SAFETY: `name_ptr` points to a valid allocation and we have exclusive access.
+        unsafe { ptr::write(name_ptr, name.as_char_ptr()) };
+    }
+
+    /// Add config to led_classdev.
+    ///
+    /// # Safety
+    ///
+    /// `ptr` has to be valid.
+    unsafe fn build_config(ptr: *mut bindings::led_classdev, config: &'a LedConfig) {
+        // SAFETY: `ptr` is pointing to a live allocation, so the deref is safe.
+        let color_ptr = unsafe { ptr::addr_of_mut!((*ptr).color) };
+        // SAFETY: `color_ptr` points to a valid allocation and we have exclusive access.
+        unsafe { ptr::write(color_ptr, config.color.into()) };
+    }
+}
+
+impl<T> Led<T>
+where
+    T: Operations,
+{
+    /// Add the Operations vtable to the `led_classdev` struct.
+    ///
+    /// # Safety
+    ///
+    /// `ptr` has to be valid.
+    unsafe fn build_vtable(ptr: *mut bindings::led_classdev) {
+        // SAFETY: `ptr` is pointing to a live allocation, so the deref is safe.
+        let max_brightness = unsafe { ptr::addr_of_mut!((*ptr).max_brightness) };
+        // SAFETY: `max_brightness` points to a valid allocation and we have exclusive access.
+        unsafe { ptr::write(max_brightness, T::MAX_BRIGHTNESS as _) };
+
+        // SAFETY: `ptr` is pointing to a live allocation, so the deref is safe.
+        let set_fn: *mut Option<_> = unsafe { ptr::addr_of_mut!((*ptr).brightness_set) };
+        // SAFETY: `set_fn` points to a valid allocation and we have exclusive access.
+        unsafe { ptr::write(set_fn, Some(brightness_set::<T>)) }
+
+        if T::HAS_BRIGHTNESS_GET {
+            // SAFETY: `ptr` is pointing to a live allocation, so the deref is safe.
+            let get_fn: *mut Option<_> = unsafe { ptr::addr_of_mut!((*ptr).brightness_get) };
+            // SAFETY: `set_fn` points to a valid allocation and we have exclusive access.
+            unsafe { ptr::write(get_fn, Some(brightness_get::<T>)) }
+        }
+
+        if T::HAS_BLINK_SET {
+            // SAFETY: `ptr` is pointing to a live allocation, so the deref is safe.
+            let blink_fn: *mut Option<_> = unsafe { ptr::addr_of_mut!((*ptr).blink_set) };
+            // SAFETY: `set_fn` points to a valid allocation and we have exclusive access.
+            unsafe { ptr::write(blink_fn, Some(blink_set::<T>)) }
+        }
+    }
+}
+
+#[pinned_drop]
+impl<T> PinnedDrop for Led<T> {
+    fn drop(self: Pin<&mut Self>) {
+        // SAEFTY: led is pointing to a live allocation
+        unsafe { bindings::led_classdev_unregister(self.led.get()) }
+    }
+}
+
+// SAFETY: `struct led_classdev` holds a mutex.
+unsafe impl<T: Send> Send for Led<T> {}
+// SAFETY: `struct led_classdev` holds a mutex.
+unsafe impl<T: Sync> Sync for Led<T> {}
+
+/// LED brightness.
+#[derive(Debug, Copy, Clone)]
+pub enum Brightness {
+    /// LED off.
+    Off,
+    /// Led set to the given value.
+    On(NonZeroU8),
+}
+
+impl Brightness {
+    /// Half LED brightness
+    // SAFETY: constant value non zero
+    pub const HALF: Self = Self::On(unsafe { NonZeroU8::new_unchecked(127) });
+    /// Full LED brightness.
+    pub const FULL: Self = Self::On(NonZeroU8::MAX);
+
+    /// Convert C brightness value to Rust brightness enum
+    fn from_c_enum(brightness: bindings::led_brightness) -> Self {
+        u8::try_from(brightness)
+            .map(NonZeroU8::new)
+            .map(|brightness| brightness.map(Self::On).unwrap_or(Self::Off))
+            .inspect_err(|_| pr_warn!("Brightness out of u8 range\n"))
+            .unwrap_or(Self::FULL)
+    }
+
+    /// Convert rust brightness enum to C value
+    fn as_c_enum(&self) -> bindings::led_brightness {
+        u8::from(self) as bindings::led_brightness
+    }
+}
+
+impl From<&Brightness> for u8 {
+    fn from(brightness: &Brightness) -> Self {
+        match brightness {
+            Brightness::Off => 0,
+            Brightness::On(v) => v.get(),
+        }
+    }
+}
+
+/// Led operations vtable.
+// TODO: add blocking variant (either via const generic or second trait)
+#[macros::vtable]
+pub trait Operations: Sized {
+    /// The maximum brightnes this led supports.
+    const MAX_BRIGHTNESS: u8;
+
+    /// Set LED brightness level.
+    /// This functions must not sleep.
+    fn brightness_set(this: &mut Led<Self>, brightness: Brightness);
+
+    /// Get the LED brightness level.
+    fn brightness_get(_this: &mut Led<Self>) -> Brightness {
+        crate::build_error(crate::error::VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Activae hardware accelerated blink, delays are in milliseconds
+    fn blink_set(
+        _this: &mut Led<Self>,
+        _delay_on: Duration,
+        _delay_off: Duration,
+    ) -> Result<(Duration, Duration)> {
+        crate::build_error(crate::error::VTABLE_DEFAULT_ERROR)
+    }
+}
+
+unsafe extern "C" fn brightness_set<T>(
+    led_cdev: *mut bindings::led_classdev,
+    brightness: bindings::led_brightness,
+) where
+    T: Operations,
+{
+    // SAFETY: By the safety requirement of this function `led_cdev` is embedded inside a `Led<T>`
+    // struct.
+    let led = unsafe { &mut *(Led::<T>::led_container_of(led_cdev.cast())) };
+    let brightness = Brightness::from_c_enum(brightness);
+    T::brightness_set(led, brightness);
+}
+
+unsafe extern "C" fn brightness_get<T>(
+    led_cdev: *mut bindings::led_classdev,
+) -> bindings::led_brightness
+where
+    T: Operations,
+{
+    // SAFETY: By the safety requirement of this function `led_cdev` is embedded inside a `Led<T>`
+    // struct.
+    let led = unsafe { &mut *(Led::<T>::led_container_of(led_cdev.cast())) };
+    T::brightness_get(led).as_c_enum()
+}
+
+unsafe extern "C" fn blink_set<T>(
+    led_cdev: *mut bindings::led_classdev,
+    delay_on: *mut c_ulong,
+    delay_off: *mut c_ulong,
+) -> i32
+where
+    T: Operations,
+{
+    from_result(|| {
+        // SAFETY: By the safety requirement of this function `led_cdev` is embedded inside a
+        // `Led<T>` struct.
+        let led = unsafe { &mut *(Led::<T>::led_container_of(led_cdev.cast())) };
+
+        // SAFETY: By the safety requirement of this function `delay_on` is pointing to a valid
+        // `c_uint`
+        let on = Duration::from_millis(unsafe { *delay_on } as u64);
+        // SAFETY: By the safety requirement of this function `delay_off` is pointing to a valid
+        // `c_uint`
+        let off = Duration::from_millis(unsafe { *delay_off } as u64);
+
+        let (on, off) = T::blink_set(led, on, off)?;
+
+        // writing back return values
+        // SAFETY: By the safety requirement of this function `delay_on` is pointing to a valid
+        // `c_uint`
+        unsafe { ptr::write(delay_on, on.as_millis() as c_ulong) };
+        // SAFETY: By the safety requirement of this function `delay_off` is pointing to a valid
+        // `c_uint`
+        unsafe { ptr::write(delay_off, off.as_millis() as c_ulong) };
+
+        Ok(0)
+    })
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index b5f4b3ce6b48..8df5f1cdf426 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -38,6 +38,8 @@
 pub mod ioctl;
 #[cfg(CONFIG_KUNIT)]
 pub mod kunit;
+#[cfg(CONFIG_NEW_LEDS)]
+pub mod leds;
 pub mod list;
 #[cfg(CONFIG_NET)]
 pub mod net;
-- 
2.46.0

[RFC PATCH 2/2] samples: rust: led sample

[Fiona Behrens]

Provide an initial sample LED driver that just prints the current
requested LED state.

This is not intended to be merged, but as a placeholder until the
abstactions for hardware access are written.

Signed-off-by: Fiona Behrens <me@kloenk.dev>
---
 samples/rust/Kconfig     |  10 ++++
 samples/rust/Makefile    |   1 +
 samples/rust/rust_led.rs | 103 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 114 insertions(+)
 create mode 100644 samples/rust/rust_led.rs

diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index b0f74a81c8f9..910f15ef6951 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -30,6 +30,16 @@ config SAMPLE_RUST_PRINT
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_LED
+       tristate "Led subsystem"
+       help
+	This option builds the Rust LED subsystem sample.
+
+	To compile this as a module, choose M here:
+	the module will be called rust_led.
+
+	If unsure, say N.
+
 config SAMPLE_RUST_HOSTPROGS
 	bool "Host programs"
 	help
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 03086dabbea4..1299ca1e9ebb 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -2,5 +2,6 @@
 
 obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
+obj-$(CONFIG_SAMPLE_RUST_LED)			+= rust_led.o
 
 subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS)		+= hostprogs
diff --git a/samples/rust/rust_led.rs b/samples/rust/rust_led.rs
new file mode 100644
index 000000000000..0085f7484ea1
--- /dev/null
+++ b/samples/rust/rust_led.rs
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0
+//! Rust LED sample.
+
+use core::time::Duration;
+
+use kernel::c_str;
+use kernel::leds::{Brightness, Color, Led, LedConfig, Operations};
+use kernel::prelude::*;
+
+module! {
+    type: RustLed,
+    name: "rust_led",
+    author: "Rust for Linux Contributors",
+    description: "Rust led sample",
+    license: "GPL",
+}
+
+/// Rust LED sample driver
+// Hold references in scope as droping would unregister the LED
+#[allow(dead_code)]
+struct RustLed {
+    /// LED which just supports on/off.
+    on_off: Pin<Box<Led<LedOnOff>>>,
+    /// LED which supports brightness levels and blinking.
+    blink: Pin<Box<Led<LedBlinking>>>,
+}
+
+impl kernel::Module for RustLed {
+    fn init(_module: &'static ThisModule) -> Result<Self> {
+        pr_info!("registering on_off led\n");
+        let on_off = Box::pin_init(
+            Led::register_with_name(
+                c_str!("sample:red:on_off"),
+                None,
+                &LedConfig { color: Color::Red },
+                LedOnOff,
+            ),
+            GFP_KERNEL,
+        )?;
+
+        let blink = Box::pin_init(
+            Led::register_with_name(
+                c_str!("sample:green:blink"),
+                None,
+                &LedConfig {
+                    color: Color::Green,
+                },
+                LedBlinking,
+            ),
+            GFP_KERNEL,
+        )?;
+
+        Ok(Self { on_off, blink })
+    }
+}
+
+struct LedOnOff;
+
+#[vtable]
+impl Operations for LedOnOff {
+    const MAX_BRIGHTNESS: u8 = 1;
+
+    fn brightness_set(_this: &mut Led<Self>, brightness: Brightness) {
+        match brightness {
+            Brightness::Off => pr_info!("Switching led off\n"),
+            Brightness::On(v) => pr_info!("Switching led on: {}\n", v.get()),
+        }
+    }
+}
+
+struct LedBlinking;
+
+impl LedBlinking {
+    const HW_DURATION: Duration = Duration::from_millis(1000);
+}
+
+#[vtable]
+impl Operations for LedBlinking {
+    const MAX_BRIGHTNESS: u8 = 255;
+
+    fn brightness_set(_this: &mut Led<Self>, brightness: Brightness) {
+        match brightness {
+            Brightness::Off => pr_info!("blinking: off\n"),
+            Brightness::On(v) => pr_info!("blinking: on at {}\n", v.get()),
+        }
+    }
+
+    fn blink_set(
+        _this: &mut Led<Self>,
+        delay_on: Duration,
+        delay_off: Duration,
+    ) -> Result<(Duration, Duration)> {
+        pr_info!("blinking: try delay {delay_on:?} {delay_off:?}\n");
+        if !(delay_on.is_zero() && delay_off.is_zero())
+            && !(delay_on == Self::HW_DURATION && delay_off == Self::HW_DURATION)
+        {
+            return Err(EINVAL);
+        }
+
+        pr_info!("blinking: setting dealy\n");
+        Ok((Self::HW_DURATION, Self::HW_DURATION))
+    }
+}
-- 
2.46.0

Re: [RFC PATCH 0/2] rust: LED abstractions

[Lee Jones]

On Wed, 09 Oct 2024, Fiona Behrens wrote:

> This RFC implements a basic LED abstraction to show how this would work with rust.
> 
> Currently this just implements a sample driver, to show how to use the abstraction, which just
> prints the requested state, supporting a on/off LED and an led with brightness level up to 255 and
> hardware blinking. I intend to write a hardware specific driver for submitting.
> 
> The abstractions is a generic struct that holds a generic driver data on which the vtable is
> implemented. Because this struct also holds the c led_classdev (include/linux/leds.h) struct this
> struct is pinned and is using pin_init to create and directly register the LED.
> Dropping the struct unregisteres the LED. I plan to also add devm functions later, but as device
> abstractions in rust are not yet that far I opted agains that for the first iteration of the LED
> abstractions.
> 
> This is currently using core::time::Duration for the blinking interval, but will likely change that
> to use the Delta time type from FUJITA Tomonori [1].
> 
> This is requiring the Opaque::try_ffi_init patch by Alice Ryhl[2] which just got merged into
> char-misc-testing.
> 
> [1]: https://lore.kernel.org/rust-for-linux/20241005122531.20298-3-fujita.tomonori@gmail.com/
> [2]: https://lore.kernel.org/rust-for-linux/20240926-b4-miscdevice-v1-1-7349c2b2837a@google.com/
> 
> Fiona Behrens (2):
>   rust: LED abstraction
>   samples: rust: led sample
> 
>  rust/kernel/leds.rs      | 399 +++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs       |   2 +
>  samples/rust/Kconfig     |  10 +
>  samples/rust/Makefile    |   1 +
>  samples/rust/rust_led.rs | 103 ++++++++++
>  5 files changed, 515 insertions(+)
>  create mode 100644 rust/kernel/leds.rs
>  create mode 100644 samples/rust/rust_led.rs

FYI: I'm not ignoring this patch-set.  On the contrary.  I'm trying to
place myself into a position where I can not only review it with some
confidence, but use it to author LED drivers.

-- 
Lee Jones [李琼斯]

Re: [RFC PATCH 0/2] rust: LED abstractions

[Fiona Behrens]

On 11 Nov 2024, at 10:41, Lee Jones wrote:

> On Wed, 09 Oct 2024, Fiona Behrens wrote:
>
>> This RFC implements a basic LED abstraction to show how this would work with rust.
>>
>> Currently this just implements a sample driver, to show how to use the abstraction, which just
>> prints the requested state, supporting a on/off LED and an led with brightness level up to 255 and
>> hardware blinking. I intend to write a hardware specific driver for submitting.
>>
>> The abstractions is a generic struct that holds a generic driver data on which the vtable is
>> implemented. Because this struct also holds the c led_classdev (include/linux/leds.h) struct this
>> struct is pinned and is using pin_init to create and directly register the LED.
>> Dropping the struct unregisteres the LED. I plan to also add devm functions later, but as device
>> abstractions in rust are not yet that far I opted agains that for the first iteration of the LED
>> abstractions.
>>
>> This is currently using core::time::Duration for the blinking interval, but will likely change that
>> to use the Delta time type from FUJITA Tomonori [1].
>>
>> This is requiring the Opaque::try_ffi_init patch by Alice Ryhl[2] which just got merged into
>> char-misc-testing.
>>
>> [1]: https://lore.kernel.org/rust-for-linux/20241005122531.20298-3-fujita.tomonori@gmail.com/
>> [2]: https://lore.kernel.org/rust-for-linux/20240926-b4-miscdevice-v1-1-7349c2b2837a@google.com/
>>
>> Fiona Behrens (2):
>>   rust: LED abstraction
>>   samples: rust: led sample
>>
>>  rust/kernel/leds.rs      | 399 +++++++++++++++++++++++++++++++++++++++
>>  rust/kernel/lib.rs       |   2 +
>>  samples/rust/Kconfig     |  10 +
>>  samples/rust/Makefile    |   1 +
>>  samples/rust/rust_led.rs | 103 ++++++++++
>>  5 files changed, 515 insertions(+)
>>  create mode 100644 rust/kernel/leds.rs
>>  create mode 100644 samples/rust/rust_led.rs
>
> FYI: I'm not ignoring this patch-set.  On the contrary.  I'm trying to
> place myself into a position where I can not only review it with some
> confidence, but use it to author LED drivers.
>

Ah amazing, the first RFC was to probe for a general would LED accept rust code, and this sounds like a yes. Nice!

Currently still working on a LED driver for a Lenovo board that does not (as of when I started, and think still not yet) have a C driver for the LED. Current things missing is a static dmi match table (idea is to hardcoded that for now) and a abstraction for port memory but working on that, and can otherwise just go around that with unsafe calls in the driver itself.

  - Fiona

> -- 
> Lee Jones [李琼斯]

Re: [RFC PATCH 1/2] rust: LED abstraction

[Marek Behún]

On Wed, Oct 09, 2024 at 12:57:58PM +0200, Fiona Behrens wrote:

> +/// Color of an LED.
> +#[derive(Copy, Clone)]
> +pub enum Color {
> +    /// White
> +    White,
> +    /// Red
> +    Red,
> +    /// Green
> +    Green,
> +    /// Blue
> +    Blue,
> +    /// Amber
> +    Amber,
> +    /// Violet
> +    Violet,
> +    /// Yellow
> +    Yellow,
> +    /// Purple
> +    Purple,
> +    /// Orange
> +    Orange,
> +    /// Pink
> +    Pink,
> +    /// Cyan
> +    Cyan,
> +    /// Lime
> +    Lime,

Why these repetitions?

> +impl TryFrom<u32> for Color {
> +    type Error = Error;
> +
> +    fn try_from(value: u32) -> Result<Self, Self::Error> {
> +        Ok(match value {
> +            bindings::LED_COLOR_ID_WHITE => Color::White,
> +            bindings::LED_COLOR_ID_RED => Color::Red,
> +            bindings::LED_COLOR_ID_GREEN => Color::Green,
> +            bindings::LED_COLOR_ID_BLUE => Color::Blue,
> +            bindings::LED_COLOR_ID_AMBER => Color::Amber,
> +            bindings::LED_COLOR_ID_VIOLET => Color::Violet,
> +            bindings::LED_COLOR_ID_YELLOW => Color::Yellow,
> +            bindings::LED_COLOR_ID_PURPLE => Color::Purple,
> +            bindings::LED_COLOR_ID_ORANGE => Color::Orange,
> +            bindings::LED_COLOR_ID_PINK => Color::Pink,
> +            bindings::LED_COLOR_ID_CYAN => Color::Cyan,
> +            bindings::LED_COLOR_ID_LIME => Color::Lime,
> +            bindings::LED_COLOR_ID_IR => Color::IR,
> +            bindings::LED_COLOR_ID_MULTI => Color::Multi,
> +            bindings::LED_COLOR_ID_RGB => Color::RGB,
> +            _ => return Err(EINVAL),
> +        })
> +    }
> +}

How does Rust compile these? If these constants compile to the same
numeric values, i.e. if
  LED_COLOR_ID_AMBER == Color::Amber,
will the compiler compile away the function?

How do enums work in Rust?

> +impl<'a, T> Led<T>

offtopic, what is 'a ? What does the ' mean? Is impl<> something like
template in c++?

> +where
> +    T: Operations + 'a,

What does + mean here?

> +/// LED brightness.
> +#[derive(Debug, Copy, Clone)]
> +pub enum Brightness {
> +    /// LED off.
> +    Off,
> +    /// Led set to the given value.
> +    On(NonZeroU8),
> +}
> +
> +impl Brightness {
> +    /// Half LED brightness
> +    // SAFETY: constant value non zero
> +    pub const HALF: Self = Self::On(unsafe { NonZeroU8::new_unchecked(127) });
> +    /// Full LED brightness.
> +    pub const FULL: Self = Self::On(NonZeroU8::MAX);

These LED_OFF, LED_ON, LED_HALF and LED_FULL are deprecated constants
that should not be used anymore. enum led_brightness will be either
uint8_t or usigned int in the future.

Is it possible to not infect Rust with these deprecated constants?

Marek

Re: [RFC PATCH 1/2] rust: LED abstraction

[Alice Ryhl]

On Sat, Nov 16, 2024 at 4:47 PM Marek Behún <kabel@kernel.org> wrote:
>
> On Wed, Oct 09, 2024 at 12:57:58PM +0200, Fiona Behrens wrote:
>
> > +/// Color of an LED.
> > +#[derive(Copy, Clone)]
> > +pub enum Color {
> > +    /// White
> > +    White,
> > +    /// Red
> > +    Red,
> > +    /// Green
> > +    Green,
> > +    /// Blue
> > +    Blue,
> > +    /// Amber
> > +    Amber,
> > +    /// Violet
> > +    Violet,
> > +    /// Yellow
> > +    Yellow,
> > +    /// Purple
> > +    Purple,
> > +    /// Orange
> > +    Orange,
> > +    /// Pink
> > +    Pink,
> > +    /// Cyan
> > +    Cyan,
> > +    /// Lime
> > +    Lime,
>
> Why these repetitions?

My guess is that it's to silence the warning about undocumented public
items. It may make sense to just silence the warning in this case.

> > +impl TryFrom<u32> for Color {
> > +    type Error = Error;
> > +
> > +    fn try_from(value: u32) -> Result<Self, Self::Error> {
> > +        Ok(match value {
> > +            bindings::LED_COLOR_ID_WHITE => Color::White,
> > +            bindings::LED_COLOR_ID_RED => Color::Red,
> > +            bindings::LED_COLOR_ID_GREEN => Color::Green,
> > +            bindings::LED_COLOR_ID_BLUE => Color::Blue,
> > +            bindings::LED_COLOR_ID_AMBER => Color::Amber,
> > +            bindings::LED_COLOR_ID_VIOLET => Color::Violet,
> > +            bindings::LED_COLOR_ID_YELLOW => Color::Yellow,
> > +            bindings::LED_COLOR_ID_PURPLE => Color::Purple,
> > +            bindings::LED_COLOR_ID_ORANGE => Color::Orange,
> > +            bindings::LED_COLOR_ID_PINK => Color::Pink,
> > +            bindings::LED_COLOR_ID_CYAN => Color::Cyan,
> > +            bindings::LED_COLOR_ID_LIME => Color::Lime,
> > +            bindings::LED_COLOR_ID_IR => Color::IR,
> > +            bindings::LED_COLOR_ID_MULTI => Color::Multi,
> > +            bindings::LED_COLOR_ID_RGB => Color::RGB,
> > +            _ => return Err(EINVAL),
> > +        })
> > +    }
> > +}
>
> How does Rust compile these? If these constants compile to the same
> numeric values, i.e. if
>   LED_COLOR_ID_AMBER == Color::Amber,
> will the compiler compile away the function?

Well, it can't compile away the part where it returns EINVAL when the
u32 is not a valid color. But other than that, these matches are
usually optimized pretty well. I just tried a few different examples
in godbolt to confirm it. See e.g.:
https://rust.godbolt.org/z/WWM7891zW

That said, this relies on the assumption that they are represented
using the same values. We probably want to change the declaration to
this:

#[derive(Copy, Clone)]
pub enum Color {
    White = bindings::LED_COLOR_ID_WHITE,
    Red = bindings::LED_COLOR_ID_RED,
    Green = bindings::LED_COLOR_ID_GREEN,
    ...
}

That way we are guaranteed that the enum uses the right values for the
enum to make the conversion free.

> How do enums work in Rust?

In this case, the enum has no fields. In that case, the enum is a
value that is only allowed to have certain values.

Enums are also allowed to have fields. In this case, you can think of
it as a discriminated union, though in some cases Rust will store it
in a more clever way. You can look up the "null pointer optimization"
for an example of that.

> > +impl<'a, T> Led<T>
>
> offtopic, what is 'a ? What does the ' mean? Is impl<> something like
> template in c++?

Things starting with a tick are lifetimes, so 'a is the name of a
lifetime. That said, this usage of lifetimes looks incorrect to me, so
I wouldn't look too closely at this instance.

As for impl<>, then yes sort of. It is the <> that makes it like a
template. When you have an `impl TypeName { ... }` block, then that
defines methods for `TypeName`, which you can call as either
`value.method(...)` or `TypeName::method(...)` depending on the
signature. When you write `impl<T>`, then this means that it is a
template (we use the word "generic" in Rust rather than "template"),
that is

impl<T> TypeName<T> { ... }

becomes the following infinite list of impl blocks:

impl TypeName<i32> { ... }
impl TypeName<u32> { ... }
impl TypeName<String> { ... }
impl TypeName<TcpStream> { ... }
// ... and so on for all possible types

This logic works anywhere that <T> appears. For example, `struct
TypeName<T> { ... }` is short-hand for the following infinite list of
structs:

struct TypeName<i32> { ... }
struct TypeName<u32> { ... }
struct TypeName<String> { ... }
struct TypeName<TcpStream> { ... }
// ... and so on for all possible types

Of course, only things in this infinite list that you actually use end
up in the final binary.

The `where T: Operations` part is a filter for the infinite list. It
restricts it so that only types `T` that implement the `Operations`
trait are present in the list; all other types are filtered out.

> > +where
> > +    T: Operations + 'a,
>
> What does + mean here?

This is the same as:
where
    T: Operations,
    T: 'a
that is, apply two filters to the infinite list I mentioned above. The
meaning of `T: 'a` when the RHS is a lifetime is that `T` must not be
a type containing a lifetime annotation shorter than 'a.

> > +/// LED brightness.
> > +#[derive(Debug, Copy, Clone)]
> > +pub enum Brightness {
> > +    /// LED off.
> > +    Off,
> > +    /// Led set to the given value.
> > +    On(NonZeroU8),
> > +}
> > +
> > +impl Brightness {
> > +    /// Half LED brightness
> > +    // SAFETY: constant value non zero
> > +    pub const HALF: Self = Self::On(unsafe { NonZeroU8::new_unchecked(127) });
> > +    /// Full LED brightness.
> > +    pub const FULL: Self = Self::On(NonZeroU8::MAX);
>
> These LED_OFF, LED_ON, LED_HALF and LED_FULL are deprecated constants
> that should not be used anymore. enum led_brightness will be either
> uint8_t or usigned int in the future.
>
> Is it possible to not infect Rust with these deprecated constants?
>
> Marek

Re: [RFC PATCH 1/2] rust: LED abstraction

[Alice Ryhl]

On Wed, Oct 9, 2024 at 12:58 PM Fiona Behrens <me@kloenk.dev> wrote:
> +impl<'a, T> Led<T>
> +where
> +    T: Operations + 'a,
> +{
> +    /// Register a new LED with a predefine name.
> +    pub fn register_with_name(
> +        name: &'a CStr,
> +        device: Option<&'a Device>,
> +        config: &'a LedConfig,
> +        data: T,
> +    ) -> impl PinInit<Self, Error> + 'a {
> +        try_pin_init!( Self {
> +            led <- Opaque::try_ffi_init(move |place: *mut bindings::led_classdev| {
> +            // SAFETY: `place` is a pointer to a live allocation, so erasing is valid.
> +            unsafe { place.write_bytes(0, 1) };
> +
> +            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
> +            unsafe { Self::build_with_name(place, name) };
> +
> +            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
> +            unsafe { Self::build_config(place, config) };
> +
> +            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
> +            unsafe { Self::build_vtable(place) };
> +
> +        let dev = device.map(|dev| dev.as_raw()).unwrap_or(ptr::null_mut());
> +            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
> +        crate::error::to_result(unsafe {
> +            bindings::led_classdev_register_ext(dev, place, ptr::null_mut())
> +        })
> +            }),
> +            data: data,
> +        })
> +    }
> +
> +    /// Add nameto the led_classdev.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `ptr` has to be valid.
> +    unsafe fn build_with_name(ptr: *mut bindings::led_classdev, name: &'a CStr) {
> +        // SAFETY: `ptr` is pointing to a live allocation, so the deref is safe.
> +        let name_ptr = unsafe { ptr::addr_of_mut!((*ptr).name) };
> +        // SAFETY: `name_ptr` points to a valid allocation and we have exclusive access.
> +        unsafe { ptr::write(name_ptr, name.as_char_ptr()) };
> +    }
> +
> +    /// Add config to led_classdev.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `ptr` has to be valid.
> +    unsafe fn build_config(ptr: *mut bindings::led_classdev, config: &'a LedConfig) {
> +        // SAFETY: `ptr` is pointing to a live allocation, so the deref is safe.
> +        let color_ptr = unsafe { ptr::addr_of_mut!((*ptr).color) };
> +        // SAFETY: `color_ptr` points to a valid allocation and we have exclusive access.
> +        unsafe { ptr::write(color_ptr, config.color.into()) };
> +    }
> +}

This usage of lifetimes looks incorrect to me. It looks like you are
trying to say that the references must be valid for longer than the
Led<T>, but what you are writing here does not enforce that. The Led
struct must be annotated with the 'a lifetime if you want that, but
I'm inclined to say you should not go for the lifetime solution in the
first place.

Alice

Re: [RFC PATCH 1/2] rust: LED abstraction

[Miguel Ojeda]

On Mon, Nov 18, 2024 at 11:19 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> signature. When you write `impl<T>`, then this means that it is a
> template (we use the word "generic" in Rust rather than "template"),

Marek: a main difference is that generics in Rust require you to spell
out everything your type needs in order to be able to use it in the
implementation, unlike C++ templates which will gladly accept any type
as long as the resulting code compiles (i.e. whether the types make
sense or not).

So in C++ you may typically do just `T`, while in Rust you typically
restrict your types with bounds and `where`s clauses like Alice shows.

I hope that clarifies a bit!

Cheers,
Miguel

Re: [RFC PATCH 1/2] rust: LED abstraction

[Fiona Behrens]

On 18 Nov 2024, at 11:22, Alice Ryhl wrote:

> On Wed, Oct 9, 2024 at 12:58 PM Fiona Behrens <me@kloenk.dev> wrote:
>> +impl<'a, T> Led<T>
>> +where
>> +    T: Operations + 'a,
>> +{
>> +    /// Register a new LED with a predefine name.
>> +    pub fn register_with_name(
>> +        name: &'a CStr,
>> +        device: Option<&'a Device>,
>> +        config: &'a LedConfig,
>> +        data: T,
>> +    ) -> impl PinInit<Self, Error> + 'a {
>> +        try_pin_init!( Self {
>> +            led <- Opaque::try_ffi_init(move |place: *mut bindings::led_classdev| {
>> +            // SAFETY: `place` is a pointer to a live allocation, so erasing is valid.
>> +            unsafe { place.write_bytes(0, 1) };
>> +
>> +            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
>> +            unsafe { Self::build_with_name(place, name) };
>> +
>> +            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
>> +            unsafe { Self::build_config(place, config) };
>> +
>> +            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
>> +            unsafe { Self::build_vtable(place) };
>> +
>> +        let dev = device.map(|dev| dev.as_raw()).unwrap_or(ptr::null_mut());
>> +            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
>> +        crate::error::to_result(unsafe {
>> +            bindings::led_classdev_register_ext(dev, place, ptr::null_mut())
>> +        })
>> +            }),
>> +            data: data,
>> +        })
>> +    }
>> +
>> +    /// Add nameto the led_classdev.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// `ptr` has to be valid.
>> +    unsafe fn build_with_name(ptr: *mut bindings::led_classdev, name: &'a CStr) {
>> +        // SAFETY: `ptr` is pointing to a live allocation, so the deref is safe.
>> +        let name_ptr = unsafe { ptr::addr_of_mut!((*ptr).name) };
>> +        // SAFETY: `name_ptr` points to a valid allocation and we have exclusive access.
>> +        unsafe { ptr::write(name_ptr, name.as_char_ptr()) };
>> +    }
>> +
>> +    /// Add config to led_classdev.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// `ptr` has to be valid.
>> +    unsafe fn build_config(ptr: *mut bindings::led_classdev, config: &'a LedConfig) {
>> +        // SAFETY: `ptr` is pointing to a live allocation, so the deref is safe.
>> +        let color_ptr = unsafe { ptr::addr_of_mut!((*ptr).color) };
>> +        // SAFETY: `color_ptr` points to a valid allocation and we have exclusive access.
>> +        unsafe { ptr::write(color_ptr, config.color.into()) };
>> +    }
>> +}
>
> This usage of lifetimes looks incorrect to me. It looks like you are
> trying to say that the references must be valid for longer than the
> Led<T>, but what you are writing here does not enforce that. The Led
> struct must be annotated with the 'a lifetime if you want that, but
> I'm inclined to say you should not go for the lifetime solution in the
> first place.

The `led_classdev_register_ext` function copies the name, therefore the idea was that the name only has to exists until the pin init function is called, which should be the case with how I used the lifetimes here

Fiona

>
> Alice

Re: [RFC PATCH 1/2] rust: LED abstraction

[Alice Ryhl]

On Thu, Nov 21, 2024 at 10:47 AM Fiona Behrens <me@kloenk.dev> wrote:
>
> On 18 Nov 2024, at 11:22, Alice Ryhl wrote:
>
> > On Wed, Oct 9, 2024 at 12:58 PM Fiona Behrens <me@kloenk.dev> wrote:
> >> +impl<'a, T> Led<T>
> >> +where
> >> +    T: Operations + 'a,
> >> +{
> >> +    /// Register a new LED with a predefine name.
> >> +    pub fn register_with_name(
> >> +        name: &'a CStr,
> >> +        device: Option<&'a Device>,
> >> +        config: &'a LedConfig,
> >> +        data: T,
> >> +    ) -> impl PinInit<Self, Error> + 'a {
> >> +        try_pin_init!( Self {
> >> +            led <- Opaque::try_ffi_init(move |place: *mut bindings::led_classdev| {
> >> +            // SAFETY: `place` is a pointer to a live allocation, so erasing is valid.
> >> +            unsafe { place.write_bytes(0, 1) };
> >> +
> >> +            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
> >> +            unsafe { Self::build_with_name(place, name) };
> >> +
> >> +            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
> >> +            unsafe { Self::build_config(place, config) };
> >> +
> >> +            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
> >> +            unsafe { Self::build_vtable(place) };
> >> +
> >> +        let dev = device.map(|dev| dev.as_raw()).unwrap_or(ptr::null_mut());
> >> +            // SAFETY: `place` is a pointer to a live allocation of `bindings::led_classdev`.
> >> +        crate::error::to_result(unsafe {
> >> +            bindings::led_classdev_register_ext(dev, place, ptr::null_mut())
> >> +        })
> >> +            }),
> >> +            data: data,
> >> +        })
> >> +    }
> >> +
> >> +    /// Add nameto the led_classdev.
> >> +    ///
> >> +    /// # Safety
> >> +    ///
> >> +    /// `ptr` has to be valid.
> >> +    unsafe fn build_with_name(ptr: *mut bindings::led_classdev, name: &'a CStr) {
> >> +        // SAFETY: `ptr` is pointing to a live allocation, so the deref is safe.
> >> +        let name_ptr = unsafe { ptr::addr_of_mut!((*ptr).name) };
> >> +        // SAFETY: `name_ptr` points to a valid allocation and we have exclusive access.
> >> +        unsafe { ptr::write(name_ptr, name.as_char_ptr()) };
> >> +    }
> >> +
> >> +    /// Add config to led_classdev.
> >> +    ///
> >> +    /// # Safety
> >> +    ///
> >> +    /// `ptr` has to be valid.
> >> +    unsafe fn build_config(ptr: *mut bindings::led_classdev, config: &'a LedConfig) {
> >> +        // SAFETY: `ptr` is pointing to a live allocation, so the deref is safe.
> >> +        let color_ptr = unsafe { ptr::addr_of_mut!((*ptr).color) };
> >> +        // SAFETY: `color_ptr` points to a valid allocation and we have exclusive access.
> >> +        unsafe { ptr::write(color_ptr, config.color.into()) };
> >> +    }
> >> +}
> >
> > This usage of lifetimes looks incorrect to me. It looks like you are
> > trying to say that the references must be valid for longer than the
> > Led<T>, but what you are writing here does not enforce that. The Led
> > struct must be annotated with the 'a lifetime if you want that, but
> > I'm inclined to say you should not go for the lifetime solution in the
> > first place.
>
> The `led_classdev_register_ext` function copies the name, therefore the idea was that the name only has to exists until the pin init function is called, which should be the case with how I used the lifetimes here

In that case you should be able to get rid of the lifetime like this:

impl<T> Led<T>
where
    T: Operations,
{
    /// Register a new LED with a predefine name.
    pub fn register_with_name(
        name: &CStr,
        device: Option<&Device>,
        config: &LedConfig,
        data: T,
    ) -> impl PinInit<Self, Error> {
        ...
    }