From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f73.google.com (mail-wm1-f73.google.com [209.85.128.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 93CAB3C345B for ; Thu, 2 Jul 2026 10:47:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.73 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782989237; cv=none; b=ZZHyjBQsQfG0AliSQDCiDrIgPZEoLssuZyRJbcaA6FXI8gnYxmRLakkCIfM/4iNcyxa6PU4OVTSykoWBlE3xQja/tdc0lQC69PefX9STAPkAtNyXgJOAGu38M53YpONJ9ztTHnpuyXCX0637PpMNX4bB7Vi+UNOvVUzbPeh9RAo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782989237; c=relaxed/simple; bh=C9b8OXYr1ZfIVQ125i9stdL0TvlKMUbAI/41Fzjjh2A=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=Ee6fBdE6DMNUGogLA/GSTMjZaO9ay0RGQlfraxoQHFhI+wvLo/HTlxAmzDmwbOIniadgTwVnRRUhV9xZHgLO2IIIf4QgaEgmO1JVB3g0cGcKtAgai3w6+s/SRPRvH3aLoIq8Pu3S12B9cmS8/3GkQpcr3lFrHTeiH+zVy1yVgW0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=UhVycY0j; arc=none smtp.client-ip=209.85.128.73 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--aliceryhl.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="UhVycY0j" Received: by mail-wm1-f73.google.com with SMTP id 5b1f17b1804b1-493b54823bdso3025205e9.0 for ; Thu, 02 Jul 2026 03:47:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1782989234; x=1783594034; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=hQQYcZxBeNQUvYJ0vw0eFdw9XqDhM+2aUNMCPPXFL7A=; b=UhVycY0jVHszZ0jxfIKWJUcWaLmpPZyGOx9T/gkrJ3Ya4Am10lYw0vUgKvOzZBKtx/ 6mLFnl4PGQDZBWxFYwydct83c0skjWBESjBA1izzBMDgwnnbTyGTyI+z+41ym/2hlNru orIxvl8qqxfkthiH3CIpsTBiifs2Hl2rhf6WHZnzXHea4/L88icTOSQHvgizm9pxw7f3 7715ngsDzGakLKIdJIxZiXOmTT0u0ik8sMocQcGTDsM9Ax2vDPZWsU5mgXZugjuyl86d BruDYbiFNYM2rEh+Ab3AKmA0QVkjIeG0gwseGtGDrsjuU3alJ6YDblIDlkO+tWH6lIX2 zDRw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782989234; x=1783594034; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=hQQYcZxBeNQUvYJ0vw0eFdw9XqDhM+2aUNMCPPXFL7A=; b=bpE1gE35Rx0UIvu/6YSXCbRum2GMlF1Qs1SJzmeOhmPFdQzFgQ+bSyhydwnnM7i93n WCtyTuRPLQigiMgiwaHEp0mzvaotCmh8ZmIorcbFYoSfVVDza21nF10+wMzEP4hI7bR0 9BbAbVSW04SL2PfvdKiaU6+sWMXbE4S1t6Zlh6dqzhiw5GxiwEnaVJThvUqjwW6VgesR 22ud3Xir9e+lii2pcrEe/3OTb0CmDi2ojwlcXPsCk+yNqCnw/q2/pSOkbOLBw9P/XX2a 0xKAVH7EsOFMxLtQLy6HsiIEFfRTVYHzVEmf7L/I1n+GL3vJp93NvvNUranc0ixsjAye mjpA== X-Forwarded-Encrypted: i=1; AFNElJ+V5Un0h+2otFfWxZX/BBfqrSBqfU6Rd1oOSZ+DVINOjXGGvCKR9BQcJTl2+as1TdwO+ssDyAReWNch@vger.kernel.org X-Gm-Message-State: AOJu0Yy77R6VKEvzGreedNuG8s3krRrgcA9eJKVVatXuQx6u4h7D7KWL aYgN8oiYaRHDCA7quqSjf0oe3Gct0lWiElQKWvFqpSOt2f120oyKfml7Eb67w3CziZIbTK5XDq+ xdtrWbkGvXtgGXnPppA== X-Received: from wmfo12.prod.google.com ([2002:a05:600c:2e0c:b0:493:b55e:f03f]) (user=aliceryhl job=prod-delivery.src-stubby-dispatcher) by 2002:a05:600c:8411:b0:493:b4a3:5ab0 with SMTP id 5b1f17b1804b1-493c8c8c80cmr57425e9.13.1782989233695; Thu, 02 Jul 2026 03:47:13 -0700 (PDT) Date: Thu, 2 Jul 2026 10:47:12 +0000 In-Reply-To: <20260629-rust_leds-v21-1-4f0f19579db5@posteo.de> Precedence: bulk X-Mailing-List: linux-leds@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20260629-rust_leds-v21-0-4f0f19579db5@posteo.de> <20260629-rust_leds-v21-1-4f0f19579db5@posteo.de> Message-ID: Subject: Re: [PATCH v21 1/3] rust: leds: add basic led classdev abstractions From: Alice Ryhl To: Markus Probst Cc: Lee Jones , Pavel Machek , Greg Kroah-Hartman , Dave Ertman , Leon Romanovsky , Miguel Ojeda , Alex Gaynor , Gary Guo , "=?utf-8?B?QmrDtnJu?= Roy Baron" , Benno Lossin , Andreas Hindborg , Trevor Gross , Danilo Krummrich , "Rafael J. Wysocki" , Bjorn Helgaas , "Krzysztof =?utf-8?Q?Wilczy=C5=84ski?=" , Boqun Feng , Daniel Almeida , Tamir Duberstein , Alexandre Courbot , "Onur =?utf-8?B?w5Z6a2Fu?=" , Ira Weiny , rust-for-linux@vger.kernel.org, linux-leds@vger.kernel.org, linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org Content-Type: text/plain; charset="utf-8" On Mon, Jun 29, 2026 at 01:10:28PM +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::DeviceBuilder` - the builder for the led class device > > * `led::Device` - a safe wrapper around `led_classdev` > > Signed-off-by: Markus Probst > --- > MAINTAINERS | 8 ++ > rust/kernel/led.rs | 288 ++++++++++++++++++++++++++++++++++++++++++++++ > rust/kernel/led/normal.rs | 230 ++++++++++++++++++++++++++++++++++++ > rust/kernel/lib.rs | 1 + > 4 files changed, 527 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 15011f5752a9..ceb2285366ff 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -14662,6 +14662,14 @@ F: drivers/leds/ > F: include/dt-bindings/leds/ > F: include/linux/leds.h > > +LED SUBSYSTEM [RUST] > +M: Markus Probst > +L: linux-leds@vger.kernel.org > +L: rust-for-linux@vger.kernel.org > +S: Maintained > +F: rust/kernel/led.rs > +F: rust/kernel/led/ > + > LEGO MINDSTORMS EV3 > R: David Lechner > S: Maintained > diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs > new file mode 100644 > index 000000000000..c92d99d68497 > --- /dev/null > +++ b/rust/kernel/led.rs > @@ -0,0 +1,288 @@ > +// 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::{ > + marker::PhantomData, > + mem::transmute, > + ptr::NonNull, // > +}; > + > +use crate::{ > + container_of, > + device::{ > + self, > + property::FwNode, > + AsBusDevice, > + Bound, // > + }, > + error::{ > + from_result, > + to_result, > + VTABLE_DEFAULT_ERROR, // > + }, > + macros::vtable, > + prelude::*, > + str::CStrExt, CStrExt is in the prelude. Please check for unnecessary imports. > diff --git a/rust/kernel/led/normal.rs b/rust/kernel/led/normal.rs > new file mode 100644 > index 000000000000..2769f690bb24 > --- /dev/null > +++ b/rust/kernel/led/normal.rs > @@ -0,0 +1,230 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +//! Led mode for the `struct led_classdev`. > +//! > +//! C header: [`include/linux/leds.h`](srctree/include/linux/leds.h) > + > +use super::*; > + > +/// The led class device representation. > +/// > +/// This structure represents the Rust abstraction for a led class device. > +#[pin_data(PinnedDrop)] > +pub struct Device<'bound, T: LedOps + 'bound> { > + #[pin] > + ops: T, > + #[pin] > + classdev: Opaque, > + _p: PhantomData<&'bound ()>, > +} > + > +impl<'a, S: DeviceBuilderState> DeviceBuilder<'a, S> { > + /// Registers a new [`Device`]. > + pub fn build<'bound: 'a, T: LedOps + 'bound>( > + self, > + parent: &'bound T::Bus, > + ops: impl PinInit + 'a, > + ) -> impl PinInit, Error> + 'a { I think it would be useful to separate out the two lifetimes more clearly. You have two sets of lifetimes: * 'bound which is the duration in which the bus device is bound. * 'a which is the duration in which the `name`/`devicename` fields are valid. And these have different constraints because 'bound is much larger than 'a. The 'bound lifetime is longer than the entire Device struct, but the 'a lifetime only needs to last for the duration of the initialization because (I assume) the strings are copied by `led_classdev_register_ext` So under that logic, I would rename 'a to 'name or something like that to indicate what it's the lifetime of. Note that if I'm wrong about the lifetime of the name strings, then this code should be changed accordingly. It looks like you're actually stashing the pointers in the led_classdev, and if that outlives this initializer, then the current lifetimes are wrong, and Device must also be annotated with 'name to indicate this additional lifetime. > + const_assert!(T::MAX_BRIGHTNESS <= i32::MAX.unsigned_abs() || !T::HAS_BRIGHTNESS_GET); > + > + try_pin_init!(Device { > + ops <- ops, > + classdev <- Opaque::try_ffi_init(|ptr: *mut bindings::led_classdev| { > + // SAFETY: `try_ffi_init` guarantees that `ptr` is valid for write. > + // `led_classdev` gets fully initialized in-place by > + // `led_classdev_register_ext` including `mutex` and `list_head`. > + unsafe { > + ptr.write(bindings::led_classdev { > + brightness_set: (!T::BLOCKING) > + .then_some(Adapter::::brightness_set_callback), > + brightness_set_blocking: T::BLOCKING > + .then_some(Adapter::::brightness_set_blocking_callback), > + brightness_get: T::HAS_BRIGHTNESS_GET > + .then_some(Adapter::::brightness_get_callback), > + blink_set: T::HAS_BLINK_SET.then_some(Adapter::::blink_set_callback), > + max_brightness: T::MAX_BRIGHTNESS, > + brightness: self.initial_brightness, > + color: self.color as u32, > + name: self.name.map_or(core::ptr::null(), CStrExt::as_char_ptr), > + ..bindings::led_classdev::default() > + }) > + }; > + > + let mut init_data = bindings::led_init_data { > + fwnode: self > + .fwnode > + .as_ref() > + .map_or(core::ptr::null_mut(), |fwnode| fwnode.as_raw()), > + default_label: core::ptr::null(), > + devicename: self > + .devicename > + .map_or(core::ptr::null(), CStrExt::as_char_ptr), > + devname_mandatory: self.devname_mandatory, > + }; > + > + // SAFETY: > + // - `parent.as_ref().as_raw()` is guaranteed to be a pointer to a valid > + // `device`. > + // - `ptr` is guaranteed to be a pointer to an initialized `led_classdev`. > + to_result(unsafe { > + bindings::led_classdev_register_ext( > + parent.as_ref().as_raw(), > + ptr, > + if self.name.is_none() { > + &raw mut init_data > + } else { > + core::ptr::null_mut() > + }, > + ) > + })?; > + > + core::mem::forget(self.fwnode); // keep the reference count incremented > + > + Ok::<_, Error>(()) > + }), > + _p: PhantomData, > + }) > + } > +} Alice