[PATCH 0/3] Update reset and poll logic for GDSCs

[PATCH 0/3] Update reset and poll logic for GDSCs

[Taniya Das]

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 | 77 +++++++++++++++++++++++++++++++++++++++++++++----
 drivers/clk/qcom/gdsc.h |  6 +++-
 2 files changed, 76 insertions(+), 7 deletions(-)

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

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

[Taniya Das]

From: Amit Nischal <anischal@codeaurora.org>

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 <tdas@codeaurora.org>
Signed-off-by: Amit Nischal <anischal@codeaurora.org>
---
 drivers/clk/qcom/gdsc.c | 22 ++++++++++++++++++++--
 drivers/clk/qcom/gdsc.h |  4 +++-
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index a4f3580..266fefa 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -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)
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index 3964834..9279278 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -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.

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

[Taniya Das]

From: Amit Nischal <anischal@codeaurora.org>

For some gdscs, it might take longer time up to 500us for
updating their status. So add support for the same by
defining a new flag 'GDS_TIMEOUT' to mark such gdsc in
order to poll their status for longer timeout value.

Signed-off-by: Amit Nischal <anischal@codeaurora.org>
Signed-off-by: Taniya Das <tdas@codeaurora.org>
---
 drivers/clk/qcom/gdsc.c | 9 +++++++--
 drivers/clk/qcom/gdsc.h | 1 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 266fefa..e89584e 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -42,6 +42,7 @@
 #define RETAIN_PERIPH		BIT(13)

 #define TIMEOUT_US		100
+#define TIMEOUT_US_GDS		500

 #define domain_to_gdsc(domain) container_of(domain, struct gdsc, pd)

@@ -67,12 +68,14 @@ static int gdsc_hwctrl(struct gdsc *sc, bool en)
 static int gdsc_poll_status(struct gdsc *sc, unsigned int reg, bool en)
 {
 	ktime_t start;
+	ktime_t timeout =
+		(sc->flags & GDS_TIMEOUT) ? TIMEOUT_US_GDS : TIMEOUT_US;

 	start = ktime_get();
 	do {
 		if (gdsc_is_enabled(sc, reg) == en)
 			return 0;
-	} while (ktime_us_delta(ktime_get(), start) < TIMEOUT_US);
+	} while (ktime_us_delta(ktime_get(), start) < timeout);

 	if (gdsc_is_enabled(sc, reg) == en)
 		return 0;
@@ -85,6 +88,8 @@ static int gdsc_toggle_logic(struct gdsc *sc, bool en)
 	int ret;
 	u32 val = en ? 0 : SW_COLLAPSE_MASK;
 	unsigned int status_reg = sc->gdscr;
+	ktime_t timeout =
+		(sc->flags & GDS_TIMEOUT) ? TIMEOUT_US_GDS : TIMEOUT_US;

 	ret = regmap_update_bits(sc->regmap, sc->gdscr, SW_COLLAPSE_MASK, val);
 	if (ret)
@@ -97,7 +102,7 @@ static int gdsc_toggle_logic(struct gdsc *sc, bool en)
 		 * right after it was disabled does not put it in an
 		 * unknown state
 		 */
-		udelay(TIMEOUT_US);
+		udelay(timeout);
 		return 0;
 	}

diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index 9279278..2e3abbf 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -55,6 +55,7 @@ struct gdsc {
 #define HW_CTRL		BIT(2)
 #define SW_RESET	BIT(3)
 #define AON_RESET	BIT(4)
+#define GDS_TIMEOUT	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.

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

[Taniya Das]

From: Amit Nischal <anischal@codeaurora.org>

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 <anischal@codeaurora.org>
Signed-off-by: Taniya Das <tdas@codeaurora.org>
---
 drivers/clk/qcom/gdsc.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
 drivers/clk/qcom/gdsc.h |  1 +
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index e89584e..e0c83ba 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -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)
@@ -83,6 +88,38 @@ static int gdsc_poll_status(struct gdsc *sc, unsigned int reg, bool en)
 	return -ETIMEDOUT;
 }

+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);
+	else
+		return !(val & GDSC_POWER_DOWN_COMPLETE);
+}
+
+static int gdsc_poll_cfg_status(struct gdsc *sc, bool en)
+{
+	ktime_t start = ktime_get();
+	ktime_t timeout =
+		(sc->flags & GDS_TIMEOUT) ? TIMEOUT_US_GDS : TIMEOUT_US;
+
+	do {
+		if (gdsc_is_enabled_by_poll_cfg_reg(sc, en) == en)
+			return 0;
+	} while (ktime_us_delta(ktime_get(), start) < timeout);
+
+	if (gdsc_is_enabled_by_poll_cfg_reg(sc, en) == en)
+		return 0;
+
+	return -ETIMEDOUT;
+}
+
 static int gdsc_toggle_logic(struct gdsc *sc, bool en)
 {
 	int ret;
@@ -106,6 +143,9 @@ static int gdsc_toggle_logic(struct gdsc *sc, bool en)
 		return 0;
 	}

+	if (sc->flags & POLL_CFG_GDSCR)
+		return gdsc_poll_cfg_status(sc, en);
+
 	if (sc->gds_hw_ctrl) {
 		status_reg = sc->gds_hw_ctrl;
 		/*
@@ -258,8 +298,12 @@ 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 (sc->flags & POLL_CFG_GDSCR) {
+			ret = gdsc_poll_cfg_status(sc, true);
+		} else {
+			reg = sc->gds_hw_ctrl ? sc->gds_hw_ctrl : sc->gdscr;
+			ret = gdsc_poll_status(sc, reg, true);
+		}
 		if (ret)
 			return ret;
 	}
diff --git a/drivers/clk/qcom/gdsc.h b/drivers/clk/qcom/gdsc.h
index 2e3abbf..a74f715 100644
--- a/drivers/clk/qcom/gdsc.h
+++ b/drivers/clk/qcom/gdsc.h
@@ -56,6 +56,7 @@ struct gdsc {
 #define SW_RESET	BIT(3)
 #define AON_RESET	BIT(4)
 #define GDS_TIMEOUT	BIT(5)
+#define POLL_CFG_GDSCR  BIT(6)
 	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.

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

[Stephen Boyd]

Quoting Taniya Das (2018-04-02 03:45:44)
> From: Amit Nischal <anischal@codeaurora.org>
> 
> For some gdscs, it might take longer time up to 500us for
> updating their status. So add support for the same by
> defining a new flag 'GDS_TIMEOUT' to mark such gdsc in
> order to poll their status for longer timeout value.
> 
> Signed-off-by: Amit Nischal <anischal@codeaurora.org>
> Signed-off-by: Taniya Das <tdas@codeaurora.org>
> ---

Let's just increase the timeout to 500 if it's required? This is a
timeout, so we're not really expecting to hit it anyway so optimizing
the uncommon case is not useful.

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

[Stephen Boyd]

Quoting Taniya Das (2018-04-02 03:45:45)
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index e89584e..e0c83ba 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -83,6 +88,38 @@ static int gdsc_poll_status(struct gdsc *sc, unsigned int reg, bool en)
>         return -ETIMEDOUT;
>  }
> 
> +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);
> +       else
> +               return !(val & GDSC_POWER_DOWN_COMPLETE);

Make this into

	if (en)
		return ...

	return ...

> +}
> +
> +static int gdsc_poll_cfg_status(struct gdsc *sc, bool en)
> +{
> +       ktime_t start = ktime_get();
> +       ktime_t timeout =
> +               (sc->flags & GDS_TIMEOUT) ? TIMEOUT_US_GDS : TIMEOUT_US;
> +
> +       do {
> +               if (gdsc_is_enabled_by_poll_cfg_reg(sc, en) == en)
> +                       return 0;
> +       } while (ktime_us_delta(ktime_get(), start) < timeout);
> +
> +       if (gdsc_is_enabled_by_poll_cfg_reg(sc, en) == en)
> +               return 0;
> +
> +       return -ETIMEDOUT;
> +}
> +
>  static int gdsc_toggle_logic(struct gdsc *sc, bool en)
>  {
>         int ret;
> @@ -106,6 +143,9 @@ static int gdsc_toggle_logic(struct gdsc *sc, bool en)
>                 return 0;
>         }
> 
> +       if (sc->flags & POLL_CFG_GDSCR)
> +               return gdsc_poll_cfg_status(sc, en);
> +
>         if (sc->gds_hw_ctrl) {
>                 status_reg = sc->gds_hw_ctrl;
>                 /*
> @@ -258,8 +298,12 @@ 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 (sc->flags & POLL_CFG_GDSCR) {
> +                       ret = gdsc_poll_cfg_status(sc, true);
> +               } else {
> +                       reg = sc->gds_hw_ctrl ? sc->gds_hw_ctrl : sc->gdscr;
> +                       ret = gdsc_poll_status(sc, reg, true);
> +               }

Maybe this can be pushed into the gdsc_poll_status() function so that
we can keep the "how do we poll status bit" logic in one place.

>                 if (ret)
>                         return ret;
>         }

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

[Taniya Das]

Hello Stephen,

Thanks for the review comments.

On 4/6/2018 4:54 AM, Stephen Boyd wrote:
> Quoting Taniya Das (2018-04-02 03:45:44)
>> From: Amit Nischal <anischal@codeaurora.org>
>>
>> For some gdscs, it might take longer time up to 500us for
>> updating their status. So add support for the same by
>> defining a new flag 'GDS_TIMEOUT' to mark such gdsc in
>> order to poll their status for longer timeout value.
>>
>> Signed-off-by: Amit Nischal <anischal@codeaurora.org>
>> Signed-off-by: Taniya Das <tdas@codeaurora.org>
>> ---
> 
> Let's just increase the timeout to 500 if it's required? This is a
> timeout, so we're not really expecting to hit it anyway so optimizing
> the uncommon case is not useful.
> --

Will fix this in the next series.

> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

--

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

[Taniya Das]

Hello Stephen,

Thanks for the review comments.

On 4/6/2018 10:10 PM, Stephen Boyd wrote:
> Quoting Taniya Das (2018-04-02 03:45:45)
>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
>> index e89584e..e0c83ba 100644
>> --- a/drivers/clk/qcom/gdsc.c
>> +++ b/drivers/clk/qcom/gdsc.c
>> @@ -83,6 +88,38 @@ static int gdsc_poll_status(struct gdsc *sc, unsigned int reg, bool en)
>>          return -ETIMEDOUT;
>>   }
>>
>> +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);
>> +       else
>> +               return !(val & GDSC_POWER_DOWN_COMPLETE);
> 
> Make this into
> 
> 	if (en)
> 		return ...
> 
> 	return ...
> 

Will fix this in the next series.

>> +}
>> +
>> +static int gdsc_poll_cfg_status(struct gdsc *sc, bool en)
>> +{
>> +       ktime_t start = ktime_get();
>> +       ktime_t timeout =
>> +               (sc->flags & GDS_TIMEOUT) ? TIMEOUT_US_GDS : TIMEOUT_US;
>> +
>> +       do {
>> +               if (gdsc_is_enabled_by_poll_cfg_reg(sc, en) == en)
>> +                       return 0;
>> +       } while (ktime_us_delta(ktime_get(), start) < timeout);
>> +
>> +       if (gdsc_is_enabled_by_poll_cfg_reg(sc, en) == en)
>> +               return 0;
>> +
>> +       return -ETIMEDOUT;
>> +}
>> +
>>   static int gdsc_toggle_logic(struct gdsc *sc, bool en)
>>   {
>>          int ret;
>> @@ -106,6 +143,9 @@ static int gdsc_toggle_logic(struct gdsc *sc, bool en)
>>                  return 0;
>>          }
>>
>> +       if (sc->flags & POLL_CFG_GDSCR)
>> +               return gdsc_poll_cfg_status(sc, en);
>> +
>>          if (sc->gds_hw_ctrl) {
>>                  status_reg = sc->gds_hw_ctrl;
>>                  /*
>> @@ -258,8 +298,12 @@ 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 (sc->flags & POLL_CFG_GDSCR) {
>> +                       ret = gdsc_poll_cfg_status(sc, true);
>> +               } else {
>> +                       reg = sc->gds_hw_ctrl ? sc->gds_hw_ctrl : sc->gdscr;
>> +                       ret = gdsc_poll_status(sc, reg, true);
>> +               }
> 
> Maybe this can be pushed into the gdsc_poll_status() function so that
> we can keep the "how do we poll status bit" logic in one place.
> 

Yes, I will push the logic in the gdsc_poll_status() in the next series.

>>                  if (ret)
>>                          return ret;
>>          }

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

--