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

[Rong Zhang]

Hi Andrew,

On Sat, 2026-04-11 at 22:42 +0200, Andrew Lunn wrote:
> > 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 think I understand your points better now.

Though my PATCH 9/9 already used a private trigger for ideapad-laptop, I
will further restrict the led_trigger_notify_hw_control_changed()
interface (PATCH 7/9) to private triggers only when I resubmit this.

Does it make sense to you?

Thanks,
Rong

> 
> 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