Skip to contentSkip to conversation

Update reset and poll logic for GDSCs

PATCH Applied Taniya Das 2 participants 7 messages Apr 9, 2018 – Apr 17, 2018

Subsystems (3)
Arm Qualcomm mailing list Common clk Qualcomm clock drivers
lore
Share & tools
lore

Taniya Das

author
#0 lore raw

v2 cover Update reset and poll logic for GDSCs

[v2]
* Addressed review comments given in v1 series

This series implements the below logic for the GDSCs

1. logic to reset the AON logic before or assert/deassert the block control reset removing the clamp io for few GDSCs on SDM845 SoC.
2. It also introduces the requirement to poll for higher timeout values for few of the GDSCs.
3. There is a new poll register for the GDSCs on SDM845 SoCs which needs to be polled for the correct hardware status of the GDSCs.

Amit Nischal (3):
clk: qcom: gdsc: Add support to reset AON and block reset logic
clk: qcom: gdsc: Add support to poll for higher timeout value
clk: qcom: gdsc: Add support to poll CFG register to check GDSC state

 drivers/clk/qcom/gdsc.c | 63 +++++++++++++++++++++++++++++++++++++++++++------
 drivers/clk/qcom/gdsc.h |  5 +++-
 2 files changed, 60 insertions(+), 8 deletions(-)

--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.

author
#1 lore raw

PATCH v2 1/3 clk: qcom: gdsc: Add support to reset AON and block reset logic

From: Amit Nischal <[email protected]>

For some of the gdsc power domains, there could be need to reset the AON logic or assert/deassert the block control reset before removing the clamp_io. Add support for the same by introducing new flags SW_RESET and AON_RESET. Both SW reset and AON reset requires to be asserted for at least 1us before being de-asserted.

Signed-off-by Taniya Das <[email protected]>
Signed-off-by Amit Nischal <[email protected]>
---
 drivers/clk/qcom/gdsc.c | 22 ++++++++++++++++++++--
 drivers/clk/qcom/gdsc.h |  4 +++-
 2 files changed, 23 insertions(+), 3 deletions(-)
drivers/clk/qcom/gdsc.c +20 -2
index a4f3580..266fefa 100644
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2015, 2017-2018, The Linux Foundation. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 and

@@ -31,6 +31,7 @@
 #define HW_CONTROL_MASK		BIT(1)
 #define SW_COLLAPSE_MASK	BIT(0)
 #define GMEM_CLAMP_IO_MASK	BIT(0)
+#define GMEM_RESET_MASK		BIT(4)
 
 /* Wait 2^n CXO cycles between all states. Here, n=2 (4 cycles). */
 #define EN_REST_WAIT_VAL	(0x2 << 20)

@@ -166,6 +167,14 @@ static inline void gdsc_assert_clamp_io(struct gdsc *sc)
 			   GMEM_CLAMP_IO_MASK, 1);
 }
 
+static inline void gdsc_assert_reset_aon(struct gdsc *sc)
+{
+	regmap_update_bits(sc->regmap, sc->clamp_io_ctrl,
+			   GMEM_RESET_MASK, 1);
+	udelay(1);
+	regmap_update_bits(sc->regmap, sc->clamp_io_ctrl,
+			   GMEM_RESET_MASK, 0);
+}
 static int gdsc_enable(struct generic_pm_domain *domain)
 {
 	struct gdsc *sc = domain_to_gdsc(domain);

@@ -174,8 +183,17 @@ static int gdsc_enable(struct generic_pm_domain *domain)
 	if (sc->pwrsts == PWRSTS_ON)
 		return gdsc_deassert_reset(sc);
 
-	if (sc->flags & CLAMP_IO)
+	if (sc->flags & SW_RESET) {
+		gdsc_assert_reset(sc);
+		udelay(1);
+		gdsc_deassert_reset(sc);
+	}
+
+	if (sc->flags & CLAMP_IO) {
+		if (sc->flags & AON_RESET)
+			gdsc_assert_reset_aon(sc);
 		gdsc_deassert_clamp_io(sc);
+	}
 
 	ret = gdsc_toggle_logic(sc, true);
 	if (ret)
drivers/clk/qcom/gdsc.h +3 -1
index 3964834..9279278 100644
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2015, 2017-2018, The Linux Foundation. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 and

@@ -53,6 +53,8 @@ struct gdsc {
 #define VOTABLE		BIT(0)
 #define CLAMP_IO	BIT(1)
 #define HW_CTRL		BIT(2)
+#define SW_RESET	BIT(3)
+#define AON_RESET	BIT(4)
 	struct reset_controller_dev	*rcdev;
 	unsigned int			*resets;
 	unsigned int			reset_count;

--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.

maintainer Applied
#2 lore raw

Re: [PATCH v2 1/3] clk: qcom: gdsc: Add support to reset AON and block reset logic

Quoting Taniya Das (2018-04-09 01:41:44)

Quoted 11 lines

From: Amit Nischal <[email protected]>

For some of the gdsc power domains, there could be need to reset the AON logic or assert/deassert the block control reset before removing the clamp_io. Add support for the same by introducing new flags SW_RESET and AON_RESET. Both SW reset and AON reset requires to be asserted for at least 1us before being de-asserted.

Signed-off-by Taniya Das <[email protected]>
Signed-off-by Amit Nischal <[email protected]>

---

Applied to clk-next

author
#3 lore raw

PATCH v2 2/3 clk: qcom: gdsc: Add support to poll for higher timeout value

In reply to Taniya Das

From: Amit Nischal <[email protected]>

For some gdscs, it might take longer time up to 500us for updating their status. Update the timeout value for all GDSC polling status.

Signed-off-by Amit Nischal <[email protected]>
Signed-off-by Taniya Das <[email protected]>
---
 drivers/clk/qcom/gdsc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/clk/qcom/gdsc.c +1 -1
index 266fefa..cb61c15 100644
@@ -41,7 +41,7 @@
 #define RETAIN_MEM		BIT(14)
 #define RETAIN_PERIPH		BIT(13)
 
-#define TIMEOUT_US		100
+#define TIMEOUT_US		500
 
 #define domain_to_gdsc(domain) container_of(domain, struct gdsc, pd)

--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.

author
#5 lore raw

PATCH v2 3/3 clk: qcom: gdsc: Add support to poll CFG register to check GDSC state

In reply to Taniya Das

From: Amit Nischal <[email protected]>

The default behavior of the GDSC enable/disable sequence is to poll the status bits of either the actual GDSCR or the corresponding HW_CTRL registers.

On targets which have support for a CFG_GDSCR register, the status bits might not show the correct state of the GDSC, especially in the disable sequence, where the status bit will be cleared even before the core is completely power collapsed. On targets with this issue, poll the power on/off bits in the CFG_GDSCR register instead to correctly determine the GDSC state.

Signed-off-by Amit Nischal <[email protected]>
Signed-off-by Taniya Das <[email protected]>
---
 drivers/clk/qcom/gdsc.c | 39 +++++++++++++++++++++++++++++++++++----
 drivers/clk/qcom/gdsc.h |  1 +
 2 files changed, 36 insertions(+), 4 deletions(-)
drivers/clk/qcom/gdsc.c +35 -4
index cb61c15..2dda2d5 100644
@@ -33,6 +33,11 @@
 #define GMEM_CLAMP_IO_MASK	BIT(0)
 #define GMEM_RESET_MASK		BIT(4)
 
+/* CFG_GDSCR */
+#define GDSC_POWER_UP_COMPLETE		BIT(16)
+#define GDSC_POWER_DOWN_COMPLETE	BIT(15)
+#define CFG_GDSCR_OFFSET		0x4
+
 /* Wait 2^n CXO cycles between all states. Here, n=2 (4 cycles). */
 #define EN_REST_WAIT_VAL	(0x2 << 20)
 #define EN_FEW_WAIT_VAL		(0x8 << 16)

@@ -64,18 +69,43 @@ static int gdsc_hwctrl(struct gdsc *sc, bool en)
 	return regmap_update_bits(sc->regmap, sc->gdscr, HW_CONTROL_MASK, val);
 }
 
+static int gdsc_is_enabled_by_poll_cfg_reg(struct gdsc *sc, bool en)
+{
+	u32 val;
+	int ret;
+
+	ret = regmap_read(sc->regmap, sc->gdscr + CFG_GDSCR_OFFSET, &val);
+	if (ret)
+		return ret;
+
+	if (en)
+		return !!(val & GDSC_POWER_UP_COMPLETE);
+
+	return !(val & GDSC_POWER_DOWN_COMPLETE);
+}
+
 static int gdsc_poll_status(struct gdsc *sc, unsigned int reg, bool en)
 {
 	ktime_t start;
 
 	start = ktime_get();
 	do {
-		if (gdsc_is_enabled(sc, reg) == en)
-			return 0;
+		if (sc->flags & POLL_CFG_GDSCR) {
+			if (gdsc_is_enabled_by_poll_cfg_reg(sc, en) == en)
+				return 0;
+		} else {
+			if (gdsc_is_enabled(sc, reg) == en)
+				return 0;
+		}
 	} while (ktime_us_delta(ktime_get(), start) < TIMEOUT_US);
 
-	if (gdsc_is_enabled(sc, reg) == en)
-		return 0;
+	if (sc->flags & POLL_CFG_GDSCR) {
+		if (gdsc_is_enabled_by_poll_cfg_reg(sc, en) == en)
+			return 0;
+	} else {
+		if (gdsc_is_enabled(sc, reg) == en)
+			return 0;
+	}
 
 	return -ETIMEDOUT;
 }

@@ -254,6 +284,7 @@ static int gdsc_disable(struct generic_pm_domain *domain)
 		udelay(1);
 
 		reg = sc->gds_hw_ctrl ? sc->gds_hw_ctrl : sc->gdscr;
+
 		ret = gdsc_poll_status(sc, reg, true);
 		if (ret)
 			return ret;
drivers/clk/qcom/gdsc.h +1 -0
index 9279278..0f992e8 100644
@@ -55,6 +55,7 @@ struct gdsc {
 #define HW_CTRL		BIT(2)
 #define SW_RESET	BIT(3)
 #define AON_RESET	BIT(4)
+#define POLL_CFG_GDSCR  BIT(5)
 	struct reset_controller_dev	*rcdev;
 	unsigned int			*resets;
 	unsigned int			reset_count;

--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.

maintainer
#6 lore raw

Re: [PATCH v2 3/3] clk: qcom: gdsc: Add support to poll CFG register to check GDSC state

Quoting Taniya Das (2018-04-09 01:41:46)

Quoted 23 lines
drivers/clk/qcom/gdsc.c +15 -0
@@ -64,18 +69,43 @@ static int gdsc_hwctrl(struct gdsc *sc, bool en)
        return regmap_update_bits(sc->regmap, sc->gdscr, HW_CONTROL_MASK, val);
 }
 
+static int gdsc_is_enabled_by_poll_cfg_reg(struct gdsc *sc, bool en)
+{
+       u32 val;
+       int ret;
+
+       ret = regmap_read(sc->regmap, sc->gdscr + CFG_GDSCR_OFFSET, &val);
+       if (ret)
+               return ret;
+
+       if (en)
+               return !!(val & GDSC_POWER_UP_COMPLETE);
+
+       return !(val & GDSC_POWER_DOWN_COMPLETE);
+}
+
 static int gdsc_poll_status(struct gdsc *sc, unsigned int reg, bool en)
 {
        ktime_t start;

drivers/clk/qcom/gdsc.c static int gdsc_hwctrl(struct gdsc *sc, bool en) · line 98

Quoted 10 lines
drivers/clk/qcom/gdsc.c +6 -2
        start = ktime_get();
        do {
-               if (gdsc_is_enabled(sc, reg) == en)
-                       return 0;
+               if (sc->flags & POLL_CFG_GDSCR) {
+                       if (gdsc_is_enabled_by_poll_cfg_reg(sc, en) == en)
+                               return 0;
+               } else {
+                       if (gdsc_is_enabled(sc, reg) == en)
+                               return 0;

Ok, I thought you would bury the is_enabled_by_poll_cfg_reg() stuff into gdsc_is_enabled() directly. Because that function is used in one more place than here, in gdsc_init(), to figure out if a GDSC is on at boot time. The sc->gds_hw_ctrl check should be able to get buried into gdsc_is_enabled() as well so that the function doesn't have to take a register at all.

drivers/clk/qcom/gdsc.c static int gdsc_hwctrl(struct gdsc *sc, bool en) · line 108

Quoted 12 lines
drivers/clk/qcom/gdsc.c +8 -2
+               }
        } while (ktime_us_delta(ktime_get(), start) < TIMEOUT_US);
 
-       if (gdsc_is_enabled(sc, reg) == en)
-               return 0;
+       if (sc->flags & POLL_CFG_GDSCR) {
+               if (gdsc_is_enabled_by_poll_cfg_reg(sc, en) == en)
+                       return 0;
+       } else {
+               if (gdsc_is_enabled(sc, reg) == en)
+                       return 0;
+       }

And then this duplicate diff wouldn't happen either.

drivers/clk/qcom/gdsc.c static int gdsc_disable(struct generic_pm_domain *domain) · line 287

Quoted 8 lines
drivers/clk/qcom/gdsc.c +1 -0
        return -ETIMEDOUT;
 }

@@ -254,6 +284,7 @@ static int gdsc_disable(struct generic_pm_domain *domain)
                udelay(1);
 
                reg = sc->gds_hw_ctrl ? sc->gds_hw_ctrl : sc->gdscr;
+

This change shouldn't be here?

Quoted 3 lines
drivers/clk/qcom/gdsc.c +0 -0
                ret = gdsc_poll_status(sc, reg, true);
                if (ret)
                        return ret;