linux-kernel - Re: [PATCH 2/7] regulator: core: Don't assume always_on when is

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 20 Nov 2018 10:17:32 -0800
From:   Doug Anderson <dianders@...omium.org>
To:     Mark Brown <broonie@...nel.org>
Cc:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        Evan Green <evgreen@...omium.org>,
        Stephen Boyd <swboyd@...omium.org>,
        Dmitry Osipenko <digetx@...il.com>, ryandcase@...omium.org,
        David Collins <collinsd@...eaurora.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/7] regulator: core: Don't assume always_on when
 is_enabled returns err

Hi,

On Tue, Nov 20, 2018 at 8:00 AM Mark Brown <broonie@...nel.org> 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