[V3,06/17] clk: tegra: pll: save and restore pll context
diff mbox series

Message ID 1560843991-24123-7-git-send-email-skomatineni@nvidia.com
State Superseded
Headers show
Series
  • SC7 entry and exit support for Tegra210
Related show

Commit Message

Sowjanya Komatineni June 18, 2019, 7:46 a.m. UTC
This patch implements save and restore of pll context.

During system suspend, core power goes off and looses the settings
of the Tegra CAR controller registers.

So during suspend entry pll rate is stored and on resume it is
restored back along with its state.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/clk/tegra/clk-pll.c | 115 ++++++++++++++++++++++++++++++++------------
 drivers/clk/tegra/clk.h     |   6 ++-
 2 files changed, 88 insertions(+), 33 deletions(-)

Comments

Thierry Reding June 18, 2019, 11:45 a.m. UTC | #1
On Tue, Jun 18, 2019 at 12:46:20AM -0700, Sowjanya Komatineni wrote:
> This patch implements save and restore of pll context.
> 
> During system suspend, core power goes off and looses the settings
> of the Tegra CAR controller registers.
> 
> So during suspend entry pll rate is stored and on resume it is
> restored back along with its state.
> 
> Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
> ---
>  drivers/clk/tegra/clk-pll.c | 115 ++++++++++++++++++++++++++++++++------------
>  drivers/clk/tegra/clk.h     |   6 ++-
>  2 files changed, 88 insertions(+), 33 deletions(-)

Nit: s/pll/PLL/ in the commit message, but it's up to Stephen and Mike
how important they consider this to be, so:

Acked-by: Thierry Reding <treding@nvidia.com>
Stephen Boyd June 25, 2019, 8:46 p.m. UTC | #2
Quoting Sowjanya Komatineni (2019-06-18 00:46:20)
> diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
> index 1583f5fc992f..4b0ed8fc6268 100644
> --- a/drivers/clk/tegra/clk-pll.c
> +++ b/drivers/clk/tegra/clk-pll.c
> @@ -1008,6 +1008,54 @@ static unsigned long clk_plle_recalc_rate(struct clk_hw *hw,
>         return rate;
>  }
>  
> +void tegra_clk_sync_state_pll(struct clk_hw *hw)
> +{
> +       if (!__clk_get_enable_count(hw->clk))
> +               clk_pll_disable(hw);
> +       else
> +               clk_pll_enable(hw);
> +}
> +
> +static int tegra_clk_pll_save_context(struct clk_hw *hw)
> +{
> +       struct tegra_clk_pll *pll = to_clk_pll(hw);
> +
> +       pll->rate = clk_hw_get_rate(hw);
> +
> +       if (!strcmp(__clk_get_name(hw->clk), "pll_mb"))
> +               pll->pllbase_ctx = pll_readl_base(pll);
> +       else if (!strcmp(__clk_get_name(hw->clk), "pll_re_vco"))
> +               pll->pllbase_ctx = pll_readl_base(pll) & (0xf << 16);
> +
> +       return 0;
> +}
> +
> +static void tegra_clk_pll_restore_context(struct clk_hw *hw)
> +{
> +       struct tegra_clk_pll *pll = to_clk_pll(hw);
> +       u32 val;
> +
> +       if (clk_pll_is_enabled(hw))
> +               return;
> +
> +       if (!strcmp(__clk_get_name(hw->clk), "pll_mb")) {

Is there any way to avoid doing a string comparison here, and instead do
something like a pointer comparison? Or maybe look at some flag in the
tegra_clk_pll to figure out what to do differently? Using a string
comparison is not too nice. Or even have different clk ops for the
different clks and then do different things in this restore clk_op?

> +               pll_writel_base(pll->pllbase_ctx, pll);
> +       } else if (!strcmp(__clk_get_name(hw->clk), "pll_re_vco")) {
> +               val = pll_readl_base(pll);
> +               val &= ~(0xf << 16);
> +               pll_writel_base(pll->pllbase_ctx | val, pll);
> +       }
> +
> +       if (pll->params->set_defaults)
> +               pll->params->set_defaults(pll);
> +
> +       clk_set_rate(hw->clk, pll->rate);

Do you need to call clk_set_rate() here to change the frequency of the
clk or just the parents of the clk, or both? I'd think that when we're
restoring the clk the cached rate of the clk would match whatever we're
restoring to, so this is a NOP. So does this do anything?

I'd prefer that the restore ops just restore the clk hardware state of
the clk_hw passed in, and not try to fix up the entire tree around a
certain clk, if that's even possible.

> +
> +       /* do not sync pllx state here. pllx is sync'd after dfll resume */
> +       if (strcmp(__clk_get_name(hw->clk), "pll_x"))
> +               tegra_clk_sync_state_pll(hw);
> +}
> +
>  const struct clk_ops tegra_clk_pll_ops = {
>         .is_enabled = clk_pll_is_enabled,
>         .enable = clk_pll_enable,
> @@ -1015,6 +1063,8 @@ const struct clk_ops tegra_clk_pll_ops = {
>         .recalc_rate = clk_pll_recalc_rate,
>         .round_rate = clk_pll_round_rate,
>         .set_rate = clk_pll_set_rate,
> +       .save_context = tegra_clk_pll_save_context,
> +       .restore_context = tegra_clk_pll_restore_context,
Sowjanya Komatineni June 25, 2019, 9:22 p.m. UTC | #3
On 6/25/19 1:46 PM, Stephen Boyd wrote:
> Quoting Sowjanya Komatineni (2019-06-18 00:46:20)
>> diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
>> index 1583f5fc992f..4b0ed8fc6268 100644
>> --- a/drivers/clk/tegra/clk-pll.c
>> +++ b/drivers/clk/tegra/clk-pll.c
>> @@ -1008,6 +1008,54 @@ static unsigned long clk_plle_recalc_rate(struct clk_hw *hw,
>>          return rate;
>>   }
>>   
>> +void tegra_clk_sync_state_pll(struct clk_hw *hw)
>> +{
>> +       if (!__clk_get_enable_count(hw->clk))
>> +               clk_pll_disable(hw);
>> +       else
>> +               clk_pll_enable(hw);
>> +}
>> +
>> +static int tegra_clk_pll_save_context(struct clk_hw *hw)
>> +{
>> +       struct tegra_clk_pll *pll = to_clk_pll(hw);
>> +
>> +       pll->rate = clk_hw_get_rate(hw);
>> +
>> +       if (!strcmp(__clk_get_name(hw->clk), "pll_mb"))
>> +               pll->pllbase_ctx = pll_readl_base(pll);
>> +       else if (!strcmp(__clk_get_name(hw->clk), "pll_re_vco"))
>> +               pll->pllbase_ctx = pll_readl_base(pll) & (0xf << 16);
>> +
>> +       return 0;
>> +}
>> +
>> +static void tegra_clk_pll_restore_context(struct clk_hw *hw)
>> +{
>> +       struct tegra_clk_pll *pll = to_clk_pll(hw);
>> +       u32 val;
>> +
>> +       if (clk_pll_is_enabled(hw))
>> +               return;
>> +
>> +       if (!strcmp(__clk_get_name(hw->clk), "pll_mb")) {
> Is there any way to avoid doing a string comparison here, and instead do
> something like a pointer comparison? Or maybe look at some flag in the
> tegra_clk_pll to figure out what to do differently? Using a string
> comparison is not too nice. Or even have different clk ops for the
> different clks and then do different things in this restore clk_op?
OK, Will update...
>> +               pll_writel_base(pll->pllbase_ctx, pll);
>> +       } else if (!strcmp(__clk_get_name(hw->clk), "pll_re_vco")) {
>> +               val = pll_readl_base(pll);
>> +               val &= ~(0xf << 16);
>> +               pll_writel_base(pll->pllbase_ctx | val, pll);
>> +       }
>> +
>> +       if (pll->params->set_defaults)
>> +               pll->params->set_defaults(pll);
>> +
>> +       clk_set_rate(hw->clk, pll->rate);
> Do you need to call clk_set_rate() here to change the frequency of the
> clk or just the parents of the clk, or both? I'd think that when we're
> restoring the clk the cached rate of the clk would match whatever we're
> restoring to, so this is a NOP. So does this do anything?
>
> I'd prefer that the restore ops just restore the clk hardware state of
> the clk_hw passed in, and not try to fix up the entire tree around a
> certain clk, if that's even possible.

On restore, need to program tegra plls rate back to the same rate as 
they were before suspend, so I am calling clk_set_rate to program pll 
m,n,p values in hw registers.

>> +
>> +       /* do not sync pllx state here. pllx is sync'd after dfll resume */
>> +       if (strcmp(__clk_get_name(hw->clk), "pll_x"))
>> +               tegra_clk_sync_state_pll(hw);
>> +}
>> +
>>   const struct clk_ops tegra_clk_pll_ops = {
>>          .is_enabled = clk_pll_is_enabled,
>>          .enable = clk_pll_enable,
>> @@ -1015,6 +1063,8 @@ const struct clk_ops tegra_clk_pll_ops = {
>>          .recalc_rate = clk_pll_recalc_rate,
>>          .round_rate = clk_pll_round_rate,
>>          .set_rate = clk_pll_set_rate,
>> +       .save_context = tegra_clk_pll_save_context,
>> +       .restore_context = tegra_clk_pll_restore_context,

Patch
diff mbox series

diff --git a/drivers/clk/tegra/clk-pll.c b/drivers/clk/tegra/clk-pll.c
index 1583f5fc992f..4b0ed8fc6268 100644
--- a/drivers/clk/tegra/clk-pll.c
+++ b/drivers/clk/tegra/clk-pll.c
@@ -1008,6 +1008,54 @@  static unsigned long clk_plle_recalc_rate(struct clk_hw *hw,
 	return rate;
 }
 
+void tegra_clk_sync_state_pll(struct clk_hw *hw)
+{
+	if (!__clk_get_enable_count(hw->clk))
+		clk_pll_disable(hw);
+	else
+		clk_pll_enable(hw);
+}
+
+static int tegra_clk_pll_save_context(struct clk_hw *hw)
+{
+	struct tegra_clk_pll *pll = to_clk_pll(hw);
+
+	pll->rate = clk_hw_get_rate(hw);
+
+	if (!strcmp(__clk_get_name(hw->clk), "pll_mb"))
+		pll->pllbase_ctx = pll_readl_base(pll);
+	else if (!strcmp(__clk_get_name(hw->clk), "pll_re_vco"))
+		pll->pllbase_ctx = pll_readl_base(pll) & (0xf << 16);
+
+	return 0;
+}
+
+static void tegra_clk_pll_restore_context(struct clk_hw *hw)
+{
+	struct tegra_clk_pll *pll = to_clk_pll(hw);
+	u32 val;
+
+	if (clk_pll_is_enabled(hw))
+		return;
+
+	if (!strcmp(__clk_get_name(hw->clk), "pll_mb")) {
+		pll_writel_base(pll->pllbase_ctx, pll);
+	} else if (!strcmp(__clk_get_name(hw->clk), "pll_re_vco")) {
+		val = pll_readl_base(pll);
+		val &= ~(0xf << 16);
+		pll_writel_base(pll->pllbase_ctx | val, pll);
+	}
+
+	if (pll->params->set_defaults)
+		pll->params->set_defaults(pll);
+
+	clk_set_rate(hw->clk, pll->rate);
+
+	/* do not sync pllx state here. pllx is sync'd after dfll resume */
+	if (strcmp(__clk_get_name(hw->clk), "pll_x"))
+		tegra_clk_sync_state_pll(hw);
+}
+
 const struct clk_ops tegra_clk_pll_ops = {
 	.is_enabled = clk_pll_is_enabled,
 	.enable = clk_pll_enable,
@@ -1015,6 +1063,8 @@  const struct clk_ops tegra_clk_pll_ops = {
 	.recalc_rate = clk_pll_recalc_rate,
 	.round_rate = clk_pll_round_rate,
 	.set_rate = clk_pll_set_rate,
+	.save_context = tegra_clk_pll_save_context,
+	.restore_context = tegra_clk_pll_restore_context,
 };
 
 const struct clk_ops tegra_clk_plle_ops = {
@@ -1802,6 +1852,27 @@  static int clk_pllu_tegra114_enable(struct clk_hw *hw)
 
 	return ret;
 }
+
+static void _clk_plle_tegra_init_parent(struct tegra_clk_pll *pll)
+{
+	u32 val, val_aux;
+
+	/* ensure parent is set to pll_ref */
+	val = pll_readl_base(pll);
+	val_aux = pll_readl(pll->params->aux_reg, pll);
+
+	if (val & PLL_BASE_ENABLE) {
+		if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
+		    (val_aux & PLLE_AUX_PLLP_SEL))
+			WARN(1, "pll_e enabled with unsupported parent %s\n",
+			     (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
+			     "pll_re_vco");
+	} else {
+		val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
+		pll_writel(val_aux, pll->params->aux_reg, pll);
+		fence_udelay(1, pll->clk_base);
+	}
+}
 #endif
 
 static struct tegra_clk_pll *_tegra_init_pll(void __iomem *clk_base,
@@ -2214,27 +2285,12 @@  struct clk *tegra_clk_register_plle_tegra114(const char *name,
 {
 	struct tegra_clk_pll *pll;
 	struct clk *clk;
-	u32 val, val_aux;
 
 	pll = _tegra_init_pll(clk_base, NULL, pll_params, lock);
 	if (IS_ERR(pll))
 		return ERR_CAST(pll);
 
-	/* ensure parent is set to pll_re_vco */
-
-	val = pll_readl_base(pll);
-	val_aux = pll_readl(pll_params->aux_reg, pll);
-
-	if (val & PLL_BASE_ENABLE) {
-		if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
-			(val_aux & PLLE_AUX_PLLP_SEL))
-			WARN(1, "pll_e enabled with unsupported parent %s\n",
-			  (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
-					"pll_re_vco");
-	} else {
-		val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
-		pll_writel(val_aux, pll_params->aux_reg, pll);
-	}
+	_clk_plle_tegra_init_parent(pll);
 
 	clk = _tegra_clk_register_pll(pll, name, parent_name, flags,
 				      &tegra_clk_plle_tegra114_ops);
@@ -2276,6 +2332,8 @@  static const struct clk_ops tegra_clk_pllss_ops = {
 	.recalc_rate = clk_pll_recalc_rate,
 	.round_rate = clk_pll_ramp_round_rate,
 	.set_rate = clk_pllxc_set_rate,
+	.save_context = tegra_clk_pll_save_context,
+	.restore_context = tegra_clk_pll_restore_context,
 };
 
 struct clk *tegra_clk_register_pllss(const char *name, const char *parent_name,
@@ -2520,11 +2578,19 @@  static void clk_plle_tegra210_disable(struct clk_hw *hw)
 		spin_unlock_irqrestore(pll->lock, flags);
 }
 
+static void tegra_clk_plle_t210_restore_context(struct clk_hw *hw)
+{
+	struct tegra_clk_pll *pll = to_clk_pll(hw);
+
+	_clk_plle_tegra_init_parent(pll);
+}
+
 static const struct clk_ops tegra_clk_plle_tegra210_ops = {
 	.is_enabled =  clk_plle_tegra210_is_enabled,
 	.enable = clk_plle_tegra210_enable,
 	.disable = clk_plle_tegra210_disable,
 	.recalc_rate = clk_pll_recalc_rate,
+	.restore_context = tegra_clk_plle_t210_restore_context,
 };
 
 struct clk *tegra_clk_register_plle_tegra210(const char *name,
@@ -2535,27 +2601,12 @@  struct clk *tegra_clk_register_plle_tegra210(const char *name,
 {
 	struct tegra_clk_pll *pll;
 	struct clk *clk;
-	u32 val, val_aux;
 
 	pll = _tegra_init_pll(clk_base, NULL, pll_params, lock);
 	if (IS_ERR(pll))
 		return ERR_CAST(pll);
 
-	/* ensure parent is set to pll_re_vco */
-
-	val = pll_readl_base(pll);
-	val_aux = pll_readl(pll_params->aux_reg, pll);
-
-	if (val & PLLE_BASE_ENABLE) {
-		if ((val_aux & PLLE_AUX_PLLRE_SEL) ||
-			(val_aux & PLLE_AUX_PLLP_SEL))
-			WARN(1, "pll_e enabled with unsupported parent %s\n",
-			  (val_aux & PLLE_AUX_PLLP_SEL) ? "pllp_out0" :
-					"pll_re_vco");
-	} else {
-		val_aux &= ~(PLLE_AUX_PLLRE_SEL | PLLE_AUX_PLLP_SEL);
-		pll_writel(val_aux, pll_params->aux_reg, pll);
-	}
+	_clk_plle_tegra_init_parent(pll);
 
 	clk = _tegra_clk_register_pll(pll, name, parent_name, flags,
 				      &tegra_clk_plle_tegra210_ops);
diff --git a/drivers/clk/tegra/clk.h b/drivers/clk/tegra/clk.h
index b47f373c35ad..581deb4f3ac0 100644
--- a/drivers/clk/tegra/clk.h
+++ b/drivers/clk/tegra/clk.h
@@ -310,6 +310,8 @@  struct tegra_clk_pll_params {
  * @pmc:	address of PMC, required to read override bits
  * @lock:	register lock
  * @params:	PLL parameters
+ * @rate:	rate during system suspend and resume
+ * @pllbase_ctx: pll base register value during suspend and resume
  */
 struct tegra_clk_pll {
 	struct clk_hw	hw;
@@ -317,6 +319,8 @@  struct tegra_clk_pll {
 	void __iomem	*pmc;
 	spinlock_t	*lock;
 	struct tegra_clk_pll_params	*params;
+	unsigned long	rate;
+	unsigned int	pllbase_ctx;
 };
 
 #define to_clk_pll(_hw) container_of(_hw, struct tegra_clk_pll, hw)
@@ -834,7 +838,7 @@  u16 tegra_pll_get_fixed_mdiv(struct clk_hw *hw, unsigned long input_rate);
 int tegra_pll_p_div_to_hw(struct tegra_clk_pll *pll, u8 p_div);
 int div_frac_get(unsigned long rate, unsigned parent_rate, u8 width,
 		 u8 frac_width, u8 flags);
-
+void tegra_clk_sync_state_pll(struct clk_hw *hw);
 
 /* Combined read fence with delay */
 #define fence_udelay(delay, reg)	\