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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 25-11-16 11:01, Pavel Machek wrote:
Hi!

In view of the above we could report hw brightness changes with POLLPRI
on brightness file, but unfortunately we can't because it is impossible
to guarantee that readout of brightness file will return the brightness
the POLLPRI was meant to notify about.

Agreed here.

That's why a separate read only file seems to be the only proper
solution.

Yes please. And lets make self-changing leds into a trigger, as
proposed, and as Hans' patch should be already doing.

Moreover, the file should return the brightness from the time
of last POLLPRI.

Not sure I agree here. Normally, kernel returns current state for
variables, does not track "old" state.

Agreed, storing the last state just unnecessarily complicates things.

So do we have a consensus on implementing a new hw_brightness_change
sysfs attribute now, which only some LEDs will have, can be polled
to detect changed done autonomously by the hardware and returns
the current / actual LED brightness when read ?

As for the modeling how the hotkey controls the LED as a trigger,
although I do like this from one pov, I can see Jacek's point that
this is confusing as there really is nothing to configure here,
where as normally a user could do "echo none > trigger" to break
the link. So I think that is best (cleanest /minimal non confusing
API) with just the hw_brightness_change sysfs-attribute and not
model this as a trigger.

That, or fall back to my latest patch-set as posted, I still like
that one the most.

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



[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux