Re: [RFC PATCH 1/9] leds: Load trigger modules on-demand if used as hw control trigger

[Rong Zhang]

Hi Andrew,

Thanks for your review.

On Tue, 2026-03-10 at 20:22 +0100, Andrew Lunn wrote:
> On Sat, Feb 28, 2026 at 03:05:58AM +0800, Rong Zhang wrote:
> > In the following patches, we are about to support hardware initiated
> > trigger transitions to/from the device's hw control trigger. In case
> > the LED hardware switches itself to hw control mode, hw control trigger
> > must be loaded before so that the transition can be processed.
> 
> This sounds backwards around.
> 
> A Linux LED starts out life as a dumb LED. You can set its brightness
> using /sys/class/leds/<foo>/brightness.
> 
> Userspace policy can then give additional meaning to the LED. It could
> blink a heartbeat, disk activity, show rf-kill status, activity on a
> serial port, what link speed eth42 has etc. The general design in
> Linux is that any LED can be used for any of these functions. You
> decide what an LED should indicate by selecting the trigger for it. To
> be able to select the trigger, the trigger needs to be already loaded.
> 
> Only once you have the trigger load, and the LED using the trigger,
> then you can think about can the trigger be offloaded to hardware.

While I agree that the current policy makes its own sense and works
well on most devices, it leads to out-of-sync software states on recent
laptops.

When the LED's hardware autonomously put itself into hw control mode,
it is offloaded to the hardware per se -- we *can't* prevent this from
happening.

The series is about how to update software state to reflect the
hardware state change. Blindly keeping the software state despite the
hardware state is hardly meaningful and makes software out-of-sync.


It's worth mentioning that the series doesn't really cause any
behavioral change compared to the current policy when we consider the
LED device's hardware state. Let's consider what will happen after LED
X has autonomously activated hw control:

[ Software state #a: trigger=none ]

Current: LED X stays in hw control mode. Trigger unchanged
(trigger=none, hence software is out-of-sync with hardware).

This series: LED X stays in hw control mode. Trigger updated to reflect
the hardware state (trigger=hw_control_trigger, offloaded=1).

[ Software state #b: trigger=other ]
[ Software state #c: trigger=hw_control_trigger, offloaded=0 ]

Current & this series: LED X stays in hw control mode until the next
trigger event. Software state unchanged and up-to-date.


And what will happen after LED X has autonomously deactivated hw
control:

[ Software state #d: trigger=hw_control_trigger, offloaded=1 ]

Current: LED X stays in software control mode. Trigger unchanged
(trigger=hw_control_trigger, offloaded=1, hence software is out-of-sync
with hardware).

This series: LED X stays in software control mode. Trigger updated to
reflect the hardware state (trigger=none).

[ Software state #e: trigger=other ]
[ Software state #f: trigger=hw_control_trigger, offloaded=0 ]

Current & this series: LED X stays in software control mode. Software
state unchanged and up-to-date.


As shown above, this series doesn't change the LED's hardware state and
it just updates the software state to notify user about that. If you'd
like to enforce software state's priority, we would have to explicitly
undo the hardware state change immediately after the LED's hardware has
autonomously activated/deactivated hw control mode.

Moreover, if an LED device doesn't autonomously change its hw control
state at all, this series should have little software/hardware impact
on them. IIUC, that's the case of all LEDs with
hw_control_trigger=netdev.

> 
> In linux, policy is in user space. If policy says led X is to be used
> with trigger Y, user space can get the needed trigger module loaded.

Just to provide some background of this patch: I decided to include it
in the series when I thought about these three scenarios:

[ Scenario #1 ]

LED X: no default trigger; hw control trigger is Y; capable for
hardware initiated trigger transition

LED W: default trigger is Y

In this case, trigger Y has been be loaded automatically due to LED W.
Hardware initiated trigger transitions from LED X can be processed
properly.

[ Scenario #2 ]

LED X: same as above

CONFIG_LEDS_TRIGGER_Y=y

In this case, trigger Y is builtin. Hardware initiated trigger
transitions from LED X can be processed properly.

Most distros have their own set of builtin triggers, so that's not
necessarily user's choice.

[ Scenario #3 ]

LED X: same as above

Other LEDs: default trigger is not Y

In this case, missing trigger Y leads to hardware initiated trigger
transitions from LED X being ignored.


Hence, not loading hw control trigger automatically leads to behavioral
divergences caused by other LED devices or distro configurations. LED X
is a victim of such divergences, but it was never intended to cause
them.

The main purpose of introducing this commit is to prevent such
divergences. We should only allow behavioral divergences when
CONFIG_LEDS_TRIGGER_Y=n or when trigger Y is blacklisted.


That being said, I am OK to drop this patch from the series. ideapad-
laptop (see [RFC PATCH 9/9]) registers a private trigger, so it doesn't
depend on this patch. AFAIK this also applies to thinkpad_acpi as long
as it implements auto brightness in a similar way.

Thanks,
Rong

> 
>      Andrew