Re: [PATCH 4/4] video: fbdev: pxafb: Add support for lcd-supply regulator

Daniel Mack
2018-06-26T08:37:58Z

On Tuesday, June 26, 2018 10:27 AM, Robert Jarzmik wrote:
> Daniel Mack <daniel@zonque.org> writes:
>
>> + if (fbi->lcd_supply && fbi->lcd_supply_enabled != on) {
> Mmh this looks weird ...
> If lcd_supply_enabled = on, then the next block is never evaluated, and the
> value of "on" is not considered in order to call regulator_disable() ...

Hmm? This early bail just avoids unbalanced calls to the regulator core,
which doesn't like that at all. IOW, the rest of this function is only
executed if the desired supply state differs from our locally cached
version.

This also worked well in my tests. Am I missing something?

>> + int ret;
>> +
>> + if (on)
>> + ret = regulator_enable(fbi->lcd_supply);
>> + else
>> + ret = regulator_disable(fbi->lcd_supply);
>
> This apart, this was a change I was expecting for pxafb, one of the 2 in my
> backlog, which is great. The second one was linking a backlight ...

That should be done with devm_of_find_backlight() I figure, and not via
a regulator. But it's trivial to do as a separate patch, yes.

Thanks,
Daniel