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 29FE93EDE5E for ; Thu, 2 Jul 2026 11:22:13 +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=1782991336; cv=none; b=WMM4nKF5Ya5FvCtC4OiT9MGlTANMZST1CpIpod8dR2UTmPkjOibtWP/Dph0wmiH1dER4zl1YPwyk8TsWwvLS+fnEAsPP5DixBdJzKrzKRxZglLpM12ys466sNFH4e2IV71KQ/VE9NJEJkd71XCPFRsJKsQnmQfqfFDn2mXyIS+M= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782991336; c=relaxed/simple; bh=h6Hjsd6YIlOqO7foxw2433GlwA8x369rcvFYjko/x58=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=C+zuggKxYq0unMMS7zzbJlatkwVueaJ07+IDo3pip6M008G6lZfKDcgvtnL40A8zLFwOlkY1FRJtFtvMPdZWYksxk7spuefyjlHfACEtxqbIa4HYCXpkadqr9PKYPnbkrBkxSIGGKJSxv4sso9Ifs0yYSAsDEVY1F0Dqq+nauTM= 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=Eh2EkCxu; 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="Eh2EkCxu" Received: by mail-wm1-f73.google.com with SMTP id 5b1f17b1804b1-490a767b782so15938385e9.2 for ; Thu, 02 Jul 2026 04:22:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1782991332; x=1783596132; 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=2ABHfhQD3F9b+S3MG2pxSXvPENP4iWnPQjn40BCzCts=; b=Eh2EkCxu6qZaJYcrUBXnS9AuMYDsFND6PrC+RoQUbqMBDifg7NevnSTSeHURJQnrur OAGxKYr2D+ZGlabR/kgftOQxLFOq+mf7DzZJwYpY2kgKvxDiC44fxLD/9FLcyjgEj+T0 +0G1Lkbpj/UfJq1T1PqP747p8ea09be5ibpZ8IMxDCnFTtSzKUesLOgL5mbJQY50iiuo ux1LYd7LP5X/rNWJptrRvIlansMcf7dJab04zuUstN2M3JSEjxWUYOuUBi4Sqhx0IJrs JSluHPP9tv2ibPa8J7VVPCx5blFWBKNyTuK4IkIceXMDAR8T9D+v/Dl59fyREvXQg46P 9cqg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782991332; x=1783596132; 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=2ABHfhQD3F9b+S3MG2pxSXvPENP4iWnPQjn40BCzCts=; b=hFef9XIwIpLlOdK9VyDHA15if+PIGgPjNqOKID1CwUQ3R1wN5hxj7L61xaF6SvGjYO Ofm5IEy5w+TkGTbp4ZxCccxWt4pmSYN7IKizasY02MEshrKHM7nsN4gJrtrbqb9Tb+bu yD2vg2g5ptGIGTE7eNaPf1dwa0WAqZmZ/oQxK13oSZSebire8xDz/Vekxt/7R/BlcmKx Ru+tXKV6S70vAGO4TksK+KqPODGd9dsYJxpVoo0gfmKANS4a+OE27y7oMLOzqCKJg58c NR0ktWbDvNafZTzZ09t96Jk1HFzeDi4odEsuYYcuVXHfyhLD54uRbnhnme1568hK9v2W BweA== X-Forwarded-Encrypted: i=1; AFNElJ+x9tfYqn69vZMcYSyofoYJJo7IYiF0ODITCVNNTvE0gAWRYfLcfEG3cxqfebizmRTN/P7Nczwo41C6xaRoKg==@vger.kernel.org X-Gm-Message-State: AOJu0Yw+Sx77LxpRnNYciN8nkZv1oHXdIEnibfvcZdF97SqZUXrJBQCP 1ACsanEVVf4rsy4Hm75sA8sYb/82HpIV/u/a2Rba42mRFEdI50sods4WVHfNQ061l22q78r9YSO tbiFWPGAUe9BQ/j/QfQ== X-Received: from wmby21.prod.google.com ([2002:a05:600c:c055:b0:493:bd67:3b3e]) (user=aliceryhl job=prod-delivery.src-stubby-dispatcher) by 2002:a05:600c:524a:b0:493:af0d:484c with SMTP id 5b1f17b1804b1-493c3df2fb5mr67180355e9.34.1782991332107; Thu, 02 Jul 2026 04:22:12 -0700 (PDT) Date: Thu, 2 Jul 2026 11:22:10 +0000 In-Reply-To: <9e020dbc48fa9f209248864cdd1b7b9ebf2eecc4.camel@posteo.de> Precedence: bulk X-Mailing-List: rust-for-linux@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> <9e020dbc48fa9f209248864cdd1b7b9ebf2eecc4.camel@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 Thu, Jul 02, 2026 at 11:00:30AM +0000, Markus Probst wrote: > On Thu, 2026-07-02 at 10:47 +0000, Alice Ryhl wrote: > > 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. > There is a > `use super::*;` in rust/kernel/led/normal.rs and > rust/kernel/led/multicolor.rs, which both make use of CStrExt. > The latter being in patch 3. I understand that you are using it. However, it's still unnecessary because you also imported prelude::*. > > > 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. > Preferably 'init, which can be used in other class device abstractions > as well if needed. Sounds good. > > > > 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. > name and devicename only need to be valid for the duration of > `led_classdev_register_ext`. Great. Alice