Re: [PATCH v5 2/6] leds: triggers: Add a keyboard backlight trigger

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 25-11-16 11:01, Pavel Machek wrote:
> Hi!
>
>> In view of the above we could report hw brightness changes with POLLPRI
>> on brightness file, but unfortunately we can't because it is impossible
>> to guarantee that readout of brightness file will return the brightness
>> the POLLPRI was meant to notify about.
>
> Agreed here.
>
>> That's why a separate read only file seems to be the only proper
>> solution.
>
> Yes please. And lets make self-changing leds into a trigger, as
> proposed, and as Hans' patch should be already doing.
>
>> Moreover, the file should return the brightness from the time
>> of last POLLPRI.
>
> Not sure I agree here. Normally, kernel returns current state for
> variables, does not track "old" state.

Agreed, storing the last state just unnecessarily complicates things.

So do we have a consensus on implementing a new hw_brightness_change
sysfs attribute now, which only some LEDs will have, can be polled
to detect changed done autonomously by the hardware and returns
the current / actual LED brightness when read ?

As for the modeling how the hotkey controls the LED as a trigger,
although I do like this from one pov, I can see Jacek's point that
this is confusing as there really is nothing to configure here,
where as normally a user could do "echo none > trigger" to break
the link. So I think that is best (cleanest /minimal non confusing
API) with just the hw_brightness_change sysfs-attribute and not
model this as a trigger.

That, or fall back to my latest patch-set as posted, I still like
that one the most.

Regards,

Hans