* [PATCH 0/2] rust: leds: add led classdev abstractions
@ 2025-10-09 17:07 Markus Probst
  2025-10-09 17:07 ` [PATCH 1/2] rust: add basic Pin<Vec<T, A>> abstractions Markus Probst
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Markus Probst @ 2025-10-09 17:07 UTC (permalink / raw)
  To: Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Lee Jones,
	Pavel Machek
  Cc: Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Boqun Feng, Gary Guo, bjorn3_gh, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Markus Probst,
	rust-for-linux, linux-kernel, linux-leds
This patch series has previously been contained in
https://lore.kernel.org/rust-for-linux/20251008181027.662616-1-markus.probst@posteo.de/T/#t
which added a rust written led driver for a microcontroller via i2c.
As the reading and writing to the i2c client via the register!
macro has not been implemented yet [1], the patch series will only
contain the additional abstractions required.
[1] https://lore.kernel.org/rust-for-linux/DDDS2V0V2NVJ.16ZKXCKUA1HUV@kernel.org/
The following changes were made:
* add basic Pin<Vec<T, A>> abstractions, that allow to initialize PinInit items with
  the gurantee that these will never be moved.
* add basic led classdev abstractions to register and unregister leds
Markus Probst (2):
  rust: add basic Pin<Vec<T, A>> abstractions
  rust: leds: add basic led classdev abstractions
 rust/kernel/alloc.rs      |   1 +
 rust/kernel/alloc/kvec.rs |  86 +++++++++++
 rust/kernel/led.rs        | 296 ++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs        |   1 +
 rust/kernel/prelude.rs    |   2 +-
 5 files changed, 385 insertions(+), 1 deletion(-)
 create mode 100644 rust/kernel/led.rs
-- 
2.49.1
^ permalink raw reply	[flat|nested] 7+ messages in thread
* [PATCH 1/2] rust: add basic Pin<Vec<T, A>> abstractions
  2025-10-09 17:07 [PATCH 0/2] rust: leds: add led classdev abstractions Markus Probst
@ 2025-10-09 17:07 ` Markus Probst
  2025-10-09 17:45   ` Onur Özkan
  2025-10-09 17:07 ` [PATCH 2/2] rust: leds: add basic led classdev abstractions Markus Probst
  2025-10-09 17:46 ` [PATCH 0/2] rust: leds: add " Onur Özkan
  2 siblings, 1 reply; 7+ messages in thread
From: Markus Probst @ 2025-10-09 17:07 UTC (permalink / raw)
  To: Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Lee Jones,
	Pavel Machek
  Cc: Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Boqun Feng, Gary Guo, bjorn3_gh, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Markus Probst,
	rust-for-linux, linux-kernel, linux-leds
Implement core Pin<Vec<T, A>> abstractions, incluing
 * `Vec::pin` and `Vec::into_pin` for constructing a `Pin<Vec<T, A>>`.
   If T does not implement `Unpin`, it values will never be moved.
 * a extension for `Pin<Vec<T, A>>` allowing PinInit to be initialied on a
   Pin<Vec>, as well as truncating and poping values from the Vec
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 rust/kernel/alloc.rs      |  1 +
 rust/kernel/alloc/kvec.rs | 86 +++++++++++++++++++++++++++++++++++++++
 rust/kernel/prelude.rs    |  2 +-
 3 files changed, 88 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
index a2c49e5494d3..9c129eaf0625 100644
--- a/rust/kernel/alloc.rs
+++ b/rust/kernel/alloc.rs
@@ -24,6 +24,7 @@
 pub use self::kvec::KVec;
 pub use self::kvec::VVec;
 pub use self::kvec::Vec;
+pub use self::kvec::PinnedVecExt;
 
 /// Indicates an allocation error.
 #[derive(Copy, Clone, PartialEq, Eq, Debug)]
diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
index 3c72e0bdddb8..d5582a7f17e9 100644
--- a/rust/kernel/alloc/kvec.rs
+++ b/rust/kernel/alloc/kvec.rs
@@ -16,11 +16,13 @@
     ops::DerefMut,
     ops::Index,
     ops::IndexMut,
+    pin::Pin,
     ptr,
     ptr::NonNull,
     slice,
     slice::SliceIndex,
 };
+use pin_init::PinInit;
 
 mod errors;
 pub use self::errors::{InsertError, PushError, RemoveError};
@@ -109,6 +111,21 @@ pub struct Vec<T, A: Allocator> {
     _p: PhantomData<A>,
 }
 
+/// Extension for Pin<Vec<T, A>>
+pub trait PinnedVecExt<T> {
+    /// Pin-initializes P and appends it to the back of the [`Vec`] instance without reallocating.
+    fn push_pin_init<E: From<PushError<P>>, P: PinInit<T, E>>(&mut self, init: P) -> Result<(), E>;
+
+    /// Shortens the vector, setting the length to `len` and drops the removed values.
+    /// If `len` is greater than or equal to the current length, this does nothing.
+    ///
+    /// This has no effect on the capacity and will not allocate.
+    fn truncate(&mut self, len: usize);
+
+    /// Removes the last element from a vector and drops it returning true, or false if it is empty.
+    fn pop(&mut self) -> bool;
+}
+
 /// Type alias for [`Vec`] with a [`Kmalloc`] allocator.
 ///
 /// # Examples
@@ -719,6 +736,18 @@ pub fn retain(&mut self, mut f: impl FnMut(&mut T) -> bool) {
         }
         self.truncate(num_kept);
     }
+
+    /// Constructs a new `Pin<Vec<T, A>>`.
+    #[inline]
+    pub fn pin(capacity: usize, flags: Flags) -> Result<Pin<Self>, AllocError> {
+        Self::with_capacity(capacity, flags).map(Pin::<Self>::from)
+    }
+
+    /// Convert a [`Vec<T,A>`] to a [`Pin<Vec<T,A>>`]. If `T` does not implement
+    /// [`Unpin`], then its values will be pinned in memory and can't be moved.
+    pub fn into_pin(this: Self) -> Pin<Self> {
+        this.into()
+    }
 }
 
 impl<T: Clone, A: Allocator> Vec<T, A> {
@@ -1294,6 +1323,63 @@ fn drop(&mut self) {
     }
 }
 
+impl<T, A: Allocator> PinnedVecExt<T> for Pin<Vec<T, A>> {
+    fn truncate(&mut self, len: usize) {
+        // SAFETY: truncate will not reallocate the Vec
+        // CAST: Pin<Ptr> is a transparent wrapper of Ptr
+        unsafe { &mut *core::ptr::from_mut(self).cast::<Vec<T, A>>() }.truncate(len);
+    }
+
+    fn push_pin_init<E: From<PushError<P>>, P: PinInit<T, E>>(&mut self, init: P) -> Result<(), E> {
+        // SAFETY: capacity, spare_capacity_mut and inc_len will not
+        // reallocate the Vec.
+        // CAST: Pin<Ptr> is a transparent wrapper of Ptr
+        let this = unsafe { &mut *core::ptr::from_mut(self).cast::<Vec<T, A>>() };
+
+        if this.len() < this.capacity() {
+            let spare = this.spare_capacity_mut();
+            // SAFETY: the length is less than the capacity, so `spare` is non-empty.
+            unsafe { init.__pinned_init(spare.get_unchecked_mut(0).as_mut_ptr())? };
+            // SAFETY: We just initialised the first spare entry, so it is safe to
+            // increase the length by 1. We also know that the new length is <= capacity.
+            unsafe { this.inc_len(1) };
+            Ok(())
+        } else {
+            Err(E::from(PushError(init)))
+        }
+    }
+
+    fn pop(&mut self) -> bool {
+        if self.is_empty() {
+            return false;
+        }
+
+        // SAFETY:
+        // - We just checked that the length is at least one.
+        // - dec_len will not reallocate the Vec
+        // CAST: Pin<Ptr> is a transparent wrapper of Ptr
+        let ptr: *mut [T] = unsafe { (*core::ptr::from_mut(self).cast::<Vec<T, A>>()).dec_len(1) };
+
+        // SAFETY: the contract of `dec_len` guarantees that the elements in `ptr` are
+        // valid elements whose ownership has been transferred to the caller.
+        unsafe { ptr::drop_in_place(ptr) };
+        true
+    }
+}
+
+impl<T, A: Allocator> From<Vec<T, A>> for Pin<Vec<T, A>> {
+    /// Converts a `Vec<T, A>` into a `Pin<Vec<T, A>>`. If `T` does not implement [`Unpin`], then
+    /// every value in v will be pinned in memory and can't be moved.
+    ///
+    /// This moves `v` into `Pin` without moving any of the values of `v` or allocating and copying
+    /// any memory.
+    fn from(v: Vec<T, A>) -> Self {
+        // SAFETY: The values wrapped inside a `Pin<Vec<T, A>>` cannot be moved or replaced as long
+        // as `T` does not implement `Unpin`.
+        unsafe { Pin::new_unchecked(v) }
+    }
+}
+
 #[macros::kunit_tests(rust_kvec_kunit)]
 mod tests {
     use super::*;
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index 25fe97aafd02..7179e2ca2a14 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -19,7 +19,7 @@
     c_ushort, c_void,
 };
 
-pub use crate::alloc::{flags::*, Box, KBox, KVBox, KVVec, KVec, VBox, VVec, Vec};
+pub use crate::alloc::{flags::*, Box, KBox, KVBox, KVVec, KVec, PinnedVecExt, VBox, VVec, Vec};
 
 #[doc(no_inline)]
 pub use macros::{export, kunit_tests, module, vtable};
-- 
2.49.1
^ permalink raw reply related	[flat|nested] 7+ messages in thread
* [PATCH 2/2] rust: leds: add basic led classdev abstractions
  2025-10-09 17:07 [PATCH 0/2] rust: leds: add led classdev abstractions Markus Probst
  2025-10-09 17:07 ` [PATCH 1/2] rust: add basic Pin<Vec<T, A>> abstractions Markus Probst
@ 2025-10-09 17:07 ` Markus Probst
  2025-10-09 17:49   ` Onur Özkan
  2025-10-10 18:36   ` Danilo Krummrich
  2025-10-09 17:46 ` [PATCH 0/2] rust: leds: add " Onur Özkan
  2 siblings, 2 replies; 7+ messages in thread
From: Markus Probst @ 2025-10-09 17:07 UTC (permalink / raw)
  To: Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Lee Jones,
	Pavel Machek
  Cc: Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Boqun Feng, Gary Guo, bjorn3_gh, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, Markus Probst,
	rust-for-linux, linux-kernel, linux-leds
Implement the core abstractions needed for led class devices, including:
* `led::Handler` - the trait for handling leds, including
  `brightness_set`
* `led::InitData` - data set for the led class device
* `led::Device` - a safe wrapper arround `led_classdev`
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
 rust/kernel/led.rs | 296 +++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs |   1 +
 2 files changed, 297 insertions(+)
 create mode 100644 rust/kernel/led.rs
diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
new file mode 100644
index 000000000000..2fddc6088e09
--- /dev/null
+++ b/rust/kernel/led.rs
@@ -0,0 +1,296 @@
+// 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::pin::Pin;
+
+use pin_init::{pin_data, pinned_drop, PinInit};
+
+use crate::{
+    alloc::KBox,
+    container_of,
+    device::{self, property::FwNode},
+    error::{code::EINVAL, from_result, to_result, Error, Result},
+    prelude::GFP_KERNEL,
+    str::CStr,
+    try_pin_init,
+    types::Opaque,
+};
+
+/// The led class device representation.
+///
+/// This structure represents the Rust abstraction for a C `struct led_classdev`.
+#[pin_data(PinnedDrop)]
+pub struct Device {
+    handler: KBox<dyn HandlerImpl>,
+    #[pin]
+    classdev: Opaque<bindings::led_classdev>,
+}
+
+/// The led init data representation.
+///
+/// This structure represents the Rust abstraction for a C `struct led_init_data`.
+#[derive(Default)]
+pub struct InitData<'a> {
+    fwnode: Option<&'a FwNode>,
+    default_label: Option<&'a CStr>,
+    devicename: Option<&'a CStr>,
+    devname_mandatory: bool,
+}
+
+impl InitData<'static> {
+    /// Creates a new [`LedInitData`]
+    pub fn new() -> Self {
+        Self::default()
+    }
+}
+
+impl<'a> InitData<'a> {
+    /// Sets the firmware node
+    pub fn fwnode<'b, 'c>(self, fwnode: &'b FwNode) -> InitData<'c>
+    where
+        'a: 'c,
+        'b: 'c,
+    {
+        InitData {
+            fwnode: Some(fwnode),
+            ..self
+        }
+    }
+
+    /// Sets a default label
+    pub fn default_label<'b, 'c>(self, label: &'b CStr) -> InitData<'c>
+    where
+        'a: 'c,
+        'b: 'c,
+    {
+        InitData {
+            default_label: Some(label),
+            ..self
+        }
+    }
+
+    /// Sets the device name
+    pub fn devicename<'b, 'c>(self, devicename: &'b CStr) -> InitData<'c>
+    where
+        'a: 'c,
+        'b: 'c,
+    {
+        InitData {
+            devicename: Some(devicename),
+            ..self
+        }
+    }
+
+    /// Sets if a device name is mandatory
+    pub fn devicename_mandatory(self, mandatory: bool) -> Self {
+        Self {
+            devname_mandatory: mandatory,
+
+            ..self
+        }
+    }
+}
+
+/// The led handler trait.
+///
+/// # Examples
+///
+///```
+/// # use kernel::{c_str, led, alloc::KBox, error::{Result, code::ENOSYS}};
+/// # use core::pin::Pin;
+///
+/// struct MyHandler;
+///
+///
+/// impl led::Handler for MyHandler {
+///     const BLOCKING = false;
+///     const MAX_BRIGHTNESS = 255;
+///
+///     fn brightness_set(&self, _brightness: u32) -> Result<()> {
+///         // Set the brightness for the led here
+///         Ok(())
+///     }
+/// }
+///
+/// fn register_my_led() -> Result<Pin<KBox<led::Device>>> {
+///     let handler = MyHandler;
+///     KBox::pin_init(led::Device::new(
+///         None,
+///         None,
+///         led::InitData::new()
+///             .default_label(c_str!("my_led")),
+///         handler,
+///     ))
+/// }
+///```
+/// Led drivers must implement this trait in order to register and handle a [`Device`].
+pub trait Handler {
+    /// If set true, [`Handler::brightness_set`] and [`Handler::blink_set`] must not sleep
+    /// and perform the operation immediately.
+    const BLOCKING: bool;
+    /// Set this to true, if [`Handler::blink_set`] is implemented.
+    const BLINK: bool = false;
+    /// The max brightness level
+    const MAX_BRIGHTNESS: u32;
+
+    /// Sets the brightness level
+    ///
+    /// See also [`Handler::BLOCKING`]
+    fn brightness_set(&self, brightness: u32) -> Result<()>;
+
+    /// 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 [`Handler::BLOCKING`]
+    fn blink_set(&self, _delay_on: &mut usize, _delay_off: &mut usize) -> Result<()> {
+        Err(EINVAL)
+    }
+}
+
+trait HandlerImpl {
+    fn brightness_set(&self, brightness: u32) -> Result<()>;
+    fn blink_set(&self, delay_on: &mut usize, delay_off: &mut usize) -> Result<()>;
+}
+
+impl<T: Handler> HandlerImpl for T {
+    fn brightness_set(&self, brightness: u32) -> Result<()> {
+        T::brightness_set(self, brightness)
+    }
+
+    fn blink_set(&self, delay_on: &mut usize, delay_off: &mut usize) -> Result<()> {
+        T::blink_set(self, delay_on, delay_off)
+    }
+}
+
+// SAFETY: A `led::Device` can be unregistered from any thread.
+unsafe impl Send for Device {}
+
+// SAFETY: `led::Device` can be shared among threads because all methods of `led::Device`
+// are thread safe.
+unsafe impl Sync for Device {}
+
+impl Device {
+    /// Registers a new led classdev.
+    ///
+    /// The [`Device`] will be unregistered and drop.
+    pub fn new<'a, T: Handler + 'static>(
+        parent: Option<&'a device::Device>,
+        init_data: InitData<'a>,
+        handler: T,
+    ) -> impl PinInit<Self, Error> + use<'a, T> {
+        try_pin_init!(Self {
+            handler <- {
+                let handler: KBox<dyn HandlerImpl> = KBox::<T>::new(handler, GFP_KERNEL)?;
+                Ok::<_, Error>(handler)
+            },
+            classdev <- Opaque::try_ffi_init(|ptr: *mut bindings::led_classdev| {
+                // SAFETY: `try_ffi_init` guarantees that `ptr` is valid for write.
+                unsafe { ptr.write(bindings::led_classdev {
+                    max_brightness: T::MAX_BRIGHTNESS,
+                    brightness_set: T::BLOCKING.then_some(
+                        brightness_set as unsafe extern "C" fn(*mut bindings::led_classdev, u32)
+                    ),
+                    brightness_set_blocking: (!T::BLOCKING).then_some(
+                        brightness_set_blocking
+                            as unsafe extern "C" fn(*mut bindings::led_classdev,u32) -> i32
+                    ),
+                    blink_set: T::BLINK.then_some(
+                        blink_set
+                            as unsafe extern "C" fn(*mut bindings::led_classdev, *mut usize,
+                                                    *mut usize) -> i32
+                    ),
+                    .. bindings::led_classdev::default()
+                }) };
+
+                let mut init_data = bindings::led_init_data {
+                    fwnode: init_data.fwnode.map_or(core::ptr::null_mut(), FwNode::as_raw),
+                    default_label: init_data.default_label
+                                            .map_or(core::ptr::null(), CStr::as_char_ptr),
+                    devicename: init_data.devicename.map_or(core::ptr::null(), CStr::as_char_ptr),
+                    devname_mandatory: init_data.devname_mandatory,
+                };
+
+                let parent = parent
+                    .map_or(core::ptr::null_mut(), device::Device::as_raw);
+
+                // SAFETY:
+                // - `parent` is guaranteed to be a pointer to a valid `device`
+                //    or a null pointer.
+                // - `ptr` is guaranteed to be a pointer to an initialized `led_classdev`.
+                to_result(unsafe {
+                    bindings::led_classdev_register_ext(parent, ptr, &mut init_data)
+                })
+            }),
+        })
+    }
+}
+
+extern "C" fn brightness_set(led_cdev: *mut bindings::led_classdev, brightness: u32) {
+    // SAFETY: `led_cdev` is a valid pointer to a `led_classdev` stored inside a `Device`.
+    let classdev = unsafe {
+        &*container_of!(
+            led_cdev.cast::<Opaque<bindings::led_classdev>>(),
+            Device,
+            classdev
+        )
+    };
+    let _ = classdev.handler.brightness_set(brightness);
+}
+
+extern "C" fn brightness_set_blocking(
+    led_cdev: *mut bindings::led_classdev,
+    brightness: u32,
+) -> i32 {
+    // SAFETY: `led_cdev` is a valid pointer to a `led_classdev` stored inside a `Device`.
+    let classdev = unsafe {
+        &*container_of!(
+            led_cdev.cast::<Opaque<bindings::led_classdev>>(),
+            Device,
+            classdev
+        )
+    };
+    from_result(|| {
+        classdev.handler.brightness_set(brightness)?;
+        Ok(0)
+    })
+}
+
+extern "C" fn blink_set(
+    led_cdev: *mut bindings::led_classdev,
+    delay_on: *mut usize,
+    delay_off: *mut usize,
+) -> i32 {
+    // SAFETY: `led_cdev` is a valid pointer to a `led_classdev` stored inside a `Device`.
+    let classdev = unsafe {
+        &*container_of!(
+            led_cdev.cast::<Opaque<bindings::led_classdev>>(),
+            Device,
+            classdev
+        )
+    };
+    from_result(|| {
+        classdev.handler.blink_set(
+            // SAFETY: `delay_on` is guaranteed to be a valid pointer to usize
+            unsafe { &mut *delay_on },
+            // SAFETY: `delay_on` is guaranteed to be a valid pointer to usize
+            unsafe { &mut *delay_off },
+        )?;
+        Ok(0)
+    })
+}
+
+#[pinned_drop]
+impl PinnedDrop for Device {
+    fn drop(self: Pin<&mut Self>) {
+        // SAFETY: The existence of `self` guarantees that `self.classdev` has previously been
+        // successfully registered with `led_classdev_register_ext`
+        unsafe { bindings::led_classdev_unregister(self.classdev.get()) };
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index e5247f584ad2..f42c60da21ae 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -97,6 +97,7 @@
 pub mod jump_label;
 #[cfg(CONFIG_KUNIT)]
 pub mod kunit;
+pub mod led;
 pub mod list;
 pub mod miscdevice;
 pub mod mm;
-- 
2.49.1
^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] rust: add basic Pin<Vec<T, A>> abstractions
  2025-10-09 17:07 ` [PATCH 1/2] rust: add basic Pin<Vec<T, A>> abstractions Markus Probst
@ 2025-10-09 17:45   ` Onur Özkan
  0 siblings, 0 replies; 7+ messages in thread
From: Onur Özkan @ 2025-10-09 17:45 UTC (permalink / raw)
  To: Markus Probst
  Cc: Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Lee Jones,
	Pavel Machek, Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Boqun Feng, Gary Guo, bjorn3_gh, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, rust-for-linux,
	linux-kernel, linux-leds
On Thu, 09 Oct 2025 17:07:56 +0000
Markus Probst <markus.probst@posteo.de> wrote:
Hi Markus,
I noticed a few typos in the code comments and the commit descriptions.
> Implement core Pin<Vec<T, A>> abstractions, incluing
>  * `Vec::pin` and `Vec::into_pin` for constructing a `Pin<Vec<T, A>>`.
>    If T does not implement `Unpin`, it values will never be moved.
>  * a extension for `Pin<Vec<T, A>>` allowing PinInit to be initialied
> on a Pin<Vec>, as well as truncating and poping values from the Vec
> 
"incluing" -> "including"
"it values will..." -> "its values will..."
"a extension" -> "an extension"
"initialied" -> "initialized"
"poping values" -> "popping values"
Thanks,
Onur
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
>  rust/kernel/alloc.rs      |  1 +
>  rust/kernel/alloc/kvec.rs | 86
> +++++++++++++++++++++++++++++++++++++++ rust/kernel/prelude.rs    |
> 2 +- 3 files changed, 88 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
> index a2c49e5494d3..9c129eaf0625 100644
> --- a/rust/kernel/alloc.rs
> +++ b/rust/kernel/alloc.rs
> @@ -24,6 +24,7 @@
>  pub use self::kvec::KVec;
>  pub use self::kvec::VVec;
>  pub use self::kvec::Vec;
> +pub use self::kvec::PinnedVecExt;
>  
>  /// Indicates an allocation error.
>  #[derive(Copy, Clone, PartialEq, Eq, Debug)]
> diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
> index 3c72e0bdddb8..d5582a7f17e9 100644
> --- a/rust/kernel/alloc/kvec.rs
> +++ b/rust/kernel/alloc/kvec.rs
> @@ -16,11 +16,13 @@
>      ops::DerefMut,
>      ops::Index,
>      ops::IndexMut,
> +    pin::Pin,
>      ptr,
>      ptr::NonNull,
>      slice,
>      slice::SliceIndex,
>  };
> +use pin_init::PinInit;
>  
>  mod errors;
>  pub use self::errors::{InsertError, PushError, RemoveError};
> @@ -109,6 +111,21 @@ pub struct Vec<T, A: Allocator> {
>      _p: PhantomData<A>,
>  }
>  
> +/// Extension for Pin<Vec<T, A>>
> +pub trait PinnedVecExt<T> {
> +    /// Pin-initializes P and appends it to the back of the [`Vec`]
> instance without reallocating.
> +    fn push_pin_init<E: From<PushError<P>>, P: PinInit<T, E>>(&mut
> self, init: P) -> Result<(), E>; +
> +    /// Shortens the vector, setting the length to `len` and drops
> the removed values.
> +    /// If `len` is greater than or equal to the current length,
> this does nothing.
> +    ///
> +    /// This has no effect on the capacity and will not allocate.
> +    fn truncate(&mut self, len: usize);
> +
> +    /// Removes the last element from a vector and drops it
> returning true, or false if it is empty.
> +    fn pop(&mut self) -> bool;
> +}
> +
>  /// Type alias for [`Vec`] with a [`Kmalloc`] allocator.
>  ///
>  /// # Examples
> @@ -719,6 +736,18 @@ pub fn retain(&mut self, mut f: impl FnMut(&mut
> T) -> bool) { }
>          self.truncate(num_kept);
>      }
> +
> +    /// Constructs a new `Pin<Vec<T, A>>`.
> +    #[inline]
> +    pub fn pin(capacity: usize, flags: Flags) -> Result<Pin<Self>,
> AllocError> {
> +        Self::with_capacity(capacity, flags).map(Pin::<Self>::from)
> +    }
> +
> +    /// Convert a [`Vec<T,A>`] to a [`Pin<Vec<T,A>>`]. If `T` does
> not implement
> +    /// [`Unpin`], then its values will be pinned in memory and
> can't be moved.
> +    pub fn into_pin(this: Self) -> Pin<Self> {
> +        this.into()
> +    }
>  }
>  
>  impl<T: Clone, A: Allocator> Vec<T, A> {
> @@ -1294,6 +1323,63 @@ fn drop(&mut self) {
>      }
>  }
>  
> +impl<T, A: Allocator> PinnedVecExt<T> for Pin<Vec<T, A>> {
> +    fn truncate(&mut self, len: usize) {
> +        // SAFETY: truncate will not reallocate the Vec
> +        // CAST: Pin<Ptr> is a transparent wrapper of Ptr
> +        unsafe { &mut *core::ptr::from_mut(self).cast::<Vec<T, A>>()
> }.truncate(len);
> +    }
> +
> +    fn push_pin_init<E: From<PushError<P>>, P: PinInit<T, E>>(&mut
> self, init: P) -> Result<(), E> {
> +        // SAFETY: capacity, spare_capacity_mut and inc_len will not
> +        // reallocate the Vec.
> +        // CAST: Pin<Ptr> is a transparent wrapper of Ptr
> +        let this = unsafe { &mut
> *core::ptr::from_mut(self).cast::<Vec<T, A>>() }; +
> +        if this.len() < this.capacity() {
> +            let spare = this.spare_capacity_mut();
> +            // SAFETY: the length is less than the capacity, so
> `spare` is non-empty.
> +            unsafe {
> init.__pinned_init(spare.get_unchecked_mut(0).as_mut_ptr())? };
> +            // SAFETY: We just initialised the first spare entry, so
> it is safe to
> +            // increase the length by 1. We also know that the new
> length is <= capacity.
> +            unsafe { this.inc_len(1) };
> +            Ok(())
> +        } else {
> +            Err(E::from(PushError(init)))
> +        }
> +    }
> +
> +    fn pop(&mut self) -> bool {
> +        if self.is_empty() {
> +            return false;
> +        }
> +
> +        // SAFETY:
> +        // - We just checked that the length is at least one.
> +        // - dec_len will not reallocate the Vec
> +        // CAST: Pin<Ptr> is a transparent wrapper of Ptr
> +        let ptr: *mut [T] = unsafe {
> (*core::ptr::from_mut(self).cast::<Vec<T, A>>()).dec_len(1) }; +
> +        // SAFETY: the contract of `dec_len` guarantees that the
> elements in `ptr` are
> +        // valid elements whose ownership has been transferred to
> the caller.
> +        unsafe { ptr::drop_in_place(ptr) };
> +        true
> +    }
> +}
> +
> +impl<T, A: Allocator> From<Vec<T, A>> for Pin<Vec<T, A>> {
> +    /// Converts a `Vec<T, A>` into a `Pin<Vec<T, A>>`. If `T` does
> not implement [`Unpin`], then
> +    /// every value in v will be pinned in memory and can't be moved.
> +    ///
> +    /// This moves `v` into `Pin` without moving any of the values
> of `v` or allocating and copying
> +    /// any memory.
> +    fn from(v: Vec<T, A>) -> Self {
> +        // SAFETY: The values wrapped inside a `Pin<Vec<T, A>>`
> cannot be moved or replaced as long
> +        // as `T` does not implement `Unpin`.
> +        unsafe { Pin::new_unchecked(v) }
> +    }
> +}
> +
>  #[macros::kunit_tests(rust_kvec_kunit)]
>  mod tests {
>      use super::*;
> diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
> index 25fe97aafd02..7179e2ca2a14 100644
> --- a/rust/kernel/prelude.rs
> +++ b/rust/kernel/prelude.rs
> @@ -19,7 +19,7 @@
>      c_ushort, c_void,
>  };
>  
> -pub use crate::alloc::{flags::*, Box, KBox, KVBox, KVVec, KVec,
> VBox, VVec, Vec}; +pub use crate::alloc::{flags::*, Box, KBox, KVBox,
> KVVec, KVec, PinnedVecExt, VBox, VVec, Vec}; 
>  #[doc(no_inline)]
>  pub use macros::{export, kunit_tests, module, vtable};
^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH 0/2] rust: leds: add led classdev abstractions
  2025-10-09 17:07 [PATCH 0/2] rust: leds: add led classdev abstractions Markus Probst
  2025-10-09 17:07 ` [PATCH 1/2] rust: add basic Pin<Vec<T, A>> abstractions Markus Probst
  2025-10-09 17:07 ` [PATCH 2/2] rust: leds: add basic led classdev abstractions Markus Probst
@ 2025-10-09 17:46 ` Onur Özkan
  2 siblings, 0 replies; 7+ messages in thread
From: Onur Özkan @ 2025-10-09 17:46 UTC (permalink / raw)
  To: Markus Probst
  Cc: Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Lee Jones,
	Pavel Machek, Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Boqun Feng, Gary Guo, bjorn3_gh, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, rust-for-linux,
	linux-kernel, linux-leds
On Thu, 09 Oct 2025 17:07:55 +0000
Markus Probst <markus.probst@posteo.de> wrote:
> This patch series has previously been contained in
> https://lore.kernel.org/rust-for-linux/20251008181027.662616-1-markus.probst@posteo.de/T/#t
> which added a rust written led driver for a microcontroller via i2c.
> 
> As the reading and writing to the i2c client via the register!
> macro has not been implemented yet [1], the patch series will only
> contain the additional abstractions required.
> 
> [1]
> https://lore.kernel.org/rust-for-linux/DDDS2V0V2NVJ.16ZKXCKUA1HUV@kernel.org/
> 
> The following changes were made:
> * add basic Pin<Vec<T, A>> abstractions, that allow to initialize
> PinInit items with the gurantee that these will never be moved.
> 
"gurantee" -> "guarantee"
> * add basic led classdev abstractions to register and unregister leds
> 
> 
> Markus Probst (2):
>   rust: add basic Pin<Vec<T, A>> abstractions
>   rust: leds: add basic led classdev abstractions
> 
>  rust/kernel/alloc.rs      |   1 +
>  rust/kernel/alloc/kvec.rs |  86 +++++++++++
>  rust/kernel/led.rs        | 296
> ++++++++++++++++++++++++++++++++++++++ rust/kernel/lib.rs        |
> 1 + rust/kernel/prelude.rs    |   2 +-
>  5 files changed, 385 insertions(+), 1 deletion(-)
>  create mode 100644 rust/kernel/led.rs
> 
^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] rust: leds: add basic led classdev abstractions
  2025-10-09 17:07 ` [PATCH 2/2] rust: leds: add basic led classdev abstractions Markus Probst
@ 2025-10-09 17:49   ` Onur Özkan
  2025-10-10 18:36   ` Danilo Krummrich
  1 sibling, 0 replies; 7+ messages in thread
From: Onur Özkan @ 2025-10-09 17:49 UTC (permalink / raw)
  To: Markus Probst
  Cc: Danilo Krummrich, Miguel Ojeda, Alex Gaynor, Lee Jones,
	Pavel Machek, Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Boqun Feng, Gary Guo, bjorn3_gh, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, rust-for-linux,
	linux-kernel, linux-leds
On Thu, 09 Oct 2025 17:07:57 +0000
Markus Probst <markus.probst@posteo.de> wrote:
> Implement the core abstractions needed for led class devices,
> including:
> 
> * `led::Handler` - the trait for handling leds, including
>   `brightness_set`
> 
> * `led::InitData` - data set for the led class device
> 
> * `led::Device` - a safe wrapper arround `led_classdev`
> 
"arround" -> "around"
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
>  rust/kernel/led.rs | 296
> +++++++++++++++++++++++++++++++++++++++++++++ rust/kernel/lib.rs |
> 1 + 2 files changed, 297 insertions(+)
>  create mode 100644 rust/kernel/led.rs
> 
> diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
> new file mode 100644
> index 000000000000..2fddc6088e09
> --- /dev/null
> +++ b/rust/kernel/led.rs
> @@ -0,0 +1,296 @@
> +// 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::pin::Pin;
> +
> +use pin_init::{pin_data, pinned_drop, PinInit};
> +
> +use crate::{
> +    alloc::KBox,
> +    container_of,
> +    device::{self, property::FwNode},
> +    error::{code::EINVAL, from_result, to_result, Error, Result},
> +    prelude::GFP_KERNEL,
> +    str::CStr,
> +    try_pin_init,
> +    types::Opaque,
> +};
> +
> +/// The led class device representation.
> +///
> +/// This structure represents the Rust abstraction for a C `struct
> led_classdev`. +#[pin_data(PinnedDrop)]
> +pub struct Device {
> +    handler: KBox<dyn HandlerImpl>,
> +    #[pin]
> +    classdev: Opaque<bindings::led_classdev>,
> +}
> +
> +/// The led init data representation.
> +///
> +/// This structure represents the Rust abstraction for a C `struct
> led_init_data`. +#[derive(Default)]
> +pub struct InitData<'a> {
> +    fwnode: Option<&'a FwNode>,
> +    default_label: Option<&'a CStr>,
> +    devicename: Option<&'a CStr>,
> +    devname_mandatory: bool,
> +}
> +
> +impl InitData<'static> {
> +    /// Creates a new [`LedInitData`]
I think you meant InitData here.
> +    pub fn new() -> Self {
> +        Self::default()
> +    }
> +}
> +
> +impl<'a> InitData<'a> {
> +    /// Sets the firmware node
> +    pub fn fwnode<'b, 'c>(self, fwnode: &'b FwNode) -> InitData<'c>
> +    where
> +        'a: 'c,
> +        'b: 'c,
> +    {
> +        InitData {
> +            fwnode: Some(fwnode),
> +            ..self
> +        }
> +    }
> +
> +    /// Sets a default label
> +    pub fn default_label<'b, 'c>(self, label: &'b CStr) ->
> InitData<'c>
> +    where
> +        'a: 'c,
> +        'b: 'c,
> +    {
> +        InitData {
> +            default_label: Some(label),
> +            ..self
> +        }
> +    }
> +
> +    /// Sets the device name
> +    pub fn devicename<'b, 'c>(self, devicename: &'b CStr) ->
> InitData<'c>
> +    where
> +        'a: 'c,
> +        'b: 'c,
> +    {
> +        InitData {
> +            devicename: Some(devicename),
> +            ..self
> +        }
> +    }
> +
> +    /// Sets if a device name is mandatory
> +    pub fn devicename_mandatory(self, mandatory: bool) -> Self {
> +        Self {
> +            devname_mandatory: mandatory,
> +
> +            ..self
> +        }
> +    }
> +}
> +
> +/// The led handler trait.
> +///
> +/// # Examples
> +///
> +///```
> +/// # use kernel::{c_str, led, alloc::KBox, error::{Result,
> code::ENOSYS}}; +/// # use core::pin::Pin;
> +///
> +/// struct MyHandler;
> +///
> +///
> +/// impl led::Handler for MyHandler {
> +///     const BLOCKING = false;
> +///     const MAX_BRIGHTNESS = 255;
> +///
> +///     fn brightness_set(&self, _brightness: u32) -> Result<()> {
> +///         // Set the brightness for the led here
> +///         Ok(())
> +///     }
> +/// }
> +///
> +/// fn register_my_led() -> Result<Pin<KBox<led::Device>>> {
> +///     let handler = MyHandler;
> +///     KBox::pin_init(led::Device::new(
> +///         None,
> +///         None,
> +///         led::InitData::new()
> +///             .default_label(c_str!("my_led")),
> +///         handler,
> +///     ))
> +/// }
> +///```
> +/// Led drivers must implement this trait in order to register and
> handle a [`Device`]. +pub trait Handler {
> +    /// If set true, [`Handler::brightness_set`] and
> [`Handler::blink_set`] must not sleep
> +    /// and perform the operation immediately.
> +    const BLOCKING: bool;
> +    /// Set this to true, if [`Handler::blink_set`] is implemented.
> +    const BLINK: bool = false;
> +    /// The max brightness level
> +    const MAX_BRIGHTNESS: u32;
> +
> +    /// Sets the brightness level
> +    ///
> +    /// See also [`Handler::BLOCKING`]
> +    fn brightness_set(&self, brightness: u32) -> Result<()>;
> +
> +    /// 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 [`Handler::BLOCKING`]
> +    fn blink_set(&self, _delay_on: &mut usize, _delay_off: &mut
> usize) -> Result<()> {
> +        Err(EINVAL)
> +    }
> +}
> +
> +trait HandlerImpl {
> +    fn brightness_set(&self, brightness: u32) -> Result<()>;
> +    fn blink_set(&self, delay_on: &mut usize, delay_off: &mut usize)
> -> Result<()>; +}
> +
> +impl<T: Handler> HandlerImpl for T {
> +    fn brightness_set(&self, brightness: u32) -> Result<()> {
> +        T::brightness_set(self, brightness)
> +    }
> +
> +    fn blink_set(&self, delay_on: &mut usize, delay_off: &mut usize)
> -> Result<()> {
> +        T::blink_set(self, delay_on, delay_off)
> +    }
> +}
> +
> +// SAFETY: A `led::Device` can be unregistered from any thread.
> +unsafe impl Send for Device {}
> +
> +// SAFETY: `led::Device` can be shared among threads because all
> methods of `led::Device` +// are thread safe.
> +unsafe impl Sync for Device {}
> +
> +impl Device {
> +    /// Registers a new led classdev.
> +    ///
> +    /// The [`Device`] will be unregistered and drop.
> +    pub fn new<'a, T: Handler + 'static>(
> +        parent: Option<&'a device::Device>,
> +        init_data: InitData<'a>,
> +        handler: T,
> +    ) -> impl PinInit<Self, Error> + use<'a, T> {
> +        try_pin_init!(Self {
> +            handler <- {
> +                let handler: KBox<dyn HandlerImpl> =
> KBox::<T>::new(handler, GFP_KERNEL)?;
> +                Ok::<_, Error>(handler)
> +            },
> +            classdev <- Opaque::try_ffi_init(|ptr: *mut
> bindings::led_classdev| {
> +                // SAFETY: `try_ffi_init` guarantees that `ptr` is
> valid for write.
> +                unsafe { ptr.write(bindings::led_classdev {
> +                    max_brightness: T::MAX_BRIGHTNESS,
> +                    brightness_set: T::BLOCKING.then_some(
> +                        brightness_set as unsafe extern "C" fn(*mut
> bindings::led_classdev, u32)
> +                    ),
> +                    brightness_set_blocking:
> (!T::BLOCKING).then_some(
> +                        brightness_set_blocking
> +                            as unsafe extern "C" fn(*mut
> bindings::led_classdev,u32) -> i32
> +                    ),
> +                    blink_set: T::BLINK.then_some(
> +                        blink_set
> +                            as unsafe extern "C" fn(*mut
> bindings::led_classdev, *mut usize,
> +                                                    *mut usize) ->
> i32
> +                    ),
> +                    .. bindings::led_classdev::default()
> +                }) };
> +
> +                let mut init_data = bindings::led_init_data {
> +                    fwnode:
> init_data.fwnode.map_or(core::ptr::null_mut(), FwNode::as_raw),
> +                    default_label: init_data.default_label
> +
> .map_or(core::ptr::null(), CStr::as_char_ptr),
> +                    devicename:
> init_data.devicename.map_or(core::ptr::null(), CStr::as_char_ptr),
> +                    devname_mandatory: init_data.devname_mandatory,
> +                };
> +
> +                let parent = parent
> +                    .map_or(core::ptr::null_mut(),
> device::Device::as_raw); +
> +                // SAFETY:
> +                // - `parent` is guaranteed to be a pointer to a
> valid `device`
> +                //    or a null pointer.
> +                // - `ptr` is guaranteed to be a pointer to an
> initialized `led_classdev`.
> +                to_result(unsafe {
> +                    bindings::led_classdev_register_ext(parent, ptr,
> &mut init_data)
> +                })
> +            }),
> +        })
> +    }
> +}
> +
> +extern "C" fn brightness_set(led_cdev: *mut bindings::led_classdev,
> brightness: u32) {
> +    // SAFETY: `led_cdev` is a valid pointer to a `led_classdev`
> stored inside a `Device`.
> +    let classdev = unsafe {
> +        &*container_of!(
> +            led_cdev.cast::<Opaque<bindings::led_classdev>>(),
> +            Device,
> +            classdev
> +        )
> +    };
> +    let _ = classdev.handler.brightness_set(brightness);
> +}
> +
> +extern "C" fn brightness_set_blocking(
> +    led_cdev: *mut bindings::led_classdev,
> +    brightness: u32,
> +) -> i32 {
> +    // SAFETY: `led_cdev` is a valid pointer to a `led_classdev`
> stored inside a `Device`.
> +    let classdev = unsafe {
> +        &*container_of!(
> +            led_cdev.cast::<Opaque<bindings::led_classdev>>(),
> +            Device,
> +            classdev
> +        )
> +    };
> +    from_result(|| {
> +        classdev.handler.brightness_set(brightness)?;
> +        Ok(0)
> +    })
> +}
> +
> +extern "C" fn blink_set(
> +    led_cdev: *mut bindings::led_classdev,
> +    delay_on: *mut usize,
> +    delay_off: *mut usize,
> +) -> i32 {
> +    // SAFETY: `led_cdev` is a valid pointer to a `led_classdev`
> stored inside a `Device`.
> +    let classdev = unsafe {
> +        &*container_of!(
> +            led_cdev.cast::<Opaque<bindings::led_classdev>>(),
> +            Device,
> +            classdev
> +        )
> +    };
> +    from_result(|| {
> +        classdev.handler.blink_set(
> +            // SAFETY: `delay_on` is guaranteed to be a valid
> pointer to usize
> +            unsafe { &mut *delay_on },
> +            // SAFETY: `delay_on` is guaranteed to be a valid
> pointer to usize
> +            unsafe { &mut *delay_off },
> +        )?;
> +        Ok(0)
> +    })
> +}
> +
> +#[pinned_drop]
> +impl PinnedDrop for Device {
> +    fn drop(self: Pin<&mut Self>) {
> +        // SAFETY: The existence of `self` guarantees that
> `self.classdev` has previously been
> +        // successfully registered with `led_classdev_register_ext`
> +        unsafe {
> bindings::led_classdev_unregister(self.classdev.get()) };
> +    }
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index e5247f584ad2..f42c60da21ae 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -97,6 +97,7 @@
>  pub mod jump_label;
>  #[cfg(CONFIG_KUNIT)]
>  pub mod kunit;
> +pub mod led;
>  pub mod list;
>  pub mod miscdevice;
>  pub mod mm;
^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] rust: leds: add basic led classdev abstractions
  2025-10-09 17:07 ` [PATCH 2/2] rust: leds: add basic led classdev abstractions Markus Probst
  2025-10-09 17:49   ` Onur Özkan
@ 2025-10-10 18:36   ` Danilo Krummrich
  1 sibling, 0 replies; 7+ messages in thread
From: Danilo Krummrich @ 2025-10-10 18:36 UTC (permalink / raw)
  To: Markus Probst
  Cc: Miguel Ojeda, Alex Gaynor, Lee Jones, Pavel Machek,
	Lorenzo Stoakes, Vlastimil Babka, Liam R. Howlett,
	Uladzislau Rezki, Boqun Feng, Gary Guo, bjorn3_gh, Benno Lossin,
	Andreas Hindborg, Alice Ryhl, Trevor Gross, rust-for-linux,
	linux-kernel, linux-leds
On Thu Oct 9, 2025 at 7:07 PM CEST, Markus Probst wrote:
> +/// The led class device representation.
> +///
> +/// This structure represents the Rust abstraction for a C `struct led_classdev`.
> +#[pin_data(PinnedDrop)]
> +pub struct Device {
> +    handler: KBox<dyn HandlerImpl>,
The corresponding callbacks already exist in struct led_classdev, please use
them instead.
> +    #[pin]
> +    classdev: Opaque<bindings::led_classdev>,
> +}
<snip>
> +// SAFETY: A `led::Device` can be unregistered from any thread.
> +unsafe impl Send for Device {}
> +
> +// SAFETY: `led::Device` can be shared among threads because all methods of `led::Device`
> +// are thread safe.
> +unsafe impl Sync for Device {}
> +
> +impl Device {
> +    /// Registers a new led classdev.
> +    ///
> +    /// The [`Device`] will be unregistered and drop.
> +    pub fn new<'a, T: Handler + 'static>(
> +        parent: Option<&'a device::Device>,
This should not be an Option, all LED devices should have a parent (bus) device.
Also, it should be a &Device<Bound>, i.e. a device that is guaranteed to be
bound to a driver.
> +        init_data: InitData<'a>,
> +        handler: T,
> +    ) -> impl PinInit<Self, Error> + use<'a, T> {
This will not compile for all supported compiler versions. For now it just has
to be:
	impl PinInit<Self, Error> + 'a
Besides that, this should return impl PinInit<Devres<Self>, Error> + 'a instead.
This will allow you to let callbacks, such as blink_set(), to provide the parent
device as &Device<Bound>.
Please see also the PWM abstractions for reference [1]. There's one difference between
LED and PWM though. Unlike PWM (and most other class device implementations) LED
combines initialization and registration of the device, hence PWM is slightly
different in its implementation details, but semantically it's the same.
[1] https://lore.kernel.org/all/20250930-rust-next-pwm-working-fan-for-sending-v15-3-5661c3090877@samsung.com/
> +        try_pin_init!(Self {
> +            handler <- {
> +                let handler: KBox<dyn HandlerImpl> = KBox::<T>::new(handler, GFP_KERNEL)?;
> +                Ok::<_, Error>(handler)
> +            },
> +            classdev <- Opaque::try_ffi_init(|ptr: *mut bindings::led_classdev| {
> +                // SAFETY: `try_ffi_init` guarantees that `ptr` is valid for write.
> +                unsafe { ptr.write(bindings::led_classdev {
This looks very odd at a first glance, but it is indeed correct. It would be
good to add a comment that the parts of struct led_classdev that need to be
initialized in place (e.g. the struct mutex and struct list_head) are
initialized by led_classdev_register_ext() (which is not exactly straight
forward).
> +                    max_brightness: T::MAX_BRIGHTNESS,
> +                    brightness_set: T::BLOCKING.then_some(
> +                        brightness_set as unsafe extern "C" fn(*mut bindings::led_classdev, u32)
> +                    ),
> +                    brightness_set_blocking: (!T::BLOCKING).then_some(
> +                        brightness_set_blocking
> +                            as unsafe extern "C" fn(*mut bindings::led_classdev,u32) -> i32
> +                    ),
> +                    blink_set: T::BLINK.then_some(
> +                        blink_set
> +                            as unsafe extern "C" fn(*mut bindings::led_classdev, *mut usize,
> +                                                    *mut usize) -> i32
> +                    ),
> +                    .. bindings::led_classdev::default()
> +                }) };
> +
> +                let mut init_data = bindings::led_init_data {
> +                    fwnode: init_data.fwnode.map_or(core::ptr::null_mut(), FwNode::as_raw),
> +                    default_label: init_data.default_label
> +                                            .map_or(core::ptr::null(), CStr::as_char_ptr),
> +                    devicename: init_data.devicename.map_or(core::ptr::null(), CStr::as_char_ptr),
> +                    devname_mandatory: init_data.devname_mandatory,
> +                };
> +
> +                let parent = parent
> +                    .map_or(core::ptr::null_mut(), device::Device::as_raw);
> +
> +                // SAFETY:
> +                // - `parent` is guaranteed to be a pointer to a valid `device`
> +                //    or a null pointer.
> +                // - `ptr` is guaranteed to be a pointer to an initialized `led_classdev`.
> +                to_result(unsafe {
> +                    bindings::led_classdev_register_ext(parent, ptr, &mut init_data)
> +                })
> +            }),
> +        })
> +    }
> +}
^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-10-10 18:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-09 17:07 [PATCH 0/2] rust: leds: add led classdev abstractions Markus Probst
2025-10-09 17:07 ` [PATCH 1/2] rust: add basic Pin<Vec<T, A>> abstractions Markus Probst
2025-10-09 17:45   ` Onur Özkan
2025-10-09 17:07 ` [PATCH 2/2] rust: leds: add basic led classdev abstractions Markus Probst
2025-10-09 17:49   ` Onur Özkan
2025-10-10 18:36   ` Danilo Krummrich
2025-10-09 17:46 ` [PATCH 0/2] rust: leds: add " Onur Özkan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).