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 11/25/2016 12:14 PM, Hans de Goede wrote:
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.

It is mandatory to secure a reliable readout of the brightness value
set by the hardware. Without it, we could end up reading brightness
set in the meantime either by trigger or by userspace. Knowing that
value can be useful in case hardware tries to lower the brightness
level set by the user for some reason (e.g. low battery voltage).

However, I'd tweak the file name a bit, to make it clear
what property the file represents - the brightness changed
by the hardware, effectively I'm proposing the property:

brightness_hw_changed

Best regards,
Jacek Anaszewski


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