Messages in this thread | | | Subject | Re: [PATCH v5 3/3] leds: Add control of the voltage/current regulator to the LED core | From | Jean-Jacques Hiblot <> | Date | Wed, 16 Oct 2019 10:13:54 +0200 |
| |
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.
| |