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

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

Hi,

On 01/24/2017 01:32 PM, Pavel Machek wrote:
> Hi!
> 
>>>> There might exist users that adjust LED brightness while having
>>>> active trigger. The best example is default-on trigger - it sets
>>>> brightness only on init, but remains active all the time. Whereas
>>>> this could be fixed, there is another case: think of changing blinking
>>>> brightness - it would be impossible.
>>>
>>> I don't see the breakage. For existing blinking and default-on
>>> triggers, existing behaviour would be kept. Difference would be that
>>> for hardware that changes triggers itself (like the keyboard light) we
>>> would present it as a trigger. And you are right -- there would be
>>> user visible changes there. You could not assign blinking, because
>>> .. it already has a trigger. But I believe it is reasonable.
>>
>> We've already received negative feedback regarding blocking
>> application of different trigger when hw brightness change
>> notifications
> 
> Actually sometimes we'll get negative feedback for anything we do. I
> still believe exposing the backlight case as a trigger is a right thing.
> 
>> are enabled. One solution to that is making the notifications
>> orthogonal to the trigger mechanism. The other possibility is to allow
>> for having more than one active trigger for a LED.
>>
>> We could think of defining a special type of persistent trigger that
>> would be always enabled.
> 
> Yes, that could be done. I'd not call it persistent trigger, but
> with right name...
> 
> #1:
> 
> Apparently some LEDs change themselves, and we can't find how or
> when. That's thinkpad battery LED, for example.
> 
> #2:
> 
> Then there are some LEDs that change themselves, and we can get
> notifications. We should make it very clear that we'll not send
> notifications kernel did itself.
> 
> So... for #1 and #2 something like
> 
> led/hardware_changes_brightness 
> 
> and for #2
> 
> led/poll_for_hardware_brightness_change
> 
> where poll() wakes the userspace up when the brightness changes, and
> read() can get new brightness...?

Well, it is almost the same as hw_brightness_change I proposed
earlier. Persistent triggers OTOH would be more generic and it would
fit for the trigger approach Hans already implemented.


-- 
Best regards,
Jacek Anaszewski