Hi,
On 15-11-16 14:28, Jacek Anaszewski wrote:
On 11/15/2016 01:06 PM, Hans de Goede wrote:
Hi,
On 15-11-16 12:48, Pavel Machek wrote:
Hi!
The LED you are talking about _has_ a trigger, implemented in
hardware. That trigger can change LED brightness behind kernel's (and
userspace's) back. Don't pretend the trigger does not exist, it does.
And when you do that, you'll have nice place to report changes to
userspace -- trigger can now export that information, and offer
poll()
interface.
Well, that sounds interesting. It is logically justifiable.
Thanks.
I initially proposed exactly this solution, with recently
added userspace LED being a trigger listener. It seems a bit
awkward though. How would you listen to the trigger events?
Trigger exposes a file in sysfs, with poll() working on that file
Hmm, a new file would give the advantage of making it easy for
userspace to see if the trigger is poll-able, this is likely
better then my own proposal I just send.
Good.
(and
probably read exposing the current brightness).
If we do this, can we please make it mirror brightness, iow
also make it writable, that will make it easier for userspace
to deal with it. We can simply re-use the existing show / store
methods for brightness for this.
Actually, echo 0 > brightness disables the trigger, IIRC. I'd avoid
that here, you want to be able to turn off the backlight but still
keep the trigger (and be notified of future changes).
True, that is easy to do the store method will just need to call
led_set_brightness_nosleep instead of led_set_brightness, this
will skip the checks to stop blinking in led_set_brightness and
otherwise is equivalent.
I suggest we call it:
trigger_brightness
And only register it when a poll-able trigger is present.
I'd call it 'current_brightness', but that's no big deal. Yes, only
registering it for poll-able triggers makes sense.
current_brightness works for me. I will take a shot a patch-set
implementing this.
Word "current" is not precise here.
It can be thought of as either last brightness set by the
user or the brightness currently written to the device
(returned by brightness file).
There is a semantic discrepancy in our requirements -
we want the file representing both permanent brightness
set by the user and brightness set by the hardware.
The two stand in contradiction to each other since
brightness set by the user can be adjusted by the hardware.
Reading the file shouldn't update brightness property of
struct led_classdev, so it shouldn't call led_update_brightness()
but it still should allow reading brightness set by the
hardware, as a result of each POLLPRI event. So in fact in
the same time it should report both according to our requirements
which is impossible. Do we need three brightness files ?
I don't think so, current_brightness actually is an accurate
name, if the brightness was last changed by writing from
sysfs, the keyboard backlight will honor that and the current_brightness
attribute will show the brightness last set through writing it,
which matches the actual current brightness of the keyboard backlight.
Likewise if it was changed with the hotkey last then the keyboard
backlight brightness will be changed and reading from current_brightness
will return the new actual brightness. Basically reading from this
file will be no different then reading from the normal "brightness"
file the difference will be in that it is poll-able and that
writing 0 turns off the LED without stopping blinking.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
