linux-kernel - Re: [PATCH v4 2/2] rust: leds: add basic led classdev abstractions

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] [day] [month] [year] [list]

Message-ID: <d3e51448678bf5a94746cc34825c35073c02d00e.camel@posteo.de>
Date: Tue, 14 Oct 2025 14:46:42 +0000
From: Markus Probst <markus.probst@...teo.de>
To: Danilo Krummrich <dakr@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>
Cc: 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>, Trevor Gross
 <tmgross@...ch.edu>, 	rust-for-linux@...r.kernel.org,
 linux-kernel@...r.kernel.org, 	linux-leds@...r.kernel.org
Subject: Re: [PATCH v4 2/2] rust: leds: add basic led classdev abstractions

On Mon, 2025-10-13 at 22:11 +0200, Danilo Krummrich wrote:
> On Mon Oct 13, 2025 at 8:34 PM CEST, Alice Ryhl wrote:
> > On Sun, Oct 12, 2025 at 02:52:39PM +0000, Markus Probst wrote:
> > > Implement the core abstractions needed for led class devices,
> > > including:
> > > 
> > > * `led::LedOps` - the trait for handling leds, including
> > >   `brightness_set`, `brightness_get` and `blink_set`
> > > 
> > > * `led::InitData` - data set for the led class device
> > > 
> > > * `led::Device` - a safe wrapper around `led_classdev`
> > > 
> > > Signed-off-by: Markus Probst <markus.probst@...teo.de>
> > 
> > > +pub trait LedOps: Send + 'static + Sized {
> > > +    /// If set true, [`LedOps::brightness_set`] and
> > > [`LedOps::blink_set`] must not sleep
> > > +    /// and perform the operation immediately.
> > > +    const BLOCKING: bool;
> > > +    /// The max brightness level
> > > +    const MAX_BRIGHTNESS: u32;
> > > +
> > > +    /// Sets the brightness level.
> > > +    ///
> > > +    /// See also [`LedOps::BLOCKING`]
> > > +    fn brightness_set(&self, brightness: u32) -> Result<()>;
> > > +
> > > +    /// Gets the current brightness level.
> > > +    fn brightness_get(&self) -> u32 {
> > > +        build_error!(VTABLE_DEFAULT_ERROR)
> > > +    }
> > > +
> > > +    /// Activates hardware accelerated blinking.
> > > +    ///
> > > +    /// delays are in milliseconds. If both are zero, a sensible
> > > default should be chosen.
> > > +    /// The caller should adjust the timings in that case and if
> > > it can't match the values
> > > +    /// specified exactly. Setting the brightness to 0 will
> > > disable the hardware accelerated
> > > +    /// blinking.
> > > +    ///
> > > +    /// See also [`LedOps::BLOCKING`]
> > > +    fn blink_set(&self, _delay_on: &mut usize, _delay_off: &mut
> > > usize) -> Result<()> {
> > > +        build_error!(VTABLE_DEFAULT_ERROR)
> > > +    }
> > 
> > These functions should probably take a &Device<Bound> argument so
> > that
> > they can use methods that require a bound device (such as IO).
> 
> Indeed!
> 
> @Markus: Note that this guarantee is given by the LED device
> registration being
> lifetime controlled by devres, while led_classdev_unregister() is
> synchronized
> against those callbacks.
> 
> For the latter, please double check that this is actually the case --
> I'm not
> familiar with the LED subsystem, I'm reviewing from driver-core
> perspective. But
> from a quick look it should be the case. :)

There is also the led_classdev->dev device, but I assume you mean the
parent of the led classdev (the one being passed on register)?

Thanks
- Markus Probst