On 01/26/2017 09:33 AM, Pali Rohár wrote: > On Wednesday 25 January 2017 22:35:53 Jacek Anaszewski wrote: >> On 01/25/2017 05:49 PM, Pali Rohár wrote: >>> On Wednesday 25 January 2017 17:11:27 Hans de Goede wrote: >>>> Some LEDs may have their brightness level changed autonomously >>>> (outside of kernel control) by hardware / firmware. This commit >>>> adds support for an optional brightness_hw_changed attribute to >>>> signal such changes to userspace (if a driver can detect them): >>>> >>>> What: /sys/class/leds/<led>/brightness_hw_changed >>>> Date: January 2017 >>>> KernelVersion: 4.11 >>>> Description: >>>> Last hardware set brightness level for this LED. Some LEDs >>>> may be changed autonomously by hardware/firmware. Only LEDs >>>> where this happens and the driver can detect this, will >>>> have this file. >>>> >>>> This file supports poll() to detect when the hardware >>>> changes the brightness. >>>> >>>> Reading this file will return the last brightness level set >>>> by the hardware, this may be different from the current >>>> brightness. > > In which situation this attribute may be different? > > I think some information should be present in this description... The first sentence in description says: "Last hardware set brightness level for this LED" - i.e. it may be different from the _actual_ brightness, which could have been set by LED class driver. >>>> Drivers which want to support this, simply add LED_BRIGHT_HW_CHANGED to >>>> their flags field and call led_classdev_notify_brightness_hw_changed() >>>> with the hardware set brightness when they detect a hardware / firmware >>>> triggered brightness change. >>>> >>>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> >>> >>> Just speculation: What about using name 'actual_brightness'? It provides >>> same output on read as actual_brightness from /sys/class/backlight/. >> >> This name is ambiguous. Propagating it to the LED subsystem only because >> it exists in backlight is not sufficient argument IMHO. > > I thought that having same name could help userspace and also to have > consistency between similar subsystems. But ok, that was just my > speculation. Please also note the "actual" would be ambiguous especially in view of my above explanation. > I have just one small suggestion in current name "brightness_hw_changed". > As it provides regular read() capability I would suggest to not indicate > "activity" (changed) in name... Without "changed" one could think that brightness file (the one we have currently) doesn't reflect hardware state. -- Best regards, Jacek Anaszewski
