Hi Jelle. On 02/14/2017 11:34 AM, Jelle Martijn Kok wrote: > > On 10-02-17 21:56, Pavel Machek wrote: >> Hi! >> >>>> + led.default_brightness = LED_OFF; >>>> + of_property_read_u32(child, "brightness", >>>> + &led.default_brightness); >>> At first you would have to submit a patch for >>> Documentation/devicetree/bindings/leds/common.txt that would add >>> brightness property. The question is whether it is really needed? >>> You can set brightness from userspace via sysfs API. >>> >>> By the way, I have a question to DT maintainers: is DT a proper >>> place for defining this type of configuration that can be set via >>> userspace scripts? Shouldn't DT describe only hardware properties and >>> constraints resulting from board configuration? >> Well, if the hardware has label "half - power, full - transmitting" on >> a LED, we might want kernel to turn it to half power on bootup. >> >> If you have a "disk activity LED" on a PC, it is driven by >> hardware. On arm notebook, it would be nice if "disk activity LED" >> worked, too. Preferably even when running fsck in init=/bin/bash >> mode. We already provide that, AFAICT, so having ability to set >> constant brightness sounds sane to me. > There is also some delay between kernel and user space where the leds > are too bright or simply off... (which is quite ugly for a simple "power > on" led) It seems to be sufficient justification for introducing the property. Please submit a patch adding DT documentation for it. I'd call it default-brightness to match the one used already in backlight subsystem. > One thing that I have seen is that several led triggers do not play > along with the brightness. For example, the "heartbeat" trigger always > sets the brightness to LED_FULL. Which negates the set brightness on > each blink... I posted the patch [0] addressing that few months ago, but I must have forgotten to reapply it to linux-leds.git after previously withdrawing it due to set_brightness_delayed work locking issues. I've just applied this patch to the for-next branch of linux-leds.git. > So would it not be an idea to add a "led_set_on_off(led, > state)" function which retains "brightness" and additionally takes an > "invert" parameter into account. This might also simplify code in other > led triggers. [0] https://patchwork.kernel.org/patch/9418701/ -- Best regards, Jacek Anaszewski -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
