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

[Andrew Lunn]

> 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