From: Brian Masney <masneyb@onstation.org>
To: Douglas Anderson <dianders@chromium.org>
Cc: Mark Brown <broonie@kernel.org>,
linux-arm-msm@vger.kernel.org, Dmitry Osipenko <digetx@gmail.com>,
evgreen@chromium.org, Liam Girdwood <lgirdwood@gmail.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] regulator: core: Clean enabling always-on regulators + their supplies
Date: Thu, 6 Dec 2018 20:31:01 -0500 [thread overview]
Message-ID: <20181207013101.GA27539@basecamp> (raw)
In-Reply-To: <20181206222318.240401-1-dianders@chromium.org>
On Thu, Dec 06, 2018 at 02:23:18PM -0800, Douglas Anderson wrote:
> At the end of regulator_resolve_supply() we have historically turned
> on our supply in some cases. This could be for one of two reasons:
>
> 1. If resolving supplies was happening before the call to
> set_machine_constraints() we needed to predict if
> set_machine_constraints() was going to turn the regulator on and we
> needed to preemptively turn the supply on.
> 2. Maybe set_machine_constraints() happened before we could resolve
> supplies (because we failed the first time to resolve) and thus we
> might need to propagate an enable that already happened up to our
> supply.
>
> Historically regulator_resolve_supply() used _regulator_is_enabled()
> to decide whether to turn on the supply.
>
> Let's change things a little bit. Specifically:
>
> 1. Let's try to enable the supply and the regulator in the same place,
> both in set_machine_constraints(). This means that we have exactly
> the same logic for enabling the supply and the regulator.
> 2. Let's properly set use_count when we enable always-on or boot-on
> regulators even for those that don't have supplies. The previous
> commit 1fc12b05895e ("regulator: core: Avoid propagating to
> supplies when possible") only did this right for regulators with
> supplies.
> 3. Let's make it clear that the only time we need to enable the supply
> in regulator_resolve_supply() is if the main regulator is currently
> in use. By using use_count (like the rest of the code) to decide
> if we're going to enable our supply we keep everything consistent.
>
> Overall the new scheme should be cleaner and easier to reason about.
> In addition to fixing regulator_summary to be more correct (because of
> the more correct use_count), this change also has the effect of no
> longer using _regulator_is_enabled() in this code path.
> _regulator_is_enabled() could return an error code for some regulators
> at bootup (like RPMh) that can't read their initial state. While one
> can argue that the design of those regulators is sub-optimal, the new
> logic sidesteps this brokenness. This fix in particular fixes
> observed problems on Qualcomm sdm845 boards which use the
> above-mentioned RPMh regulator. Those problems were made worse by
> commit 1fc12b05895e ("regulator: core: Avoid propagating to supplies
> when possible") because now we'd think at bootup that the SD
> regulators were already enabled and we'd never try them again.
>
> Fixes: 1fc12b05895e ("regulator: core: Avoid propagating to supplies when possible")
> Reported-by: Evan Green <evgreen@chromium.org>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
Looks good on the Nexus 5 (qcom msm8974).
Tested-by: Brian Masney <masneyb@onstation.org>
next prev parent reply other threads:[~2018-12-07 1:31 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-06 22:23 [PATCH] regulator: core: Clean enabling always-on regulators + their supplies Douglas Anderson
2018-12-07 1:31 ` Brian Masney [this message]
2018-12-10 15:43 ` Mark Brown
2018-12-10 16:32 ` Doug Anderson
2018-12-11 0:52 ` Mark Brown
2018-12-11 1:00 ` Mark Brown
2018-12-11 2:40 ` Doug Anderson
2018-12-11 14:06 ` Mark Brown
2018-12-11 16:21 ` Doug Anderson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181207013101.GA27539@basecamp \
--to=masneyb@onstation.org \
--cc=broonie@kernel.org \
--cc=dianders@chromium.org \
--cc=digetx@gmail.com \
--cc=evgreen@chromium.org \
--cc=lgirdwood@gmail.com \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).