Re: [PATCH 2/7] regulator: core: Don't assume always_on when is_enabled returns err

[Doug Anderson]

Hi,

On Tue, Nov 20, 2018 at 8:00 AM Mark Brown <broonie@xxxxxxxxxx> wrote:
>
> On Mon, Nov 19, 2018 at 04:26:49PM -0800, Douglas Anderson wrote:
>
> > At boot sometimes regulators (like qcom-rpmh-regulator) will return
> > -EINVAL if we don't know the enable state of the regulator.  We
> > shouldn't take this to mean that the regulator is an always-on
> > regulator, but that was what was happening since "-EINVAL" is non-zero
> > and a few places in the code were not properly checking for errors.
>
> This still results in breakage - if the regulator fails to read when
> we're initializing then we won't flag the regualtor as always_on, the
> regulator would need to tell the framework when it becomes readable.

Hrm.  Are you sure it's broken?  I'll admit that I'm a little fuzzy
about why a regulator that happens to be enabled when its supplies get
resolved should be considered always-on.  Can you give me a hint about
why it works that way?

My best guess was that the code was assuming that when we were
resolving supplies it was super early and the only way the regulator
could be enabled at that point is if either:
A) the regulator ops have no is_enabled()
B) we called _regulator_do_enable() in set_machine_constraints()

If that's true then my patch definitely helps things.  Case A) isn't
an issue for qcom-rpmh-regulator.  For case B) once we see the first
call to set the enable state of the regulator then we'll definitely
return 1 from is_enabled.  That means we'll be able to tell the
difference between a regulator that should be considered always-on and
one that can't.

Did that make sense, or am I spouting nonsense again?


> This hardware really is fragile :(

No kidding.

-Doug