All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marek Szyprowski <m.szyprowski@samsung.com>
To: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org,
	Mark Brown <broonie@kernel.org>,
	Dmitry Osipenko <digetx@gmail.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Lucas Stach <l.stach@pengutronix.de>,
	Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	Viresh Kumar <viresh.kumar@linaro.org>,
	peron.clem@gmail.com, Nishanth Menon <nm@ti.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Vincent Guittot <vincent.guittot@linaro.org>,
	Rafael Wysocki <rjw@rjwysocki.net>,
	linux-samsung-soc@vger.kernel.org,
	Chanwoo Choi <cw00.choi@samsung.com>
Subject: [PATCH] regulator: do not balance regulators without constraints
Date: Thu, 28 May 2020 15:11:30 +0200	[thread overview]
Message-ID: <20200528131130.17984-1-m.szyprowski@samsung.com> (raw)
In-Reply-To: CGME20200528131144eucas1p121b9151996fa3f780a5028f68c69d5ba@eucas1p1.samsung.com

Balancing coupled regulators must wait until the clients for all of the
coupled regualtors set their constraints, otherwise the balancing code
might change the voltage of the not-yet-constrained regulator to the
value below the bootloader-configured operation point, what might cause a
system crash.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---

This is probably a generalization of the issue aleady observed and
reported here:
https://lore.kernel.org/linux-samsung-soc/20191008101709.qVNy8eijBi0LynOteWFMnTg4GUwKG599n6OyYoX1Abs@z/
https://lore.kernel.org/lkml/20191017102758.8104-1-m.szyprowski@samsung.com/
https://lore.kernel.org/linux-pm/cover.1589528491.git.viresh.kumar@linaro.org/

The problem is with "vdd_int" regulator coupled with "vdd_arm" on Odroid
XU3/XU4 boards family. "vdd_arm" is handled by CPUfreq. "vdd_int" is
handled by devfreq. CPUfreq initialized quite early during boot and it
starts changing OPPs and "vdd_arm" value. Sometimes CPU activity during
boot goes down and some low-frequency OPPs are selected, what in turn
causes lowering "vdd_arm". This happens before devfreq applies its
requirements on "vdd_int". Regulator balancing code reduces "vdd_arm"
voltage value, what in turn causes lowering "vdd_int" value to the lowest
possible value. This is much below the operation point of the wcore bus,
which still runs at the highest frequency.

The issue was hard to notice because in the most cases the board managed
to boot properly, even when the regulator was set to lowest value allowed
by the regulator constraints. However, it caused some random issues,
which can be observed as "Unhandled prefetch abort" or low USB stability.

I know that adding more and more special cases to the generic code is not
the best idea, but so far I see no other way to fix this issue. The only
other solution that comes to my mind is admiting that it is not possible
to have generic regulator coupler and this needs board-specific code in
all cases. Such code might take care of those corner cases if they are
critical.

Best regards,
Marek Szyprowski
---
 drivers/regulator/core.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 941783a14b45..c1d77d44186b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3697,10 +3697,21 @@ static int regulator_balance_voltage(struct regulator_dev *rdev,
 			 * the coupled voltages.
 			 */
 			int optimal_uV = 0, optimal_max_uV = 0, current_uV = 0;
+			int cons_uV = 0, cons_max_uV = INT_MAX;
 
 			if (test_bit(i, &c_rdev_done))
 				continue;
 
+			ret = regulator_check_consumers(c_rdevs[i],
+						&cons_uV,
+						&cons_max_uV, state);
+			if (ret < 0)
+				goto out;
+
+			/* no constraints set - ignore */
+			if (cons_uV == 0)
+				continue;
+
 			ret = regulator_get_optimal_voltage(c_rdevs[i],
 							    &current_uV,
 							    &optimal_uV,
-- 
2.17.1


       reply	other threads:[~2020-05-28 13:11 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20200528131144eucas1p121b9151996fa3f780a5028f68c69d5ba@eucas1p1.samsung.com>
2020-05-28 13:11 ` Marek Szyprowski [this message]
2020-05-28 13:43   ` [PATCH] regulator: do not balance regulators without constraints Mark Brown
2020-05-29  5:45     ` Marek Szyprowski
2020-05-29 11:10       ` Mark Brown

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=20200528131130.17984-1-m.szyprowski@samsung.com \
    --to=m.szyprowski@samsung.com \
    --cc=b.zolnierkie@samsung.com \
    --cc=broonie@kernel.org \
    --cc=cw00.choi@samsung.com \
    --cc=digetx@gmail.com \
    --cc=krzk@kernel.org \
    --cc=l.stach@pengutronix.de \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=nm@ti.com \
    --cc=peron.clem@gmail.com \
    --cc=rjw@rjwysocki.net \
    --cc=sboyd@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.