Re: [PATCH v5 3/3] leds: Add control of the voltage/current regulator to the LED core

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 14/10/2019 20:48, Jacek Anaszewski wrote:
On 10/14/19 2:38 PM, Daniel Thompson wrote:
On Mon, Oct 14, 2019 at 12:49:07PM +0200, Jean-Jacques Hiblot wrote:
On 13/10/2019 14:09, Pavel Machek wrote:
Hi!

I must say I'm not a big fan of this change.
It adds a bunch of code to the LED core and gives small
functionality in a reward. It may also influence maximum
software blinking rate, so I'd rather avoid calling
regulator_enable/disable when timer trigger is set.

It will of course require more code.

Since AFAIR Pavel was original proponent of this change then
I'd like to see his opinion before we move on to discussing
possible improvements to this patch.
Was I?

Okay, this series looks quite confusing to me. First, 1/3 looks
"interesting" (would have to analyze it way more).

Second, 3/3... So we have a LED driver _and_ a regulator? So yes, the
chip driving a LED is usually ... voltage/current regulator. What is
second regulator doing there? Is that a common setup?
This is quite common with current-sink LED drivers.

The setup looks like this:

+-----------+
|           |
| Regulator |
|           +------------------------+
|           |                        |
+-----------+                      __|__
                                    \   /
          +---------------------+    \ / led
          |                     |     V
          |    Led Driver       |   --+--
          |                     |     |
          |                     |     |
          |                +----------+
          |              \      |
          |               \     |
          |                +    |
          |                |    |
          +---------------------+
                           |
                        +--+--+
                        ///////


Only the regulator usually does not supply only one LED.
Given questions have been raised about the complexity of the change I
wondered whether, for a system with this topology, the regulator
could/should be enabled when the LED driver driver probes?
And this is how are doing that now.

Moreover, just after seeing your ASCII art it has become obvious to me
that we can't simply do regulator_disable() while setting brightness to
LED_OFF since it can result in powering off the LED controller, which
would need to be reconfigured on next transition to
brightness > LED_OFF.

That is a problem only if the LED driver is powered by the same regulator, which is not the case in the diagram.

This series make sense only for boards where LEDs have a dedicated voltage rail or can be modeled this way.

My use case is a LED panel driven by a LED current-sink controller that uses both a PWM-style control for brightness AND a active-low enable pin. If the enable pin is not HIGH, the panel is never completely dark even if the LED brightness is set to 0. I modeled this switch with a fixed-voltage regulator  which is a common way of doing it (it is after all how this things is done inside the panel).

JJ
It looks that the idea is flawed I'm afraid.



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux