All of lore.kernel.org
 help / color / mirror / Atom feed
From: Douglas Anderson <dianders@chromium.org>
To: Mark Brown <broonie@kernel.org>
Cc: Andrew Halaney <ahalaney@redhat.com>,
	Douglas Anderson <dianders@chromium.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: [RFT PATCH] regulator: core: Require regulator drivers to check uV for get_optimum_mode()
Date: Tue, 23 Aug 2022 13:16:34 -0700	[thread overview]
Message-ID: <20220823131629.RFT.1.I137e6bef4f6d517be7b081be926059321102fd3d@changeid> (raw)

The get_optimum_mode() for regulator drivers is passed the input
voltage and output voltage as well as the current. This is because, in
theory, the optimum mode can depend on all three things.

It turns out that for all regulator drivers in mainline only the
current is looked at when implementing get_optimum_mode(). None of the
drivers take the input or output voltage into account. Despite the
fact that none of the drivers take the input or output voltage into
account, though, the regulator framework will error out before calling
into get_optimum_mode() if it doesn't know the input or output
voltage.

The above behavior turned out to be a probelm for some boards when we
landed commit efb0cb50c427 ("regulator: qcom-rpmh: Implement
get_optimum_mode(), not set_load()"). Before that change we'd have no
problems running drms_uA_update() for RPMH regulators even if a
regulator's input or output voltage was unknown. After that change
drms_uA_update() started to fail. This is because typically boards
using RPMH regulators don't model the input supplies of RPMH
regulators. Input supplies for RPMH regulators nearly always come from
the output of other RPMH regulators (or always-on regulators) and RPMH
firmware is initialized with this knowledge and handles enabling (and
adjusting the voltage of) input supplies. While we could model the
parent/child relationship of the regulators in Linux, many boards
don't bother since it adds extra overhead.

Let's change the regulator core to make things work again. Now if we
fail to get the input or output voltage we'll still call into
get_optimum_mode() and we'll just pass error codes in for input_uV
and/or output_uV parameters.

Since no existing regulator drivers even look at input_uV and
output_uV we don't need to add this error handling anywhere right
now. We'll add some comments in the core so that it's obvious that (if
regulator drivers care) it's up to them to add the checks.

Reported-by: Andrew Halaney <ahalaney@redhat.com>
Fixes: efb0cb50c427 ("regulator: qcom-rpmh: Implement get_optimum_mode(), not set_load()")
Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 drivers/regulator/core.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 5b5da14976c2..0bc4b9b0a885 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -979,10 +979,13 @@ static int drms_uA_update(struct regulator_dev *rdev)
 	} else {
 		/* get output voltage */
 		output_uV = regulator_get_voltage_rdev(rdev);
-		if (output_uV <= 0) {
-			rdev_err(rdev, "invalid output voltage found\n");
-			return -EINVAL;
-		}
+
+		/*
+		 * Don't return an error; if regulator driver cares about
+		 * output_uV then it's up to the driver to validate.
+		 */
+		if (output_uV <= 0)
+			rdev_dbg(rdev, "invalid output voltage found\n");
 
 		/* get input voltage */
 		input_uV = 0;
@@ -990,10 +993,13 @@ static int drms_uA_update(struct regulator_dev *rdev)
 			input_uV = regulator_get_voltage(rdev->supply);
 		if (input_uV <= 0)
 			input_uV = rdev->constraints->input_uV;
-		if (input_uV <= 0) {
-			rdev_err(rdev, "invalid input voltage found\n");
-			return -EINVAL;
-		}
+
+		/*
+		 * Don't return an error; if regulator driver cares about
+		 * input_uV then it's up to the driver to validate.
+		 */
+		if (input_uV <= 0)
+			rdev_dbg(rdev, "invalid input voltage found\n");
 
 		/* now get the optimum mode for our new total regulator load */
 		mode = rdev->desc->ops->get_optimum_mode(rdev, input_uV,
-- 
2.37.2.609.g9ff673ca1a-goog


             reply	other threads:[~2022-08-23 20:35 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-23 20:16 Douglas Anderson [this message]
2022-08-23 22:14 ` [RFT PATCH] regulator: core: Require regulator drivers to check uV for get_optimum_mode() Andrew Halaney
2022-08-23 22:32   ` Doug Anderson
2022-08-23 22:49     ` Mark Brown
2022-08-24 21:24       ` 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=20220823131629.RFT.1.I137e6bef4f6d517be7b081be926059321102fd3d@changeid \
    --to=dianders@chromium.org \
    --cc=ahalaney@redhat.com \
    --cc=broonie@kernel.org \
    --cc=lgirdwood@gmail.com \
    --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 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.