Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
From:   Stephan Gerhold <stephan.gerhold@kernkonzept.com>
Date:   Wed, 04 Oct 2023 16:17:17 +0200
Subject: [PATCH RFC 1/2] regulator: core: Disable unused regulators with
 unknown status
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
Message-Id: <20231004-reg-smd-unused-v1-1-5d682493d555@kernkonzept.com>
References: <20231004-reg-smd-unused-v1-0-5d682493d555@kernkonzept.com>
In-Reply-To: <20231004-reg-smd-unused-v1-0-5d682493d555@kernkonzept.com>
To:     Mark Brown <broonie@kernel.org>
Cc:     Liam Girdwood <lgirdwood@gmail.com>,
        Andy Gross <agross@kernel.org>,
        Bjorn Andersson <andersson@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        Stephan Gerhold <stephan@gerhold.net>,
        Stephan Gerhold <stephan.gerhold@kernkonzept.com>
Precedence: bulk

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@kernkonzept.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. :)
---
 drivers/regulator/core.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 3137e40fcd3e..182e3727651a 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -6207,8 +6207,13 @@ static int regulator_late_cleanup(struct device *dev, void *data)
 	if (rdev->use_count)
 		goto unlock;
 
-	/* If reading the status failed, assume that it's off. */
-	if (_regulator_is_enabled(rdev) <= 0)
+	/*
+	 * If reading the status failed, assume that it's off.
+	 * If the current status is unknown (-EINVAL), assume that the
+	 * regulator might be on and try to explicitly disable it.
+	 */
+	ret = _regulator_is_enabled(rdev);
+	if (ret <= 0 && ret != -EINVAL)
 		goto unlock;
 
 	if (have_full_constraints()) {

-- 
2.39.2