[PATCH 0/2] clk: qcom: gdsc: Update retain_ff sequence and timeout for GDSC

[PATCH 0/2] clk: qcom: gdsc: Update retain_ff sequence and timeout for GDSC

[Taniya Das]

The retain_ff bit should be updated for a GDSC when it is under SW
control and ON. The current sequence needs to be fixed as the GDSC
needs to update retention and is moved to HW control which does not
guarantee the GDSC to be in enabled state.

During the GDSC FSM state, the GDSC hardware waits for an ACK and the
timeout for the ACK is 2000us as per design requirements.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
Taniya Das (2):
      clk: qcom: gdsc: Set retain_ff before moving to HW CTRL
      clk: qcom: gdsc: Update the status poll timeout for GDSC

 drivers/clk/qcom/gdsc.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)
---
base-commit: c674aa7c289e51659e40dda0f954886ef7f80042
change-id: 20250212-gdsc_fixes-77e8b8e27e2f

Best regards,
-- 
Taniya Das <quic_tdas@quicinc.com>

[PATCH 1/2] clk: qcom: gdsc: Set retain_ff before moving to HW CTRL

[Taniya Das]

Enable the retain_ff_enable bit of GDSCR only if the GDSC is already ON.
Once the GDSCR moves to HW control, SW no longer can determine the state
of the GDSCR and setting the retain_ff bit could destroy all the register
contents we intended to save.
Therefore, move the retain_ff configuration before switching the GDSC to
HW trigger mode.

Cc: stable@vger.kernel.org
Fixes: 173722995cdb ("clk: qcom: gdsc: Add support to enable retention of GSDCR")
Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
 drivers/clk/qcom/gdsc.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index fa5fe4c2a2ee7786c2e8858f3e41301f639e5d59..1fd40b755082a4658cca581e920635e1bdfbc922 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -292,6 +292,9 @@ static int gdsc_enable(struct generic_pm_domain *domain)
 	 */
 	udelay(1);
 
+	if (sc->flags & RETAIN_FF_ENABLE)
+		gdsc_retain_ff_on(sc);
+
 	/* Turn on HW trigger mode if supported */
 	if (sc->flags & HW_CTRL) {
 		ret = gdsc_hwctrl(sc, true);
@@ -308,9 +311,6 @@ static int gdsc_enable(struct generic_pm_domain *domain)
 		udelay(1);
 	}
 
-	if (sc->flags & RETAIN_FF_ENABLE)
-		gdsc_retain_ff_on(sc);
-
 	return 0;
 }
 
@@ -457,13 +457,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
@@ -471,6 +464,14 @@ static int gdsc_init(struct gdsc *sc)
 		 */
 		if (sc->flags & RETAIN_FF_ENABLE)
 			gdsc_retain_ff_on(sc);
+
+		/* Turn on HW trigger mode if supported */
+		if (sc->flags & HW_CTRL) {
+			ret = gdsc_hwctrl(sc, true);
+			if (ret < 0)
+				goto err_disable_supply;
+		}
+
 	} else if (sc->flags & ALWAYS_ON) {
 		/* If ALWAYS_ON GDSCs are not ON, turn them ON */
 		gdsc_enable(&sc->pd);

-- 
2.48.1

[PATCH 2/2] clk: qcom: gdsc: Update the status poll timeout for GDSC

[Taniya Das]

During the GDSC FSM state, the GDSC hardware waits for an ACK from the
respective subsystem core. In some scenarios, this ACK can be delayed.
To handle such delays, increase the GDSC status poll timeout from 1500us
to 2000us as per the design recommendation.

Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
---
 drivers/clk/qcom/gdsc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 1fd40b755082a4658cca581e920635e1bdfbc922..2e3128ed16c4d78f1bca799b4e716e9034b4a6b9 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -46,7 +46,7 @@
 #define RETAIN_MEM		BIT(14)
 #define RETAIN_PERIPH		BIT(13)
 
-#define STATUS_POLL_TIMEOUT_US	1500
+#define STATUS_POLL_TIMEOUT_US	2000
 #define TIMEOUT_US		500
 
 #define domain_to_gdsc(domain) container_of(domain, struct gdsc, pd)

-- 
2.48.1

Re: [PATCH 1/2] clk: qcom: gdsc: Set retain_ff before moving to HW CTRL

[Imran Shaik]

On 2/14/2025 9:56 AM, Taniya Das wrote:
> Enable the retain_ff_enable bit of GDSCR only if the GDSC is already ON.
> Once the GDSCR moves to HW control, SW no longer can determine the state
> of the GDSCR and setting the retain_ff bit could destroy all the register
> contents we intended to save.
> Therefore, move the retain_ff configuration before switching the GDSC to
> HW trigger mode.
> 
> Cc: stable@vger.kernel.org
> Fixes: 173722995cdb ("clk: qcom: gdsc: Add support to enable retention of GSDCR")
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> ---
>  drivers/clk/qcom/gdsc.c | 21 +++++++++++----------
>  1 file changed, 11 insertions(+), 10 deletions(-)
> 

Reviewed-by: Imran Shaik <quic_imrashai@quicinc.com>
Tested-by: Imran Shaik <quic_imrashai@quicinc.com> # on QCS8300

Re: [PATCH 2/2] clk: qcom: gdsc: Update the status poll timeout for GDSC

[Imran Shaik]

On 2/14/2025 9:57 AM, Taniya Das wrote:
> During the GDSC FSM state, the GDSC hardware waits for an ACK from the
> respective subsystem core. In some scenarios, this ACK can be delayed.
> To handle such delays, increase the GDSC status poll timeout from 1500us
> to 2000us as per the design recommendation.
> 
> Signed-off-by: Taniya Das <quic_tdas@quicinc.com>
> ---
>  drivers/clk/qcom/gdsc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Imran Shaik <quic_imrashai@quicinc.com>
Tested-by: Imran Shaik <quic_imrashai@quicinc.com> # on QCS8300

Re: [PATCH 0/2] clk: qcom: gdsc: Update retain_ff sequence and timeout for GDSC

[Bjorn Andersson]

On Fri, 14 Feb 2025 09:56:58 +0530, Taniya Das wrote:
> The retain_ff bit should be updated for a GDSC when it is under SW
> control and ON. The current sequence needs to be fixed as the GDSC
> needs to update retention and is moved to HW control which does not
> guarantee the GDSC to be in enabled state.
> 
> During the GDSC FSM state, the GDSC hardware waits for an ACK and the
> timeout for the ACK is 2000us as per design requirements.
> 
> [...]

Applied, thanks!

[1/2] clk: qcom: gdsc: Set retain_ff before moving to HW CTRL
      commit: 25708f73ff171bb4171950c9f4be5aa8504b8459
[2/2] clk: qcom: gdsc: Update the status poll timeout for GDSC
      commit: 172320f5ead5d1a0eed14472ce84146221c75675

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>