Hello Jeff, On Sun, Jan 26, 2020 at 07:42:39PM +0000, Jeff LaBundy wrote: > My only concern here is whether or not there happen to be some hardware > out in the world whose PWM-related registers power on to a random state > and unknowingly depended on brightness being forced to zero. This might happen, and is (AFAIK) the same for other LED drivers. > The other folks on the thread probably have some better view into this, > but I wonder if the safest option would be to adopt the "default-state" > property from leds/common.txt, and only forgo the call to led_pwm_set() > if the property is set to "keep". I think it would be sane to add support for parsing the default-state property to the LED core. I was surprised to learn that this for now is only implemented in a few LED drivers. > I did test this and can confirm that the LED's state prior to the driver > being loaded is preserved. However as you've warned, the brightness read > back from user space is zero and does not match the actual brightness of > the LED. That is something that the core should handle, too. If there is no .get_brightness callback and 0 is assumed, calling .set_brightness to 0 to ensure a right assumption sounds like the right thing to me. > Understanding that it's more work, I wonder if this patch is not safe if > we don't also add a brightness_get callback in case there were any cases > where user space makes some decisions based on brightness == 0? > > At any rate this patch does what is advertised, and so: > > Tested-by: Jeff LaBundy <jeff@xxxxxxxxxxx> Thanks for your feedback. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |
