Hi,
On 20-11-16 15:59, Pali Rohár wrote:
On Thursday 17 November 2016 23:24:36 Hans de Goede wrote:
In some cases it may be desirable for userspace to be notified when
a trigger event happens. This commit adds support for a poll-able
current_brightness trigger specific sysfs attribute which triggers
may register:
What: /sys/class/leds/<led>/current_brightness
Date: November 2016
KernelVersion: 4.10
Description:
Triggers which support it may register a current_brightness
file. This file supports poll() to detect when the trigger
modifies the brightness of the LED.
Reading this file will always return the current brightness
of the LED.
Writing this file sets the current brightness of the LED,
without influencing the trigger.
Personally I do not like this new sysfs attribute...
Now when somebody look at /sys/class/leds/<something>/, the first thing
which say would be:
"What the hell, why there are two files (brightness and
current_brightness) for changing LED level? And which should I use?"
If I understood correctly we need to handle two things:
1) Provide poll() for userspace when LED level is changed (either by HW
or other user call)
>
2) Deal with fact that on _some_ hardware, special key is hardwired to
change LED level
So why for 1) we cannot use existing sysfs file "brightness"? I do not
see any problem with it.
That was our first attempt at this, but because the brightness may also
be changed by triggers / blink-timers, we need to wakeup poll() in those
cases too (anything else would be inconsistent) and doing such a wakeup
in that case has turned out to cause too much overhead in some cases
(even if userspace is not listening), specifically the idle power uses
on some systems got multiplied by a factor of 5 or more.
So this approach was rejected.
And for 2) we even do not know on which machines is such hardwired
feature enabled. Yes, on _some_ (not *all*) Dell machines there is Fn
key combination which changes level of one LED device. But kernel does
not know if hardware on which is running is doing such thing or not.
Some machines do not have to have key for such action and we do not know
it.
And what about situation when somebody wants to configure e.g. mouse
movement (or keypress) trigger to enable/disable LED device (which
belongs to keyboard brightness)? In this case user explicitly know that
his Fn+Space change level of LED device, so can be careful to not press
it. With your read-only trigger you basically disable such (I think
useful) feature.
This has already been discussed and the third patch in the set, which is
the one making the trigger read-only has been dropped.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
