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

[Pavel Machek <pavel@ucw.cz>]

[-- Attachment #1: Type: text/plain, Size: 2917 bytes --]

Hi!

> >>Triggers are not limited to periodic blinking or reporting cpu
> >>activity. There is also oneshot trigger that can be used e.g. when
> >>user touches the screen, as Pali mentioned.
> >
> >Using oneshot trigger for this would be pretty strange.
> 
> It was only an example to mention other than periodic triggers.
> You could have a trigger that just turns the LED permanently on
> after user touches the screen.

Well.. triggers kind of assume they control the LED. They were not
prepared to deal with hardware changing the brightness behind their
back.

> >Notice that in
> >some cases (thinkpad battery led, for example) we either have firmware
> >controls the LED (but then software can't control it) or we have
> >software controlling the LED (but then we don't know what firmware
> >would put there). Maybe keyboard backlight can be controlled
> >"simultaneously" by both software and firmware, but there are
> >certainly LEDs that can't handle that, and IMO it would be nice to
> >have same interface.
> >
> >>>Well.. actually... I think this is a little bit over complex and
> >>>probably unneccessary. I'd let Hans implement whatever he thinks is
> >>>easiest.
> >>
> >>I'd say this is the trigger approach which is a bit convoluted.
> >
> >In my eyes trigger approach is neccessary at least for some hardware,
> >and things it pretty clear: trigger on == LED changes without
> >userspace involvement. trigger off == userspace controls the LED.
> 
> It is likely that it would break many existing users.

Can you elaborate on that?

I just tried with leds on thinkpad

root@duo:/sys/class/leds/tpacpi::power# echo 1 > brightness
root@duo:/sys/class/leds/tpacpi::power# echo 0 > brightness
root@duo:/sys/class/leds/tpacpi::power# cat trigger
[none] bluetooth-power kbd-scrollock kbd-numlock kbd-capslock
kbd-kanalock kbd-shiftlock kbd-altgrlock kbd-ctrllock kbd-altlock
kbd-shiftllock kbd-shiftrlock kbd-ctrlllock kbd-ctrlrlock AC-online
BAT0-charging-or-full BAT0-charging BAT0-full
BAT0-charging-blink-full-solid rfkill0 phy0rx phy0tx phy0assoc
phy0radio phy0tpt mmc0 timer pattern rfkill1 hci0-power rfkill74
heartbeat
root@duo:/sys/class/leds/tpacpi::power#

I can control the LED from userspace, but then there's no way to put
the LED back to firmware control. That's just broken.

Do you have a proposal how to handle that?

> >So I'd do the trigger here. It is same way we can handle LEDs on
> >thinkpad. Yes, it means you won't be able to do oneshot trigger on
> >backlight. I don't think that's a huge problem.
> 
> There have been voices in this discussion claiming the opposite. :-)

Well, lets ignore those voices until the voices understand the current
design :-).

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]