On 10/20/2016 10:41 AM, Hans de Goede wrote:
Hi,
On 20-10-16 08:42, Jacek Anaszewski wrote:
Hi Hans,
How about exploiting recently added userspace driver for the LED
subsystem ((drivers/leds/uleds.c, available in linux-next) instead?
If the LED class device to be observed implemented its internal trigger
for generating kernel events upon device brightness change, then the
userspace could create virtual LED class device, register on the
trigger and poll the obtained file descriptor.
See tools/leds/uledmon.c.
Erm, this feels like a very wrong way to go about this, so now you
want the led_classdev to send a new trigger to be picked up by a fake-led
driven from userspace ? Having a led_classdev send triggers feels quite
wrong, and having a u-led which will not really be a led at all, just a
way to listen to the trigger seems wrong to me too.
Generally the intention behind introducing userspace LED class driver
was to have a means for intercepting kernel LED events. I agree that
having LED class device generating trigger events is awkward. It was
just the first thing that came to my mind seeing the idea of polling,
and having the fresh memory of userspace LED driver.
I am inclined to accept your patch, but it will need thorough testing
to check if there will be no unpleasant side effects when one or more
processes will be polling the brightness sysfs file and in the
meantime other process(es) will write to it.
Also notifying only about brightness change events not caused by
writing brightness file is counter intuitive if we are polling
brightness file. What about brightness changes caused by using
led_set_brightness() API, without mediation of brightness file?
If we want to notify only brightness changes originating from
hardware, maybe it would be a good idea to add a dedicated
sysfs file? It could appear only if relevant option in the
kernel config was turned on.
No this really is the wrong way to do this IMHO.
We already have a well defined interface to wait for sysfs attribute
changes for devices which export a sysfs interfaces, which is doing
POLL_PRI on the sysfs attribute.
Looking at the discussion between me and Pali I can see a clear
consensus on the semantics of the poll here, we will notify any
POLL_PRI listeners on long-lived changes to the brightness, either
done by the hw autonomously or those done by a sysfs brightness write.
Temporary brightness changes caused by triggers and/or blinking will
not lead to a notify.
If we can all agree on these semantics, then I believe that this will
be a good interface to deal with this.
Regards,
Hans
Best regards,
Jacek Anaszewski
On 10/19/2016 06:07 PM, Hans de Goede wrote:
Hi,
On 19-10-16 15:59, Pali Rohár wrote:
On Wednesday 19 October 2016 15:33:54 Hans de Goede wrote:
+ itself and the driver can detect this. Changes done by
+ kernel triggers / software blinking and writing the
brightness
+ file are not signalled.
Why? In case you have desktop application which show current brightness
level and you change manually it via echo something > /sys/ then that
application does not have any information about your change. And so it
show old value...
Well it seems like a bad idea to me to notify on changes caused by
triggers and blinking, even though those are visible under sysfs
if polling the brightness attribute. At which point it seemed to make
sense to only notify on changes done autonomously by the hardware,
rather then from any software (running on the main CPU).
As for specifically not notifying on write, the sysfs interface is root
only,
so usually there will be a single daemon controlling the brightness,
and any users can go through that daemon and it can distribute change
messages itself (and will do so for the POLL_PRI events).
Since the usual use case is a single writing process, which is also
the same single process listening for POLL_PRI, it seems unnecessary
to me to notify the process about the write it has just done.
But if people think that we should consider multiple simultaneous
users (which seems like a bad idea to me, because of coordination
issues, but the API does allow it) and therefor should notify
of the brightness change on a write, I'm fine with doing a new
version implementing those semantics instead.
Either way notifying on brightness changes caused by triggers /
blinking seems like a bad idea to me.
Regards,
Hans
--
Best regards,
Jacek Anaszewski
--
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
