lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251009204922.084da00b@nimda.home>
Date: Thu, 9 Oct 2025 20:49:22 +0300
From: Onur Özkan <work@...rozkan.dev>
To: Markus Probst <markus.probst@...teo.de>
Cc: Danilo Krummrich <dakr@...nel.org>, Miguel Ojeda <ojeda@...nel.org>,
 Alex Gaynor <alex.gaynor@...il.com>, Lee Jones <lee@...nel.org>, Pavel
 Machek <pavel@...nel.org>, Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 Vlastimil Babka <vbabka@...e.cz>, "Liam R. Howlett"
 <Liam.Howlett@...cle.com>, Uladzislau Rezki <urezki@...il.com>, Boqun Feng
 <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
 bjorn3_gh@...tonmail.com, Benno Lossin <lossin@...nel.org>, Andreas
 Hindborg <a.hindborg@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>, Trevor
 Gross <tmgross@...ch.edu>, rust-for-linux@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-leds@...r.kernel.org
Subject: Re: [PATCH 2/2] rust: leds: add basic led classdev abstractions

On Thu, 09 Oct 2025 17:07:57 +0000
Markus Probst <markus.probst@...teo.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@...teo.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;


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ