Messages in this thread | | | Subject | Re: [PATCH 3/4] backlight: add led-backlight driver | From | Jean-Jacques Hiblot <> | Date | Fri, 5 Jul 2019 12:33:33 +0200 |
| |
Pavel
On 05/07/2019 12:08, Pavel Machek wrote: > Hi! > >>>>> Also still relevant is whether the LED device is being correctly >>>>> modelled if the act of turning on the LED doesn't, in fact, turn the LED >>>>> on. Is it *really* a correct implementation of an LED device that >>>>> setting it to LED_FULL using sysfs doesn't cause it to light up? >>>> What I understood from the discussion between Rob and Tomi is that the >>>> child-node of the LED controller should be considered a backlight device, >>>> not a simple LED. I'm not sure if the sysfs interface is still relevant in >>>> that case. Maybe it should just be disabled by the backlight driver >>>> (possible with led_sysfs_disable()) >>> led_sysfs_disable() sounds like a sensible change but that's not quite >>> what I mean. >>> >>> It is more a thought experiment to see if the power control *should* be >>> implemented by the backlight. Consider what happens if we *don't* >>> enable CONFIG_BACKLIGHT_LED in the kernel: we would still have an LED >>> device and it would not work correctly. >>> >>> In other words I naively expect turning on an LED using the LED API >>> (any of them, sysfs or kernel) to result in the LED turning on. >>> Implementing a workaround in the client for what appears to be >>> something missing in the LED driver strikes me as odd. Why shouldn't >>> the regulator be managed in the LED driver? >> I see your point. Indeed having the regulator handled in the LED-core makes >> sense in a lot of situations >> >> I'll think about it. > For the record, I also believe regulator and enable gpio should be > handled in the core.
I am working on adding the regulator to the led core.
I don't really want to add a GPIO enable to the core though. If needed it can be described as a GPIO-enabled regulator up(/down)stream the regular regulator.
JJ
> > Pavel > PS please trim down the quoted text.
| |