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

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

 



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...

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.

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

-- 
Pali Rohár
pali.rohar@xxxxxxxxx

Attachment: signature.asc
Description: This is a digitally signed message part.

------------------------------------------------------------------------------
_______________________________________________
ibm-acpi-devel mailing list
ibm-acpi-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel

[Index of Archives]     [Linux ACPI]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Photo]     [Yosemite Photos]     [Yosemite Advice]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]

  Powered by Linux