linux-kernel - [PATCH] clk: qcom: gdsc: Disable HW control until supported

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230112135224.3837820-1-quic_bjorande@quicinc.com>
Date:   Thu, 12 Jan 2023 05:52:24 -0800
From:   Bjorn Andersson <quic_bjorande@...cinc.com>
To:     Bjorn Andersson <andersson@...nel.org>,
        Andy Gross <agross@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Taniya Das <quic_tdas@...cinc.com>,
        Abel Vesa <abel.vesa@...aro.org>
CC:     <linux-arm-msm@...r.kernel.org>, <linux-clk@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: [PATCH] clk: qcom: gdsc: Disable HW control until supported

Software normally uses the SW_COLLAPSE bit to collapse a GDSC, but in
some scenarios it's beneficial to let the hardware perform this without
software intervention.

This is done by configuring the GDSC in "hardware control" state, in
which case the SW_COLLAPSE bit is ignored and some hardware signal is
relies upon instead.

The GDSCs are modelled as power-domains in Linux and as such it's
reasonable to assume that the device drivers intend for the hardware
block to be accessible when their power domain is active.

But in the current implementation, any GDSC that is marked to support
hardware control, gets hardware control unconditionally while the
client driver requests it to be active. It's therefor conceivable that
the hardware collapses a GDSC while Linux is accessing resources
depending on it.

There are ongoing discussions about how to properly expose this control
to the client drivers, but until conclusion in that discussion is
reached, the safer option would be to keep the GDSC in software control
mode.

Signed-off-by: Bjorn Andersson <quic_bjorande@...cinc.com>
---
 drivers/clk/qcom/gdsc.c | 48 ++++++-----------------------------------
 1 file changed, 7 insertions(+), 41 deletions(-)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 9e4d6ce891aa..6d3b36a52a48 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -291,22 +291,6 @@ static int gdsc_enable(struct generic_pm_domain *domain)
 	 */
 	udelay(1);
 
-	/* Turn on HW trigger mode if supported */
-	if (sc->flags & HW_CTRL) {
-		ret = gdsc_hwctrl(sc, true);
-		if (ret)
-			return ret;
-		/*
-		 * Wait for the GDSC to go through a power down and
-		 * up cycle.  In case a firmware ends up polling status
-		 * bits for the gdsc, it might read an 'on' status before
-		 * the GDSC can finish the power cycle.
-		 * We wait 1us before returning to ensure the firmware
-		 * can't immediately poll the status bits.
-		 */
-		udelay(1);
-	}
-
 	if (sc->flags & RETAIN_FF_ENABLE)
 		gdsc_retain_ff_on(sc);
 
@@ -321,24 +305,6 @@ static int gdsc_disable(struct generic_pm_domain *domain)
 	if (sc->pwrsts == PWRSTS_ON)
 		return gdsc_assert_reset(sc);
 
-	/* Turn off HW trigger mode if supported */
-	if (sc->flags & HW_CTRL) {
-		ret = gdsc_hwctrl(sc, false);
-		if (ret < 0)
-			return ret;
-		/*
-		 * Wait for the GDSC to go through a power down and
-		 * up cycle.  In case we end up polling status
-		 * bits for the gdsc before the power cycle is completed
-		 * it might read an 'on' status wrongly.
-		 */
-		udelay(1);
-
-		ret = gdsc_poll_status(sc, GDSC_ON);
-		if (ret)
-			return ret;
-	}
-
 	if (sc->pwrsts & PWRSTS_OFF)
 		gdsc_clear_mem_on(sc);
 
@@ -419,13 +385,6 @@ static int gdsc_init(struct gdsc *sc)
 				goto err_disable_supply;
 		}
 
-		/* Turn on HW trigger mode if supported */
-		if (sc->flags & HW_CTRL) {
-			ret = gdsc_hwctrl(sc, true);
-			if (ret < 0)
-				goto err_disable_supply;
-		}
-
 		/*
 		 * Make sure the retain bit is set if the GDSC is already on,
 		 * otherwise we end up turning off the GDSC and destroying all
@@ -439,6 +398,13 @@ static int gdsc_init(struct gdsc *sc)
 		on = true;
 	}
 
+	/* Disable HW trigger mode until propertly supported */
+	if (sc->flags & HW_CTRL) {
+		ret = gdsc_hwctrl(sc, false);
+		if (ret < 0)
+			return ret;
+	}
+
 	if (on || (sc->pwrsts & PWRSTS_RET))
 		gdsc_force_mem_on(sc);
 	else
-- 
2.37.3