Re: [PATCH 3/4] backlight: add led-backlight driver

From: Jean-Jacques Hiblot
Date: Fri Jul 05 2019 - 06:33:52 EST


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.