* Re: [RFC PATCH 1/9] leds: Load trigger modules on-demand if used as hw control trigger
From: Andrew Lunn @ 2026-04-11 20:42 UTC (permalink / raw)
To: Mark Pearson
Cc: Rong Zhang, Lee Jones, Pavel Machek, Thomas Weißschuh,
Benson Leung, Guenter Roeck, Marek Behún, Derek J . Clark,
Hans de Goede, Ilpo Järvinen, Ike Panhc, Vishnu Sankar,
Vishnu Sankar, linux-kernel, linux-leds, chrome-platform,
platform-driver-x86@vger.kernel.org
In-Reply-To: <7437cc09-7958-4172-aec2-4429cfa98141@app.fastmail.com>
> Hi maintainers - a gentle nudge on this one. Would like some
> direction if we can move ahead with the series. If not, what is
> required?
> On our side I know Vishnu has implemented the thinkpad driver using
> this and confirmed the design works there too.
> Would like to get this upstream so we can start working with the
> user-space folk on updating their pieces.
I'm not the Maintainer here, so i would not be too unhappy if i was
ignored.
I've had some time to think about this. I guess you are going to go
with a Linux LED, whatever. With that assumption in place, a trigger
makes sense. But i think it needs to be a special sort of trigger, and
a special sort of LED.
This is not a general purpose LED, which is a basic assumption for
Linux LEDs. You cannot make it blink for netdev packets, shift lock,
heartbeat, etc, because Linux does not control it, the EC does. Linux
can ask the EC to set it to some state, but the EC might change it,
and notify Linux afterwards. That is not the normal behaviour of a
Linux LED.
Also, it does not make sense to allow the trigger to be bound to any
other LED.
The trigger and the LED are tightly bound together. So i would put
them in the same driver. There are triggers which live outside of
drivers/leds/trigger/. So it is not a problem it lives somewhere
else. It also solves the ordering problem you had, getting the trigger
registered. Since it lives in the same driver as the LED, you know it
is registered, you do it before registering the LED.
I think you need some additional logic in the core. This LED must have
this trigger. I would look at using the is_visible() attribute
callback to make the trigger file in sysfs invisible for this LED, so
it cannot be changed. I would somehow get this trigger attached to the
LED when it is created. The trigger_relevant() call needs to be
extended so that you cannot attach this trigger to any other LED.
I would also think about naming. The behaviour is i guess specific to
X86 laptops? Do Apple laptops have the same behaviour? What about
Snapdragon X Series laptops? Chromebooks? USB keyboards? What about
things like the Unihertz Titan 2?
Somebody in the future might produce a generic solution. Linux
controls the LED. The Linux input system gets the key presses and
sends them to the trigger. You can bind an iio ambient sensor to the
trigger, etc. So you should leave the name space open for other
implementations.
Andrew
^ permalink raw reply
* Re: [PATCH v7 1/2] platform: Add initial synology microp driver
From: Onur Özkan @ 2026-04-11 16:55 UTC (permalink / raw)
To: Markus Probst via B4 Relay
Cc: Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue,
Lee Jones, Pavel Machek, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Greg Kroah-Hartman, platform-driver-x86, linux-leds,
devicetree, linux-kernel, rust-for-linux, Markus Probst
In-Reply-To: <20260411-synology_microp_initial-v7-1-9a3a094e763a@posteo.de>
On Sat, 11 Apr 2026 17:27:34 +0200
Markus Probst via B4 Relay <devnull+markus.probst.posteo.de@kernel.org> wrote:
> From: Markus Probst <markus.probst@posteo.de>
>
> Add a initial synology microp driver, written in Rust.
> The driver targets a microcontroller found in Synology NAS devices. It
> currently only supports controlling of the power led, status led, alert
> led and usb led. Other components such as fan control or handling
> on-device buttons will be added once the required rust abstractions are
> there.
>
> This driver can be used both on arm and x86, thus it goes into the root
> directory of drivers/platform.
>
> Tested successfully on a Synology DS923+.
>
> Signed-off-by: Markus Probst <markus.probst@posteo.de>
> ---
> MAINTAINERS | 5 +
> drivers/platform/Kconfig | 2 +
> drivers/platform/Makefile | 1 +
> drivers/platform/synology_microp/Kconfig | 13 +
> drivers/platform/synology_microp/Makefile | 3 +
> drivers/platform/synology_microp/TODO | 7 +
> drivers/platform/synology_microp/command.rs | 55 ++++
> drivers/platform/synology_microp/led.rs | 276 +++++++++++++++++++++
> drivers/platform/synology_microp/model.rs | 49 ++++
> .../platform/synology_microp/synology_microp.rs | 109 ++++++++
> 10 files changed, 520 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a667f4efb66e..78c99d831431 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -25554,6 +25554,11 @@ F: drivers/dma-buf/sync_*
> F: include/linux/sync_file.h
> F: include/uapi/linux/sync_file.h
>
> +SYNOLOGY MICROP DRIVER
> +M: Markus Probst <markus.probst@posteo.de>
> +S: Maintained
> +F: drivers/platform/synology_microp/
> +
> SYNOPSYS ARC ARCHITECTURE
> M: Vineet Gupta <vgupta@kernel.org>
> L: linux-snps-arc@lists.infradead.org
> diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
> index 312788f249c9..996050566a4a 100644
> --- a/drivers/platform/Kconfig
> +++ b/drivers/platform/Kconfig
> @@ -22,3 +22,5 @@ source "drivers/platform/arm64/Kconfig"
> source "drivers/platform/raspberrypi/Kconfig"
>
> source "drivers/platform/wmi/Kconfig"
> +
> +source "drivers/platform/synology_microp/Kconfig"
> diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
> index fa322e7f8716..2381872e9133 100644
> --- a/drivers/platform/Makefile
> +++ b/drivers/platform/Makefile
> @@ -15,3 +15,4 @@ obj-$(CONFIG_SURFACE_PLATFORMS) += surface/
> obj-$(CONFIG_ARM64_PLATFORM_DEVICES) += arm64/
> obj-$(CONFIG_BCM2835_VCHIQ) += raspberrypi/
> obj-$(CONFIG_ACPI_WMI) += wmi/
> +obj-$(CONFIG_SYNOLOGY_MICROP) += synology_microp/
> diff --git a/drivers/platform/synology_microp/Kconfig b/drivers/platform/synology_microp/Kconfig
> new file mode 100644
> index 000000000000..7c4d8f2808f0
> --- /dev/null
> +++ b/drivers/platform/synology_microp/Kconfig
> @@ -0,0 +1,13 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +config SYNOLOGY_MICROP
> + tristate "Synology Microp driver"
> + depends on LEDS_CLASS && LEDS_CLASS_MULTICOLOR
> + depends on RUST_SERIAL_DEV_BUS_ABSTRACTIONS
> + help
> + Enable support for the MCU found in Synology NAS devices.
> +
> + This is needed to properly shutdown and reboot the device, as well as
> + additional functionality like fan and LED control.
> +
> + This driver is work in progress and may not be fully functional.
> diff --git a/drivers/platform/synology_microp/Makefile b/drivers/platform/synology_microp/Makefile
> new file mode 100644
> index 000000000000..63585ccf76e4
> --- /dev/null
> +++ b/drivers/platform/synology_microp/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-y += synology_microp.o
> diff --git a/drivers/platform/synology_microp/TODO b/drivers/platform/synology_microp/TODO
> new file mode 100644
> index 000000000000..1961a33115db
> --- /dev/null
> +++ b/drivers/platform/synology_microp/TODO
> @@ -0,0 +1,7 @@
> +TODO:
> +- add missing components:
> + - handle on-device buttons (Power, Factory reset, "USB Copy")
> + - handle fan failure
> + - beeper
> + - fan speed control
> + - correctly perform device power-off and restart on Synology devices
> diff --git a/drivers/platform/synology_microp/command.rs b/drivers/platform/synology_microp/command.rs
> new file mode 100644
> index 000000000000..5b3dd715afac
> --- /dev/null
> +++ b/drivers/platform/synology_microp/command.rs
> @@ -0,0 +1,55 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use kernel::{
> + device::Bound,
> + error::Result,
> + serdev, //
> +};
> +
> +use crate::led;
> +
> +#[derive(Copy, Clone)]
Copy and Clone seem never used, please drop them (also see [1]).
[1]: https://rust-for-linux.zulipchat.com/#narrow/channel/509436-Nova/topic/clone.2Fcopy.20additions
> +#[expect(
> + clippy::enum_variant_names,
> + reason = "future variants will not end with Led"
> +)]
> +pub(crate) enum Command {
> + PowerLed(led::State),
> + StatusLed(led::StatusLedColor, led::State),
> + AlertLed(led::State),
> + UsbLed(led::State),
> + EsataLed(led::State),
> +}
> +
> +impl Command {
> + pub(crate) fn write(self, dev: &serdev::Device<Bound>) -> Result {
> + dev.write_all(
> + match self {
> + Self::PowerLed(led::State::On) => &[0x34],
> + Self::PowerLed(led::State::Blink) => &[0x35],
> + Self::PowerLed(led::State::Off) => &[0x36],
> +
> + Self::StatusLed(_, led::State::Off) => &[0x37],
> + Self::StatusLed(led::StatusLedColor::Green, led::State::On) => &[0x38],
> + Self::StatusLed(led::StatusLedColor::Green, led::State::Blink) => &[0x39],
> + Self::StatusLed(led::StatusLedColor::Orange, led::State::On) => &[0x3A],
> + Self::StatusLed(led::StatusLedColor::Orange, led::State::Blink) => &[0x3B],
> +
> + Self::AlertLed(led::State::On) => &[0x4C, 0x41, 0x31],
> + Self::AlertLed(led::State::Blink) => &[0x4C, 0x41, 0x32],
> + Self::AlertLed(led::State::Off) => &[0x4C, 0x41, 0x33],
> +
> + Self::UsbLed(led::State::On) => &[0x40],
> + Self::UsbLed(led::State::Blink) => &[0x41],
> + Self::UsbLed(led::State::Off) => &[0x42],
> +
> + Self::EsataLed(led::State::On) => &[0x4C, 0x45, 0x31],
> + Self::EsataLed(led::State::Blink) => &[0x4C, 0x45, 0x32],
> + Self::EsataLed(led::State::Off) => &[0x4C, 0x45, 0x33],
> + },
> + serdev::Timeout::Max,
> + )?;
> + dev.wait_until_sent(serdev::Timeout::Max);
> + Ok(())
> + }
> +}
> diff --git a/drivers/platform/synology_microp/led.rs b/drivers/platform/synology_microp/led.rs
> new file mode 100644
> index 000000000000..a78a95588456
> --- /dev/null
> +++ b/drivers/platform/synology_microp/led.rs
> @@ -0,0 +1,276 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use kernel::{
> + device::Bound,
> + devres::{
> + self,
> + Devres, //
> + },
> + led::{
> + self,
> + LedOps,
> + MultiColorSubLed, //
> + },
> + new_mutex,
> + prelude::*,
> + serdev,
> + str::CString,
> + sync::Mutex, //
> +};
> +use pin_init::pin_init_scope;
> +
> +use crate::{
> + command::Command,
> + model::Model, //
> +};
> +
> +#[pin_data]
> +pub(crate) struct Data {
> + #[pin]
> + status: Devres<led::MultiColorDevice<StatusLedHandler>>,
> + power_name: CString,
> + #[pin]
> + power: Devres<led::Device<LedHandler>>,
> +}
> +
> +impl Data {
> + pub(super) fn register<'a>(
> + dev: &'a serdev::Device<Bound>,
> + model: &'a Model,
> + ) -> impl PinInit<Self, Error> + 'a {
> + pin_init_scope(move || {
> + if let Some(color) = model.led_alert {
> + let name = CString::try_from_fmt(fmt!("{}:alarm", color.as_c_str().to_str()?))?;
> + devres::register(
> + dev.as_ref(),
> + led::DeviceBuilder::new().color(color).name(&name).build(
> + dev,
> + try_pin_init!(LedHandler {
> + blink <- new_mutex!(false),
> + command: Command::AlertLed,
> + }),
> + ),
> + GFP_KERNEL,
> + )?;
> + }
> +
> + if model.led_usb_copy {
> + devres::register(
> + dev.as_ref(),
> + led::DeviceBuilder::new()
> + .color(led::Color::Green)
> + .name(c"green:usb")
> + .build(
> + dev,
> + try_pin_init!(LedHandler {
> + blink <- new_mutex!(false),
> + command: Command::UsbLed,
> + }),
> + ),
> + GFP_KERNEL,
> + )?;
> + }
> +
> + if model.led_esata {
> + devres::register(
> + dev.as_ref(),
> + led::DeviceBuilder::new()
> + .color(led::Color::Green)
> + .name(c"green:esata")
> + .build(
> + dev,
> + try_pin_init!(LedHandler {
> + blink <- new_mutex!(false),
> + command: Command::EsataLed,
> + }),
> + ),
> + GFP_KERNEL,
> + )?;
> + }
> +
> + Ok(try_pin_init!(Self {
> + status <- led::DeviceBuilder::new()
> + .color(led::Color::Multi)
> + .name(c"multicolor:status")
> + .build_multicolor(
> + dev,
> + try_pin_init!(StatusLedHandler {
> + blink <- new_mutex!(false),
> + }),
> + StatusLedHandler::SUBLEDS,
> + ),
> + power_name: CString::try_from_fmt(fmt!(
> + "{}:power",
> + model.led_power.as_c_str().to_str()?
> + ))?,
> + power <- led::DeviceBuilder::new()
> + .color(model.led_power)
> + .name(power_name)
> + .build(
> + dev,
> + try_pin_init!(LedHandler {
> + blink <- new_mutex!(true),
> + command: Command::PowerLed,
> + }),
> + ),
> + }))
> + })
> + }
> +}
> +
> +#[derive(Copy, Clone)]
> +pub(crate) enum StatusLedColor {
> + Green,
> + Orange,
> +}
> +
> +#[derive(Copy, Clone)]
> +pub(crate) enum State {
> + On,
> + Blink,
> + Off,
> +}
> +
> +#[pin_data]
> +struct LedHandler {
> + #[pin]
> + blink: Mutex<bool>,
> + command: fn(State) -> Command,
> +}
> +
> +#[vtable]
> +impl LedOps for LedHandler {
> + type Bus = serdev::Device<Bound>;
> + type Mode = led::Normal;
> + const BLOCKING: bool = true;
> + const MAX_BRIGHTNESS: u32 = 1;
> +
> + fn brightness_set(
> + &self,
> + dev: &Self::Bus,
> + _classdev: &led::Device<Self>,
> + brightness: u32,
> + ) -> Result<()> {
> + let mut blink = self.blink.lock();
> + (self.command)(if brightness == 0 {
> + *blink = false;
> + State::Off
> + } else if *blink {
> + State::Blink
> + } else {
> + State::On
> + })
> + .write(dev)?;
> +
> + Ok(())
> + }
> +
> + fn blink_set(
> + &self,
> + dev: &Self::Bus,
> + _classdev: &led::Device<Self>,
> + delay_on: &mut usize,
> + delay_off: &mut usize,
> + ) -> Result<()> {
> + let mut blink = self.blink.lock();
> +
> + (self.command)(if *delay_on == 0 && *delay_off != 0 {
> + State::Off
> + } else if *delay_on != 0 && *delay_off == 0 {
> + State::On
> + } else {
> + *blink = true;
> + *delay_on = 167;
> + *delay_off = 167;
Perhaps using a named constant with a simple comment instead of a hard-coded
integer would help to clarify what 167 is.
- Onur
> +
> + State::Blink
> + })
> + .write(dev)
> + }
> +}
> +
> +#[pin_data]
> +struct StatusLedHandler {
> + #[pin]
> + blink: Mutex<bool>,
> +}
> +
> +impl StatusLedHandler {
> + const SUBLEDS: &[MultiColorSubLed] = &[
> + MultiColorSubLed::new(led::Color::Green).initial_intensity(1),
> + MultiColorSubLed::new(led::Color::Orange),
> + ];
> +}
> +
> +#[vtable]
> +impl LedOps for StatusLedHandler {
> + type Bus = serdev::Device<Bound>;
> + type Mode = led::MultiColor;
> + const BLOCKING: bool = true;
> + const MAX_BRIGHTNESS: u32 = 1;
> +
> + fn brightness_set(
> + &self,
> + dev: &Self::Bus,
> + classdev: &led::MultiColorDevice<Self>,
> + brightness: u32,
> + ) -> Result<()> {
> + let mut blink = self.blink.lock();
> + if brightness == 0 {
> + *blink = false;
> + }
> +
> + let (color, subled_brightness) = if classdev.subleds()[1].intensity == 0 {
> + (StatusLedColor::Green, classdev.subleds()[0].brightness)
> + } else {
> + (StatusLedColor::Orange, classdev.subleds()[1].brightness)
> + };
> +
> + Command::StatusLed(
> + color,
> + if subled_brightness == 0 {
> + State::Off
> + } else if *blink {
> + State::Blink
> + } else {
> + State::On
> + },
> + )
> + .write(dev)
> + }
> +
> + fn blink_set(
> + &self,
> + dev: &Self::Bus,
> + classdev: &led::MultiColorDevice<Self>,
> + delay_on: &mut usize,
> + delay_off: &mut usize,
> + ) -> Result<()> {
> + let mut blink = self.blink.lock();
> + *blink = true;
> +
> + let (color, subled_intensity) = if classdev.subleds()[1].intensity == 0 {
> + (StatusLedColor::Green, classdev.subleds()[0].intensity)
> + } else {
> + (StatusLedColor::Orange, classdev.subleds()[1].intensity)
> + };
> + Command::StatusLed(
> + color,
> + if *delay_on == 0 && *delay_off != 0 {
> + *blink = false;
> + State::Off
> + } else if subled_intensity == 0 {
> + State::Off
> + } else if *delay_on != 0 && *delay_off == 0 {
> + *blink = false;
> + State::On
> + } else {
> + *delay_on = 167;
> + *delay_off = 167;
> +
> + State::Blink
> + },
> + )
> + .write(dev)
> + }
> +}
> diff --git a/drivers/platform/synology_microp/model.rs b/drivers/platform/synology_microp/model.rs
> new file mode 100644
> index 000000000000..715d8840f56b
> --- /dev/null
> +++ b/drivers/platform/synology_microp/model.rs
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +use kernel::led::Color;
> +
> +pub(crate) struct Model {
> + pub(crate) led_power: Color,
> + pub(crate) led_alert: Option<Color>,
> + pub(crate) led_usb_copy: bool,
> + pub(crate) led_esata: bool,
> +}
> +
> +impl Model {
> + pub(super) const fn new() -> Self {
> + Self {
> + led_power: Color::Blue,
> + led_alert: None,
> + led_usb_copy: false,
> + led_esata: false,
> + }
> + }
> +
> + pub(super) const fn led_power(self, color: Color) -> Self {
> + Self {
> + led_power: color,
> + ..self
> + }
> + }
> +
> + pub(super) const fn led_alert(self, color: Color) -> Self {
> + Self {
> + led_alert: Some(color),
> + ..self
> + }
> + }
> +
> + pub(super) const fn led_esata(self) -> Self {
> + Self {
> + led_esata: true,
> + ..self
> + }
> + }
> +
> + pub(super) const fn led_usb_copy(self) -> Self {
> + Self {
> + led_usb_copy: true,
> + ..self
> + }
> + }
> +}
> diff --git a/drivers/platform/synology_microp/synology_microp.rs b/drivers/platform/synology_microp/synology_microp.rs
> new file mode 100644
> index 000000000000..f02c4dade76c
> --- /dev/null
> +++ b/drivers/platform/synology_microp/synology_microp.rs
> @@ -0,0 +1,109 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Synology Microp driver
> +
> +use kernel::{
> + device,
> + led::Color,
> + of::{
> + DeviceId,
> + IdTable, //
> + },
> + of_device_table,
> + prelude::*,
> + serdev, //
> +};
> +use pin_init::pin_init_scope;
> +
> +use crate::model::Model;
> +
> +pub(crate) mod command;
> +mod led;
> +mod model;
> +
> +kernel::module_serdev_device_driver! {
> + type: SynologyMicropDriver,
> + name: "synology_microp",
> + authors: ["Markus Probst <markus.probst@posteo.de>"],
> + description: "Synology Microp driver",
> + license: "GPL v2",
> +}
> +
> +#[rustfmt::skip]
> +of_device_table!(
> + OF_TABLE,
> + MODULE_OF_TABLE,
> + Model,
> + [
> + // apollolake
> + (DeviceId::new(c"synology,ds918p-microp"), Model::new()),
> +
> + // evansport
> + (DeviceId::new(c"synology,ds214play-microp"), Model::new()),
> +
> + // geminilakenk
> + (DeviceId::new(c"synology,ds225p-microp"), Model::new().led_usb_copy()),
> + (DeviceId::new(c"synology,ds425p-microp"), Model::new()),
> +
> + // pineview
> + (DeviceId::new(c"synology,ds710p-microp"), Model::new().led_esata()),
> + (DeviceId::new(c"synology,ds1010p-microp"), Model::new().led_alert(Color::Orange)),
> +
> + // r1000
> + (DeviceId::new(c"synology,ds923p-microp"), Model::new()),
> + (DeviceId::new(c"synology,ds723p-microp"), Model::new()),
> + (DeviceId::new(c"synology,ds1522p-microp"), Model::new()),
> + (DeviceId::new(c"synology,rs422p-microp"), Model::new().led_power(Color::Green)),
> +
> + // r1000nk
> + (DeviceId::new(c"synology,ds725p-microp"), Model::new()),
> +
> + // rtd1296
> + (DeviceId::new(c"synology,ds118-microp"), Model::new()),
> +
> + // rtd1619b
> + (DeviceId::new(c"synology,ds124-microp"), Model::new()),
> + (DeviceId::new(c"synolody,ds223-microp"), Model::new().led_usb_copy()),
> + (DeviceId::new(c"synology,ds223j-microp"), Model::new()),
> +
> + // v1000
> + (DeviceId::new(c"synology,ds1823xsp-microp"), Model::new()),
> + (DeviceId::new(c"synology,rs822p-microp"), Model::new().led_power(Color::Green)),
> + (DeviceId::new(c"synology,rs1221p-microp"), Model::new().led_power(Color::Green)),
> + (DeviceId::new(c"synology,rs1221rpp-microp"), Model::new().led_power(Color::Green)),
> +
> + // v1000nk
> + (DeviceId::new(c"synology,ds925p-microp"), Model::new()),
> + (DeviceId::new(c"synology,ds1525p-microp"), Model::new()),
> + (DeviceId::new(c"synology,ds1825p-microp"), Model::new()),
> + ]
> +);
> +
> +#[pin_data]
> +struct SynologyMicropDriver {
> + #[pin]
> + led: led::Data,
> +}
> +
> +#[vtable]
> +impl serdev::Driver for SynologyMicropDriver {
> + type IdInfo = Model;
> + const OF_ID_TABLE: Option<IdTable<Self::IdInfo>> = Some(&OF_TABLE);
> +
> + fn probe(
> + dev: &serdev::Device<device::Core>,
> + model: Option<&Model>,
> + ) -> impl PinInit<Self, kernel::error::Error> {
> + pin_init_scope(move || {
> + let model = model.ok_or(EINVAL)?;
> +
> + dev.set_baudrate(9600).map_err(|_| EINVAL)?;
> + dev.set_flow_control(false);
> + dev.set_parity(serdev::Parity::None)?;
> +
> + Ok(try_pin_init!(Self {
> + led <- led::Data::register(dev, model),
> + }))
> + })
> + }
> +}
>
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [RFC PATCH 1/9] leds: Load trigger modules on-demand if used as hw control trigger
From: Mark Pearson @ 2026-04-11 15:31 UTC (permalink / raw)
To: Rong Zhang, Andrew Lunn, Lee Jones
Cc: Pavel Machek, Thomas Weißschuh, Benson Leung, Guenter Roeck,
Marek Behún, Derek J . Clark, Hans de Goede,
Ilpo Järvinen, Ike Panhc, Vishnu Sankar, Vishnu Sankar,
linux-kernel, linux-leds, chrome-platform,
platform-driver-x86@vger.kernel.org
In-Reply-To: <2ac0c46c-805d-4749-a6e0-94b16b104a72@app.fastmail.com>
On Sat, Mar 14, 2026, at 3:02 PM, Mark Pearson wrote:
> On Fri, Mar 13, 2026, at 10:01 AM, Rong Zhang wrote:
>> Hi Mark,
>>
>> On Thu, 2026-03-12 at 17:46 -0400, Mark Pearson wrote:
>>>
>>> On Thu, Mar 12, 2026, at 2:01 PM, Rong Zhang wrote:
>>> > Hi Andrew,
>>> >
>>> > On Wed, 2026-03-11 at 22:29 +0100, Andrew Lunn wrote:
>>> > > > We just can't prevent the EC from responding to the Fn+Space shortcut.
>>> > > > So it's essentially user's choice to switch to the hw control trigger
>>> > > > and make it offloaded to hardware (sorry if my cover letter and replies
>>> > > > didn't express this well).
>>> > >
>>> > > Do you have any control over the EC?
>>> > >
>>> > > You have a two bosses dilemma. You need to eliminate one of the
>>> > > bosses. Either the EC controls the LED, or Linux does. Having both
>>> > > controlling it is just going to work out badly.
>>> >
>>> > I agree that the manufacturers designed the interface poorly :-/
>>> >
>>> > I am not affiliated with any laptop manufacturers so I am just speaking
>>> > for myself:
>>> >
>>> > IMO the real boss is the user. Both the shortcut (Fn+Space) and the
>>> > ACPI interface are just "message channels" for the EC to know about the
>>> > user's choice.
>>> >
>>> > Being able to press such a shortcut always implies that the user is
>>> > physically in front of the device. In this case it no longer about
>>> > whether Linux or the EC controls the LED, but both should respect
>>> > user's choice. That was why brightness_hw_changed was introduced to
>>> > respect user's choice and pass it to the userspace. So far there has
>>> > been ~10 drivers utilizing the brightness_hw_changed interface.
>>> >
>>> I am affiliated with a laptop manufacturer :) Happy to take suggestions on what should be improved or is missing (can't promise anything but happy to consider it and take it for review).
>>>
>>> We can set the brightness, get the status, and the FW sends events when it changes - all supported on Linux (for Lenovo devices). This looks like a pretty decent API to me. What is it missing?
>>
>> IMO, one missing thing is that there's no approach for the OS to
>> prevent the EC from responding to Fn+Space. Hence no 100% pure software
>> control is possible. We end up have a mixed SW + EC control.
>>
>> Another missing thing is that there's no approach for the OS to
>> query/set the ALS-to-brightness curve (or trip points, whatever you
>> call it) of the EC driven auto brightness mode. Therefore, should we
>> have a kernel mode ALS-based software trigger, we would never know if
>> our curve could be offloaded.
>>
>> That being said, I don't think either of these two missing things is a
>> big deal, since most laptops (if not all) are just designed to work
>> like this and I don't think we would have a kernel mode ALS-based
>> software trigger any soon. No 100% pure software control wasn't, isn't
>> and shouldn't be a blocker of using an LED classdev. As I've said,
>> that's exactly why brightness_hw_changed was introduced.
>>
> Got it - thanks for the input.
>
> My personal opinion (not feedback from the FW team)
> - disabling the FN+spacebar is not a useful feature. It's not exactly
> something you would trigger accidentally. I can't think when people
> would need to disable it rather than just choosing to not use it.
> - Programming the curve would be more interesting - but becomes a bit
> of a pro-user use case. Happy to suggest it, but it's a nice-to-have
> feature IMO.
>
> Agreed that neither of these seem a valid reason to block the implementation.
>
>>>
>>> I don't understand the two bosses issue I'm afraid. The user ('Linux' in your description?) tells the EC what it wants the LED to be, and the EC does it. The EC is not a boss.
>>
>> The "user" means the one (i.e., a human) who is using the device. And
>> "message channels" mean:
>>
>> User -> pressing Fn+Space -> EC -> update keyboard backlight
>>
>> User -> LED classdev / manufacturer utilities -> ACPI -> EC -> update
>> keyboard backlight
>>
>> They are all about the user, as a human, tells their intention to the
>> device. Of course there may be some userspace software or kernel
>> trigger blinking the LED, but again, that's still the user's choice and
>> intention. Hence I don't think EC is a boss either, and the user is the
>> real boss.
>>
> I think we're in agreement :)
>
>>>
>>> > >
>>> > > > As my previous reply said, it's common that an LED driver can't prevent
>>> > > > hardware from changing its state autonomously. Prior to the
>>> > > > introduction of auto brightness mode, brightness_hw_changed is enough
>>> > > > to handle this. The issue only emerges when recent models start to
>>> > > > provide an auto brightness mode based on the ALS sensor.
>>> > >
>>> > > Do you have a software driven brightness mode based on an ALS? What
>>> > > API do you use to control this? Can you use that API, and accelerate
>>> > > it?
>>> >
>>> > All devices I've seen implement an EC driven auto brightness mode based
>>> > on an ALS.
>>> >
>>> > @Mark, do you know any device implementing a software driven auto
>>> > brightness mode?
>>> >
>>>
>>> I don't - to my knowledge in auto mode it's always driven by the HW/FW.
>>
>> Thanks.
>>
>>>
>>> If there was a SW approach it would read the sensor and set the brightness to low/medium/high (and not to auto) so I'm struggling to understand the issue here. What am I missing?
>>
>> My understanding of Andrew's words (see also his previous replies) is:
>>
>> hw control trigger is fundamentally an accelerated (offloaded)
>> software trigger. Only if there is a software-driven ALS-based
>> trigger and the curve matches the FW one can we treat the auto
>> brightness mode as a hw control trigger.
>>
>> But those laptops with an ALS and keyboard backlight are not designed
>> to work like this, and the curve may be very specific to the ALS and
>> the luminance of the keyboard backlight. So I asked you to confirm if
>> there is any device designed to use an software driven auto brightness
>> mode (even under Windows).
>>
>> Hmm, forgot to ask about that... Is there any device comes with ALS-
>> based auto brightness mode but Linux cannot read the the ALS? If such
>> devices exist, the "accelerated software trigger" model is no longer
>> relevant.
>>
> I don't know of any SW driven auto brightness mode.
>
> Afraid I also don't know about reading the ALS. I'll see if I can find
> out - but I don't think it's directly important to this series which is
> about supporting the new HW automated feature.
> We're not trying to implement an automated SW based control feature
> with this series :)
>
>> Also that's why we have private LED triggers -- it's useful when the
>> HW/FW has its own triggering functionality. For example, "cros-ec-led"
>> has a private trigger and declares it as its hw control trigger.
>>
>
> ack
>
>>>
>>> > >
>>> > > > FYI, desktop environments (e.g., GNOME, KDE) can control the backlight
>>> > > > brightness of keyboards through sliders and heavily depend on
>>> > > > brightness_hw_changed to update the sliders and display OSD once the
>>> > > > shortcut is pressed.
>>> > >
>>> > > Hold up. Terminology problem. I'm a networking guy, i know networking
>>> > > terms. By slider, do you mean a software scroll bar sort of thing?
>>> >
>>> > Yes. See
>>> > https://blogs.kde.org/2024/09/04/brightness-controls-for-all-your-displays/
>>> >
>>> > (it was about display backlight but it also showed the keyboard one in
>>> > the same image)
>>> >
>>> > > I'm
>>> > > an XFCE users. I can control the display backlight with a slider on
>>> > > the battery charge applet. And i can use Fn F4/F5. I've not seen a
>>> > > software scroll bar for the keyboard backlight, but i think
>>> > > <CTRL><SPC> allows me to change the keyboard backlight.
>>> > >
>>> > > So we have a slider, which is purely software, Linux. And we have key
>>> > > presses, which you are calling shortcut, which the EC acts on, and
>>> > > might tell Linux, maybe, but not about the key press, but the action
>>> > > the EC took because of the key press.
>>> >
>>> > "might tell", "maybe"
>>> >
>>> > It always tells the OS that the state of keyboard backlight has
>>> > changed.
>>> >
>>> > >
>>> > > You have some API to the EC to ask it nicely to act on the software
>>> > > slide, but it is the EC which really controls the LED, not Linux.
>>> > >
>>> > > To me a Linux LED is a poor fit for what you want, and i think a
>>> > > trigger is even worse. The problems you have are because the
>>> > > LED+trigger model, plus using the hardware for acceleration, does not
>>> > > fit with the EC actually controlling the hardware.
>>> > >
>>> > > I would suggest you look at the API the EC exports and find a better
>>> > > model for it.
>>> >
>>> > An LED classdev may be unable to perfectly fit this, but nothing is
>>> > perfect and so far it's the best thing we have. It's a fortunate to
>>> > have the LED subsystem. Windows, without a similar interface, ends up
>>> > being filled with disgusting software pre-installed by the
>>> > manufacturer.
>>> >
>>>
>>> Afraid I don't understand what we are debating here.
>>>
>>> Isn't the whole goal of this patch to make it so LED classdev is a better fit to address missing functionality? Why would switching to something else (I have no idea what) be better? Especially given the the keyboard backlight is currently a LED device, and changing that would potentially break things for users.
>>>
>>> From my perspective if I could just tear this out and have a Lenovo only keyboard_backlight implementation under (for example) /sys/devices/thinkpad_acpi it would be so much easier. But I don't think it is the right thing to do. My experience is if we define a common approach then all vendors will use it going forward - which is better for the Linux experience overall.
>>> Or we don't have fully implemented features for Linux users? That's kinda sucky.
>>
>> I agreed with you. Just some supplemental information:
>>
>> ideapad-laptop has an custom attribute "fn_lock". This used to be
>> the only way for userspace to turn on/off FnLock. The attribute does
>> not support any notification mechanism.
>>
>> Since devices with FnLock also come with an LED indicating the status
>> of FnLock, an LED classdev has been added with a new
>> LED_FUNCTION_FNLOCK macro for dt-bindings and UAPI. See commit
>> 7ab6c64663a0 ("dt-bindings: leds: Add LED_FUNCTION_FNLOCK") and commit
>> 07f48f668fac ("platform/x86: ideapad-laptop: add FnLock LED class
>> device"). Since then, userspace receives notifications through
>> brightness_hw_changed when the user presses Fn+Esc.
>>
>> I think this shows that an LED classdev, as a common interface, has its
>> vitality even when being used in a very specific use case.
>>
>>>
>>> I don't think the two bosses argument is valid (or at least I don't understand it). Are there any other critical implementation details that make this a poor choice and will bite us in the long run?
>>>
>>> I personally find the implementation more complicated than I originally expected, but having looked at it and understood better what Rong was proposing I understand the benefits and I think it works. We're still checking it out on Thinkpad to confirm that, but this patch is a RFC so I think that's part of the process.
>>>
>>> > IMO the presence of brightness_hw_changed proves that an LED classdev,
>>> > as long as appropriate interfaces are provided, can work well with such
>>> > hardware. And I don't think there is too much difference between EC
>>> > setting a static brightness value due to a shortcut being pressed and
>>> > EC turning on/off the auto brightness mode due to the same shortcut --
>>> > if we can handle the former well, we can also implement a similar
>>> > mechanism for the latter.
>>> >
>>> >
>>> > Do you have any recommendations for a "better model"?
>>> >
>>> > Did you mean do not register LED classdevs at all? This isn't really
>>> > viable and will break userspace. Some drivers has been using LED
>>> > classdevs for keyboard backlight for over a decade. And many
>>> > `*::kbd_backlight' LEDs rely on brightness_hw_changed, so it's very
>>> > common that we can't take 100% control over EC. LED classdevs and EC-
>>> > controlled keyboard backlight have lived in harmony for a long time.
>>> >
>>> > If we still register the keybaord backlight as an LED classdev but use
>>> > a custom attribute to report/set the auto brightness mode. IMO this is
>>> > even uglier than LED+trigger, as writing to such a non-brightness
>>> > attribute will interfere with the brightness attribute and the active
>>> > trigger and vice versa. Even if we rule out the EC's action, such
>>> > interference still exists as long as users use the attribute.
>>> >
>>> >
>>> > As for the software-vs-hardware priority issue, how about adding an
>>> > attribute "hw_change_policy" so that users can select if the software
>>> > state should be always reimposed to hardware?
>>>
>>> Is this needed? When wouldn't this be the case?
>>>
>>> If the SW sets a specific brightness that should become the setting. It would override any previous choices and tell the HW that is what is wanted now - don't change it (until the user says otherwise).
>>> If we're in auto mode and the HW changes the brightness - it doesn't change the setting from auto mode, just the reported brightness level if queried.
>>
>> My understanding of Andrew's words is:
>>
>> Linux LED should always be the boss. Tell the HW: don't change it *even
>> if the user presses Fn+Space*. Failing to accomplish this means that we
>> are in "a two bosses dilemma", hence "a Linux LED is a poor fit for
>> what you want" and "a trigger is even worse".
>>
>> Since Andrew cares about the software's precedence, the best thing we
>> can do is adding an attribute for users to select their preference. The
>> attribute's default value must not be reimpose, otherwise it breaks
>> existing userspace programs relying on brightness_hw_changed and
>> confuses most users.
>>
>
> For the current HW, the attribute is not going to be supported. There's
> no way to disable FN+space as it goes directly to the BIOS (which then
> notifies the OS).
>
> I guess the OS can get the notification and then say 'nope - despite
> the fact they just pressed the FN + spacebar, they actually don't want
> anything to change', and revert the setting to the previous setting.
> If the user had to specifically set the attribute to do this it's fine
> - but it feels like something that nobody would ever use and a whole
> bunch of extra complexity, to me
>
>> But yeah, personally I don't think it's needed either. It's been 9
>> years since the introduction of brightness_hw_changed, and there's no
>> complaint about HW/FW overriding the software configured brightness.
>> After all, it's the user who decides to press the shortcut and asks the
>> EC to change the brightness or turn on/off the auto brightness mode.
>>
>
> I think you and I are both in agreement.
>
> Andrew - are you still against the current proposal or have your
> concerns been addressed?
>
> How do we proceed on finding something that we can move forward with?
>
Hi maintainers - a gentle nudge on this one. Would like some direction if we can move ahead with the series. If not, what is required?
On our side I know Vishnu has implemented the thinkpad driver using this and confirmed the design works there too.
Would like to get this upstream so we can start working with the user-space folk on updating their pieces.
Thanks
Mark
^ permalink raw reply
* [PATCH v7 1/2] platform: Add initial synology microp driver
From: Markus Probst via B4 Relay @ 2026-04-11 15:27 UTC (permalink / raw)
To: Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue,
Lee Jones, Pavel Machek, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Greg Kroah-Hartman
Cc: platform-driver-x86, linux-leds, devicetree, linux-kernel,
rust-for-linux, Markus Probst
In-Reply-To: <20260411-synology_microp_initial-v7-0-9a3a094e763a@posteo.de>
From: Markus Probst <markus.probst@posteo.de>
Add a initial synology microp driver, written in Rust.
The driver targets a microcontroller found in Synology NAS devices. It
currently only supports controlling of the power led, status led, alert
led and usb led. Other components such as fan control or handling
on-device buttons will be added once the required rust abstractions are
there.
This driver can be used both on arm and x86, thus it goes into the root
directory of drivers/platform.
Tested successfully on a Synology DS923+.
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
MAINTAINERS | 5 +
drivers/platform/Kconfig | 2 +
drivers/platform/Makefile | 1 +
drivers/platform/synology_microp/Kconfig | 13 +
drivers/platform/synology_microp/Makefile | 3 +
drivers/platform/synology_microp/TODO | 7 +
drivers/platform/synology_microp/command.rs | 55 ++++
drivers/platform/synology_microp/led.rs | 276 +++++++++++++++++++++
drivers/platform/synology_microp/model.rs | 49 ++++
.../platform/synology_microp/synology_microp.rs | 109 ++++++++
10 files changed, 520 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index a667f4efb66e..78c99d831431 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25554,6 +25554,11 @@ F: drivers/dma-buf/sync_*
F: include/linux/sync_file.h
F: include/uapi/linux/sync_file.h
+SYNOLOGY MICROP DRIVER
+M: Markus Probst <markus.probst@posteo.de>
+S: Maintained
+F: drivers/platform/synology_microp/
+
SYNOPSYS ARC ARCHITECTURE
M: Vineet Gupta <vgupta@kernel.org>
L: linux-snps-arc@lists.infradead.org
diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
index 312788f249c9..996050566a4a 100644
--- a/drivers/platform/Kconfig
+++ b/drivers/platform/Kconfig
@@ -22,3 +22,5 @@ source "drivers/platform/arm64/Kconfig"
source "drivers/platform/raspberrypi/Kconfig"
source "drivers/platform/wmi/Kconfig"
+
+source "drivers/platform/synology_microp/Kconfig"
diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
index fa322e7f8716..2381872e9133 100644
--- a/drivers/platform/Makefile
+++ b/drivers/platform/Makefile
@@ -15,3 +15,4 @@ obj-$(CONFIG_SURFACE_PLATFORMS) += surface/
obj-$(CONFIG_ARM64_PLATFORM_DEVICES) += arm64/
obj-$(CONFIG_BCM2835_VCHIQ) += raspberrypi/
obj-$(CONFIG_ACPI_WMI) += wmi/
+obj-$(CONFIG_SYNOLOGY_MICROP) += synology_microp/
diff --git a/drivers/platform/synology_microp/Kconfig b/drivers/platform/synology_microp/Kconfig
new file mode 100644
index 000000000000..7c4d8f2808f0
--- /dev/null
+++ b/drivers/platform/synology_microp/Kconfig
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config SYNOLOGY_MICROP
+ tristate "Synology Microp driver"
+ depends on LEDS_CLASS && LEDS_CLASS_MULTICOLOR
+ depends on RUST_SERIAL_DEV_BUS_ABSTRACTIONS
+ help
+ Enable support for the MCU found in Synology NAS devices.
+
+ This is needed to properly shutdown and reboot the device, as well as
+ additional functionality like fan and LED control.
+
+ This driver is work in progress and may not be fully functional.
diff --git a/drivers/platform/synology_microp/Makefile b/drivers/platform/synology_microp/Makefile
new file mode 100644
index 000000000000..63585ccf76e4
--- /dev/null
+++ b/drivers/platform/synology_microp/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-y += synology_microp.o
diff --git a/drivers/platform/synology_microp/TODO b/drivers/platform/synology_microp/TODO
new file mode 100644
index 000000000000..1961a33115db
--- /dev/null
+++ b/drivers/platform/synology_microp/TODO
@@ -0,0 +1,7 @@
+TODO:
+- add missing components:
+ - handle on-device buttons (Power, Factory reset, "USB Copy")
+ - handle fan failure
+ - beeper
+ - fan speed control
+ - correctly perform device power-off and restart on Synology devices
diff --git a/drivers/platform/synology_microp/command.rs b/drivers/platform/synology_microp/command.rs
new file mode 100644
index 000000000000..5b3dd715afac
--- /dev/null
+++ b/drivers/platform/synology_microp/command.rs
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use kernel::{
+ device::Bound,
+ error::Result,
+ serdev, //
+};
+
+use crate::led;
+
+#[derive(Copy, Clone)]
+#[expect(
+ clippy::enum_variant_names,
+ reason = "future variants will not end with Led"
+)]
+pub(crate) enum Command {
+ PowerLed(led::State),
+ StatusLed(led::StatusLedColor, led::State),
+ AlertLed(led::State),
+ UsbLed(led::State),
+ EsataLed(led::State),
+}
+
+impl Command {
+ pub(crate) fn write(self, dev: &serdev::Device<Bound>) -> Result {
+ dev.write_all(
+ match self {
+ Self::PowerLed(led::State::On) => &[0x34],
+ Self::PowerLed(led::State::Blink) => &[0x35],
+ Self::PowerLed(led::State::Off) => &[0x36],
+
+ Self::StatusLed(_, led::State::Off) => &[0x37],
+ Self::StatusLed(led::StatusLedColor::Green, led::State::On) => &[0x38],
+ Self::StatusLed(led::StatusLedColor::Green, led::State::Blink) => &[0x39],
+ Self::StatusLed(led::StatusLedColor::Orange, led::State::On) => &[0x3A],
+ Self::StatusLed(led::StatusLedColor::Orange, led::State::Blink) => &[0x3B],
+
+ Self::AlertLed(led::State::On) => &[0x4C, 0x41, 0x31],
+ Self::AlertLed(led::State::Blink) => &[0x4C, 0x41, 0x32],
+ Self::AlertLed(led::State::Off) => &[0x4C, 0x41, 0x33],
+
+ Self::UsbLed(led::State::On) => &[0x40],
+ Self::UsbLed(led::State::Blink) => &[0x41],
+ Self::UsbLed(led::State::Off) => &[0x42],
+
+ Self::EsataLed(led::State::On) => &[0x4C, 0x45, 0x31],
+ Self::EsataLed(led::State::Blink) => &[0x4C, 0x45, 0x32],
+ Self::EsataLed(led::State::Off) => &[0x4C, 0x45, 0x33],
+ },
+ serdev::Timeout::Max,
+ )?;
+ dev.wait_until_sent(serdev::Timeout::Max);
+ Ok(())
+ }
+}
diff --git a/drivers/platform/synology_microp/led.rs b/drivers/platform/synology_microp/led.rs
new file mode 100644
index 000000000000..a78a95588456
--- /dev/null
+++ b/drivers/platform/synology_microp/led.rs
@@ -0,0 +1,276 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use kernel::{
+ device::Bound,
+ devres::{
+ self,
+ Devres, //
+ },
+ led::{
+ self,
+ LedOps,
+ MultiColorSubLed, //
+ },
+ new_mutex,
+ prelude::*,
+ serdev,
+ str::CString,
+ sync::Mutex, //
+};
+use pin_init::pin_init_scope;
+
+use crate::{
+ command::Command,
+ model::Model, //
+};
+
+#[pin_data]
+pub(crate) struct Data {
+ #[pin]
+ status: Devres<led::MultiColorDevice<StatusLedHandler>>,
+ power_name: CString,
+ #[pin]
+ power: Devres<led::Device<LedHandler>>,
+}
+
+impl Data {
+ pub(super) fn register<'a>(
+ dev: &'a serdev::Device<Bound>,
+ model: &'a Model,
+ ) -> impl PinInit<Self, Error> + 'a {
+ pin_init_scope(move || {
+ if let Some(color) = model.led_alert {
+ let name = CString::try_from_fmt(fmt!("{}:alarm", color.as_c_str().to_str()?))?;
+ devres::register(
+ dev.as_ref(),
+ led::DeviceBuilder::new().color(color).name(&name).build(
+ dev,
+ try_pin_init!(LedHandler {
+ blink <- new_mutex!(false),
+ command: Command::AlertLed,
+ }),
+ ),
+ GFP_KERNEL,
+ )?;
+ }
+
+ if model.led_usb_copy {
+ devres::register(
+ dev.as_ref(),
+ led::DeviceBuilder::new()
+ .color(led::Color::Green)
+ .name(c"green:usb")
+ .build(
+ dev,
+ try_pin_init!(LedHandler {
+ blink <- new_mutex!(false),
+ command: Command::UsbLed,
+ }),
+ ),
+ GFP_KERNEL,
+ )?;
+ }
+
+ if model.led_esata {
+ devres::register(
+ dev.as_ref(),
+ led::DeviceBuilder::new()
+ .color(led::Color::Green)
+ .name(c"green:esata")
+ .build(
+ dev,
+ try_pin_init!(LedHandler {
+ blink <- new_mutex!(false),
+ command: Command::EsataLed,
+ }),
+ ),
+ GFP_KERNEL,
+ )?;
+ }
+
+ Ok(try_pin_init!(Self {
+ status <- led::DeviceBuilder::new()
+ .color(led::Color::Multi)
+ .name(c"multicolor:status")
+ .build_multicolor(
+ dev,
+ try_pin_init!(StatusLedHandler {
+ blink <- new_mutex!(false),
+ }),
+ StatusLedHandler::SUBLEDS,
+ ),
+ power_name: CString::try_from_fmt(fmt!(
+ "{}:power",
+ model.led_power.as_c_str().to_str()?
+ ))?,
+ power <- led::DeviceBuilder::new()
+ .color(model.led_power)
+ .name(power_name)
+ .build(
+ dev,
+ try_pin_init!(LedHandler {
+ blink <- new_mutex!(true),
+ command: Command::PowerLed,
+ }),
+ ),
+ }))
+ })
+ }
+}
+
+#[derive(Copy, Clone)]
+pub(crate) enum StatusLedColor {
+ Green,
+ Orange,
+}
+
+#[derive(Copy, Clone)]
+pub(crate) enum State {
+ On,
+ Blink,
+ Off,
+}
+
+#[pin_data]
+struct LedHandler {
+ #[pin]
+ blink: Mutex<bool>,
+ command: fn(State) -> Command,
+}
+
+#[vtable]
+impl LedOps for LedHandler {
+ type Bus = serdev::Device<Bound>;
+ type Mode = led::Normal;
+ const BLOCKING: bool = true;
+ const MAX_BRIGHTNESS: u32 = 1;
+
+ fn brightness_set(
+ &self,
+ dev: &Self::Bus,
+ _classdev: &led::Device<Self>,
+ brightness: u32,
+ ) -> Result<()> {
+ let mut blink = self.blink.lock();
+ (self.command)(if brightness == 0 {
+ *blink = false;
+ State::Off
+ } else if *blink {
+ State::Blink
+ } else {
+ State::On
+ })
+ .write(dev)?;
+
+ Ok(())
+ }
+
+ fn blink_set(
+ &self,
+ dev: &Self::Bus,
+ _classdev: &led::Device<Self>,
+ delay_on: &mut usize,
+ delay_off: &mut usize,
+ ) -> Result<()> {
+ let mut blink = self.blink.lock();
+
+ (self.command)(if *delay_on == 0 && *delay_off != 0 {
+ State::Off
+ } else if *delay_on != 0 && *delay_off == 0 {
+ State::On
+ } else {
+ *blink = true;
+ *delay_on = 167;
+ *delay_off = 167;
+
+ State::Blink
+ })
+ .write(dev)
+ }
+}
+
+#[pin_data]
+struct StatusLedHandler {
+ #[pin]
+ blink: Mutex<bool>,
+}
+
+impl StatusLedHandler {
+ const SUBLEDS: &[MultiColorSubLed] = &[
+ MultiColorSubLed::new(led::Color::Green).initial_intensity(1),
+ MultiColorSubLed::new(led::Color::Orange),
+ ];
+}
+
+#[vtable]
+impl LedOps for StatusLedHandler {
+ type Bus = serdev::Device<Bound>;
+ type Mode = led::MultiColor;
+ const BLOCKING: bool = true;
+ const MAX_BRIGHTNESS: u32 = 1;
+
+ fn brightness_set(
+ &self,
+ dev: &Self::Bus,
+ classdev: &led::MultiColorDevice<Self>,
+ brightness: u32,
+ ) -> Result<()> {
+ let mut blink = self.blink.lock();
+ if brightness == 0 {
+ *blink = false;
+ }
+
+ let (color, subled_brightness) = if classdev.subleds()[1].intensity == 0 {
+ (StatusLedColor::Green, classdev.subleds()[0].brightness)
+ } else {
+ (StatusLedColor::Orange, classdev.subleds()[1].brightness)
+ };
+
+ Command::StatusLed(
+ color,
+ if subled_brightness == 0 {
+ State::Off
+ } else if *blink {
+ State::Blink
+ } else {
+ State::On
+ },
+ )
+ .write(dev)
+ }
+
+ fn blink_set(
+ &self,
+ dev: &Self::Bus,
+ classdev: &led::MultiColorDevice<Self>,
+ delay_on: &mut usize,
+ delay_off: &mut usize,
+ ) -> Result<()> {
+ let mut blink = self.blink.lock();
+ *blink = true;
+
+ let (color, subled_intensity) = if classdev.subleds()[1].intensity == 0 {
+ (StatusLedColor::Green, classdev.subleds()[0].intensity)
+ } else {
+ (StatusLedColor::Orange, classdev.subleds()[1].intensity)
+ };
+ Command::StatusLed(
+ color,
+ if *delay_on == 0 && *delay_off != 0 {
+ *blink = false;
+ State::Off
+ } else if subled_intensity == 0 {
+ State::Off
+ } else if *delay_on != 0 && *delay_off == 0 {
+ *blink = false;
+ State::On
+ } else {
+ *delay_on = 167;
+ *delay_off = 167;
+
+ State::Blink
+ },
+ )
+ .write(dev)
+ }
+}
diff --git a/drivers/platform/synology_microp/model.rs b/drivers/platform/synology_microp/model.rs
new file mode 100644
index 000000000000..715d8840f56b
--- /dev/null
+++ b/drivers/platform/synology_microp/model.rs
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use kernel::led::Color;
+
+pub(crate) struct Model {
+ pub(crate) led_power: Color,
+ pub(crate) led_alert: Option<Color>,
+ pub(crate) led_usb_copy: bool,
+ pub(crate) led_esata: bool,
+}
+
+impl Model {
+ pub(super) const fn new() -> Self {
+ Self {
+ led_power: Color::Blue,
+ led_alert: None,
+ led_usb_copy: false,
+ led_esata: false,
+ }
+ }
+
+ pub(super) const fn led_power(self, color: Color) -> Self {
+ Self {
+ led_power: color,
+ ..self
+ }
+ }
+
+ pub(super) const fn led_alert(self, color: Color) -> Self {
+ Self {
+ led_alert: Some(color),
+ ..self
+ }
+ }
+
+ pub(super) const fn led_esata(self) -> Self {
+ Self {
+ led_esata: true,
+ ..self
+ }
+ }
+
+ pub(super) const fn led_usb_copy(self) -> Self {
+ Self {
+ led_usb_copy: true,
+ ..self
+ }
+ }
+}
diff --git a/drivers/platform/synology_microp/synology_microp.rs b/drivers/platform/synology_microp/synology_microp.rs
new file mode 100644
index 000000000000..f02c4dade76c
--- /dev/null
+++ b/drivers/platform/synology_microp/synology_microp.rs
@@ -0,0 +1,109 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Synology Microp driver
+
+use kernel::{
+ device,
+ led::Color,
+ of::{
+ DeviceId,
+ IdTable, //
+ },
+ of_device_table,
+ prelude::*,
+ serdev, //
+};
+use pin_init::pin_init_scope;
+
+use crate::model::Model;
+
+pub(crate) mod command;
+mod led;
+mod model;
+
+kernel::module_serdev_device_driver! {
+ type: SynologyMicropDriver,
+ name: "synology_microp",
+ authors: ["Markus Probst <markus.probst@posteo.de>"],
+ description: "Synology Microp driver",
+ license: "GPL v2",
+}
+
+#[rustfmt::skip]
+of_device_table!(
+ OF_TABLE,
+ MODULE_OF_TABLE,
+ Model,
+ [
+ // apollolake
+ (DeviceId::new(c"synology,ds918p-microp"), Model::new()),
+
+ // evansport
+ (DeviceId::new(c"synology,ds214play-microp"), Model::new()),
+
+ // geminilakenk
+ (DeviceId::new(c"synology,ds225p-microp"), Model::new().led_usb_copy()),
+ (DeviceId::new(c"synology,ds425p-microp"), Model::new()),
+
+ // pineview
+ (DeviceId::new(c"synology,ds710p-microp"), Model::new().led_esata()),
+ (DeviceId::new(c"synology,ds1010p-microp"), Model::new().led_alert(Color::Orange)),
+
+ // r1000
+ (DeviceId::new(c"synology,ds923p-microp"), Model::new()),
+ (DeviceId::new(c"synology,ds723p-microp"), Model::new()),
+ (DeviceId::new(c"synology,ds1522p-microp"), Model::new()),
+ (DeviceId::new(c"synology,rs422p-microp"), Model::new().led_power(Color::Green)),
+
+ // r1000nk
+ (DeviceId::new(c"synology,ds725p-microp"), Model::new()),
+
+ // rtd1296
+ (DeviceId::new(c"synology,ds118-microp"), Model::new()),
+
+ // rtd1619b
+ (DeviceId::new(c"synology,ds124-microp"), Model::new()),
+ (DeviceId::new(c"synolody,ds223-microp"), Model::new().led_usb_copy()),
+ (DeviceId::new(c"synology,ds223j-microp"), Model::new()),
+
+ // v1000
+ (DeviceId::new(c"synology,ds1823xsp-microp"), Model::new()),
+ (DeviceId::new(c"synology,rs822p-microp"), Model::new().led_power(Color::Green)),
+ (DeviceId::new(c"synology,rs1221p-microp"), Model::new().led_power(Color::Green)),
+ (DeviceId::new(c"synology,rs1221rpp-microp"), Model::new().led_power(Color::Green)),
+
+ // v1000nk
+ (DeviceId::new(c"synology,ds925p-microp"), Model::new()),
+ (DeviceId::new(c"synology,ds1525p-microp"), Model::new()),
+ (DeviceId::new(c"synology,ds1825p-microp"), Model::new()),
+ ]
+);
+
+#[pin_data]
+struct SynologyMicropDriver {
+ #[pin]
+ led: led::Data,
+}
+
+#[vtable]
+impl serdev::Driver for SynologyMicropDriver {
+ type IdInfo = Model;
+ const OF_ID_TABLE: Option<IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+
+ fn probe(
+ dev: &serdev::Device<device::Core>,
+ model: Option<&Model>,
+ ) -> impl PinInit<Self, kernel::error::Error> {
+ pin_init_scope(move || {
+ let model = model.ok_or(EINVAL)?;
+
+ dev.set_baudrate(9600).map_err(|_| EINVAL)?;
+ dev.set_flow_control(false);
+ dev.set_parity(serdev::Parity::None)?;
+
+ Ok(try_pin_init!(Self {
+ led <- led::Data::register(dev, model),
+ }))
+ })
+ }
+}
--
2.52.0
^ permalink raw reply related
* [PATCH v7 2/2] dt-bindings: embedded-controller: Add synology microp devices
From: Markus Probst via B4 Relay @ 2026-04-11 15:27 UTC (permalink / raw)
To: Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue,
Lee Jones, Pavel Machek, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Greg Kroah-Hartman
Cc: platform-driver-x86, linux-leds, devicetree, linux-kernel,
rust-for-linux, Markus Probst
In-Reply-To: <20260411-synology_microp_initial-v7-0-9a3a094e763a@posteo.de>
From: Markus Probst <markus.probst@posteo.de>
Add the Synology Microp devicetree bindings. Those devices are
microcontrollers found on Synology NAS devices. They are connected to a
serial port on the host device.
Those devices are used to control certain LEDs, fan speeds, a beeper, to
handle buttons, fan failures and to properly shutdown and reboot the
device.
The device has a different feature set depending on the Synology NAS
model, like having different number of fans, buttons and leds. Depending
on the architecture of the model, they also need a different system
shutdown behaviour.
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
.../synology,ds923p-microp.yaml | 92 ++++++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 93 insertions(+)
diff --git a/Documentation/devicetree/bindings/embedded-controller/synology,ds923p-microp.yaml b/Documentation/devicetree/bindings/embedded-controller/synology,ds923p-microp.yaml
new file mode 100644
index 000000000000..0a8fb1d8f314
--- /dev/null
+++ b/Documentation/devicetree/bindings/embedded-controller/synology,ds923p-microp.yaml
@@ -0,0 +1,92 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/embedded-controller/synology,ds923p-microp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synology NAS on-board Microcontroller
+
+maintainers:
+ - Markus Probst <markus.probst@posteo.de>
+
+description: |
+ Synology Microp is a microcontroller found in Synology NAS devices.
+ It is connected to a serial port on the host device.
+
+ It is necessary to properly shutdown and reboot the NAS device and
+ provides additional functionality such as led control, fan speed control,
+ a beeper and buttons on the NAS device.
+
+properties:
+ compatible:
+ enum:
+ - synology,ds923p-microp
+ - synology,ds918p-microp
+ - synology,ds214play-microp
+ - synology,ds225p-microp
+ - synology,ds425p-microp
+ - synology,ds710p-microp
+ - synology,ds1010p-microp
+ - synology,ds723p-microp
+ - synology,ds1522p-microp
+ - synology,rs422p-microp
+ - synology,ds725p-microp
+ - synology,ds118-microp
+ - synology,ds124-microp
+ - synology,ds223-microp
+ - synology,ds223j-microp
+ - synology,ds1823xsp-microp
+ - synology,rs822p-microp
+ - synology,rs1221p-microp
+ - synology,rs1221rpp-microp
+ - synology,ds925p-microp
+ - synology,ds1525p-microp
+ - synology,ds1825p-microp
+
+ fan-failure-gpios:
+ description: GPIOs needed to determine which fans stopped working on a fan failure event.
+ minItems: 2
+ maxItems: 3
+
+required:
+ - compatible
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - synology,ds214play-microp
+ - synology,ds225p-microp
+ - synology,ds710p-microp
+ - synology,ds723p-microp
+ - synology,ds725p-microp
+ - synology,ds118-microp
+ - synology,ds124-microp
+ - synology,ds223-microp
+ - synology,ds223j-microp
+ - synology,ds1823xsp-microp
+ - synology,rs822p-microp
+ - synology,rs1221p-microp
+ - synology,rs1221rpp-microp
+ - synology,ds1825p-microp
+ then:
+ properties:
+ fan-failure-gpios: false
+ else:
+ required:
+ - fan-failure-gpios
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/leds/common.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ embedded-controller {
+ compatible = "synology,ds923p-microp";
+
+ fan-failure-gpios = <&gpio 68 GPIO_ACTIVE_HIGH>, <&gpio 69 GPIO_ACTIVE_HIGH>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index 78c99d831431..72075c9a2016 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25557,6 +25557,7 @@ F: include/uapi/linux/sync_file.h
SYNOLOGY MICROP DRIVER
M: Markus Probst <markus.probst@posteo.de>
S: Maintained
+F: Documentation/devicetree/bindings/embedded-controller/synology,ds923p-microp.yaml
F: drivers/platform/synology_microp/
SYNOPSYS ARC ARCHITECTURE
--
2.52.0
^ permalink raw reply related
* [PATCH v7 0/2] Introduce Synology Microp driver
From: Markus Probst via B4 Relay @ 2026-04-11 15:27 UTC (permalink / raw)
To: Hans de Goede, Ilpo Järvinen, Bryan O'Donoghue,
Lee Jones, Pavel Machek, Miguel Ojeda, Boqun Feng, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Greg Kroah-Hartman
Cc: platform-driver-x86, linux-leds, devicetree, linux-kernel,
rust-for-linux, Markus Probst
Synology uses a microcontroller in their NAS devices connected to a
serial port to control certain LEDs, fan speeds, a beeper, to handle
proper shutdown and restart, buttons and fan failures.
This patch series depends on the rust led abstraction [1] and the rust
serdev abstraction [2].
This is only a initial version of the driver able to control LEDs.
The following rust abstractions would be required, to implement the
remaining features:
- hwmon (include/linux/hwmon.h)
- input (include/linux/input.h)
- sysoff handler + hardware protection shutdown (include/linux/reboot.h)
[1] https://lore.kernel.org/rust-for-linux/20260329-rust_leds-v13-0-21a599c5b2d1@posteo.de/
[2] https://lore.kernel.org/rust-for-linux/20260411-rust_serdev-v4-0-845e960c6627@posteo.de/
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
Changes in v7:
- remove list of compatible ids from commit msg
- explain what makes the different models not compatible in the commit msg
- remove unnecessary examples
- Link to v6: https://lore.kernel.org/r/20260405-synology_microp_initial-v6-0-08fde474b6c9@posteo.de
Changes in v6:
- moved devicetree bindings patch at the end of the set
- remove several patches
- move of id table from model.rs to synology_microp.rs
- remove the model! macro
- use if blocks in devicetree schema to narrow down the
fan-failure-gpios property
- add multiple devicetree examples to test if blocks
- Link to v5: https://lore.kernel.org/r/20260329-synology_microp_initial-v5-0-27cb80bdf591@posteo.de
Changes in v5:
- add esata led support
- use different compatible for each model
- add visibility modifier to of_device_table macro
- fix match data missing when using PRP0001
- Link to v4: https://lore.kernel.org/r/20260320-synology_microp_initial-v4-0-0423ddb83ca4@posteo.de
Changes in v4:
- convert to monolithic driver and moved it into drivers/platform
- removed mfd rust abstraction
- moved dt-bindings to embedded-controller
- Link to v3: https://lore.kernel.org/r/20260313-synology_microp_initial-v3-0-ad6ac463a201@posteo.de
Changes in v3:
- remove `default n` from Kconfig entry, as n is the default already.
- select RUST_SERIAL_DEV_BUS_ABSTRACTIONS in Kconfig
- add mfd rust abstraction
- split core and led parts into their own driver. It should now be considered a
MFD device.
- split led part of dt binding into its own file
- Link to v2: https://lore.kernel.org/r/20260308-synology_microp_initial-v2-0-9389963f31c5@posteo.de
Changes in v2:
- fix missing tabs in MAINTAINERS file
- remove word binding from patch subject
- add missing signed-off-by
- add missing help entry in Kconfig
- add missing spdx license headers
- remove no-check{,-cpu}-fan properties from the dt-bindings and replace
them with the check_fan module parameter
- use patternProperties for leds in dt-bindings
- license dt-binding as GPL-2.0-only OR BSD-2-Clause
- move driver from staging tree into mfd tree and mark it as work in
progress inside Kconfig
- only register alert and usb led if fwnode is present
- Link to v1: https://lore.kernel.org/r/20260306-synology_microp_initial-v1-0-fcffede6448c@posteo.de
---
Markus Probst (2):
platform: Add initial synology microp driver
dt-bindings: embedded-controller: Add synology microp devices
.../synology,ds923p-microp.yaml | 92 +++++++
MAINTAINERS | 6 +
drivers/platform/Kconfig | 2 +
drivers/platform/Makefile | 1 +
drivers/platform/synology_microp/Kconfig | 13 +
drivers/platform/synology_microp/Makefile | 3 +
drivers/platform/synology_microp/TODO | 7 +
drivers/platform/synology_microp/command.rs | 55 ++++
drivers/platform/synology_microp/led.rs | 276 +++++++++++++++++++++
drivers/platform/synology_microp/model.rs | 49 ++++
.../platform/synology_microp/synology_microp.rs | 109 ++++++++
11 files changed, 613 insertions(+)
---
base-commit: 0e5d0a0b5ca6ea4e391d6786266405c5871e0151
change-id: 20260306-synology_microp_initial-0f7dac7b7496
prerequisite-change-id: 20251217-rust_serdev-ee5481e9085c:v4
prerequisite-patch-id: 52b17274481cc770c257d8f95335293eca32a2c5
prerequisite-patch-id: eec47e5051640d08bcd34a9670b98804449cad52
prerequisite-patch-id: f24b68c71c3f69371e8ac0251efca0a023b31cc4
prerequisite-patch-id: d0686cf451ef899a06d468adfba51ccd84e6ff98
prerequisite-change-id: 20251114-rust_leds-a959f7c2f7f9:v13
prerequisite-patch-id: 818700f22dcb9676157c985f82762d7c607b861e
prerequisite-patch-id: b15ffa7d95d9260151bfb116b259c4473f721c82
prerequisite-patch-id: 8c47e0d107530f577a1be0b79f8ee791f95d3cbe
^ permalink raw reply
* [PATCH RESEND v13 3/3] rust: leds: add multicolor classdev abstractions
From: Markus Probst @ 2026-04-11 15:07 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rafael J. Wysocki, Bjorn Helgaas,
Krzysztof Wilczyński, Boqun Feng, Boqun Feng
Cc: rust-for-linux, linux-leds, linux-kernel, linux-pci,
Markus Probst
In-Reply-To: <20260411-rust_leds-v13-0-1208a2821deb@posteo.de>
Implement the abstractions needed for multicolor led class devices,
including:
* `led::MultiColor` - the led mode implementation
* `MultiColorSubLed` - a safe wrapper arround `mc_subled`
* `led::MultiColorDevice` - a safe wrapper around `led_classdev_mc`
* `led::DeviceBuilder::build_multicolor` - a function to register a new
multicolor led class device
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
rust/bindings/bindings_helper.h | 1 +
rust/kernel/led.rs | 30 +++-
rust/kernel/led/multicolor.rs | 387 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 417 insertions(+), 1 deletion(-)
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 083cc44aa952..3171e3e6351c 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -63,6 +63,7 @@
#include <linux/ioport.h>
#include <linux/jiffies.h>
#include <linux/jump_label.h>
+#include <linux/led-class-multicolor.h>
#include <linux/mdio.h>
#include <linux/mm.h>
#include <linux/miscdevice.h>
diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
index 5035563d68a3..a862d88cab29 100644
--- a/rust/kernel/led.rs
+++ b/rust/kernel/led.rs
@@ -33,8 +33,12 @@
}, //
};
+#[cfg(CONFIG_LEDS_CLASS_MULTICOLOR)]
+mod multicolor;
mod normal;
+#[cfg(CONFIG_LEDS_CLASS_MULTICOLOR)]
+pub use multicolor::{MultiColor, MultiColorDevice, MultiColorSubLed};
pub use normal::{Device, Normal};
/// The name of the led is determined by the driver.
@@ -279,7 +283,24 @@ pub enum Color {
Violet = bindings::LED_COLOR_ID_VIOLET,
Yellow = bindings::LED_COLOR_ID_YELLOW,
Ir = bindings::LED_COLOR_ID_IR,
+ #[cfg_attr(
+ CONFIG_LEDS_CLASS_MULTICOLOR,
+ doc = "Use this color for a [`MultiColor`] led."
+ )]
+ #[cfg_attr(
+ not(CONFIG_LEDS_CLASS_MULTICOLOR),
+ doc = "Use this color for a `MultiColor` led."
+ )]
+ /// If the led supports RGB, use [`Color::Rgb`] instead.
Multi = bindings::LED_COLOR_ID_MULTI,
+ #[cfg_attr(
+ CONFIG_LEDS_CLASS_MULTICOLOR,
+ doc = "Use this color for a [`MultiColor`] led with rgb support."
+ )]
+ #[cfg_attr(
+ not(CONFIG_LEDS_CLASS_MULTICOLOR),
+ doc = "Use this color for a `MultiColor` led with rgb support."
+ )]
Rgb = bindings::LED_COLOR_ID_RGB,
Purple = bindings::LED_COLOR_ID_PURPLE,
Orange = bindings::LED_COLOR_ID_ORANGE,
@@ -319,7 +340,14 @@ fn try_from(value: u32) -> core::result::Result<Self, Self::Error> {
///
/// Each led mode has its own led class device type with different capabilities.
///
-/// See [`Normal`].
+#[cfg_attr(
+ CONFIG_LEDS_CLASS_MULTICOLOR,
+ doc = "See [`Normal`] and [`MultiColor`]."
+)]
+#[cfg_attr(
+ not(CONFIG_LEDS_CLASS_MULTICOLOR),
+ doc = "See [`Normal`] and `MultiColor`."
+)]
pub trait Mode: private::Sealed {
/// The class device for the led mode.
type Device<T: LedOps<Mode = Self>>;
diff --git a/rust/kernel/led/multicolor.rs b/rust/kernel/led/multicolor.rs
new file mode 100644
index 000000000000..726fdaf068cb
--- /dev/null
+++ b/rust/kernel/led/multicolor.rs
@@ -0,0 +1,387 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Led mode for the `struct led_classdev_mc`.
+//!
+//! C header: [`include/linux/led-class-multicolor.h`](srctree/include/linux/led-class-multicolor.h)
+
+use crate::alloc::KVec;
+
+use super::*;
+
+/// The led mode for the `struct led_classdev_mc`. Leds with this mode can have multiple colors.
+pub enum MultiColor {}
+impl Mode for MultiColor {
+ type Device<T: LedOps<Mode = Self>> = MultiColorDevice<T>;
+}
+impl private::Sealed for MultiColor {}
+
+/// The multicolor sub led info representation.
+///
+/// This structure represents the Rust abstraction for a C `struct mc_subled`.
+#[repr(C)]
+#[derive(Copy, Clone, Debug)]
+#[non_exhaustive]
+pub struct MultiColorSubLed {
+ /// the color of the sub led
+ pub color: Color,
+ /// the brightness of the sub led.
+ ///
+ /// The value will be automatically calculated.
+ /// See `MultiColor::pre_brightness_set`.
+ pub brightness: u32,
+ /// the intensity of the sub led.
+ pub intensity: u32,
+ /// arbitrary data for the driver to store.
+ pub channel: u32,
+}
+
+// We directly pass a reference to the `subled_info` field in `led_classdev_mc` to the driver via
+// `Device::subleds()`.
+// We need safeguards to ensure `MultiColorSubLed` and `mc_subled` stay identical.
+const _: () = {
+ use core::mem::offset_of;
+
+ const fn assert_same_type<T>(_: &T, _: &T) {}
+
+ let rust_zeroed = MultiColorSubLed {
+ color: Color::White,
+ brightness: 0,
+ intensity: 0,
+ channel: 0,
+ };
+ let c_zeroed = bindings::mc_subled {
+ color_index: 0,
+ brightness: 0,
+ intensity: 0,
+ channel: 0,
+ };
+
+ assert!(offset_of!(MultiColorSubLed, color) == offset_of!(bindings::mc_subled, color_index));
+ assert_same_type(&0u32, &c_zeroed.color_index);
+
+ assert!(
+ offset_of!(MultiColorSubLed, brightness) == offset_of!(bindings::mc_subled, brightness)
+ );
+ assert_same_type(&rust_zeroed.brightness, &c_zeroed.brightness);
+
+ assert!(offset_of!(MultiColorSubLed, intensity) == offset_of!(bindings::mc_subled, intensity));
+ assert_same_type(&rust_zeroed.intensity, &c_zeroed.intensity);
+
+ assert!(offset_of!(MultiColorSubLed, channel) == offset_of!(bindings::mc_subled, channel));
+ assert_same_type(&rust_zeroed.channel, &c_zeroed.channel);
+
+ assert!(size_of::<MultiColorSubLed>() == size_of::<bindings::mc_subled>());
+};
+
+impl MultiColorSubLed {
+ /// Create a new multicolor sub led info.
+ pub const fn new(color: Color) -> Self {
+ Self {
+ color,
+ brightness: 0,
+ intensity: 0,
+ channel: 0,
+ }
+ }
+
+ /// Set arbitrary data for the driver.
+ pub const fn channel(mut self, channel: u32) -> Self {
+ self.channel = channel;
+ self
+ }
+
+ /// Set the initial intensity of the subled.
+ pub const fn initial_intensity(mut self, intensity: u32) -> Self {
+ self.intensity = intensity;
+ self
+ }
+}
+
+/// The multicolor led class device representation.
+///
+/// This structure represents the Rust abstraction for a multicolor led class device.
+#[pin_data(PinnedDrop)]
+pub struct MultiColorDevice<T: LedOps<Mode = MultiColor>> {
+ #[pin]
+ ops: T,
+ #[pin]
+ classdev: Opaque<bindings::led_classdev_mc>,
+}
+
+impl<'a, S: DeviceBuilderState> DeviceBuilder<'a, S> {
+ /// Registers a new [`MulticolorDevice`].
+ pub fn build_multicolor<T: LedOps<Mode = MultiColor>>(
+ self,
+ parent: &'a T::Bus,
+ ops: impl PinInit<T, Error> + 'a,
+ subleds: &'a [MultiColorSubLed],
+ ) -> impl PinInit<Devres<MultiColorDevice<T>>, Error> + 'a {
+ Devres::new(
+ parent.as_ref(),
+ try_pin_init!(MultiColorDevice {
+ ops <- ops,
+ classdev <- Opaque::try_ffi_init(|ptr: *mut bindings::led_classdev_mc| {
+ let mut used = 0;
+ if subleds.iter().any(|subled| {
+ let bit = 1 << (subled.color as u32);
+ if (used & bit) != 0 {
+ true
+ } else {
+ used |= bit;
+ false
+ }
+ }) {
+ dev_err!(parent.as_ref(), "duplicate color in multicolor led\n");
+ return Err(EINVAL);
+ }
+ let mut subleds_vec = KVec::new();
+ subleds_vec.extend_from_slice(subleds, GFP_KERNEL)?;
+ let (subled_info, num_colors, capacity) = subleds_vec.into_raw_parts();
+ debug_assert_eq!(num_colors, capacity);
+
+ // SAFETY: `try_ffi_init` guarantees that `ptr` is valid for write.
+ // `led_classdev_mc` gets fully initialized in-place by
+ // `led_classdev_multicolor_register_ext` including `mutex` and `list_head`.
+ unsafe {
+ ptr.write(bindings::led_classdev_mc {
+ led_cdev: bindings::led_classdev {
+ brightness_set: (!T::BLOCKING)
+ .then_some(Adapter::<T>::brightness_set_callback),
+ brightness_set_blocking: T::BLOCKING
+ .then_some(Adapter::<T>::brightness_set_blocking_callback),
+ brightness_get: T::HAS_BRIGHTNESS_GET
+ .then_some(Adapter::<T>::brightness_get_callback),
+ blink_set: T::HAS_BLINK_SET
+ .then_some(Adapter::<T>::blink_set_callback),
+ max_brightness: T::MAX_BRIGHTNESS,
+ brightness: self.initial_brightness,
+ default_trigger: self
+ .default_trigger
+ .map_or(core::ptr::null(), CStrExt::as_char_ptr),
+ color: self.color as u32,
+ name: self.name.map_or(core::ptr::null(), CStrExt::as_char_ptr),
+ ..bindings::led_classdev::default()
+ },
+ num_colors: u32::try_from(num_colors)?,
+ // CAST: The safeguards in the const block ensure that
+ // `MultiColorSubLed` has an identical layout to `mc_subled`.
+ subled_info: subled_info.cast::<bindings::mc_subled>(),
+ })
+ };
+
+ 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_mc`.
+ to_result(unsafe {
+ bindings::led_classdev_multicolor_register_ext(
+ parent.as_ref().as_raw(),
+ ptr,
+ if self.name.is_none() {
+ &raw mut init_data
+ } else {
+ core::ptr::null_mut()
+ },
+ )
+ })
+ .inspect_err(|_err| {
+ // SAFETY: `subled_info` is guaranteed to be a valid array pointer to
+ // `mc_subled` with the length and capacity of `num_colors`.
+ drop(unsafe { KVec::from_raw_parts(subled_info, num_colors, num_colors) });
+ })?;
+
+ core::mem::forget(self.fwnode); // keep the reference count incremented
+
+ Ok::<_, Error>(())
+ }),
+ }),
+ )
+ }
+}
+
+impl<T: LedOps<Mode = MultiColor>> MultiColorDevice<T> {
+ /// # Safety
+ /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+ /// `led::MultiColorDevice`.
+ unsafe fn from_raw<'a>(led_cdev: *mut bindings::led_classdev) -> &'a Self {
+ // SAFETY: The function's contract guarantees that `led_cdev` points to a `led_classdev`
+ // field embedded within a valid `led::MultiColorDevice`. `container_of!` can therefore
+ // safely calculate the address of the containing struct.
+ let led_mc_cdev = unsafe { container_of!(led_cdev, bindings::led_classdev_mc, led_cdev) };
+
+ // SAFETY: It is guaranteed that `led_mc_cdev` points to a `led_classdev_mc`
+ // field embedded within a valid `led::MultiColorDevice`. `container_of!` can therefore
+ // safely calculate the address of the containing struct.
+ unsafe { &*container_of!(Opaque::cast_from(led_mc_cdev), Self, classdev) }
+ }
+
+ fn parent(&self) -> &device::Device<Bound> {
+ // SAFETY: `self.classdev.get()` is guaranteed to be a valid pointer to `led_classdev_mc`.
+ unsafe { device::Device::from_raw((*(*self.classdev.get()).led_cdev.dev).parent) }
+ }
+
+ /// Returns the subleds passed to [`Device::new_multicolor`].
+ pub fn subleds(&self) -> &[MultiColorSubLed] {
+ // SAFETY: The existence of `self` guarantees that `self.classdev.get()` is a pointer to a
+ // valid `led_classdev_mc`.
+ let raw = unsafe { &*self.classdev.get() };
+ // SAFETY: `raw.subled_info` is a valid pointer to `mc_subled[num_colors]`.
+ // CAST: The safeguards in the const block ensure that `MultiColorSubLed` has an identical
+ // layout to `mc_subled`.
+ unsafe {
+ core::slice::from_raw_parts(
+ raw.subled_info.cast::<MultiColorSubLed>(),
+ raw.num_colors as usize,
+ )
+ }
+ }
+}
+
+// SAFETY: A `led::MultiColorDevice` can be unregistered from any thread.
+unsafe impl<T: LedOps<Mode = MultiColor> + Send> Send for MultiColorDevice<T> {}
+
+// SAFETY: `led::MultiColorDevice` can be shared among threads because all methods of `led::Device`
+// are thread safe.
+unsafe impl<T: LedOps<Mode = MultiColor> + Sync> Sync for MultiColorDevice<T> {}
+
+struct Adapter<T: LedOps<Mode = MultiColor>> {
+ _p: PhantomData<T>,
+}
+
+impl<T: LedOps<Mode = MultiColor>> Adapter<T> {
+ /// # Safety
+ /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+ /// `led::MultiColorDevice`.
+ /// This function is called on setting the brightness of a led.
+ unsafe extern "C" fn brightness_set_callback(
+ led_cdev: *mut bindings::led_classdev,
+ brightness: u32,
+ ) {
+ // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
+ // `led_classdev` embedded within a `led::MultiColorDevice`.
+ let classdev = unsafe { MultiColorDevice::<T>::from_raw(led_cdev) };
+ // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
+ let parent = unsafe { T::Bus::from_device(classdev.parent()) };
+
+ // SAFETY: `classdev.classdev.get()` is guaranteed to be a pointer to a valid
+ // `led_classdev_mc`.
+ unsafe { bindings::led_mc_calc_color_components(classdev.classdev.get(), brightness) };
+
+ let _ = classdev.ops.brightness_set(parent, classdev, brightness);
+ }
+
+ /// # Safety
+ /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+ /// `led::MultiColorDevice`.
+ /// This function is called on setting the brightness of a led immediately.
+ unsafe extern "C" fn brightness_set_blocking_callback(
+ led_cdev: *mut bindings::led_classdev,
+ brightness: u32,
+ ) -> i32 {
+ from_result(|| {
+ // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
+ // `led_classdev` embedded within a `led::MultiColorDevice`.
+ let classdev = unsafe { MultiColorDevice::<T>::from_raw(led_cdev) };
+ // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
+ let parent = unsafe { T::Bus::from_device(classdev.parent()) };
+
+ // SAFETY: `classdev.classdev.get()` is guaranteed to be a pointer to a valid
+ // `led_classdev_mc`.
+ unsafe { bindings::led_mc_calc_color_components(classdev.classdev.get(), brightness) };
+
+ classdev.ops.brightness_set(parent, classdev, brightness)?;
+ Ok(0)
+ })
+ }
+
+ /// # Safety
+ /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+ /// `led::MultiColorDevice`.
+ /// This function is called on getting the brightness of a led.
+ unsafe extern "C" fn brightness_get_callback(led_cdev: *mut bindings::led_classdev) -> u32 {
+ // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
+ // `led_classdev` embedded within a `led::MultiColorDevice`.
+ let classdev = unsafe { MultiColorDevice::<T>::from_raw(led_cdev) };
+ // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
+ let parent = unsafe { T::Bus::from_device(classdev.parent()) };
+
+ classdev.ops.brightness_get(parent, classdev)
+ }
+
+ /// # Safety
+ /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+ /// `led::MultiColorDevice`.
+ /// `delay_on` and `delay_off` must be valid pointers to `usize` and have
+ /// exclusive access for the period of this function.
+ /// This function is called on enabling hardware accelerated blinking.
+ unsafe extern "C" fn blink_set_callback(
+ led_cdev: *mut bindings::led_classdev,
+ delay_on: *mut usize,
+ delay_off: *mut usize,
+ ) -> i32 {
+ from_result(|| {
+ // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
+ // `led_classdev` embedded within a `led::MultiColorDevice`.
+ let classdev = unsafe { MultiColorDevice::<T>::from_raw(led_cdev) };
+ // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
+ let parent = unsafe { T::Bus::from_device(classdev.parent()) };
+
+ classdev.ops.blink_set(
+ parent,
+ classdev,
+ // SAFETY: The function's contract guarantees that `delay_on` points to a `usize`
+ // and is exclusive for the period of this function.
+ unsafe { &mut *delay_on },
+ // SAFETY: The function's contract guarantees that `delay_off` points to a `usize`
+ // and is exclusive for the period of this function.
+ unsafe { &mut *delay_off },
+ )?;
+ Ok(0)
+ })
+ }
+}
+
+#[pinned_drop]
+impl<T: LedOps<Mode = MultiColor>> PinnedDrop for MultiColorDevice<T> {
+ fn drop(self: Pin<&mut Self>) {
+ let raw = self.classdev.get();
+ // SAFETY: The existence of `self` guarantees that `self.classdev.get()` is a pointer to a
+ // valid `led_classdev_mc`.
+ let dev: &device::Device = unsafe { device::Device::from_raw((*raw).led_cdev.dev) };
+
+ let _fwnode = dev
+ .fwnode()
+ // SAFETY: the reference count of `fwnode` has previously been
+ // incremented in `led::Device::new`.
+ .map(|fwnode| unsafe { ARef::from_raw(NonNull::from(fwnode)) });
+
+ // SAFETY: The existence of `self` guarantees that `self.classdev` has previously been
+ // successfully registered with `led_classdev_multicolor_register_ext`.
+ unsafe { bindings::led_classdev_multicolor_unregister(raw) };
+
+ // SAFETY: `raw` is guaranteed to be a valid pointer to `led_classdev_mc`.
+ let led_cdev = unsafe { &*raw };
+
+ // SAFETY: `subled_info` is guaranteed to be a valid array pointer to `mc_subled` with the
+ // length and capacity of `led_cdev.num_colors`. See `led::MulticolorDevice::new`.
+ drop(unsafe {
+ KVec::from_raw_parts(
+ led_cdev.subled_info,
+ led_cdev.num_colors as usize,
+ led_cdev.num_colors as usize,
+ )
+ });
+ }
+}
--
2.52.0
^ permalink raw reply related
* [PATCH RESEND v13 2/3] rust: leds: add Mode trait
From: Markus Probst @ 2026-04-11 15:07 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rafael J. Wysocki, Bjorn Helgaas,
Krzysztof Wilczyński, Boqun Feng, Boqun Feng
Cc: rust-for-linux, linux-leds, linux-kernel, linux-pci,
Markus Probst
In-Reply-To: <20260411-rust_leds-v13-0-1208a2821deb@posteo.de>
Add the `led::Mode` trait to allow for other types of led class devices
in `led::LedOps`.
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
rust/kernel/led.rs | 28 ++++++++++++++++++++++++----
rust/kernel/led/normal.rs | 24 ++++++++++++++++--------
2 files changed, 40 insertions(+), 12 deletions(-)
diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
index 1fba512a804c..5035563d68a3 100644
--- a/rust/kernel/led.rs
+++ b/rust/kernel/led.rs
@@ -35,7 +35,7 @@
mod normal;
-pub use normal::Device;
+pub use normal::{Device, Normal};
/// The name of the led is determined by the driver.
pub enum Named {}
@@ -177,6 +177,7 @@ pub fn name(self, name: &'a CStr) -> Self {
/// #[vtable]
/// impl led::LedOps for MyLedOps {
/// type Bus = platform::Device<device::Bound>;
+/// type Mode = led::Normal;
/// const BLOCKING: bool = false;
/// const MAX_BRIGHTNESS: u32 = 255;
///
@@ -209,6 +210,11 @@ pub trait LedOps: Send + 'static + Sized {
#[allow(private_bounds)]
type Bus: AsBusDevice<Bound>;
+ /// The led mode to use.
+ ///
+ /// See [`Mode`].
+ type Mode: Mode;
+
/// If set true, [`LedOps::brightness_set`] and [`LedOps::blink_set`] must perform the
/// operation immediately. If set false, they must not sleep.
const BLOCKING: bool;
@@ -221,12 +227,16 @@ pub trait LedOps: Send + 'static + Sized {
fn brightness_set(
&self,
dev: &Self::Bus,
- classdev: &Device<Self>,
+ classdev: &<Self::Mode as Mode>::Device<Self>,
brightness: u32,
) -> Result<()>;
/// Gets the current brightness level.
- fn brightness_get(&self, dev: &Self::Bus, classdev: &Device<Self>) -> u32 {
+ fn brightness_get(
+ &self,
+ dev: &Self::Bus,
+ classdev: &<Self::Mode as Mode>::Device<Self>,
+ ) -> u32 {
let _ = (dev, classdev);
build_error!(VTABLE_DEFAULT_ERROR)
}
@@ -242,7 +252,7 @@ fn brightness_get(&self, dev: &Self::Bus, classdev: &Device<Self>) -> u32 {
fn blink_set(
&self,
dev: &Self::Bus,
- classdev: &Device<Self>,
+ classdev: &<Self::Mode as Mode>::Device<Self>,
delay_on: &mut usize,
delay_off: &mut usize,
) -> Result<()> {
@@ -305,6 +315,16 @@ fn try_from(value: u32) -> core::result::Result<Self, Self::Error> {
}
}
+/// The led mode.
+///
+/// Each led mode has its own led class device type with different capabilities.
+///
+/// See [`Normal`].
+pub trait Mode: private::Sealed {
+ /// The class device for the led mode.
+ type Device<T: LedOps<Mode = Self>>;
+}
+
mod private {
pub trait Sealed {}
}
diff --git a/rust/kernel/led/normal.rs b/rust/kernel/led/normal.rs
index bd239f186c64..dda247145f25 100644
--- a/rust/kernel/led/normal.rs
+++ b/rust/kernel/led/normal.rs
@@ -6,11 +6,19 @@
use super::*;
+/// The led mode for the `struct led_classdev`. Leds with this mode can only have a fixed color.
+pub enum Normal {}
+
+impl Mode for Normal {
+ type Device<T: LedOps<Mode = Self>> = Device<T>;
+}
+impl private::Sealed for Normal {}
+
/// The led class device representation.
///
/// This structure represents the Rust abstraction for a led class device.
#[pin_data(PinnedDrop)]
-pub struct Device<T: LedOps> {
+pub struct Device<T: LedOps<Mode = Normal>> {
#[pin]
ops: T,
#[pin]
@@ -19,7 +27,7 @@ pub struct Device<T: LedOps> {
impl<'a, S: DeviceBuilderState> DeviceBuilder<'a, S> {
/// Registers a new [`Device`].
- pub fn build<T: LedOps>(
+ pub fn build<T: LedOps<Mode = Normal>>(
self,
parent: &'a T::Bus,
ops: impl PinInit<T, Error> + 'a,
@@ -89,7 +97,7 @@ pub fn build<T: LedOps>(
}
}
-impl<T: LedOps> Device<T> {
+impl<T: LedOps<Mode = Normal>> Device<T> {
/// # Safety
/// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
/// `led::Device`.
@@ -107,17 +115,17 @@ fn parent(&self) -> &device::Device<Bound> {
}
// SAFETY: A `led::Device` can be unregistered from any thread.
-unsafe impl<T: LedOps + Send> Send for Device<T> {}
+unsafe impl<T: LedOps<Mode = Normal> + Send> Send for Device<T> {}
// SAFETY: `led::Device` can be shared among threads because all methods of `led::Device`
// are thread safe.
-unsafe impl<T: LedOps + Sync> Sync for Device<T> {}
+unsafe impl<T: LedOps<Mode = Normal> + Sync> Sync for Device<T> {}
-struct Adapter<T: LedOps> {
+struct Adapter<T: LedOps<Mode = Normal>> {
_p: PhantomData<T>,
}
-impl<T: LedOps> Adapter<T> {
+impl<T: LedOps<Mode = Normal>> Adapter<T> {
/// # Safety
/// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
/// `led::Device`.
@@ -203,7 +211,7 @@ impl<T: LedOps> Adapter<T> {
}
#[pinned_drop]
-impl<T: LedOps> PinnedDrop for Device<T> {
+impl<T: LedOps<Mode = Normal>> PinnedDrop for Device<T> {
fn drop(self: Pin<&mut Self>) {
let raw = self.classdev.get();
// SAFETY: The existence of `self` guarantees that `self.classdev.get()` is a pointer to a
--
2.52.0
^ permalink raw reply related
* [PATCH RESEND v13 1/3] rust: leds: add basic led classdev abstractions
From: Markus Probst @ 2026-04-11 15:07 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rafael J. Wysocki, Bjorn Helgaas,
Krzysztof Wilczyński, Boqun Feng, Boqun Feng
Cc: rust-for-linux, linux-leds, linux-kernel, linux-pci,
Markus Probst
In-Reply-To: <20260411-rust_leds-v13-0-1208a2821deb@posteo.de>
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 <markus.probst@posteo.de>
---
MAINTAINERS | 8 ++
rust/kernel/led.rs | 310 ++++++++++++++++++++++++++++++++++++++++++++++
rust/kernel/led/normal.rs | 223 +++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 1 +
4 files changed, 542 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 7d10988cbc62..83b5a45de729 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14439,6 +14439,14 @@ F: drivers/leds/
F: include/dt-bindings/leds/
F: include/linux/leds.h
+LED SUBSYSTEM [RUST]
+M: Markus Probst <markus.probst@posteo.de>
+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 <david@lechnology.com>
S: Maintained
diff --git a/rust/kernel/led.rs b/rust/kernel/led.rs
new file mode 100644
index 000000000000..1fba512a804c
--- /dev/null
+++ b/rust/kernel/led.rs
@@ -0,0 +1,310 @@
+// 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, //
+ },
+ devres::Devres,
+ error::{
+ from_result,
+ to_result,
+ VTABLE_DEFAULT_ERROR, //
+ },
+ macros::vtable,
+ prelude::*,
+ str::CStrExt,
+ types::{
+ ARef,
+ Opaque, //
+ }, //
+};
+
+mod normal;
+
+pub use normal::Device;
+
+/// The name of the led is determined by the driver.
+pub enum Named {}
+/// The name of the led is determined by its fwnode.
+pub enum Unnamed {}
+
+/// How the name of the led should be determined.
+pub trait DeviceBuilderState: private::Sealed {}
+
+impl DeviceBuilderState for Named {}
+impl private::Sealed for Named {}
+impl DeviceBuilderState for Unnamed {}
+impl private::Sealed for Unnamed {}
+
+/// The builder to register a led class device.
+///
+/// See [`LedOps`].
+pub struct DeviceBuilder<'a, S> {
+ fwnode: Option<ARef<FwNode>>,
+ name: Option<&'a CStr>,
+ devicename: Option<&'a CStr>,
+ devname_mandatory: bool,
+ initial_brightness: u32,
+ default_trigger: Option<&'a CStr>,
+ color: Color,
+ _p: PhantomData<S>,
+}
+
+impl<S: DeviceBuilderState> DeviceBuilder<'static, S> {
+ /// Creates a new [`DeviceBuilder`].
+ #[inline]
+ #[expect(
+ clippy::new_without_default,
+ reason = "no need and derive is prevented by S"
+ )]
+ pub fn new() -> Self {
+ Self {
+ fwnode: None,
+ name: None,
+ devicename: None,
+ devname_mandatory: false,
+ initial_brightness: 0,
+ default_trigger: None,
+ color: Color::default(),
+ _p: PhantomData,
+ }
+ }
+}
+
+impl<'a> DeviceBuilder<'a, Unnamed> {
+ /// Sets the firmware node.
+ #[inline]
+ pub fn fwnode(self, fwnode: Option<ARef<FwNode>>) -> Self {
+ Self { fwnode, ..self }
+ }
+
+ /// Sets the device name.
+ #[inline]
+ pub fn devicename(self, devicename: &'a CStr) -> Self {
+ Self {
+ devicename: Some(devicename),
+ ..self
+ }
+ }
+
+ /// Sets if a device name is mandatory.
+ #[inline]
+ pub fn devicename_mandatory(self, mandatory: bool) -> Self {
+ Self {
+ devname_mandatory: mandatory,
+ ..self
+ }
+ }
+}
+
+impl<'a, S: DeviceBuilderState> DeviceBuilder<'a, S> {
+ /// Sets the initial brightness value for the led.
+ ///
+ /// The default brightness is 0.
+ /// If [`LedOps::brightness_get`] is implemented, this value will be ignored.
+ #[inline]
+ pub fn initial_brightness(self, brightness: u32) -> Self {
+ Self {
+ initial_brightness: brightness,
+ ..self
+ }
+ }
+
+ /// Set the default led trigger.
+ ///
+ /// This value can be overwritten by the "linux,default-trigger" fwnode property.
+ #[inline]
+ pub fn default_trigger(self, trigger: &'a CStr) -> Self {
+ Self {
+ default_trigger: Some(trigger),
+ ..self
+ }
+ }
+
+ /// Sets the color of the led.
+ ///
+ /// This value can be overwritten by the "color" fwnode property.
+ #[inline]
+ pub fn color(self, color: Color) -> Self {
+ Self { color, ..self }
+ }
+}
+
+impl<'a> DeviceBuilder<'a, Named> {
+ /// Sets the name of the led.
+ ///
+ /// Setting this will prevent the fwnode from being used and prevents automatic name
+ /// composition.
+ #[inline]
+ pub fn name(self, name: &'a CStr) -> Self {
+ Self {
+ name: Some(name),
+ ..self
+ }
+ }
+}
+
+/// Trait defining the operations for a LED driver.
+///
+/// # Examples
+/// ```
+/// use kernel::{
+/// device,
+/// devres::Devres,
+/// led,
+/// macros::vtable,
+/// platform,
+/// prelude::*, //
+/// };
+///
+/// struct MyLedOps;
+///
+///
+/// #[vtable]
+/// impl led::LedOps for MyLedOps {
+/// type Bus = platform::Device<device::Bound>;
+/// const BLOCKING: bool = false;
+/// const MAX_BRIGHTNESS: u32 = 255;
+///
+/// fn brightness_set(
+/// &self,
+/// _dev: &platform::Device<device::Bound>,
+/// _classdev: &led::Device<Self>,
+/// _brightness: u32
+/// ) -> Result<()> {
+/// // Set the brightness for the led here
+/// Ok(())
+/// }
+/// }
+///
+/// fn register_my_led(
+/// parent: &platform::Device<device::Bound>,
+/// ) -> Result<Pin<KBox<Devres<led::Device<MyLedOps>>>>> {
+/// KBox::pin_init(led::DeviceBuilder::new()
+/// .name(c"white:test")
+/// .build(
+/// parent,
+/// Ok(MyLedOps),
+/// ), GFP_KERNEL)
+/// }
+/// ```
+/// Led drivers must implement this trait in order to register and handle a [`Device`].
+#[vtable]
+pub trait LedOps: Send + 'static + Sized {
+ /// The bus device required by the implementation.
+ #[allow(private_bounds)]
+ type Bus: AsBusDevice<Bound>;
+
+ /// If set true, [`LedOps::brightness_set`] and [`LedOps::blink_set`] must perform the
+ /// operation immediately. If set false, they must not sleep.
+ const BLOCKING: bool;
+ /// The max brightness level.
+ const MAX_BRIGHTNESS: u32;
+
+ /// Sets the brightness level.
+ ///
+ /// See also [`LedOps::BLOCKING`].
+ fn brightness_set(
+ &self,
+ dev: &Self::Bus,
+ classdev: &Device<Self>,
+ brightness: u32,
+ ) -> Result<()>;
+
+ /// Gets the current brightness level.
+ fn brightness_get(&self, dev: &Self::Bus, classdev: &Device<Self>) -> u32 {
+ let _ = (dev, classdev);
+ 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,
+ dev: &Self::Bus,
+ classdev: &Device<Self>,
+ delay_on: &mut usize,
+ delay_off: &mut usize,
+ ) -> Result<()> {
+ let _ = (dev, classdev, delay_on, delay_off);
+ build_error!(VTABLE_DEFAULT_ERROR)
+ }
+}
+
+/// Led colors.
+#[derive(Copy, Clone, Debug, Default)]
+#[repr(u32)]
+#[non_exhaustive]
+#[expect(
+ missing_docs,
+ reason = "it shouldn't be necessary to document each color"
+)]
+pub enum Color {
+ #[default]
+ White = bindings::LED_COLOR_ID_WHITE,
+ Red = bindings::LED_COLOR_ID_RED,
+ Green = bindings::LED_COLOR_ID_GREEN,
+ Blue = bindings::LED_COLOR_ID_BLUE,
+ Amber = bindings::LED_COLOR_ID_AMBER,
+ Violet = bindings::LED_COLOR_ID_VIOLET,
+ Yellow = bindings::LED_COLOR_ID_YELLOW,
+ Ir = bindings::LED_COLOR_ID_IR,
+ Multi = bindings::LED_COLOR_ID_MULTI,
+ Rgb = bindings::LED_COLOR_ID_RGB,
+ Purple = bindings::LED_COLOR_ID_PURPLE,
+ Orange = bindings::LED_COLOR_ID_ORANGE,
+ Pink = bindings::LED_COLOR_ID_PINK,
+ Cyan = bindings::LED_COLOR_ID_CYAN,
+ Lime = bindings::LED_COLOR_ID_LIME,
+}
+static_assert!(bindings::LED_COLOR_ID_MAX == 15);
+
+impl Color {
+ /// Name of the color.
+ pub fn as_c_str(self) -> &'static CStr {
+ // SAFETY:
+ // - `self as u8` is a valid led color id.
+ // - `led_get_color_name` always returns a valid C string pointer.
+ unsafe { CStr::from_char_ptr(bindings::led_get_color_name(self as u8)) }
+ }
+}
+
+impl TryFrom<u32> for Color {
+ type Error = Error;
+
+ fn try_from(value: u32) -> core::result::Result<Self, Self::Error> {
+ if value < bindings::LED_COLOR_ID_MAX {
+ // SAFETY:
+ // - `Color` is represented as `u32`
+ // - the static_assert above guarantees that no additional color has been added
+ // - `value` is guaranteed to be in the color id range
+ Ok(unsafe { transmute::<u32, Color>(value) })
+ } else {
+ Err(EINVAL)
+ }
+ }
+}
+
+mod private {
+ pub trait Sealed {}
+}
diff --git a/rust/kernel/led/normal.rs b/rust/kernel/led/normal.rs
new file mode 100644
index 000000000000..bd239f186c64
--- /dev/null
+++ b/rust/kernel/led/normal.rs
@@ -0,0 +1,223 @@
+// 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<T: LedOps> {
+ #[pin]
+ ops: T,
+ #[pin]
+ classdev: Opaque<bindings::led_classdev>,
+}
+
+impl<'a, S: DeviceBuilderState> DeviceBuilder<'a, S> {
+ /// Registers a new [`Device`].
+ pub fn build<T: LedOps>(
+ self,
+ parent: &'a T::Bus,
+ ops: impl PinInit<T, Error> + 'a,
+ ) -> impl PinInit<Devres<Device<T>>, Error> + 'a {
+ Devres::new(
+ parent.as_ref(),
+ 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::<T>::brightness_set_callback),
+ brightness_set_blocking: T::BLOCKING
+ .then_some(Adapter::<T>::brightness_set_blocking_callback),
+ brightness_get: T::HAS_BRIGHTNESS_GET
+ .then_some(Adapter::<T>::brightness_get_callback),
+ blink_set: T::HAS_BLINK_SET.then_some(Adapter::<T>::blink_set_callback),
+ max_brightness: T::MAX_BRIGHTNESS,
+ brightness: self.initial_brightness,
+ default_trigger: self
+ .default_trigger
+ .map_or(core::ptr::null(), CStrExt::as_char_ptr),
+ 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>(())
+ }),
+ }),
+ )
+ }
+}
+
+impl<T: LedOps> Device<T> {
+ /// # Safety
+ /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+ /// `led::Device`.
+ unsafe fn from_raw<'a>(led_cdev: *mut bindings::led_classdev) -> &'a Self {
+ // SAFETY: The function's contract guarantees that `led_cdev` points to a `led_classdev`
+ // field embedded within a valid `led::Device`. `container_of!` can therefore
+ // safely calculate the address of the containing struct.
+ unsafe { &*container_of!(Opaque::cast_from(led_cdev), Self, classdev) }
+ }
+
+ fn parent(&self) -> &device::Device<Bound> {
+ // SAFETY: `self.classdev.get()` is guaranteed to be a valid pointer to `led_classdev`.
+ unsafe { device::Device::from_raw((*(*self.classdev.get()).dev).parent) }
+ }
+}
+
+// SAFETY: A `led::Device` can be unregistered from any thread.
+unsafe impl<T: LedOps + Send> Send for Device<T> {}
+
+// SAFETY: `led::Device` can be shared among threads because all methods of `led::Device`
+// are thread safe.
+unsafe impl<T: LedOps + Sync> Sync for Device<T> {}
+
+struct Adapter<T: LedOps> {
+ _p: PhantomData<T>,
+}
+
+impl<T: LedOps> Adapter<T> {
+ /// # Safety
+ /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+ /// `led::Device`.
+ /// This function is called on setting the brightness of a led.
+ unsafe extern "C" fn brightness_set_callback(
+ led_cdev: *mut bindings::led_classdev,
+ brightness: u32,
+ ) {
+ // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
+ // `led_classdev` embedded within a `led::Device`.
+ let classdev = unsafe { Device::<T>::from_raw(led_cdev) };
+ // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
+ let parent = unsafe { T::Bus::from_device(classdev.parent()) };
+
+ let _ = classdev.ops.brightness_set(parent, classdev, brightness);
+ }
+
+ /// # Safety
+ /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+ /// `led::Device`.
+ /// This function is called on setting the brightness of a led immediately.
+ unsafe extern "C" fn brightness_set_blocking_callback(
+ led_cdev: *mut bindings::led_classdev,
+ brightness: u32,
+ ) -> i32 {
+ from_result(|| {
+ // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
+ // `led_classdev` embedded within a `led::Device`.
+ let classdev = unsafe { Device::<T>::from_raw(led_cdev) };
+ // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
+ let parent = unsafe { T::Bus::from_device(classdev.parent()) };
+
+ classdev.ops.brightness_set(parent, classdev, brightness)?;
+ Ok(0)
+ })
+ }
+
+ /// # Safety
+ /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+ /// `led::Device`.
+ /// This function is called on getting the brightness of a led.
+ unsafe extern "C" fn brightness_get_callback(led_cdev: *mut bindings::led_classdev) -> u32 {
+ // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
+ // `led_classdev` embedded within a `led::Device`.
+ let classdev = unsafe { Device::<T>::from_raw(led_cdev) };
+ // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
+ let parent = unsafe { T::Bus::from_device(classdev.parent()) };
+
+ classdev.ops.brightness_get(parent, classdev)
+ }
+
+ /// # Safety
+ /// `led_cdev` must be a valid pointer to a `led_classdev` embedded within a
+ /// `led::Device`.
+ /// `delay_on` and `delay_off` must be valid pointers to `usize` and have
+ /// exclusive access for the period of this function.
+ /// This function is called on enabling hardware accelerated blinking.
+ unsafe extern "C" fn blink_set_callback(
+ led_cdev: *mut bindings::led_classdev,
+ delay_on: *mut usize,
+ delay_off: *mut usize,
+ ) -> i32 {
+ from_result(|| {
+ // SAFETY: The function's contract guarantees that `led_cdev` is a valid pointer to a
+ // `led_classdev` embedded within a `led::Device`.
+ let classdev = unsafe { Device::<T>::from_raw(led_cdev) };
+ // SAFETY: `classdev.parent()` is guaranteed to be contained in `T::Bus`.
+ let parent = unsafe { T::Bus::from_device(classdev.parent()) };
+
+ classdev.ops.blink_set(
+ parent,
+ classdev,
+ // SAFETY: The function's contract guarantees that `delay_on` points to a `usize`
+ // and is exclusive for the period of this function.
+ unsafe { &mut *delay_on },
+ // SAFETY: The function's contract guarantees that `delay_off` points to a `usize`
+ // and is exclusive for the period of this function.
+ unsafe { &mut *delay_off },
+ )?;
+ Ok(0)
+ })
+ }
+}
+
+#[pinned_drop]
+impl<T: LedOps> PinnedDrop for Device<T> {
+ fn drop(self: Pin<&mut Self>) {
+ let raw = self.classdev.get();
+ // SAFETY: The existence of `self` guarantees that `self.classdev.get()` is a pointer to a
+ // valid `led_classdev`.
+ let dev: &device::Device = unsafe { device::Device::from_raw((*raw).dev) };
+
+ let _fwnode = dev
+ .fwnode()
+ // SAFETY: the reference count of `fwnode` has previously been
+ // incremented in `led::Device::new`.
+ .map(|fwnode| unsafe { ARef::from_raw(NonNull::from(fwnode)) });
+
+ // SAFETY: The existence of `self` guarantees that `self.classdev` has previously been
+ // successfully registered with `led_classdev_register_ext`.
+ unsafe { bindings::led_classdev_unregister(raw) };
+ }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index d93292d47420..d0e30d3733dd 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -115,6 +115,7 @@
pub mod jump_label;
#[cfg(CONFIG_KUNIT)]
pub mod kunit;
+pub mod led;
pub mod list;
pub mod maple_tree;
pub mod miscdevice;
--
2.52.0
^ permalink raw reply related
* [PATCH RESEND v13 0/3] rust: leds: add led classdev abstractions
From: Markus Probst @ 2026-04-11 15:07 UTC (permalink / raw)
To: Lee Jones, Pavel Machek, Greg Kroah-Hartman, Dave Ertman,
Ira Weiny, Leon Romanovsky, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Danilo Krummrich, Rafael J. Wysocki, Bjorn Helgaas,
Krzysztof Wilczyński, Boqun Feng, Boqun Feng
Cc: rust-for-linux, linux-leds, linux-kernel, linux-pci,
Markus Probst
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 led classdev abstractions to register and unregister leds
* add basic led classdev abstractions to register and unregister
multicolor leds
Changes since v12:
* add `led::DeviceBuilder::name()` and `DeviceBuilderState'
* add `led::Color::as_c_str`
Changes since v11:
* use `led::DeviceBuilder` instead of `led::InitData`
* use static_assert instead of const { assert!(...) }
* restructured patches to avoid moving `led::Device` from
rust/kernel/led.rs to rust/kernel/led/normal.rs in the 2. patch
Changes since v10:
* allow in-place initialization of `LedOps`
* run rustfmt for code inside `try_pin_init!`
Changes since v9:
* add missing periods in documentation
* duplicate `led::Device` and `led::Adapter` instead of using a complex
trait
* fix imports not using prelude
* adapt to CStr change
* documented `led::Color::Multi` and `led::Color::Rgb`
Changes since v8:
* accept `Option<ARef<Fwnode>>` in `led::InitData::fwnode()`
* make functions in `MultiColorSubLed` const
* drop the "rust: Add trait to convert a device reference to a bus
device reference" patch, as it has been picked into driver-core
Changes since v7:
* adjusted import style
* added classdev parameter to callback functions in `LedOps`
* implement `led::Color`
* extend `led::InitData` with
- initial_brightness
- default_trigger
- default_color
* split generic and normal led classdev abstractions up (see patch 3/4)
* add multicolor led class device abstractions (see patch 4/4)
* added MAINTAINERS entry
Changes since v6:
* fixed typos
* improved documentation
Changes since v5:
* rename `IntoBusDevice` trait into `AsBusDevice`
* fix documentation about `LedOps::BLOCKING`
* removed dependency on i2c bindings
* added `AsBusDevice` implementation for `platform::Device`
* removed `device::Device` fallback implementation
* document that `AsBusDevice` must not be used by drivers and is
intended for bus and class device abstractions only.
Changes since v4:
* add abstraction to convert a device reference to a bus device
reference
* require the bus device as parent device and provide it in class device
callbacks
* remove Pin<Vec<_>> abstraction (as not relevant for the led
abstractions)
* fixed formatting in `led::Device::new`
* fixed `LedOps::BLOCKING` did the inverse effect
Changes since v3:
* fixed kunit tests failing because of example in documentation
Changes since v2:
* return `Devres` on `led::Device` creation
* replace KBox<T> with T in struct definition
* increment and decrement reference-count of fwnode
* make a device parent mandatory for led classdev creation
* rename `led::Handler` to `led::LedOps`
* add optional `brightness_get` function to `led::LedOps`
* use `#[vtable]` instead of `const BLINK: bool`
* use `Opaque::cast_from` instead of casting a pointer
* improve documentation
* improve support for older rust versions
* use `&Device<Bound>` for parent
Changes since v1:
* fixed typos noticed by Onur Özkan
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
Markus Probst (3):
rust: leds: add basic led classdev abstractions
rust: leds: add Mode trait
rust: leds: add multicolor classdev abstractions
MAINTAINERS | 8 +
rust/bindings/bindings_helper.h | 1 +
rust/kernel/led.rs | 358 +++++++++++++++++++++++++++++++++++++
rust/kernel/led/multicolor.rs | 387 ++++++++++++++++++++++++++++++++++++++++
rust/kernel/led/normal.rs | 231 ++++++++++++++++++++++++
rust/kernel/lib.rs | 1 +
6 files changed, 986 insertions(+)
---
base-commit: d1d81e9d1a4dd846aee9ae77ff9ecc2800d72148
change-id: 20251114-rust_leds-a959f7c2f7f9
-----BEGIN PGP SIGNATURE-----
iQJPBAABCAA5FiEEgnQYxPSsWOdyMMRzNHYf+OetQ9IFAmnJX+IbFIAAAAAABAAO
bWFudTIsMi41KzEuMTEsMiwyAAoJEDR2H/jnrUPSKfEQAIr+nl9WCSty+mWQlJqB
D9B4+xLku25rFvvSCv/j7vEtL0VF5Rk5GJKFOhEEYg642WR6Os+VuTNnEOVAWTbK
qNfDNOP9hrn75anyf1HEOduwdsW9sTafrfF8gq36y80q0zVNWqD2SbYJ1A+Ri2Hw
uvJATuDnxMyBL7lAIPhshPjDHjO9wO7qiW6wSVY0RydD21tQZJkag3vg9gUG0Od7
v5DtM57QNf5h/7GXmC0oZ07h3Ua5ZSFNFrHBDm7MAt7YU8TllQcD1sL7f7OhcFDh
LzsQYm+A+VNQ9bL4/SdBeKGug7bEFshJywowdp6noRNJPR1Y2lCy1i3D85PnlM+j
h/NuFg6nUb1NupcB4J+ibzx/eL+80ablZDAqq5OKiskvDxiiTrQw4kC1TGlh+3rL
tFqqynKTU40N31nXRPHTG3bIfZsb3GHvXxEkJ1s1ufKKhk9WZKhCDOuSdduYrZX7
50kR5ptLn3YysJr1JTtt7xgT0ToSaxbre8ZWEKLCG8j8t2ONFDdJSskci+1VOUm3
FCkRLKILcnqFrQv7vnIa2tK6Xlg6jwcGQaVDB6KUIImGp8FatM0R6qP0iKM535+L
qAeYagvMjTUgwZXLIuUhwHWVk+1V54VjLlmBx6xfJ0a3MlfmtgdalruWfRAItSCq
ZjXW80wg0v3k/fsCPU5J+hY1
=OlH0
-----END PGP SIGNATURE-----
--
Markus Probst <markus.probst@posteo.de>
^ permalink raw reply
* [lee-leds:for-leds-next-next] BUILD SUCCESS 770edd8e8e5bed961af2ca6ab397052046d1d774
From: kernel test robot @ 2026-04-11 5:37 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-leds
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git for-leds-next-next
branch HEAD: 770edd8e8e5bed961af2ca6ab397052046d1d774 leds: pca9532: Don't stop blinking for non-zero brightness
elapsed time: 731m
configs tested: 169
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc-15.2.0
alpha allyesconfig gcc-15.2.0
alpha defconfig gcc-15.2.0
arc allmodconfig clang-16
arc allnoconfig gcc-15.2.0
arc allyesconfig clang-23
arc defconfig gcc-15.2.0
arc randconfig-001-20260411 gcc-12.5.0
arc randconfig-002-20260411 gcc-12.5.0
arm allnoconfig gcc-15.2.0
arm allyesconfig clang-16
arm defconfig gcc-15.2.0
arm randconfig-001-20260411 gcc-12.5.0
arm randconfig-002-20260411 gcc-12.5.0
arm randconfig-003-20260411 gcc-12.5.0
arm randconfig-004-20260411 gcc-12.5.0
arm shmobile_defconfig gcc-15.2.0
arm64 allmodconfig clang-23
arm64 allnoconfig gcc-15.2.0
arm64 defconfig gcc-15.2.0
arm64 randconfig-001-20260411 clang-23
arm64 randconfig-002-20260411 clang-23
arm64 randconfig-003-20260411 clang-23
arm64 randconfig-004-20260411 clang-23
csky allmodconfig gcc-15.2.0
csky allnoconfig gcc-15.2.0
csky defconfig gcc-15.2.0
csky randconfig-001-20260411 clang-23
csky randconfig-002-20260411 clang-23
hexagon allmodconfig gcc-15.2.0
hexagon allnoconfig gcc-15.2.0
hexagon defconfig gcc-15.2.0
hexagon randconfig-001-20260411 gcc-14.3.0
hexagon randconfig-002-20260411 gcc-14.3.0
i386 allmodconfig clang-20
i386 allnoconfig gcc-15.2.0
i386 allyesconfig clang-20
i386 buildonly-randconfig-001-20260411 gcc-14
i386 buildonly-randconfig-002-20260411 gcc-14
i386 buildonly-randconfig-003-20260411 gcc-14
i386 buildonly-randconfig-004-20260411 gcc-14
i386 buildonly-randconfig-005-20260411 gcc-14
i386 buildonly-randconfig-006-20260411 gcc-14
i386 defconfig gcc-15.2.0
i386 randconfig-001-20260411 clang-20
i386 randconfig-002-20260411 clang-20
i386 randconfig-003-20260411 clang-20
i386 randconfig-004-20260411 clang-20
i386 randconfig-005-20260411 clang-20
i386 randconfig-006-20260411 clang-20
i386 randconfig-007-20260411 clang-20
i386 randconfig-011-20260411 clang-20
i386 randconfig-012-20260411 clang-20
i386 randconfig-013-20260411 clang-20
i386 randconfig-014-20260411 clang-20
i386 randconfig-015-20260411 clang-20
i386 randconfig-016-20260411 clang-20
i386 randconfig-017-20260411 clang-20
loongarch allmodconfig clang-23
loongarch allnoconfig gcc-15.2.0
loongarch defconfig clang-19
loongarch randconfig-001-20260411 gcc-14.3.0
loongarch randconfig-002-20260411 gcc-14.3.0
m68k allmodconfig gcc-15.2.0
m68k allnoconfig gcc-15.2.0
m68k allyesconfig clang-16
m68k defconfig clang-19
microblaze allnoconfig gcc-15.2.0
microblaze allyesconfig gcc-15.2.0
microblaze defconfig clang-19
mips allmodconfig gcc-15.2.0
mips allnoconfig gcc-15.2.0
mips allyesconfig gcc-15.2.0
nios2 allmodconfig clang-23
nios2 allnoconfig clang-23
nios2 defconfig clang-19
nios2 randconfig-001-20260411 gcc-14.3.0
nios2 randconfig-002-20260411 gcc-14.3.0
openrisc allmodconfig clang-23
openrisc allnoconfig clang-23
openrisc defconfig gcc-15.2.0
parisc allmodconfig gcc-15.2.0
parisc allnoconfig clang-23
parisc allyesconfig clang-19
parisc defconfig gcc-15.2.0
parisc randconfig-001-20260411 gcc-11.5.0
parisc randconfig-002-20260411 gcc-11.5.0
parisc64 defconfig clang-19
powerpc allmodconfig gcc-15.2.0
powerpc allnoconfig clang-23
powerpc randconfig-001-20260411 gcc-11.5.0
powerpc randconfig-002-20260411 gcc-11.5.0
powerpc64 randconfig-001-20260411 gcc-11.5.0
powerpc64 randconfig-002-20260411 gcc-11.5.0
riscv allmodconfig clang-23
riscv allnoconfig clang-23
riscv allyesconfig clang-16
riscv defconfig gcc-15.2.0
riscv randconfig-001-20260411 gcc-10.5.0
riscv randconfig-002-20260411 gcc-10.5.0
s390 allmodconfig clang-19
s390 allnoconfig clang-23
s390 allyesconfig gcc-15.2.0
s390 defconfig gcc-15.2.0
s390 randconfig-001-20260411 gcc-10.5.0
s390 randconfig-002-20260411 gcc-10.5.0
sh allmodconfig gcc-15.2.0
sh allnoconfig clang-23
sh allyesconfig clang-19
sh defconfig gcc-14
sh randconfig-001-20260411 gcc-10.5.0
sh randconfig-002-20260411 gcc-10.5.0
sparc allnoconfig clang-23
sparc defconfig gcc-15.2.0
sparc randconfig-001-20260411 gcc-14
sparc randconfig-002-20260411 gcc-14
sparc64 allmodconfig clang-23
sparc64 defconfig gcc-14
sparc64 randconfig-001-20260411 gcc-14
sparc64 randconfig-002-20260411 gcc-14
um allmodconfig clang-19
um allnoconfig clang-23
um allyesconfig gcc-15.2.0
um defconfig gcc-14
um i386_defconfig gcc-14
um randconfig-001-20260411 gcc-14
um randconfig-002-20260411 gcc-14
um x86_64_defconfig gcc-14
x86_64 allmodconfig clang-20
x86_64 allnoconfig clang-23
x86_64 allyesconfig clang-20
x86_64 buildonly-randconfig-001-20260411 gcc-14
x86_64 buildonly-randconfig-002-20260411 gcc-14
x86_64 buildonly-randconfig-003-20260411 gcc-14
x86_64 buildonly-randconfig-004-20260411 gcc-14
x86_64 buildonly-randconfig-005-20260411 gcc-14
x86_64 buildonly-randconfig-006-20260411 gcc-14
x86_64 defconfig gcc-14
x86_64 kexec clang-20
x86_64 randconfig-001-20260411 gcc-14
x86_64 randconfig-002-20260411 gcc-14
x86_64 randconfig-003-20260411 gcc-14
x86_64 randconfig-004-20260411 gcc-14
x86_64 randconfig-005-20260411 gcc-14
x86_64 randconfig-006-20260411 gcc-14
x86_64 randconfig-011-20260411 clang-20
x86_64 randconfig-012-20260411 clang-20
x86_64 randconfig-013-20260411 clang-20
x86_64 randconfig-014-20260411 clang-20
x86_64 randconfig-015-20260411 clang-20
x86_64 randconfig-016-20260411 clang-20
x86_64 randconfig-071-20260411 clang-20
x86_64 randconfig-072-20260411 clang-20
x86_64 randconfig-073-20260411 clang-20
x86_64 randconfig-074-20260411 clang-20
x86_64 randconfig-075-20260411 clang-20
x86_64 randconfig-076-20260411 clang-20
x86_64 rhel-9.4 clang-20
x86_64 rhel-9.4-bpf gcc-14
x86_64 rhel-9.4-func clang-20
x86_64 rhel-9.4-kselftests clang-20
x86_64 rhel-9.4-kunit gcc-14
x86_64 rhel-9.4-ltp gcc-14
x86_64 rhel-9.4-rust clang-20
xtensa allnoconfig clang-23
xtensa allyesconfig clang-23
xtensa randconfig-001-20260411 gcc-14
xtensa randconfig-002-20260411 gcc-14
xtensa smp_lx200_defconfig gcc-15.2.0
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2] dt-binding: leds: publish common bindings under dual license
From: Gergo Koteles @ 2026-04-10 21:35 UTC (permalink / raw)
To: Corvin Köhne, linux-kernel
Cc: open list:LED SUBSYSTEM, Pavel Machek,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Corvin Köhne, Ashley Towns, Dan Murphy, INAGAKI Hiroshi,
Jacek Anaszewski, Olliver Schinagl, Pavel Machek,
Rafał Miłecki, Roderick Colenbrander,
Krzysztof Kozlowski
In-Reply-To: <20260408062942.7128-1-corvin.koehne@gmail.com>
On Wed, 2026-04-08 at 08:29 +0200, Corvin Köhne wrote:
> From: Corvin Köhne <c.koehne@beckhoff.com>
>
> Changes leds/common.h DT binding header file to be published under GPLv2
> or BSD-2-Clause license terms. This change allows this common LED
> bindings header file to be used in software components as bootloaders
> and OSes that are not published under GPLv2 terms.
>
> All contributors to leds/common.h file in copy.
>
> Cc: Ashley Towns <mail@ashleytowns.id.au>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Gergo Koteles <soyer@irl.hu>
> Cc: INAGAKI Hiroshi <musashino.open@gmail.com>
> Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Olliver Schinagl <oliver@schinagl.nl>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Rafał Miłecki <rafal@milecki.pl>
> Cc: Roderick Colenbrander <roderick@gaikai.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Corvin Köhne <c.koehne@beckhoff.com>
> ---
> include/dt-bindings/leds/common.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> index 4f017bea0123..b7bafbaf7df3 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -1,4 +1,4 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
> /*
> * This header provides macros for the common LEDs device tree bindings.
> *
I don't know if a single line change counts, but it's fine with me.
Acked-by: Gergo Koteles <soyer@irl.hu>
^ permalink raw reply
* Re: [PATCH v2] dt-binding: leds: publish common bindings under dual license
From: Jacek Anaszewski @ 2026-04-10 18:06 UTC (permalink / raw)
To: Corvin Köhne, linux-kernel
Cc: open list:LED SUBSYSTEM, Pavel Machek,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Corvin Köhne, Ashley Towns, Gergo Koteles, INAGAKI Hiroshi,
Olliver Schinagl, Pavel Machek, Rafał Miłecki,
Roderick Colenbrander, Krzysztof Kozlowski
In-Reply-To: <20260408062942.7128-1-corvin.koehne@gmail.com>
On 4/8/26 8:29 AM, Corvin Köhne wrote:
> From: Corvin Köhne <c.koehne@beckhoff.com>
>
> Changes leds/common.h DT binding header file to be published under GPLv2
> or BSD-2-Clause license terms. This change allows this common LED
> bindings header file to be used in software components as bootloaders
> and OSes that are not published under GPLv2 terms.
>
> All contributors to leds/common.h file in copy.
>
> Cc: Ashley Towns <mail@ashleytowns.id.au>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Gergo Koteles <soyer@irl.hu>
> Cc: INAGAKI Hiroshi <musashino.open@gmail.com>
> Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Olliver Schinagl <oliver@schinagl.nl>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Rafał Miłecki <rafal@milecki.pl>
> Cc: Roderick Colenbrander <roderick@gaikai.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Corvin Köhne <c.koehne@beckhoff.com>
> ---
> include/dt-bindings/leds/common.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/dt-bindings/leds/common.h b/include/dt-bindings/leds/common.h
> index 4f017bea0123..b7bafbaf7df3 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -1,4 +1,4 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
> /*
> * This header provides macros for the common LEDs device tree bindings.
> *
Acked-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH 04/61] ext4: Prefer IS_ERR_OR_NULL over manual NULL check
From: Theodore Ts'o @ 2026-04-10 15:18 UTC (permalink / raw)
To: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Philipp Hahn
Cc: Theodore Ts'o, Andreas Dilger
In-Reply-To: <20260310-b4-is_err_or_null-v1-4-bd63b656022d@avm.de>
On Tue, 10 Mar 2026 12:48:30 +0100, Philipp Hahn wrote:
> Prefer using IS_ERR_OR_NULL() over using IS_ERR() and a manual NULL
> check.
>
> Change generated with coccinelle.
Applied, thanks!
[04/61] ext4: Prefer IS_ERR_OR_NULL over manual NULL check
commit: 1d749e110277ce4103f27bd60d6181e52c0cc1e3
Best regards,
--
Theodore Ts'o <tytso@mit.edu>
^ permalink raw reply
* Re: [PATCH v2] dt-binding: leds: publish common bindings under dual license
From: Rob Herring @ 2026-04-10 13:22 UTC (permalink / raw)
To: Corvin Köhne
Cc: linux-kernel, open list:LED SUBSYSTEM, Pavel Machek,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Lee Jones, Krzysztof Kozlowski, Conor Dooley, Corvin Köhne,
Ashley Towns, Dan Murphy, Gergo Koteles, INAGAKI Hiroshi,
Jacek Anaszewski, Olliver Schinagl, Pavel Machek,
Rafał Miłecki, Roderick Colenbrander,
Krzysztof Kozlowski
In-Reply-To: <20260408062942.7128-1-corvin.koehne@gmail.com>
On Wed, Apr 08, 2026 at 08:29:42AM +0200, Corvin Köhne wrote:
> From: Corvin Köhne <c.koehne@beckhoff.com>
>
> Changes leds/common.h DT binding header file to be published under GPLv2
> or BSD-2-Clause license terms. This change allows this common LED
> bindings header file to be used in software components as bootloaders
> and OSes that are not published under GPLv2 terms.
>
> All contributors to leds/common.h file in copy.
>
> Cc: Ashley Towns <mail@ashleytowns.id.au>
I don't think a one line change is copyright-able work.
> Cc: Dan Murphy <dmurphy@ti.com>
Dan doesn't appear in git blame.
> Cc: Gergo Koteles <soyer@irl.hu>
Another oneliner.
> Cc: INAGAKI Hiroshi <musashino.open@gmail.com>
only 3 lines... Shrug
> Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Olliver Schinagl <oliver@schinagl.nl>
Just adding more colors to the list.
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Rafał Miłecki <rafal@milecki.pl>
> Cc: Roderick Colenbrander <roderick@gaikai.com>
5 lines of basically the same thing.
So really, I think it is mainly Jacek's and Pavel's acks we need.
Rob
^ permalink raw reply
* Re: [PATCH v2] dt-binding: leds: publish common bindings under dual license
From: Rafał Miłecki @ 2026-04-10 11:53 UTC (permalink / raw)
To: Corvin Köhne
Cc: linux-kernel, linux-leds, Pavel Machek, devicetree, Lee Jones,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Corvin Köhne,
Ashley Towns, Dan Murphy, Gergo Koteles, INAGAKI Hiroshi,
Jacek Anaszewski, Olliver Schinagl, Pavel Machek,
Roderick Colenbrander, Krzysztof Kozlowski
In-Reply-To: <20260408062942.7128-1-corvin.koehne@gmail.com>
On 2026-04-08 08:29, Corvin Köhne wrote:
> Changes leds/common.h DT binding header file to be published under
> GPLv2
> or BSD-2-Clause license terms. This change allows this common LED
> bindings header file to be used in software components as bootloaders
> and OSes that are not published under GPLv2 terms.
>
> All contributors to leds/common.h file in copy.
Acked-by: Rafał Miłecki <rafal@milecki.pl>
> Cc: Ashley Towns <mail@ashleytowns.id.au>
> Cc: Dan Murphy <dmurphy@ti.com>
> Cc: Gergo Koteles <soyer@irl.hu>
> Cc: INAGAKI Hiroshi <musashino.open@gmail.com>
> Cc: Jacek Anaszewski <jacek.anaszewski@gmail.com>
> Cc: Olliver Schinagl <oliver@schinagl.nl>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Rafał Miłecki <rafal@milecki.pl>
> Cc: Roderick Colenbrander <roderick@gaikai.com>
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Corvin Köhne <c.koehne@beckhoff.com>
> ---
> include/dt-bindings/leds/common.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/dt-bindings/leds/common.h
> b/include/dt-bindings/leds/common.h
> index 4f017bea0123..b7bafbaf7df3 100644
> --- a/include/dt-bindings/leds/common.h
> +++ b/include/dt-bindings/leds/common.h
> @@ -1,4 +1,4 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> +/* SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) */
> /*
> * This header provides macros for the common LEDs device tree
> bindings.
> *
--
Rafał Miłecki
^ permalink raw reply
* [PATCH net-next v4 2/2] net: pse-pd: add LED trigger support via notification path
From: Carlo Szelinsky @ 2026-04-10 12:44 UTC (permalink / raw)
To: Oleksij Rempel, Kory Maincent
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Krzysztof Kozlowski, Krzysztof Kozlowski,
Conor Dooley, Rob Herring, netdev, linux-kernel, linux-leds,
Carlo Szelinsky, kernel test robot
In-Reply-To: <20260410124428.809943-1-github@szelinsky.de>
Add per-PI "delivering" and "enabled" LED triggers to the PSE core
subsystem. LED state is updated from the shared pse_handle_events()
function whenever the IRQ or poll path detects a state change, as well
as from the regulator enable/disable paths so that host-initiated
admin state changes via ethtool are immediately reflected.
Both C33 and PoDL power status and admin state are checked so that LED
triggers work for both controller types.
Trigger names use dev_name(dev) (e.g. "pse-1-003c:port0:delivering")
to ensure uniqueness when multiple PSE controllers are present on the
same system.
Initial LED state is queried at registration time so already-active
ports are reflected immediately without waiting for a hardware event.
LED trigger registration is performed before adding the controller to
the global list, avoiding a race where an IRQ or poll event could
invoke pse_led_update() on a partially initialized trigger.
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202603251254.o5PqMBRU-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202603251250.cuMCk5Yv-lkp@intel.com/
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
drivers/net/pse-pd/pse_core.c | 134 +++++++++++++++++++++++++++++++++-
include/linux/pse-pd/pse.h | 22 ++++++
2 files changed, 154 insertions(+), 2 deletions(-)
diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index f641a6fa087f..dfc84340afb9 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -12,9 +12,10 @@
#include <linux/phy.h>
#include <linux/pse-pd/pse.h>
#include <linux/regulator/driver.h>
+#include <linux/leds.h>
+#include <linux/workqueue.h>
#include <linux/regulator/machine.h>
#include <linux/rtnetlink.h>
-#include <linux/workqueue.h>
#include <net/net_trackers.h>
#define PSE_PW_D_LIMIT INT_MAX
@@ -670,6 +671,111 @@ static int _pse_pi_delivery_power_sw_pw_ctrl(struct pse_controller_dev *pcdev,
return 0;
}
+#if IS_ENABLED(CONFIG_LEDS_TRIGGERS)
+/**
+ * pse_led_update - Update LED triggers for a PI based on current state
+ * @pcdev: PSE controller device
+ * @id: PI index
+ *
+ * Queries the current power status and admin state of the PI and
+ * fires LED trigger events on state changes. Called from the
+ * notification path and the regulator enable/disable paths.
+ *
+ * Must be called with pcdev->lock held.
+ */
+static void pse_led_update(struct pse_controller_dev *pcdev, int id)
+{
+ struct pse_pi_led_triggers *trigs;
+ struct pse_pw_status pw_status = {};
+ struct pse_admin_state admin_state = {};
+ bool delivering, enabled;
+
+ if (!pcdev->pi_led_trigs)
+ return;
+
+ trigs = &pcdev->pi_led_trigs[id];
+ if (!trigs->delivering.name)
+ return;
+
+ if (pcdev->ops->pi_get_pw_status(pcdev, id, &pw_status))
+ return;
+ if (pcdev->ops->pi_get_admin_state(pcdev, id, &admin_state))
+ return;
+
+ delivering = pw_status.c33_pw_status ==
+ ETHTOOL_C33_PSE_PW_D_STATUS_DELIVERING ||
+ pw_status.podl_pw_status ==
+ ETHTOOL_PODL_PSE_PW_D_STATUS_DELIVERING;
+ enabled = admin_state.c33_admin_state ==
+ ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED ||
+ admin_state.podl_admin_state ==
+ ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED;
+
+ if (trigs->last_delivering != delivering) {
+ trigs->last_delivering = delivering;
+ led_trigger_event(&trigs->delivering,
+ delivering ? LED_FULL : LED_OFF);
+ }
+
+ if (trigs->last_enabled != enabled) {
+ trigs->last_enabled = enabled;
+ led_trigger_event(&trigs->enabled,
+ enabled ? LED_FULL : LED_OFF);
+ }
+}
+
+static int pse_led_triggers_register(struct pse_controller_dev *pcdev)
+{
+ struct device *dev = pcdev->dev;
+ const char *dev_id;
+ int i, ret;
+
+ dev_id = dev_name(dev);
+
+ pcdev->pi_led_trigs = devm_kcalloc(dev, pcdev->nr_lines,
+ sizeof(*pcdev->pi_led_trigs),
+ GFP_KERNEL);
+ if (!pcdev->pi_led_trigs)
+ return -ENOMEM;
+
+ for (i = 0; i < pcdev->nr_lines; i++) {
+ struct pse_pi_led_triggers *trigs = &pcdev->pi_led_trigs[i];
+
+ /* Skip PIs not described in device tree */
+ if (!pcdev->no_of_pse_pi && !pcdev->pi[i].np)
+ continue;
+
+ trigs->delivering.name = devm_kasprintf(dev, GFP_KERNEL,
+ "pse-%s:port%d:delivering",
+ dev_id, i);
+ if (!trigs->delivering.name)
+ return -ENOMEM;
+
+ ret = devm_led_trigger_register(dev, &trigs->delivering);
+ if (ret)
+ return ret;
+
+ trigs->enabled.name = devm_kasprintf(dev, GFP_KERNEL,
+ "pse-%s:port%d:enabled",
+ dev_id, i);
+ if (!trigs->enabled.name)
+ return -ENOMEM;
+
+ ret = devm_led_trigger_register(dev, &trigs->enabled);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+#else
+static inline void pse_led_update(struct pse_controller_dev *pcdev, int id) {}
+static int pse_led_triggers_register(struct pse_controller_dev *pcdev)
+{
+ return 0;
+}
+#endif /* CONFIG_LEDS_TRIGGERS */
+
static int pse_pi_enable(struct regulator_dev *rdev)
{
struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
@@ -695,6 +801,7 @@ static int pse_pi_enable(struct regulator_dev *rdev)
pcdev->pi[id].admin_state_enabled = 1;
ret = 0;
}
+ pse_led_update(pcdev, id);
mutex_unlock(&pcdev->lock);
return ret;
}
@@ -702,6 +809,7 @@ static int pse_pi_enable(struct regulator_dev *rdev)
ret = ops->pi_enable(pcdev, id);
if (!ret)
pcdev->pi[id].admin_state_enabled = 1;
+ pse_led_update(pcdev, id);
mutex_unlock(&pcdev->lock);
return ret;
@@ -719,6 +827,7 @@ static int pse_pi_disable(struct regulator_dev *rdev)
ret = _pse_pi_disable(pcdev, id);
if (!ret)
pi->admin_state_enabled = 0;
+ pse_led_update(pcdev, id);
mutex_unlock(&pcdev->lock);
return 0;
@@ -1108,6 +1217,20 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
if (ret)
return ret;
+ ret = pse_led_triggers_register(pcdev);
+ if (ret) {
+ dev_warn(pcdev->dev, "Failed to register LED triggers: %d\n",
+ ret);
+ }
+
+ /* Query initial LED state for all PIs so already-active ports
+ * are reflected immediately without waiting for a hardware event.
+ */
+ for (i = 0; i < pcdev->nr_lines; i++) {
+ if (pcdev->no_of_pse_pi || pcdev->pi[i].np)
+ pse_led_update(pcdev, i);
+ }
+
mutex_lock(&pse_list_mutex);
list_add(&pcdev->list, &pse_controller_list);
mutex_unlock(&pse_list_mutex);
@@ -1265,7 +1388,14 @@ static void pse_handle_events(struct pse_controller_dev *pcdev,
struct pse_ntf ntf = {};
int ret;
- /* Do nothing PI not described */
+ /* Update LEDs for described PIs regardless of consumer state.
+ * LED triggers are registered at controller init, before any
+ * PHY claims a PSE control, so rdev may still be NULL here.
+ */
+ if (pcdev->no_of_pse_pi || pcdev->pi[i].np)
+ pse_led_update(pcdev, i);
+
+ /* Skip regulator/netlink path for PIs without consumers */
if (!pcdev->pi[i].rdev)
continue;
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index 44d5d10e239d..0058636a6299 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -10,6 +10,7 @@
#include <linux/kfifo.h>
#include <uapi/linux/ethtool.h>
#include <uapi/linux/ethtool_netlink_generated.h>
+#include <linux/leds.h>
#include <linux/regulator/driver.h>
/* Maximum current in uA according to IEEE 802.3-2022 Table 145-1 */
@@ -266,6 +267,23 @@ struct pse_pi {
int pw_allocated_mW;
};
+#if IS_ENABLED(CONFIG_LEDS_TRIGGERS)
+/**
+ * struct pse_pi_led_triggers - LED trigger state for a PSE PI
+ *
+ * @delivering: LED trigger for power delivering state
+ * @enabled: LED trigger for admin enabled state
+ * @last_delivering: cached delivering state for change detection
+ * @last_enabled: cached enabled state for change detection
+ */
+struct pse_pi_led_triggers {
+ struct led_trigger delivering;
+ struct led_trigger enabled;
+ bool last_delivering;
+ bool last_enabled;
+};
+#endif
+
/**
* struct pse_ntf - PSE notification element
*
@@ -303,6 +321,7 @@ struct pse_ntf {
* @ntf_work: workqueue for PSE notification management
* @ntf_fifo: PSE notifications FIFO
* @ntf_fifo_lock: protect @ntf_fifo writer
+ * @pi_led_trigs: per-PI LED trigger state array
*/
struct pse_controller_dev {
const struct pse_controller_ops *ops;
@@ -327,6 +346,9 @@ struct pse_controller_dev {
struct work_struct ntf_work;
DECLARE_KFIFO_PTR(ntf_fifo, struct pse_ntf);
spinlock_t ntf_fifo_lock; /* Protect @ntf_fifo writer */
+#if IS_ENABLED(CONFIG_LEDS_TRIGGERS)
+ struct pse_pi_led_triggers *pi_led_trigs;
+#endif
};
/**
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v4 1/2] net: pse-pd: add devm_pse_poll_helper()
From: Carlo Szelinsky @ 2026-04-10 12:44 UTC (permalink / raw)
To: Oleksij Rempel, Kory Maincent
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Krzysztof Kozlowski, Krzysztof Kozlowski,
Conor Dooley, Rob Herring, netdev, linux-kernel, linux-leds,
Carlo Szelinsky
In-Reply-To: <20260410124428.809943-1-github@szelinsky.de>
Extract the common event handling loop from pse_isr() into a shared
pse_handle_events() function, and add a generic poll-based alternative
to the IRQ path for PSE controllers that lack interrupt support or
have IRQ lines not wired on the board.
The new devm_pse_poll_helper() function sets up a delayed work that
periodically calls the driver's map_event callback to detect state
changes, feeding events into the existing ntf_fifo / pse_send_ntf_worker
notification pipeline. This reuses the same pse_irq_desc interface as
the IRQ path — the driver provides a map_event callback that populates
per-PI notification arrays.
The poll worker uses system_freezable_wq to avoid running during system
suspend when the underlying hardware (e.g. I2C bus) may be inaccessible.
Work cancellation on teardown is handled via devm_add_action_or_reset()
to ensure the delayed work is cancelled before poll_notifs is freed
by devres, avoiding a use-after-free when devm_pse_poll_helper() is
called after devm_pse_controller_register() (devres LIFO ordering).
The poll interval defaults to 500ms if not set by the driver, balancing
responsiveness against bus load (e.g. I2C).
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
drivers/net/pse-pd/pse_core.c | 164 +++++++++++++++++++++++++++-------
include/linux/pse-pd/pse.h | 12 +++
2 files changed, 146 insertions(+), 30 deletions(-)
diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 3beaaaeec9e1..f641a6fa087f 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -14,10 +14,18 @@
#include <linux/regulator/driver.h>
#include <linux/regulator/machine.h>
#include <linux/rtnetlink.h>
+#include <linux/workqueue.h>
#include <net/net_trackers.h>
#define PSE_PW_D_LIMIT INT_MAX
+/*
+ * Default poll interval for controllers without IRQ support.
+ * 500ms provides a reasonable trade-off between responsiveness
+ * (event detection, PD detection) and I2C bus utilization.
+ */
+#define PSE_DEFAULT_POLL_INTERVAL_MS 500
+
static DEFINE_MUTEX(pse_list_mutex);
static LIST_HEAD(pse_controller_list);
static DEFINE_XARRAY_ALLOC(pse_pw_d_map);
@@ -1238,66 +1246,103 @@ static int pse_set_config_isr(struct pse_controller_dev *pcdev, int id,
}
/**
- * pse_isr - IRQ handler for PSE
- * @irq: irq number
- * @data: pointer to user interrupt structure
+ * pse_handle_events - Process PSE events for all PIs
+ * @pcdev: a pointer to the PSE controller device
+ * @notifs: per-PI notification array
+ * @notifs_mask: bitmask of PIs with events
*
- * Return: irqreturn_t - status of IRQ
+ * Common event handling shared between IRQ and poll paths.
+ * Caller must hold pcdev->lock.
*/
-static irqreturn_t pse_isr(int irq, void *data)
+static void pse_handle_events(struct pse_controller_dev *pcdev,
+ unsigned long *notifs,
+ unsigned long notifs_mask)
{
- struct pse_controller_dev *pcdev;
- unsigned long notifs_mask = 0;
- struct pse_irq_desc *desc;
- struct pse_irq *h = data;
- int ret, i;
-
- desc = &h->desc;
- pcdev = h->pcdev;
-
- /* Clear notifs mask */
- memset(h->notifs, 0, pcdev->nr_lines * sizeof(*h->notifs));
- mutex_lock(&pcdev->lock);
- ret = desc->map_event(irq, pcdev, h->notifs, ¬ifs_mask);
- if (ret || !notifs_mask) {
- mutex_unlock(&pcdev->lock);
- return IRQ_NONE;
- }
+ int i;
for_each_set_bit(i, ¬ifs_mask, pcdev->nr_lines) {
- unsigned long notifs, rnotifs;
+ unsigned long pi_notifs, rnotifs;
struct pse_ntf ntf = {};
+ int ret;
/* Do nothing PI not described */
if (!pcdev->pi[i].rdev)
continue;
- notifs = h->notifs[i];
+ pi_notifs = notifs[i];
if (pse_pw_d_is_sw_pw_control(pcdev, pcdev->pi[i].pw_d)) {
- ret = pse_set_config_isr(pcdev, i, notifs);
+ ret = pse_set_config_isr(pcdev, i, pi_notifs);
if (ret)
- notifs |= ETHTOOL_PSE_EVENT_SW_PW_CONTROL_ERROR;
+ pi_notifs |= ETHTOOL_PSE_EVENT_SW_PW_CONTROL_ERROR;
}
- dev_dbg(h->pcdev->dev,
- "Sending PSE notification EVT 0x%lx\n", notifs);
+ dev_dbg(pcdev->dev,
+ "Sending PSE notification EVT 0x%lx\n", pi_notifs);
- ntf.notifs = notifs;
+ ntf.notifs = pi_notifs;
ntf.id = i;
kfifo_in_spinlocked(&pcdev->ntf_fifo, &ntf, 1,
&pcdev->ntf_fifo_lock);
schedule_work(&pcdev->ntf_work);
- rnotifs = pse_to_regulator_notifs(notifs);
+ rnotifs = pse_to_regulator_notifs(pi_notifs);
regulator_notifier_call_chain(pcdev->pi[i].rdev, rnotifs,
NULL);
}
+}
+
+/**
+ * pse_isr - IRQ handler for PSE
+ * @irq: irq number
+ * @data: pointer to user interrupt structure
+ *
+ * Return: irqreturn_t - status of IRQ
+ */
+static irqreturn_t pse_isr(int irq, void *data)
+{
+ struct pse_controller_dev *pcdev;
+ unsigned long notifs_mask = 0;
+ struct pse_irq *h = data;
+ int ret;
+ pcdev = h->pcdev;
+
+ /* Clear notifs mask */
+ memset(h->notifs, 0, pcdev->nr_lines * sizeof(*h->notifs));
+ mutex_lock(&pcdev->lock);
+ ret = h->desc.map_event(irq, pcdev, h->notifs, ¬ifs_mask);
+ if (ret || !notifs_mask) {
+ mutex_unlock(&pcdev->lock);
+ return IRQ_NONE;
+ }
+
+ pse_handle_events(pcdev, h->notifs, notifs_mask);
mutex_unlock(&pcdev->lock);
return IRQ_HANDLED;
}
+static void pse_poll_worker(struct work_struct *work)
+{
+ struct pse_controller_dev *pcdev =
+ container_of(work, struct pse_controller_dev,
+ poll_work.work);
+ unsigned long notifs_mask = 0;
+ int ret;
+
+ memset(pcdev->poll_notifs, 0,
+ pcdev->nr_lines * sizeof(*pcdev->poll_notifs));
+ mutex_lock(&pcdev->lock);
+ ret = pcdev->poll_desc.map_event(0, pcdev, pcdev->poll_notifs,
+ ¬ifs_mask);
+ if (!ret && notifs_mask)
+ pse_handle_events(pcdev, pcdev->poll_notifs, notifs_mask);
+ mutex_unlock(&pcdev->lock);
+
+ queue_delayed_work(system_freezable_wq, &pcdev->poll_work,
+ msecs_to_jiffies(pcdev->poll_interval_ms));
+}
+
/**
* devm_pse_irq_helper - Register IRQ based PSE event notifier
* @pcdev: a pointer to the PSE
@@ -1351,6 +1396,65 @@ int devm_pse_irq_helper(struct pse_controller_dev *pcdev, int irq,
}
EXPORT_SYMBOL_GPL(devm_pse_irq_helper);
+static void pse_poll_cancel(void *data)
+{
+ struct pse_controller_dev *pcdev = data;
+
+ cancel_delayed_work_sync(&pcdev->poll_work);
+}
+
+/**
+ * devm_pse_poll_helper - Register poll-based PSE event notifier
+ * @pcdev: a pointer to the PSE controller device
+ * @d: PSE event description (uses same pse_irq_desc as IRQ path)
+ *
+ * For PSE controllers without IRQ support or with IRQ not wired. Sets
+ * up a delayed work that periodically calls the driver's map_event
+ * callback to detect state changes, feeding events into the standard
+ * notification pipeline.
+ *
+ * The poll worker uses system_freezable_wq to ensure it does not run
+ * during system suspend while the hardware may be inaccessible.
+ *
+ * Return: 0 on success and errno on failure
+ */
+int devm_pse_poll_helper(struct pse_controller_dev *pcdev,
+ const struct pse_irq_desc *d)
+{
+ struct device *dev = pcdev->dev;
+ int ret;
+
+ if (!d || !d->map_event || !d->name)
+ return -EINVAL;
+
+ pcdev->poll_desc = *d;
+ pcdev->poll_notifs = devm_kcalloc(dev, pcdev->nr_lines,
+ sizeof(*pcdev->poll_notifs),
+ GFP_KERNEL);
+ if (!pcdev->poll_notifs)
+ return -ENOMEM;
+
+ if (!pcdev->poll_interval_ms)
+ pcdev->poll_interval_ms = PSE_DEFAULT_POLL_INTERVAL_MS;
+
+ INIT_DELAYED_WORK(&pcdev->poll_work, pse_poll_worker);
+ pcdev->polling = true;
+
+ /* Register devm action to cancel poll work before poll_notifs is
+ * freed by devres. This ensures correct teardown ordering since
+ * devm_pse_poll_helper() is called after devm_pse_controller_register().
+ */
+ ret = devm_add_action_or_reset(dev, pse_poll_cancel, pcdev);
+ if (ret)
+ return ret;
+
+ queue_delayed_work(system_freezable_wq, &pcdev->poll_work,
+ msecs_to_jiffies(pcdev->poll_interval_ms));
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(devm_pse_poll_helper);
+
/* PSE control section */
static void __pse_control_release(struct kref *kref)
diff --git a/include/linux/pse-pd/pse.h b/include/linux/pse-pd/pse.h
index 4e5696cfade7..44d5d10e239d 100644
--- a/include/linux/pse-pd/pse.h
+++ b/include/linux/pse-pd/pse.h
@@ -292,6 +292,11 @@ struct pse_ntf {
* @pi: table of PSE PIs described in this controller device
* @no_of_pse_pi: flag set if the pse_pis devicetree node is not used
* @irq: PSE interrupt
+ * @polling: flag indicating poll-based event detection is active
+ * @poll_interval_ms: poll interval in milliseconds
+ * @poll_work: delayed work for poll-based event detection
+ * @poll_desc: copy of the driver's event descriptor for polling
+ * @poll_notifs: per-PI notification scratch space for poll worker
* @pis_prio_max: Maximum value allowed for the PSE PIs priority
* @supp_budget_eval_strategies: budget evaluation strategies supported
* by the PSE
@@ -312,6 +317,11 @@ struct pse_controller_dev {
struct pse_pi *pi;
bool no_of_pse_pi;
int irq;
+ bool polling;
+ unsigned int poll_interval_ms;
+ struct delayed_work poll_work;
+ struct pse_irq_desc poll_desc;
+ unsigned long *poll_notifs;
unsigned int pis_prio_max;
u32 supp_budget_eval_strategies;
struct work_struct ntf_work;
@@ -345,6 +355,8 @@ int devm_pse_controller_register(struct device *dev,
struct pse_controller_dev *pcdev);
int devm_pse_irq_helper(struct pse_controller_dev *pcdev, int irq,
int irq_flags, const struct pse_irq_desc *d);
+int devm_pse_poll_helper(struct pse_controller_dev *pcdev,
+ const struct pse_irq_desc *d);
struct pse_control *of_pse_control_get(struct device_node *node,
struct phy_device *phydev);
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v4 0/2] net: pse-pd: add poll path and LED trigger support
From: Carlo Szelinsky @ 2026-04-10 12:44 UTC (permalink / raw)
To: Oleksij Rempel, Kory Maincent
Cc: Andrew Lunn, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Krzysztof Kozlowski, Krzysztof Kozlowski,
Conor Dooley, Rob Herring, netdev, linux-kernel, linux-leds,
Carlo Szelinsky
Thanks to Kory, Oleksij, Krzysztof and Andrew for all the helpful
feedback on earlier versions - really appreciate the time you put
into reviewing this.
This series adds poll-based event detection and LED trigger support
to the PSE core subsystem.
Patch 1 introduces the poll path independently of LED support,
so it can be tested in isolation on boards with and without IRQ
configured.
Patch 2 adds LED triggers that hook into the shared event handling
path introduced by patch 1.
Note: pse_handle_events() and the existing pse_isr() pass notifs_mask
as a single unsigned long, which limits the bitmask to BITS_PER_LONG
PI lines. This is a pre-existing constraint in the IRQ path and is
sufficient for all current PSE controllers (max 48 ports vs 64-bit
unsigned long), but may need to be converted to DECLARE_BITMAP() if
future hardware exceeds this limit.
Changes since v3:
- Dropped the dt-bindings poll-interval-ms patch: the poll interval
is a driver decision, not a hardware property (Krzysztof)
- Removed of_property_read_u32() for poll-interval-ms from
devm_pse_poll_helper(); the 500ms default is now hardcoded but
drivers can override pcdev->poll_interval_ms before calling the
helper
- Rebased on net-next/main
Changes since v2:
- Based on net-next/main, added net-next subject prefix
- Added --base tree information
- Added CC for devicetree list and DT maintainers
- Collected Reviewed-by from Kory Maincent on patch 1/3
- Fixed build error when CONFIG_LEDS_TRIGGERS is disabled:
moved LED registration before list_add(), removing the
pcdev->pi_led_trigs = NULL assignment on conditionally
compiled struct member (reported by kernel test robot)
- Fixed use-after-free on device unbind: poll work is now
cancelled via devm_add_action_or_reset() to ensure correct
devres teardown ordering (poll_work cancelled before
poll_notifs is freed)
- Used system_freezable_wq for poll worker to prevent hardware
access during system suspend
- Added PoDL power status and admin state checks to LED triggers
so they work for both C33 and PoDL controller types
- Used dev_name(dev) for LED trigger names to ensure uniqueness
across multiple PSE controllers (of_node->name can be generic)
- Added initial LED state query at registration so already-active
ports are reflected immediately
- Added pse_led_update() calls in regulator enable/disable paths
so ethtool admin state changes are reflected in LEDs
- Moved LED trigger registration before list_add() to prevent
race where IRQ/poll could invoke pse_led_update() on partially
initialized triggers
Changes since v1:
- Split single patch into 3 separate patches
- Extracted pse_handle_events() and devm_pse_poll_helper() as a
standalone poll path (patches 1-2), testable without LED code
- Added DT binding for poll-interval-ms as a separate patch
- Renamed led-poll-interval-ms to poll-interval-ms for generic use
- Fire LED triggers from the notification path rather than a
separate poll loop
Tested on Realtek RTL9303 with HS104 PoE chip, poll path only
(without IRQ configured). Verified PD connect/disconnect notifications
and LED trigger state changes.
Link: https://lore.kernel.org/all/20260329153124.2823980-1-github@szelinsky.de/
Link: https://lore.kernel.org/all/20260323201225.1836561-1-github@szelinsky.de/
Link: https://lore.kernel.org/all/20260314235916.2391678-1-github@szelinsky.de/
Carlo Szelinsky (2):
net: pse-pd: add devm_pse_poll_helper()
net: pse-pd: add LED trigger support via notification path
drivers/net/pse-pd/pse_core.c | 296 ++++++++++++++++++++++++++++++----
include/linux/pse-pd/pse.h | 34 ++++
2 files changed, 299 insertions(+), 31 deletions(-)
base-commit: b3e69fc3196fc421e26196e7792f17b0463edc6f
--
2.43.0
^ permalink raw reply
* RE: [PATCH v5 2/2] leds: ltc3220: Add Support for LTC3220 18 channel LED Driver
From: Escala, Edelweise @ 2026-04-10 9:17 UTC (permalink / raw)
To: Lee Jones
Cc: Pavel Machek, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-leds@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <DS0PR03MB72281D6EEF372B2E5C73533EED7BA@DS0PR03MB7228.namprd03.prod.outlook.com>
Hello Lee,
I would like to know your recommendation regarding this query before sending in a new version.
Right now what I've done is add comment which references the binding file.
> > > + if (aggregated_led_found && num_leds > 1)
> > > + return dev_err_probe(&client->dev, -EINVAL,
> > > + "Aggregated LED must be the only LED
> > node\n");
> >
> > Must it? Why? Where does it say that?
>
> Aggregated LED mode uses the hardware's "quick-write" feature which
> broadcasts writes to all 18 channels simultaneously. This is a hardware limitation
> - when quick-write mode is enabled, writing to LED channel 1 automatically
> updates ALL channels. Controlling LED individually is still possible however if LED
> 1 is changed all LED value will change.
>
> The device tree binding currently supports two mutually exclusive modes:
> - Multiple independent LED nodes (quick-write disabled), OR
> - Single aggregated LED node with led-sources (quick-write enabled)
>
> This aggregated LED approach was suggested in v2 review:
> https://lore.kernel.org/all/20260112-ltc3220-driver-v2-0-
> d043058fc4df@analog.com/
>
> However, we'd like your recommendation on this design. Would it be better to:
> 1. Keep the aggregated LED mode with hardware quick-write2. Drop aggregated
> mode and let userspace control all 18 LEDs individually
> (userspace can loop to set brightness if synchronized control is needed)
>
Thank you,
Edelweise Escala
^ permalink raw reply
* [PATCH v3 0/1] leds: Introduce the multi_max_intensity sysfs attribute
From: Armin Wolf @ 2026-04-09 21:06 UTC (permalink / raw)
To: lee, pavel
Cc: linux-kernel, corbet, skhan, linux-leds, linux-doc,
jacek.anaszewski, pobrn, m.tretter, wse
This patch series was born out of of a mailing list thread [1] where
i asked how to properly model a RGB LED as a multicolor LED. Said
LED has some exotic properties:
1. 5 global brightness levels.
2. 50 intensity levels for each R/G/B color components.
The current sysfs interface mandates that the maximum intensity value
for each color component should be the same as the maximum global
brightness. This makes sense for LEDs that only emulate global
brightness using led_mc_calc_color_components(), but causes problems
for LEDs that perform global brightness control in hardware.
Faking a maximum global brightness of 50 will not work in this case,
as the hardware can change the global brightness on its own. Userspace
applications might also prefer to know the true maximum brightness
value.
Because of this i decided to add a new sysfs attribute called
"multi_max_intensity". This attribute is similar to the
"max_brightness" sysfs attribute, except that it targets the intensity
values inside the "multi_intensity" sysfs atribute. I also decided to
cap intensity values comming from userspace to said maximum intensity
values to relieve drivers from doing it themself. This was already
proposed in a unrelated patch [2] and might break some misbehaving
userspace applications that do not respect max_brightness.
#include <linux/module.h>
#include <linux/init.h>
#include <linux/led-class-multicolor.h>
static int test_brightness_set_blocking(struct led_classdev *led_cdev,
enum led_brightness brightness)
{
struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(led_cdev);
for (int i = 0; i < mc_cdev->num_colors; i++) {
if (mc_cdev->subled_info[i].intensity > 30)
return -EIO;
}
return 0;
}
static struct mc_subled subleds[] = {
{
.color_index = LED_COLOR_ID_RED,
.max_intensity = 0,
.channel = 1,
},
{
.color_index = LED_COLOR_ID_GREEN,
.max_intensity = 0,
.channel = 2,
},
{
.color_index = LED_COLOR_ID_BLUE,
.max_intensity = 0,
.channel = 3,
},
};
static struct led_classdev_mc led_mc_cdev = {
.led_cdev = {
.max_brightness = 255,
.color = LED_COLOR_ID_MULTI,
.flags = LED_CORE_SUSPENDRESUME | LED_REJECT_NAME_CONFLICT,
.brightness_set_blocking = test_brightness_set_blocking,
},
.num_colors = ARRAY_SIZE(subleds),
.subled_info = subleds,
};
static int __init test_init(void)
{
struct led_init_data init_data = {
.devicename = "test-led",
.default_label = "multicolor:" LED_FUNCTION_KBD_BACKLIGHT,
.devname_mandatory = true,
};
return led_classdev_multicolor_register_ext(NULL, &led_mc_cdev, &init_data);
}
module_init(test_init);
static void __exit test_exit(void)
{
led_classdev_multicolor_unregister(&led_mc_cdev);
}
module_exit(test_exit);
MODULE_AUTHOR("Armin Wolf <W_Armin@gmx.de>");
MODULE_DESCRIPTION("Multicolor LED test device");
MODULE_LICENSE("GPL");
[1] https://lore.kernel.org/linux-leds/2d91a44e-fce2-42dc-b529-133ab4a191f0@gmx.de/
[2] https://lore.kernel.org/linux-leds/20260123-leds-multicolor-limit-intensity-v1-1-b37761c2fdfd@pengutronix.de/
Changes since v2:
- add Reviewed-by tags
- fix spelling mistake
Changes since v1:
- use sysfs_emit_at()
- fix documentation issues
Changes since RFC:
- rework documentation
- drop useless defines
- reduce amount of driver code churn
Armin Wolf (1):
leds: Introduce the multi_max_intensity sysfs attribute
.../ABI/testing/sysfs-class-led-multicolor | 19 ++++++--
Documentation/leds/leds-class-multicolor.rst | 21 ++++++++-
drivers/leds/led-class-multicolor.c | 47 ++++++++++++++++++-
drivers/leds/leds-lp50xx.c | 1 +
drivers/leds/rgb/leds-ncp5623.c | 4 +-
include/linux/led-class-multicolor.h | 30 +++++++++++-
6 files changed, 113 insertions(+), 9 deletions(-)
--
2.39.5
^ permalink raw reply
* [PATCH v3 1/1] leds: Introduce the multi_max_intensity sysfs attribute
From: Armin Wolf @ 2026-04-09 21:06 UTC (permalink / raw)
To: lee, pavel
Cc: linux-kernel, corbet, skhan, linux-leds, linux-doc,
jacek.anaszewski, pobrn, m.tretter, wse
In-Reply-To: <20260409210629.9934-1-W_Armin@gmx.de>
Some multicolor LEDs support global brightness control in hardware,
meaning that the maximum intensity of the color components is not
connected to the maximum global brightness. Such LEDs cannot be
described properly by the current multicolor LED class interface,
because it assumes that the maximum intensity of each color component
is described by the maximum global brightness of the LED.
Fix this by introducing a new sysfs attribute called
"multi_max_intensity" holding the maximum intensity values for the
color components of a multicolor LED class device. Drivers can use
the new max_intensity field inside struct mc_subled to tell the
multicolor LED class code about those values. Intensity values written
by userspace applications will be limited to this maximum value.
Drivers for multicolor LEDs that do not support global brightness
control in hardware might still want to use the maximum global LED
brightness supplied via devicetree as the maximum intensity of each
individual color component. Such drivers should set max_intensity
to 0 so that the multicolor LED core can act accordingly.
The lp50xx and ncp5623 LED drivers already use hardware-based control
for the global LED brightness. Modify those drivers to correctly
initalize .max_intensity to avoid being limited to the maximum global
brightness supplied via devicetree.
Reviewed-by: Werner Sembach <wse@tuxedocomputers.com>
Reviewed-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
.../ABI/testing/sysfs-class-led-multicolor | 19 ++++++--
Documentation/leds/leds-class-multicolor.rst | 21 ++++++++-
drivers/leds/led-class-multicolor.c | 47 ++++++++++++++++++-
drivers/leds/leds-lp50xx.c | 1 +
drivers/leds/rgb/leds-ncp5623.c | 4 +-
include/linux/led-class-multicolor.h | 30 +++++++++++-
6 files changed, 113 insertions(+), 9 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-led-multicolor b/Documentation/ABI/testing/sysfs-class-led-multicolor
index 16fc827b10cb..197da3e775b4 100644
--- a/Documentation/ABI/testing/sysfs-class-led-multicolor
+++ b/Documentation/ABI/testing/sysfs-class-led-multicolor
@@ -16,9 +16,22 @@ Date: March 2020
KernelVersion: 5.9
Contact: Dan Murphy <dmurphy@ti.com>
Description: read/write
- This file contains array of integers. Order of components is
- described by the multi_index array. The maximum intensity should
- not exceed /sys/class/leds/<led>/max_brightness.
+ This file contains an array of integers. The order of components
+ is described by the multi_index array. The maximum intensity value
+ supported by each color component is described by the multi_max_intensity
+ file. Writing intensity values larger than the maximum value of a
+ given color component will result in those values being clamped.
+
+ For additional details please refer to
+ Documentation/leds/leds-class-multicolor.rst.
+
+What: /sys/class/leds/<led>/multi_max_intensity
+Date: March 2026
+KernelVersion: 7.1
+Contact: Armin Wolf <W_Armin@gmx.de>
+Description: read
+ This file contains an array of integers describing the maximum
+ intensity value for each intensity component.
For additional details please refer to
Documentation/leds/leds-class-multicolor.rst.
diff --git a/Documentation/leds/leds-class-multicolor.rst b/Documentation/leds/leds-class-multicolor.rst
index c6b47b4093c4..68340644f80b 100644
--- a/Documentation/leds/leds-class-multicolor.rst
+++ b/Documentation/leds/leds-class-multicolor.rst
@@ -25,10 +25,14 @@ color name to indexed value.
The ``multi_index`` file is an array that contains the string list of the colors as
they are defined in each ``multi_*`` array file.
-The ``multi_intensity`` is an array that can be read or written to for the
+The ``multi_intensity`` file is an array that can be read or written to for the
individual color intensities. All elements within this array must be written in
order for the color LED intensities to be updated.
+The ``multi_max_intensity`` file is an array that contains the maximum intensity
+value supported by each color intensity. Intensity values above this will be
+automatically clamped into the supported range.
+
Directory Layout Example
========================
.. code-block:: console
@@ -38,6 +42,7 @@ Directory Layout Example
-r--r--r-- 1 root root 4096 Oct 19 16:16 max_brightness
-r--r--r-- 1 root root 4096 Oct 19 16:16 multi_index
-rw-r--r-- 1 root root 4096 Oct 19 16:16 multi_intensity
+ -r--r--r-- 1 root root 4096 Oct 19 16:16 multi_max_intensity
..
@@ -104,3 +109,17 @@ the color LED group.
128
..
+
+Writing intensity values larger than the maximum specified in ``multi_max_intensity``
+will result in those values being clamped into the supported range.
+
+.. code-block:: console
+
+ # cat /sys/class/leds/multicolor:status/multi_max_intensity
+ 255 255 255
+
+ # echo 512 512 512 > /sys/class/leds/multicolor:status/multi_intensity
+ # cat /sys/class/leds/multicolor:status/multi_intensity
+ 255 255 255
+
+..
diff --git a/drivers/leds/led-class-multicolor.c b/drivers/leds/led-class-multicolor.c
index 6b671f3f9c61..8d763b1ae76f 100644
--- a/drivers/leds/led-class-multicolor.c
+++ b/drivers/leds/led-class-multicolor.c
@@ -7,10 +7,28 @@
#include <linux/init.h>
#include <linux/led-class-multicolor.h>
#include <linux/math.h>
+#include <linux/minmax.h>
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
+static unsigned int led_mc_get_max_intensity(struct led_classdev_mc *mcled_cdev, size_t index)
+{
+ unsigned int max_intensity;
+
+ /* The maximum global brightness value might still be changed by
+ * led_classdev_register_ext() using devicetree properties. This
+ * prevents us from changing subled_info[X].max_intensity when
+ * registering a multicolor LED class device, so we have to do
+ * this during runtime.
+ */
+ max_intensity = mcled_cdev->subled_info[index].max_intensity;
+ if (max_intensity)
+ return max_intensity;
+
+ return mcled_cdev->led_cdev.max_brightness;
+}
+
int led_mc_calc_color_components(struct led_classdev_mc *mcled_cdev,
enum led_brightness brightness)
{
@@ -27,6 +45,27 @@ int led_mc_calc_color_components(struct led_classdev_mc *mcled_cdev,
}
EXPORT_SYMBOL_GPL(led_mc_calc_color_components);
+static ssize_t multi_max_intensity_show(struct device *dev,
+ struct device_attribute *intensity_attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev);
+ unsigned int max_intensity;
+ int len = 0;
+ int i;
+
+ for (i = 0; i < mcled_cdev->num_colors; i++) {
+ max_intensity = led_mc_get_max_intensity(mcled_cdev, i);
+ len += sysfs_emit_at(buf, len, "%u", max_intensity);
+ if (i < mcled_cdev->num_colors - 1)
+ len += sprintf(buf + len, " ");
+ }
+
+ buf[len++] = '\n';
+ return len;
+}
+static DEVICE_ATTR_RO(multi_max_intensity);
+
static ssize_t multi_intensity_store(struct device *dev,
struct device_attribute *intensity_attr,
const char *buf, size_t size)
@@ -35,6 +74,7 @@ static ssize_t multi_intensity_store(struct device *dev,
struct led_classdev_mc *mcled_cdev = lcdev_to_mccdev(led_cdev);
int nrchars, offset = 0;
unsigned int intensity_value[LED_COLOR_ID_MAX];
+ unsigned int max_intensity;
int i;
ssize_t ret;
@@ -56,8 +96,10 @@ static ssize_t multi_intensity_store(struct device *dev,
goto err_out;
}
- for (i = 0; i < mcled_cdev->num_colors; i++)
- mcled_cdev->subled_info[i].intensity = intensity_value[i];
+ for (i = 0; i < mcled_cdev->num_colors; i++) {
+ max_intensity = led_mc_get_max_intensity(mcled_cdev, i);
+ mcled_cdev->subled_info[i].intensity = min(intensity_value[i], max_intensity);
+ }
if (!test_bit(LED_BLINK_SW, &led_cdev->work_flags))
led_set_brightness(led_cdev, led_cdev->brightness);
@@ -111,6 +153,7 @@ static ssize_t multi_index_show(struct device *dev,
static DEVICE_ATTR_RO(multi_index);
static struct attribute *led_multicolor_attrs[] = {
+ &dev_attr_multi_max_intensity.attr,
&dev_attr_multi_intensity.attr,
&dev_attr_multi_index.attr,
NULL,
diff --git a/drivers/leds/leds-lp50xx.c b/drivers/leds/leds-lp50xx.c
index e2a9c8592953..69c3550f1a31 100644
--- a/drivers/leds/leds-lp50xx.c
+++ b/drivers/leds/leds-lp50xx.c
@@ -525,6 +525,7 @@ static int lp50xx_probe_dt(struct lp50xx *priv)
}
mc_led_info[multi_index].color_index = color_id;
+ mc_led_info[multi_index].max_intensity = 255;
num_colors++;
}
diff --git a/drivers/leds/rgb/leds-ncp5623.c b/drivers/leds/rgb/leds-ncp5623.c
index 85d6be6fff2b..f2528f06507d 100644
--- a/drivers/leds/rgb/leds-ncp5623.c
+++ b/drivers/leds/rgb/leds-ncp5623.c
@@ -56,8 +56,7 @@ static int ncp5623_brightness_set(struct led_classdev *cdev,
for (int i = 0; i < mc_cdev->num_colors; i++) {
ret = ncp5623_write(ncp->client,
NCP5623_PWM_REG(mc_cdev->subled_info[i].channel),
- min(mc_cdev->subled_info[i].intensity,
- NCP5623_MAX_BRIGHTNESS));
+ mc_cdev->subled_info[i].intensity);
if (ret)
return ret;
}
@@ -190,6 +189,7 @@ static int ncp5623_probe(struct i2c_client *client)
goto release_led_node;
subled_info[ncp->mc_dev.num_colors].channel = reg;
+ subled_info[ncp->mc_dev.num_colors].max_intensity = NCP5623_MAX_BRIGHTNESS;
subled_info[ncp->mc_dev.num_colors++].color_index = color_index;
}
diff --git a/include/linux/led-class-multicolor.h b/include/linux/led-class-multicolor.h
index db9f34c6736e..45469388bb1a 100644
--- a/include/linux/led-class-multicolor.h
+++ b/include/linux/led-class-multicolor.h
@@ -9,10 +9,31 @@
#include <linux/leds.h>
#include <dt-bindings/leds/common.h>
+/**
+ * struct mc_subled - Color component description.
+ * @color_index: Color ID.
+ * @brightness: Scaled intensity.
+ * @intensity: Current intensity.
+ * @max_intensity: Maximum supported intensity value.
+ * @channel: Channel index.
+ *
+ * Describes a color component of a multicolor LED. Many multicolor LEDs
+ * do no support global brightness control in hardware, so they use
+ * the brightness field in connection with led_mc_calc_color_components()
+ * to perform the intensity scaling in software.
+ * Such drivers should set max_intensity to 0 to signal the multicolor LED core
+ * that the maximum global brightness of the LED class device should be used for
+ * limiting incoming intensity values.
+ *
+ * Multicolor LEDs that do support global brightness control in hardware
+ * should instead set max_intensity to the maximum intensity value supported
+ * by the hardware for a given color component.
+ */
struct mc_subled {
unsigned int color_index;
unsigned int brightness;
unsigned int intensity;
+ unsigned int max_intensity;
unsigned int channel;
};
@@ -53,7 +74,14 @@ int led_classdev_multicolor_register_ext(struct device *parent,
*/
void led_classdev_multicolor_unregister(struct led_classdev_mc *mcled_cdev);
-/* Calculate brightness for the monochrome LED cluster */
+/**
+ * led_mc_calc_color_components() - Calculates component brightness values of a LED cluster.
+ * @mcled_cdev - Multicolor LED class device of the LED cluster.
+ * @brightness - Global brightness of the LED cluster.
+ *
+ * Calculates the brightness values for each color component of a monochrome LED cluster,
+ * see Documentation/leds/leds-class-multicolor.rst for details.
+ */
int led_mc_calc_color_components(struct led_classdev_mc *mcled_cdev,
enum led_brightness brightness);
--
2.39.5
^ permalink raw reply related
* Re: [PATCH v2 1/1] leds: Introduce the multi_max_intensity sysfs attribute
From: Jacek Anaszewski @ 2026-04-09 20:04 UTC (permalink / raw)
To: Armin Wolf, lee, pavel
Cc: linux-kernel, corbet, skhan, linux-leds, linux-doc, wse, pobrn,
m.tretter
In-Reply-To: <20260331191619.3729-2-W_Armin@gmx.de>
Hi Armin,
On 3/31/26 9:16 PM, Armin Wolf wrote:
> Some multicolor LEDs support global brightness control in hardware,
> meaning that the maximum intensity of the color components is not
> connected to the maximum global brightness. Such LEDs cannot be
> described properly by the current multicolor LED class interface,
> because it assumes that the maximum intensity of each color component
> is described by the maximum global brightness of the LED.
>
> Fix this by introducing a new sysfs attribute called
> "multi_max_intensity" holding the maximum intensity values for the
> color components of a multicolor LED class device. Drivers can use
> the new max_intensity field inside struct mc_subled to tell the
> multicolor LED class code about those values. Intensity values written
> by userspace applications will be limited to this maximum value.
>
> Drivers for multicolor LEDs that do not support global brightness
> control in hardware might still want to use the maximum global LED
> brightness supplied via devicetree as the maximum intensity of each
> individual color component. Such drivers should set max_intensity
> to 0 so that the multicolor LED core can act accordingly.
>
> The lp50xx and ncp5623 LED drivers already use hardware-based control
> for the global LED brightness. Modify those drivers to correctly
> initalize .max_intensity to avoid being limited to the maximum global
> brightness supplied via devicetree.
>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
> .../ABI/testing/sysfs-class-led-multicolor | 19 ++++++--
> Documentation/leds/leds-class-multicolor.rst | 21 ++++++++-
> drivers/leds/led-class-multicolor.c | 47 ++++++++++++++++++-
> drivers/leds/leds-lp50xx.c | 1 +
> drivers/leds/rgb/leds-ncp5623.c | 4 +-
> include/linux/led-class-multicolor.h | 30 +++++++++++-
> 6 files changed, 113 insertions(+), 9 deletions(-)
[...]
> diff --git a/include/linux/led-class-multicolor.h b/include/linux/led-class-multicolor.h
> index db9f34c6736e..6f89d92566b2 100644
> --- a/include/linux/led-class-multicolor.h
> +++ b/include/linux/led-class-multicolor.h
> @@ -9,10 +9,31 @@
> #include <linux/leds.h>
> #include <dt-bindings/leds/common.h>
>
> +/**
> + * struct mc_subled - Color component description.
> + * @color_index: Color ID.
> + * @brightness: Scaled intensity.
> + * @intensity: Current intensity.
> + * @max_intensity: Maximum supported intensity value.
> + * @channel: Channel index.
> + *
> + * Describes a color component of a multicolor LED. Many multicolor LEDs
> + * do no support gobal brightness control in hardware, so they use
s/gobal/global/
> + * the brightness field in connection with led_mc_calc_color_components()
> + * to perform the intensity scaling in software.
> + * Such drivers should set max_intensity to 0 to signal the multicolor LED core
> + * that the maximum global brightness of the LED class device should be used for
> + * limiting incoming intensity values.
> + *
> + * Multicolor LEDs that do support global brightness control in hardware
> + * should instead set max_intensity to the maximum intensity value supported
> + * by the hardware for a given color component.
> + */
> struct mc_subled {
> unsigned int color_index;
> unsigned int brightness;
> unsigned int intensity;
> + unsigned int max_intensity;
> unsigned int channel;
> };
>
> @@ -53,7 +74,14 @@ int led_classdev_multicolor_register_ext(struct device *parent,
> */
> void led_classdev_multicolor_unregister(struct led_classdev_mc *mcled_cdev);
>
> -/* Calculate brightness for the monochrome LED cluster */
> +/**
> + * led_mc_calc_color_components() - Calculates component brightness values of a LED cluster.
> + * @mcled_cdev - Multicolor LED class device of the LED cluster.
> + * @brightness - Global brightness of the LED cluster.
> + *
> + * Calculates the brightness values for each color component of a monochrome LED cluster,
> + * see Documentation/leds/leds-class-multicolor.rst for details.
> + */
> int led_mc_calc_color_components(struct led_classdev_mc *mcled_cdev,
> enum led_brightness brightness);
>
Reviewed-by: Jacek Anaszewski <jacek.anaszewski@gmail.com>
--
Best regards,
Jacek Anaszewski
^ permalink raw reply
* Re: [PATCH 00/61] treewide: Use IS_ERR_OR_NULL over manual NULL check - refactor
From: Al Viro @ 2026-04-09 18:16 UTC (permalink / raw)
To: Philipp Hahn
Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Julia Lawall, Nicolas Palix, Chris Mason,
David Sterba, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
Theodore Ts'o, Andreas Dilger, Steve French, Paulo Alcantara,
Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Bharath SM,
Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
Christian Schoenebeck, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
Sandeep Dhavale, Hongbo Li, Chunhai Guo, Miklos Szeredi,
Konstantin Komarov, Andreas Gruenbacher, Kees Cook, Tony Luck,
Guilherme G. Piccoli, Jan Kara, Phillip Lougher,
Christian Brauner, Jan Kara, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Tejun Heo, David Vernet, Andrea Righi,
Changwoo Min, Ingo Molnar, Peter Zijlstra, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Ben Segall, Mel Gorman,
Valentin Schneider, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin, Sylwester Nawrocki, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, Max Filippov,
Paolo Bonzini, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Alasdair Kergon, Mike Snitzer,
Mikulas Patocka, Benjamin Marzinski, David S. Miller, David Ahern,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, Jamal Hadi Salim, Jiri Pirko,
Marcelo Ricardo Leitner, Xin Long, Trond Myklebust,
Anna Schumaker, Chuck Lever, Jeff Layton, NeilBrown,
Olga Kornievskaia, Dai Ngo, Jon Maloy, Johannes Berg,
Catalin Marinas, Russell King, John Crispin, Thomas Bogendoerfer,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Zhenyu Wang,
Zhi Wang, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Alex Deucher, Christian König, Sandy Huang,
Heiko Stübner, Andy Yan, Igor Russkikh, Andrew Lunn,
Pavan Chebbi, Michael Chan, Potnuri Bharat Teja, Tony Nguyen,
Przemek Kitszel, Taras Chornyi, Maxime Coquelin, Alexandre Torgue,
Iyappan Subramanian, Keyur Chudgar, Quan Nguyen, Heiner Kallweit,
Marc Zyngier, Thomas Gleixner, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, Vinod Koul, Linus Walleij, Ulf Hansson,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Martin K. Petersen,
Eduardo Valentin, Keerthy, Rafael J. Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Alex Williamson, Mark Greer,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Shuah Khan, Kieran Bingham, Mauro Carvalho Chehab, Joerg Roedel,
Will Deacon, Robin Murphy, Lee Jones, Pavel Machek, Dave Penkler,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Justin Sanders, Jens Axboe, Georgi Djakov, Michael Turquette,
Stephen Boyd, Philipp Zabel, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Pali Rohár, Dmitry Torokhov
In-Reply-To: <20260310-b4-is_err_or_null-v1-0-bd63b656022d@avm.de>
On Tue, Mar 10, 2026 at 12:48:26PM +0100, Philipp Hahn wrote:
> While doing some static code analysis I stumbled over a common pattern,
> where IS_ERR() is combined with a NULL check. For that there is
> IS_ERR_OR_NULL().
... and valid uses of IS_ERR_OR_NULL are rare as hen teeth.
Most of those are "I'm not sure how this function returns an
error, let's use that just in case".
Please, do not introduce more of that crap.
^ permalink raw reply
* Re: [PATCH] leds: pca955x: kzalloc + kcalloc to single kzalloc
From: Rosen Penev @ 2026-04-09 17:32 UTC (permalink / raw)
To: Lee Jones
Cc: linux-leds, Pavel Machek, Kees Cook, Gustavo A. R. Silva,
open list,
open list:KERNEL HARDENING (not covered by other areas):Keyword:b__counted_by(_le|_be)?b
In-Reply-To: <20260409124417.GA3290953@google.com>
On Thu, Apr 9, 2026 at 5:44 AM Lee Jones <lee@kernel.org> wrote:
>
> On Thu, 26 Mar 2026, Rosen Penev wrote:
>
> > On Thu, Mar 26, 2026 at 4:33 AM Lee Jones <lee@kernel.org> wrote:
> > >
> > > On Thu, 19 Mar 2026, Rosen Penev wrote:
> > >
> > > > Two fewer allocations as a result.
> > > >
> > > > Required placing some structs before others as flexible array members
> > > > require a complete definition. Declaration is not enough.
> > > >
> > > > Added __counted_by support for one of the structs for extra runtime
> > > > analysis.
> > > >
> > > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > > > ---
> > > > drivers/leds/leds-pca955x.c | 45 ++++++++++++++-----------------------
> > > > 1 file changed, 17 insertions(+), 28 deletions(-)
> > > >
> > > > diff --git a/drivers/leds/leds-pca955x.c b/drivers/leds/leds-pca955x.c
> > > > index 2007fe6217ec..ee5f02eaa3c9 100644
> > > > --- a/drivers/leds/leds-pca955x.c
> > > > +++ b/drivers/leds/leds-pca955x.c
> > > > @@ -112,19 +112,6 @@ static const struct pca955x_chipdef pca955x_chipdefs[] = {
> > > > },
> > > > };
> > > >
> > > > -struct pca955x {
> > > > - struct mutex lock;
> > > > - struct pca955x_led *leds;
> > > > - const struct pca955x_chipdef *chipdef;
> > > > - struct i2c_client *client;
> > > > - unsigned long active_blink;
> > > > - unsigned long active_pins;
> > > > - unsigned long blink_period;
> > > > -#ifdef CONFIG_LEDS_PCA955X_GPIO
> > > > - struct gpio_chip gpio;
> > > > -#endif
> > > > -};
> > > > -
> > > > struct pca955x_led {
> > > > struct pca955x *pca955x;
> > > > struct led_classdev led_cdev;
> > > > @@ -137,8 +124,21 @@ struct pca955x_led {
> > > > #define led_to_pca955x(l) container_of(l, struct pca955x_led, led_cdev)
> > > >
> > > > struct pca955x_platform_data {
> > > > - struct pca955x_led *leds;
> > > > int num_leds;
> > > > + struct pca955x_led leds[] __counted_by(num_leds);
>
> I'm not sure we should we be embedding the full 'struct pca955x_led'
> (which contains runtime framework structures like 'struct led_classdev')
> directly inside the platform data? Besides platform data should ideally
> be restricted to static configuration properties rather than runtime
> state.
>
> > > Where is the memory allocated to this now?
> > The kzalloc call was updated to use struct_size to allocate it
> > together with the struct.
> > >
> > > Why do we need this in both structs?
> > kzalloc and kcalloc can be combined in both locations.
>
> I believe my original question might have been misunderstood. I was
> asking about the architectural design choice. Why does the same 'leds'
> array need to exist in both the 'pca955x' runtime structure and the
> 'pca955x_platform_data' configuration structure? Could we look at
> eliminating this redundancy to avoid storing duplicate data?
From what I see, this bit of code handles missing pdata.
>
> > >
> > > > +};
> > > > +
> > > > +struct pca955x {
> > > > + struct mutex lock;
> > > > + const struct pca955x_chipdef *chipdef;
> > > > + struct i2c_client *client;
> > > > + unsigned long active_blink;
> > > > + unsigned long active_pins;
> > > > + unsigned long blink_period;
> > > > +#ifdef CONFIG_LEDS_PCA955X_GPIO
> > > > + struct gpio_chip gpio;
> > > > +#endif
>
> #ifdef statements are to be avoided in C files.
>
> Please find another way to do this (if it's required).
This is not my code.
>
> > > > + struct pca955x_led leds[];
> > > > };
> > > >
> > > > /* 8 bits per input register */
> > > > @@ -542,15 +542,11 @@ pca955x_get_pdata(struct i2c_client *client, const struct pca955x_chipdef *chip)
> > > > if (count > chip->bits)
> > > > return ERR_PTR(-ENODEV);
> > > >
> > > > - pdata = devm_kzalloc(&client->dev, sizeof(*pdata), GFP_KERNEL);
> > > > + pdata = devm_kzalloc(&client->dev, struct_size(pdata, leds, chip->bits), GFP_KERNEL);
> > > > if (!pdata)
> > > > return ERR_PTR(-ENOMEM);
> > > >
> > > > - pdata->leds = devm_kcalloc(&client->dev,
> > > > - chip->bits, sizeof(struct pca955x_led),
> > > > - GFP_KERNEL);
> > > > - if (!pdata->leds)
> > > > - return ERR_PTR(-ENOMEM);
> > > > + pdata->num_leds = chip->bits;
> > > >
> > > > device_for_each_child_node(&client->dev, child) {
> > > > u32 reg;
> > > > @@ -568,8 +564,6 @@ pca955x_get_pdata(struct i2c_client *client, const struct pca955x_chipdef *chip)
> > > > fwnode_property_read_u32(child, "type", &led->type);
> > > > }
> > > >
> > > > - pdata->num_leds = chip->bits;
> > > > -
> > > > return pdata;
> > > > }
> > > >
> > > > @@ -623,15 +617,10 @@ static int pca955x_probe(struct i2c_client *client)
> > > > return -ENODEV;
> > > > }
> > > >
> > > > - pca955x = devm_kzalloc(&client->dev, sizeof(*pca955x), GFP_KERNEL);
> > > > + pca955x = devm_kzalloc(&client->dev, struct_size(pca955x, leds, chip->bits), GFP_KERNEL);
> > > > if (!pca955x)
> > > > return -ENOMEM;
> > > >
> > > > - pca955x->leds = devm_kcalloc(&client->dev, chip->bits,
> > > > - sizeof(*pca955x_led), GFP_KERNEL);
> > > > - if (!pca955x->leds)
> > > > - return -ENOMEM;
> > > > -
> > > > i2c_set_clientdata(client, pca955x);
> > > >
> > > > mutex_init(&pca955x->lock);
> > > > --
> > > > 2.53.0
> > > >
> > >
> > > --
> > > Lee Jones [李琼斯]
> >
>
> --
> Lee Jones [李琼斯]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox