lkml.org 
[lkml]   [2018]   [Dec]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
SubjectApplied "regulator: core: Clean enabling always-on regulators + their supplies" to the regulator tree
Date
The patch

regulator: core: Clean enabling always-on regulators + their supplies

has been applied to the regulator tree at

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

From 4c9ed27f79fe4fb46db42dd0c6e69328371f2f3d Mon Sep 17 00:00:00 2001
From: Douglas Anderson <dianders@chromium.org>
Date: Tue, 11 Dec 2018 08:17:19 -0800
Subject: [PATCH] regulator: core: Clean enabling always-on regulators + their
supplies

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>
Tested-by: Brian Masney <masneyb@onstation.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/regulator/core.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 4981ed71686d..925fa0f5d7cb 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1356,11 +1356,21 @@ static int set_machine_constraints(struct regulator_dev *rdev,
* and we have control then make sure it is enabled.
*/
if (rdev->constraints->always_on || rdev->constraints->boot_on) {
+ if (rdev->supply) {
+ ret = regulator_enable(rdev->supply);
+ if (ret < 0) {
+ _regulator_put(rdev->supply);
+ rdev->supply = NULL;
+ return ret;
+ }
+ }
+
ret = _regulator_do_enable(rdev);
if (ret < 0 && ret != -EINVAL) {
rdev_err(rdev, "failed to enable\n");
return ret;
}
+ rdev->use_count++;
}

if ((rdev->constraints->ramp_delay || rdev->constraints->ramp_disable)
@@ -1822,15 +1832,18 @@ static int regulator_resolve_supply(struct regulator_dev *rdev)
return ret;
}

- /* Cascade always-on state to supply */
- if (_regulator_is_enabled(rdev)) {
+ /*
+ * In set_machine_constraints() we may have turned this regulator on
+ * but we couldn't propagate to the supply if it hadn't been resolved
+ * yet. Do it now.
+ */
+ if (rdev->use_count) {
ret = regulator_enable(rdev->supply);
if (ret < 0) {
_regulator_put(rdev->supply);
rdev->supply = NULL;
return ret;
}
- rdev->use_count = 1;
}

return 0;
--
2.19.0.rc2
\
 
 \ /
  Last update: 2018-12-11 21:38    [W:0.874 / U:0.168 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site