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:26 PM, Pali Rohár wrote:
On Friday 25 November 2016 12:14:56 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.

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.

I can accept with this solution (no trigger, event on new sysfs file
which returns current/actual brightness state, new sysfs file only for
devices which can report brightness state).

But I'm not sure if it is really fixing that original problem with high
power usage...

High power consumption occurred because in that solution every call
to __led_set_brightness{_blocking} triggered the POLLPRI.
In the discussed approach it will not be the case.

As wrote in some previous emails, consider "actual_brightness" sysfs
name which is already used for this purpose by backlight subsystem --
because for consistency. backlight devices have: actual_brightness,
brightness, max_brightness.

I don't like it very much but I could accept it eventually if
other people deem it accurate.

--
Best regards,
Jacek Anaszewski
--
To unsubscribe from this list: send the line "unsubscribe linux-leds" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux