linux-kernel - Re: [PATCH RFC 1/2] regulator: core: Disable unused regulators with unknown status

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZSRg1H772gMTl-d3@gerhold.net>
Date:   Mon, 9 Oct 2023 22:21:40 +0200
From:   Stephan Gerhold <stephan@...hold.net>
To:     Konrad Dybcio <konrad.dybcio@...aro.org>
Cc:     Stephan Gerhold <stephan.gerhold@...nkonzept.com>,
        Mark Brown <broonie@...nel.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH RFC 1/2] regulator: core: Disable unused regulators with
 unknown status

On Fri, Oct 06, 2023 at 11:11:48PM +0200, Konrad Dybcio wrote:
> On 4.10.2023 16:17, Stephan Gerhold wrote:
> > Some regulator drivers do not provide a way to check if the regulator is
> > currently enabled or not. That does not necessarily mean that the
> > regulator is always-on. For example, the regulators managed by the RPM
> > firmware on Qualcomm platforms can be either on or off during boot but
> > the initial state is not known. To sync the state the regulator should
> > get either explicitly enabled or explicitly disabled.
> > 
> > Enabling all regulators unconditionally is not safe, because we might
> > not know which voltages are safe. The devices supplied by those
> > regulators might also require a special power-up sequence where the
> > regulators are turned on in a certain order or with specific delay.
> > 
> > Disabling all unused regulators is safer. If the regulator is already
> > off it will just stay that way. If the regulator is on, disabling it
> > explicitly allows the firmware to turn it off for reduced power
> > consumption.
> > 
> > The regulator core already has functionality for disabling unused
> > regulators. However, at the moment it assumes that all regulators where
> > the .is_enabled() callback fails are actually off. There is no way to
> > return a special value for the "unknown" state to explicitly ask for
> > disabling those regulators.
> > 
> > Some drivers (e.g. qcom-rpmh-regulator.c) return -EINVAL for the case
> > where the initial status is unknown. Use that return code to assume the
> > initial status is unknown and try to explicitly disable the regulator
> > in that case.
> > 
> > Signed-off-by: Stephan Gerhold <stephan.gerhold@...nkonzept.com>
> > ---
> > Instead of -EINVAL we could also use a different return code to indicate
> > the initial status is unknown. Or maybe there is some other option that
> > would be easier? This is working for me but I'm sending it as RFC to get
> > more feedback. :)
>
> -EOPNOTSUPP for "doesn't support getting is_enabled state"?
> 

The way it is implemented right now the Qualcomm SMD RPM regulator does
actually support getting the .is_enabled() state. It is only unable to
determine the initial state during boot. Once the regulator has been
enabled by some consumer for the first time the .is_enabled() callback
starts returning the expected results.

Typically -EOPNOTSUPP is used when the driver callback (or similar) is
not implemented at all. I'm not sure if using -EOPNOTSUPP for the
"temporarily unable to determine state" purpose would be misleading.

Thanks,
Stephan