[PATCH 0/9] clk: pistachio: Assorted changes

[PATCH 0/9] clk: pistachio: Assorted changes

[Ezequiel Garcia]

This patchset contains a bunch of clock changes for the Pistachio
clock driver.

The nine patches in this series are not really related, but I wasn't
sure it was worth to prepare a separate patchset for each group.
However, if this makes it harder to review, I can send different groups
of related patches.

Here's a brief summary of the patch groups:

Patches 1 and 2 clean up the PLL lock handling.

Patch 3 implements PLL rate adjustment, and in particular allows to support
small (i.e. neighbor-constrained) changes of the fractional PLL rate.

Patch 4 to 7 implements MIPS PLL rate change propagation and introduces
a table of MIPS PLL rate parameters.

Patch 8 adds some very useful sanity checks on integer and fractions PLL
set_rate(), to make sure the parameters are modified only when it's legal
to do so.

Patch 9 fixes the list of critical clocks.

None of these are urgent fixes so this is all v4.2 material.

Damien Horsley (1):
  clk: pistachio: Correct critical clock list

Ezequiel Garcia (7):
  clk: pistachio: Add a pll_lock() helper for clarity
  clk: pistachio: Lock the PLL when enabled upon rate change
  clk: pistachio: Implement PLL rate adjustment
  clk: pistachio: Extend DIV_F to pass clk_flags as well
  clk: pistachio: Add a MUX_F macro to pass clk_flags
  clk: pistachio: Propagate rate changes in the MIPS PLL clock sub-tree
  clk: pistachio: Add a rate table for the MIPS PLL

Kevin Cernekee (1):
  clk: pistachio: Add sanity checks on PLL configuration

 drivers/clk/pistachio/clk-pistachio.c |  63 ++++++++-----
 drivers/clk/pistachio/clk-pll.c       | 161 +++++++++++++++++++++++++++-------
 drivers/clk/pistachio/clk.c           |   5 +-
 drivers/clk/pistachio/clk.h           |  33 ++++++-
 4 files changed, 205 insertions(+), 57 deletions(-)

-- 
2.3.3

[PATCH 0/9] clk: pistachio: Assorted changes

[Ezequiel Garcia]

This patchset contains a bunch of clock changes for the Pistachio
clock driver.

The nine patches in this series are not really related, but I wasn't
sure it was worth to prepare a separate patchset for each group.
However, if this makes it harder to review, I can send different groups
of related patches.

Here's a brief summary of the patch groups:

Patches 1 and 2 clean up the PLL lock handling.

Patch 3 implements PLL rate adjustment, and in particular allows to support
small (i.e. neighbor-constrained) changes of the fractional PLL rate.

Patch 4 to 7 implements MIPS PLL rate change propagation and introduces
a table of MIPS PLL rate parameters.

Patch 8 adds some very useful sanity checks on integer and fractions PLL
set_rate(), to make sure the parameters are modified only when it's legal
to do so.

Patch 9 fixes the list of critical clocks.

None of these are urgent fixes so this is all v4.2 material.

Damien Horsley (1):
  clk: pistachio: Correct critical clock list

Ezequiel Garcia (7):
  clk: pistachio: Add a pll_lock() helper for clarity
  clk: pistachio: Lock the PLL when enabled upon rate change
  clk: pistachio: Implement PLL rate adjustment
  clk: pistachio: Extend DIV_F to pass clk_flags as well
  clk: pistachio: Add a MUX_F macro to pass clk_flags
  clk: pistachio: Propagate rate changes in the MIPS PLL clock sub-tree
  clk: pistachio: Add a rate table for the MIPS PLL

Kevin Cernekee (1):
  clk: pistachio: Add sanity checks on PLL configuration

 drivers/clk/pistachio/clk-pistachio.c |  63 ++++++++-----
 drivers/clk/pistachio/clk-pll.c       | 161 +++++++++++++++++++++++++++-------
 drivers/clk/pistachio/clk.c           |   5 +-
 drivers/clk/pistachio/clk.h           |  33 ++++++-
 4 files changed, 205 insertions(+), 57 deletions(-)

-- 
2.3.3

[PATCH 1/9] clk: pistachio: Add a pll_lock() helper for clarity

[Ezequiel Garcia]

This commit adds a pll_lock() helper making the code more readable.
Cosmetic change only, no functionality changes.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pll.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pll.c b/drivers/clk/pistachio/clk-pll.c
index de53756..9ce1be7 100644
--- a/drivers/clk/pistachio/clk-pll.c
+++ b/drivers/clk/pistachio/clk-pll.c
@@ -67,6 +67,12 @@ static inline void pll_writel(struct pistachio_clk_pll *pll, u32 val, u32 reg)
 	writel(val, pll->base + reg);
 }
 
+static inline void pll_lock(struct pistachio_clk_pll *pll)
+{
+	while (!(pll_readl(pll, PLL_STATUS) & PLL_STATUS_LOCK))
+		cpu_relax();
+}
+
 static inline u32 do_div_round_closest(u64 dividend, u32 divisor)
 {
 	dividend += divisor / 2;
@@ -178,8 +184,7 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 		(params->postdiv2 << PLL_FRAC_CTRL2_POSTDIV2_SHIFT);
 	pll_writel(pll, val, PLL_CTRL2);
 
-	while (!(pll_readl(pll, PLL_STATUS) & PLL_STATUS_LOCK))
-		cpu_relax();
+	pll_lock(pll);
 
 	if (!was_enabled)
 		pll_gf40lp_frac_disable(hw);
@@ -288,8 +293,7 @@ static int pll_gf40lp_laint_set_rate(struct clk_hw *hw, unsigned long rate,
 		(params->postdiv2 << PLL_INT_CTRL1_POSTDIV2_SHIFT);
 	pll_writel(pll, val, PLL_CTRL1);
 
-	while (!(pll_readl(pll, PLL_STATUS) & PLL_STATUS_LOCK))
-		cpu_relax();
+	pll_lock(pll);
 
 	if (!was_enabled)
 		pll_gf40lp_laint_disable(hw);
-- 
2.3.3

[PATCH 1/9] clk: pistachio: Add a pll_lock() helper for clarity

[Ezequiel Garcia]

This commit adds a pll_lock() helper making the code more readable.
Cosmetic change only, no functionality changes.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pll.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pll.c b/drivers/clk/pistachio/clk-pll.c
index de53756..9ce1be7 100644
--- a/drivers/clk/pistachio/clk-pll.c
+++ b/drivers/clk/pistachio/clk-pll.c
@@ -67,6 +67,12 @@ static inline void pll_writel(struct pistachio_clk_pll *pll, u32 val, u32 reg)
 	writel(val, pll->base + reg);
 }
 
+static inline void pll_lock(struct pistachio_clk_pll *pll)
+{
+	while (!(pll_readl(pll, PLL_STATUS) & PLL_STATUS_LOCK))
+		cpu_relax();
+}
+
 static inline u32 do_div_round_closest(u64 dividend, u32 divisor)
 {
 	dividend += divisor / 2;
@@ -178,8 +184,7 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 		(params->postdiv2 << PLL_FRAC_CTRL2_POSTDIV2_SHIFT);
 	pll_writel(pll, val, PLL_CTRL2);
 
-	while (!(pll_readl(pll, PLL_STATUS) & PLL_STATUS_LOCK))
-		cpu_relax();
+	pll_lock(pll);
 
 	if (!was_enabled)
 		pll_gf40lp_frac_disable(hw);
@@ -288,8 +293,7 @@ static int pll_gf40lp_laint_set_rate(struct clk_hw *hw, unsigned long rate,
 		(params->postdiv2 << PLL_INT_CTRL1_POSTDIV2_SHIFT);
 	pll_writel(pll, val, PLL_CTRL1);
 
-	while (!(pll_readl(pll, PLL_STATUS) & PLL_STATUS_LOCK))
-		cpu_relax();
+	pll_lock(pll);
 
 	if (!was_enabled)
 		pll_gf40lp_laint_disable(hw);
-- 
2.3.3

[PATCH 2/9] clk: pistachio: Lock the PLL when enabled upon rate change

[Ezequiel Garcia]

Currently, when the rate is changed, the driver makes sure the
PLL is enabled before doing so. This is done because the PLL
cannot be locked while disabled. Once locked, the drivers
returns the PLL to its previous enable/disable state.

This is a bit cumbersome, and can be simplified.

This commit reworks the .set_rate() functions for the integer
and fractional PLLs. Upon rate change, the PLL is now locked
only if it's already enabled.

Also, the driver locks the PLL on .enable(). This makes sure
the PLL is locked when enabled, and not locked when disabled.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pll.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pll.c b/drivers/clk/pistachio/clk-pll.c
index 9ce1be7..f12d520 100644
--- a/drivers/clk/pistachio/clk-pll.c
+++ b/drivers/clk/pistachio/clk-pll.c
@@ -130,6 +130,8 @@ static int pll_gf40lp_frac_enable(struct clk_hw *hw)
 	val &= ~PLL_FRAC_CTRL4_BYPASS;
 	pll_writel(pll, val, PLL_CTRL4);
 
+	pll_lock(pll);
+
 	return 0;
 }
 
@@ -155,17 +157,13 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 {
 	struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
 	struct pistachio_pll_rate_table *params;
-	bool was_enabled;
+	int enabled = pll_gf40lp_frac_is_enabled(hw);
 	u32 val;
 
 	params = pll_get_params(pll, parent_rate, rate);
 	if (!params)
 		return -EINVAL;
 
-	was_enabled = pll_gf40lp_frac_is_enabled(hw);
-	if (!was_enabled)
-		pll_gf40lp_frac_enable(hw);
-
 	val = pll_readl(pll, PLL_CTRL1);
 	val &= ~((PLL_CTRL1_REFDIV_MASK << PLL_CTRL1_REFDIV_SHIFT) |
 		 (PLL_CTRL1_FBDIV_MASK << PLL_CTRL1_FBDIV_SHIFT));
@@ -184,10 +182,8 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 		(params->postdiv2 << PLL_FRAC_CTRL2_POSTDIV2_SHIFT);
 	pll_writel(pll, val, PLL_CTRL2);
 
-	pll_lock(pll);
-
-	if (!was_enabled)
-		pll_gf40lp_frac_disable(hw);
+	if (enabled)
+		pll_lock(pll);
 
 	return 0;
 }
@@ -246,6 +242,8 @@ static int pll_gf40lp_laint_enable(struct clk_hw *hw)
 	val &= ~PLL_INT_CTRL2_BYPASS;
 	pll_writel(pll, val, PLL_CTRL2);
 
+	pll_lock(pll);
+
 	return 0;
 }
 
@@ -271,17 +269,13 @@ static int pll_gf40lp_laint_set_rate(struct clk_hw *hw, unsigned long rate,
 {
 	struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
 	struct pistachio_pll_rate_table *params;
-	bool was_enabled;
+	int enabled = pll_gf40lp_laint_is_enabled(hw);
 	u32 val;
 
 	params = pll_get_params(pll, parent_rate, rate);
 	if (!params)
 		return -EINVAL;
 
-	was_enabled = pll_gf40lp_laint_is_enabled(hw);
-	if (!was_enabled)
-		pll_gf40lp_laint_enable(hw);
-
 	val = pll_readl(pll, PLL_CTRL1);
 	val &= ~((PLL_CTRL1_REFDIV_MASK << PLL_CTRL1_REFDIV_SHIFT) |
 		 (PLL_CTRL1_FBDIV_MASK << PLL_CTRL1_FBDIV_SHIFT) |
@@ -293,10 +287,8 @@ static int pll_gf40lp_laint_set_rate(struct clk_hw *hw, unsigned long rate,
 		(params->postdiv2 << PLL_INT_CTRL1_POSTDIV2_SHIFT);
 	pll_writel(pll, val, PLL_CTRL1);
 
-	pll_lock(pll);
-
-	if (!was_enabled)
-		pll_gf40lp_laint_disable(hw);
+	if (enabled)
+		pll_lock(pll);
 
 	return 0;
 }
-- 
2.3.3

[PATCH 2/9] clk: pistachio: Lock the PLL when enabled upon rate change

[Ezequiel Garcia]

Currently, when the rate is changed, the driver makes sure the
PLL is enabled before doing so. This is done because the PLL
cannot be locked while disabled. Once locked, the drivers
returns the PLL to its previous enable/disable state.

This is a bit cumbersome, and can be simplified.

This commit reworks the .set_rate() functions for the integer
and fractional PLLs. Upon rate change, the PLL is now locked
only if it's already enabled.

Also, the driver locks the PLL on .enable(). This makes sure
the PLL is locked when enabled, and not locked when disabled.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pll.c | 28 ++++++++++------------------
 1 file changed, 10 insertions(+), 18 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pll.c b/drivers/clk/pistachio/clk-pll.c
index 9ce1be7..f12d520 100644
--- a/drivers/clk/pistachio/clk-pll.c
+++ b/drivers/clk/pistachio/clk-pll.c
@@ -130,6 +130,8 @@ static int pll_gf40lp_frac_enable(struct clk_hw *hw)
 	val &= ~PLL_FRAC_CTRL4_BYPASS;
 	pll_writel(pll, val, PLL_CTRL4);
 
+	pll_lock(pll);
+
 	return 0;
 }
 
@@ -155,17 +157,13 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 {
 	struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
 	struct pistachio_pll_rate_table *params;
-	bool was_enabled;
+	int enabled = pll_gf40lp_frac_is_enabled(hw);
 	u32 val;
 
 	params = pll_get_params(pll, parent_rate, rate);
 	if (!params)
 		return -EINVAL;
 
-	was_enabled = pll_gf40lp_frac_is_enabled(hw);
-	if (!was_enabled)
-		pll_gf40lp_frac_enable(hw);
-
 	val = pll_readl(pll, PLL_CTRL1);
 	val &= ~((PLL_CTRL1_REFDIV_MASK << PLL_CTRL1_REFDIV_SHIFT) |
 		 (PLL_CTRL1_FBDIV_MASK << PLL_CTRL1_FBDIV_SHIFT));
@@ -184,10 +182,8 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 		(params->postdiv2 << PLL_FRAC_CTRL2_POSTDIV2_SHIFT);
 	pll_writel(pll, val, PLL_CTRL2);
 
-	pll_lock(pll);
-
-	if (!was_enabled)
-		pll_gf40lp_frac_disable(hw);
+	if (enabled)
+		pll_lock(pll);
 
 	return 0;
 }
@@ -246,6 +242,8 @@ static int pll_gf40lp_laint_enable(struct clk_hw *hw)
 	val &= ~PLL_INT_CTRL2_BYPASS;
 	pll_writel(pll, val, PLL_CTRL2);
 
+	pll_lock(pll);
+
 	return 0;
 }
 
@@ -271,17 +269,13 @@ static int pll_gf40lp_laint_set_rate(struct clk_hw *hw, unsigned long rate,
 {
 	struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
 	struct pistachio_pll_rate_table *params;
-	bool was_enabled;
+	int enabled = pll_gf40lp_laint_is_enabled(hw);
 	u32 val;
 
 	params = pll_get_params(pll, parent_rate, rate);
 	if (!params)
 		return -EINVAL;
 
-	was_enabled = pll_gf40lp_laint_is_enabled(hw);
-	if (!was_enabled)
-		pll_gf40lp_laint_enable(hw);
-
 	val = pll_readl(pll, PLL_CTRL1);
 	val &= ~((PLL_CTRL1_REFDIV_MASK << PLL_CTRL1_REFDIV_SHIFT) |
 		 (PLL_CTRL1_FBDIV_MASK << PLL_CTRL1_FBDIV_SHIFT) |
@@ -293,10 +287,8 @@ static int pll_gf40lp_laint_set_rate(struct clk_hw *hw, unsigned long rate,
 		(params->postdiv2 << PLL_INT_CTRL1_POSTDIV2_SHIFT);
 	pll_writel(pll, val, PLL_CTRL1);
 
-	pll_lock(pll);
-
-	if (!was_enabled)
-		pll_gf40lp_laint_disable(hw);
+	if (enabled)
+		pll_lock(pll);
 
 	return 0;
 }
-- 
2.3.3

[PATCH 3/9] clk: pistachio: Implement PLL rate adjustment

[Ezequiel Garcia]

This commit implements small rate changes to the fractional PLL.
This is done using the PLL frac parameter. The .set_rate function
first finds the parameters associated to the closest nominal rate.

Then the new rate is set, using parameters from the table entry,
except for the frac parameter, which is calculated from the rate
using the fractional PLL rate formula.

Using .round_rate, the driver guarantees that only rates near
a table nominal rate is applied. To this extent, add two parameters
fout_min and fout_max, which allows to define the allowed rate
adjustment.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pll.c | 48 +++++++++++++++++++++++++++++++----------
 drivers/clk/pistachio/clk.h     |  2 ++
 2 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pll.c b/drivers/clk/pistachio/clk-pll.c
index f12d520..cf000bb 100644
--- a/drivers/clk/pistachio/clk-pll.c
+++ b/drivers/clk/pistachio/clk-pll.c
@@ -90,29 +90,50 @@ static struct pistachio_pll_rate_table *
 pll_get_params(struct pistachio_clk_pll *pll, unsigned long fref,
 	       unsigned long fout)
 {
-	unsigned int i;
+	unsigned int i, best;
+	unsigned long err, best_err = ~0;
 
 	for (i = 0; i < pll->nr_rates; i++) {
-		if (pll->rates[i].fref == fref && pll->rates[i].fout == fout)
-			return &pll->rates[i];
+		err = abs(pll->rates[i].fout - fout);
+		if (pll->rates[i].fref == fref && err < best_err) {
+			best = i;
+			best_err = err;
+		}
 	}
 
-	return NULL;
+	return &pll->rates[best];
 }
 
 static long pll_round_rate(struct clk_hw *hw, unsigned long rate,
 			   unsigned long *parent_rate)
 {
 	struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
-	unsigned int i;
+	unsigned int i, best;
+	unsigned long err, best_err = ~0;
 
 	for (i = 0; i < pll->nr_rates; i++) {
-		if (i > 0 && pll->rates[i].fref == *parent_rate &&
-		    pll->rates[i].fout <= rate)
-			return pll->rates[i - 1].fout;
+		err = abs(pll->rates[i].fout - rate);
+		if (pll->rates[i].fref == *parent_rate && err < best_err) {
+			best = i;
+			best_err = err;
+		}
 	}
 
-	return pll->rates[0].fout;
+	/* Make sure fout_{min,max} parameters have sane values */
+	if (!pll->rates[best].fout_min)
+		pll->rates[best].fout_min = pll->rates[best].fout;
+	if (!pll->rates[best].fout_max)
+		pll->rates[best].fout_max = pll->rates[best].fout;
+
+	/*
+	 * If the chosen rate is within the maximum allowed PLL adjustment
+	 * then we accept it.
+	 * Otherwise, just return the closest nominal table rate.
+	 */
+	if (rate <= pll->rates[best].fout_max &&
+	    rate >= pll->rates[best].fout_min)
+		return rate;
+	return pll->rates[best].fout;
 }
 
 static int pll_gf40lp_frac_enable(struct clk_hw *hw)
@@ -158,12 +179,17 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 	struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
 	struct pistachio_pll_rate_table *params;
 	int enabled = pll_gf40lp_frac_is_enabled(hw);
-	u32 val;
+	u32 val, frac;
 
 	params = pll_get_params(pll, parent_rate, rate);
 	if (!params)
 		return -EINVAL;
 
+	/* Calculate the frac parameter */
+	frac = rate * params->refdiv * params->postdiv1 * params->postdiv2;
+	frac -= (params->fbdiv * parent_rate);
+	frac = do_div_round_closest((u64)frac << 24, parent_rate);
+
 	val = pll_readl(pll, PLL_CTRL1);
 	val &= ~((PLL_CTRL1_REFDIV_MASK << PLL_CTRL1_REFDIV_SHIFT) |
 		 (PLL_CTRL1_FBDIV_MASK << PLL_CTRL1_FBDIV_SHIFT));
@@ -177,7 +203,7 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 		  PLL_FRAC_CTRL2_POSTDIV1_SHIFT) |
 		 (PLL_FRAC_CTRL2_POSTDIV2_MASK <<
 		  PLL_FRAC_CTRL2_POSTDIV2_SHIFT));
-	val |= (params->frac << PLL_FRAC_CTRL2_FRAC_SHIFT) |
+	val |= (frac << PLL_FRAC_CTRL2_FRAC_SHIFT) |
 		(params->postdiv1 << PLL_FRAC_CTRL2_POSTDIV1_SHIFT) |
 		(params->postdiv2 << PLL_FRAC_CTRL2_POSTDIV2_SHIFT);
 	pll_writel(pll, val, PLL_CTRL2);
diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
index 52fabbc..ea48d15 100644
--- a/drivers/clk/pistachio/clk.h
+++ b/drivers/clk/pistachio/clk.h
@@ -97,6 +97,8 @@ struct pistachio_fixed_factor {
 struct pistachio_pll_rate_table {
 	unsigned long fref;
 	unsigned long fout;
+	unsigned long fout_min;
+	unsigned long fout_max;
 	unsigned int refdiv;
 	unsigned int fbdiv;
 	unsigned int postdiv1;
-- 
2.3.3

[PATCH 3/9] clk: pistachio: Implement PLL rate adjustment

[Ezequiel Garcia]

This commit implements small rate changes to the fractional PLL.
This is done using the PLL frac parameter. The .set_rate function
first finds the parameters associated to the closest nominal rate.

Then the new rate is set, using parameters from the table entry,
except for the frac parameter, which is calculated from the rate
using the fractional PLL rate formula.

Using .round_rate, the driver guarantees that only rates near
a table nominal rate is applied. To this extent, add two parameters
fout_min and fout_max, which allows to define the allowed rate
adjustment.

Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pll.c | 48 +++++++++++++++++++++++++++++++----------
 drivers/clk/pistachio/clk.h     |  2 ++
 2 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pll.c b/drivers/clk/pistachio/clk-pll.c
index f12d520..cf000bb 100644
--- a/drivers/clk/pistachio/clk-pll.c
+++ b/drivers/clk/pistachio/clk-pll.c
@@ -90,29 +90,50 @@ static struct pistachio_pll_rate_table *
 pll_get_params(struct pistachio_clk_pll *pll, unsigned long fref,
 	       unsigned long fout)
 {
-	unsigned int i;
+	unsigned int i, best;
+	unsigned long err, best_err = ~0;
 
 	for (i = 0; i < pll->nr_rates; i++) {
-		if (pll->rates[i].fref == fref && pll->rates[i].fout == fout)
-			return &pll->rates[i];
+		err = abs(pll->rates[i].fout - fout);
+		if (pll->rates[i].fref == fref && err < best_err) {
+			best = i;
+			best_err = err;
+		}
 	}
 
-	return NULL;
+	return &pll->rates[best];
 }
 
 static long pll_round_rate(struct clk_hw *hw, unsigned long rate,
 			   unsigned long *parent_rate)
 {
 	struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
-	unsigned int i;
+	unsigned int i, best;
+	unsigned long err, best_err = ~0;
 
 	for (i = 0; i < pll->nr_rates; i++) {
-		if (i > 0 && pll->rates[i].fref == *parent_rate &&
-		    pll->rates[i].fout <= rate)
-			return pll->rates[i - 1].fout;
+		err = abs(pll->rates[i].fout - rate);
+		if (pll->rates[i].fref == *parent_rate && err < best_err) {
+			best = i;
+			best_err = err;
+		}
 	}
 
-	return pll->rates[0].fout;
+	/* Make sure fout_{min,max} parameters have sane values */
+	if (!pll->rates[best].fout_min)
+		pll->rates[best].fout_min = pll->rates[best].fout;
+	if (!pll->rates[best].fout_max)
+		pll->rates[best].fout_max = pll->rates[best].fout;
+
+	/*
+	 * If the chosen rate is within the maximum allowed PLL adjustment
+	 * then we accept it.
+	 * Otherwise, just return the closest nominal table rate.
+	 */
+	if (rate <= pll->rates[best].fout_max &&
+	    rate >= pll->rates[best].fout_min)
+		return rate;
+	return pll->rates[best].fout;
 }
 
 static int pll_gf40lp_frac_enable(struct clk_hw *hw)
@@ -158,12 +179,17 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 	struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
 	struct pistachio_pll_rate_table *params;
 	int enabled = pll_gf40lp_frac_is_enabled(hw);
-	u32 val;
+	u32 val, frac;
 
 	params = pll_get_params(pll, parent_rate, rate);
 	if (!params)
 		return -EINVAL;
 
+	/* Calculate the frac parameter */
+	frac = rate * params->refdiv * params->postdiv1 * params->postdiv2;
+	frac -= (params->fbdiv * parent_rate);
+	frac = do_div_round_closest((u64)frac << 24, parent_rate);
+
 	val = pll_readl(pll, PLL_CTRL1);
 	val &= ~((PLL_CTRL1_REFDIV_MASK << PLL_CTRL1_REFDIV_SHIFT) |
 		 (PLL_CTRL1_FBDIV_MASK << PLL_CTRL1_FBDIV_SHIFT));
@@ -177,7 +203,7 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 		  PLL_FRAC_CTRL2_POSTDIV1_SHIFT) |
 		 (PLL_FRAC_CTRL2_POSTDIV2_MASK <<
 		  PLL_FRAC_CTRL2_POSTDIV2_SHIFT));
-	val |= (params->frac << PLL_FRAC_CTRL2_FRAC_SHIFT) |
+	val |= (frac << PLL_FRAC_CTRL2_FRAC_SHIFT) |
 		(params->postdiv1 << PLL_FRAC_CTRL2_POSTDIV1_SHIFT) |
 		(params->postdiv2 << PLL_FRAC_CTRL2_POSTDIV2_SHIFT);
 	pll_writel(pll, val, PLL_CTRL2);
diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
index 52fabbc..ea48d15 100644
--- a/drivers/clk/pistachio/clk.h
+++ b/drivers/clk/pistachio/clk.h
@@ -97,6 +97,8 @@ struct pistachio_fixed_factor {
 struct pistachio_pll_rate_table {
 	unsigned long fref;
 	unsigned long fout;
+	unsigned long fout_min;
+	unsigned long fout_max;
 	unsigned int refdiv;
 	unsigned int fbdiv;
 	unsigned int postdiv1;
-- 
2.3.3

Re: [PATCH 3/9] clk: pistachio: Implement PLL rate adjustment

[Andrew Bresticker]

On Thu, May 21, 2015 at 4:57 PM, Ezequiel Garcia
<ezequiel.garcia@imgtec.com> wrote:
> This commit implements small rate changes to the fractional PLL.
> This is done using the PLL frac parameter. The .set_rate function
> first finds the parameters associated to the closest nominal rate.
>
> Then the new rate is set, using parameters from the table entry,
> except for the frac parameter, which is calculated from the rate
> using the fractional PLL rate formula.
>
> Using .round_rate, the driver guarantees that only rates near
> a table nominal rate is applied. To this extent, add two parameters
> fout_min and fout_max, which allows to define the allowed rate
> adjustment.
>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> ---
>  drivers/clk/pistachio/clk-pll.c | 48 +++++++++++++++++++++++++++++++----------
>  drivers/clk/pistachio/clk.h     |  2 ++
>  2 files changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/clk/pistachio/clk-pll.c b/drivers/clk/pistachio/clk-pll.c
> index f12d520..cf000bb 100644
> --- a/drivers/clk/pistachio/clk-pll.c
> +++ b/drivers/clk/pistachio/clk-pll.c
> @@ -90,29 +90,50 @@ static struct pistachio_pll_rate_table *
>  pll_get_params(struct pistachio_clk_pll *pll, unsigned long fref,
>                unsigned long fout)
>  {
> -       unsigned int i;
> +       unsigned int i, best;
> +       unsigned long err, best_err = ~0;
>
>         for (i = 0; i < pll->nr_rates; i++) {
> -               if (pll->rates[i].fref == fref && pll->rates[i].fout == fout)
> -                       return &pll->rates[i];
> +               err = abs(pll->rates[i].fout - fout);
> +               if (pll->rates[i].fref == fref && err < best_err) {
> +                       best = i;
> +                       best_err = err;
> +               }
>         }
>
> -       return NULL;
> +       return &pll->rates[best];
>  }
>
>  static long pll_round_rate(struct clk_hw *hw, unsigned long rate,
>                            unsigned long *parent_rate)
>  {
>         struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
> -       unsigned int i;
> +       unsigned int i, best;
> +       unsigned long err, best_err = ~0;
>
>         for (i = 0; i < pll->nr_rates; i++) {
> -               if (i > 0 && pll->rates[i].fref == *parent_rate &&
> -                   pll->rates[i].fout <= rate)
> -                       return pll->rates[i - 1].fout;
> +               err = abs(pll->rates[i].fout - rate);
> +               if (pll->rates[i].fref == *parent_rate && err < best_err) {
> +                       best = i;
> +                       best_err = err;
> +               }
>         }
>
> -       return pll->rates[0].fout;
> +       /* Make sure fout_{min,max} parameters have sane values */
> +       if (!pll->rates[best].fout_min)
> +               pll->rates[best].fout_min = pll->rates[best].fout;
> +       if (!pll->rates[best].fout_max)
> +               pll->rates[best].fout_max = pll->rates[best].fout;

Perhaps the rate tables should just always populate fout_min and fout_max?

[PATCH 4/9] clk: pistachio: Extend DIV_F to pass clk_flags as well

[Ezequiel Garcia]

As preparation work to support MIPS PLL rate change propagation, this
commit extends the DIV_F macro to pass clk_flags in addition to div_flags.

Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pistachio.c | 24 ++++++++++++------------
 drivers/clk/pistachio/clk.c           |  3 ++-
 drivers/clk/pistachio/clk.h           |  7 +++++--
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pistachio.c b/drivers/clk/pistachio/clk-pistachio.c
index 8c0fe88..47e2fb1 100644
--- a/drivers/clk/pistachio/clk-pistachio.c
+++ b/drivers/clk/pistachio/clk-pistachio.c
@@ -61,13 +61,13 @@ static struct pistachio_div pistachio_divs[] __initdata = {
 	    0x204, 2),
 	DIV(CLK_MIPS_DIV, "mips_div", "mips_internal_div", 0x208, 8),
 	DIV_F(CLK_AUDIO_DIV, "audio_div", "audio_mux",
-		0x20c, 8, CLK_DIVIDER_ROUND_CLOSEST),
+		0x20c, 8, 0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(CLK_I2S_DIV, "i2s_div", "audio_pll_mux",
-		0x210, 8, CLK_DIVIDER_ROUND_CLOSEST),
+		0x210, 8, 0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(CLK_SPDIF_DIV, "spdif_div", "audio_pll_mux",
-		0x214, 8, CLK_DIVIDER_ROUND_CLOSEST),
+		0x214, 8, 0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(CLK_AUDIO_DAC_DIV, "audio_dac_div", "audio_pll_mux",
-		0x218, 8, CLK_DIVIDER_ROUND_CLOSEST),
+		0x218, 8, 0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV(CLK_RPU_V_DIV, "rpu_v_div", "rpu_v_pll_mux", 0x21c, 2),
 	DIV(CLK_RPU_L_DIV, "rpu_l_div", "rpu_l_mux", 0x220, 2),
 	DIV(CLK_RPU_SLEEP_DIV, "rpu_sleep_div", "xtal", 0x224, 10),
@@ -75,13 +75,13 @@ static struct pistachio_div pistachio_divs[] __initdata = {
 	DIV(CLK_USB_PHY_DIV, "usb_phy_div", "sys_internal_div", 0x22c, 6),
 	DIV(CLK_ENET_DIV, "enet_div", "enet_mux", 0x230, 6),
 	DIV_F(CLK_UART0_INTERNAL_DIV, "uart0_internal_div", "sys_pll_mux",
-	      0x234, 3, CLK_DIVIDER_ROUND_CLOSEST),
+	      0x234, 3, 0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(CLK_UART0_DIV, "uart0_div", "uart0_internal_div", 0x238, 10,
-	      CLK_DIVIDER_ROUND_CLOSEST),
+	      0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(CLK_UART1_INTERNAL_DIV, "uart1_internal_div", "sys_pll_mux",
-	      0x23c, 3, CLK_DIVIDER_ROUND_CLOSEST),
+	      0x23c, 3, 0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(CLK_UART1_DIV, "uart1_div", "uart1_internal_div", 0x240, 10,
-	      CLK_DIVIDER_ROUND_CLOSEST),
+	      0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV(CLK_SYS_INTERNAL_DIV, "sys_internal_div", "sys_pll_mux", 0x244, 3),
 	DIV(CLK_SPI0_INTERNAL_DIV, "spi0_internal_div", "sys_pll_mux",
 	    0x248, 3),
@@ -226,13 +226,13 @@ static struct pistachio_div pistachio_periph_divs[] __initdata = {
 	DIV(PERIPH_CLK_COUNTER_SLOW_DIV, "counter_slow_div",
 	    "counter_slow_pre_div", 0x118, 7),
 	DIV_F(PERIPH_CLK_IR_PRE_DIV, "ir_pre_div", "periph_sys", 0x11c, 7,
-	      CLK_DIVIDER_ROUND_CLOSEST),
+	      0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(PERIPH_CLK_IR_DIV, "ir_div", "ir_pre_div", 0x120, 7,
-	      CLK_DIVIDER_ROUND_CLOSEST),
+	      0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(PERIPH_CLK_WD_PRE_DIV, "wd_pre_div", "periph_sys", 0x124, 7,
-	      CLK_DIVIDER_ROUND_CLOSEST),
+	      0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(PERIPH_CLK_WD_DIV, "wd_div", "wd_pre_div", 0x128, 7,
-	      CLK_DIVIDER_ROUND_CLOSEST),
+	      0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV(PERIPH_CLK_PDM_PRE_DIV, "pdm_pre_div", "periph_sys", 0x12c, 7),
 	DIV(PERIPH_CLK_PDM_DIV, "pdm_div", "pdm_pre_div", 0x130, 7),
 	DIV(PERIPH_CLK_PWM_PRE_DIV, "pwm_pre_div", "periph_sys", 0x134, 7),
diff --git a/drivers/clk/pistachio/clk.c b/drivers/clk/pistachio/clk.c
index 85faa83..380879b 100644
--- a/drivers/clk/pistachio/clk.c
+++ b/drivers/clk/pistachio/clk.c
@@ -99,7 +99,8 @@ void pistachio_clk_register_div(struct pistachio_clk_provider *p,
 
 	for (i = 0; i < num; i++) {
 		clk = clk_register_divider(NULL, div[i].name, div[i].parent,
-					   0, p->base + div[i].reg, 0,
+					   div[i].clk_flags,
+					   p->base + div[i].reg, 0,
 					   div[i].width, div[i].div_flags,
 					   NULL);
 		p->clk_data.clks[div[i].id] = clk;
diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
index ea48d15..1589227 100644
--- a/drivers/clk/pistachio/clk.h
+++ b/drivers/clk/pistachio/clk.h
@@ -54,6 +54,7 @@ struct pistachio_div {
 	unsigned int id;
 	unsigned long reg;
 	unsigned int width;
+	unsigned int clk_flags;
 	unsigned int div_flags;
 	const char *name;
 	const char *parent;
@@ -64,17 +65,19 @@ struct pistachio_div {
 		.id		= _id,				\
 		.reg		= _reg,				\
 		.width		= _width,			\
+		.clk_flags	= 0,				\
 		.div_flags	= 0,				\
 		.name		= _name,			\
 		.parent		= _pname,			\
 	}
 
-#define DIV_F(_id, _name, _pname, _reg, _width, _div_flags)	\
+#define DIV_F(_id, _name, _pname, _reg, _width, _clkf, _divf)	\
 	{							\
 		.id		= _id,				\
 		.reg		= _reg,				\
 		.width		= _width,			\
-		.div_flags	= _div_flags,			\
+		.clk_flags	= _clkf,			\
+		.div_flags	= _divf,			\
 		.name		= _name,			\
 		.parent		= _pname,			\
 	}
-- 
2.3.3

[PATCH 4/9] clk: pistachio: Extend DIV_F to pass clk_flags as well

[Ezequiel Garcia]

As preparation work to support MIPS PLL rate change propagation, this
commit extends the DIV_F macro to pass clk_flags in addition to div_flags.

Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pistachio.c | 24 ++++++++++++------------
 drivers/clk/pistachio/clk.c           |  3 ++-
 drivers/clk/pistachio/clk.h           |  7 +++++--
 3 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pistachio.c b/drivers/clk/pistachio/clk-pistachio.c
index 8c0fe88..47e2fb1 100644
--- a/drivers/clk/pistachio/clk-pistachio.c
+++ b/drivers/clk/pistachio/clk-pistachio.c
@@ -61,13 +61,13 @@ static struct pistachio_div pistachio_divs[] __initdata = {
 	    0x204, 2),
 	DIV(CLK_MIPS_DIV, "mips_div", "mips_internal_div", 0x208, 8),
 	DIV_F(CLK_AUDIO_DIV, "audio_div", "audio_mux",
-		0x20c, 8, CLK_DIVIDER_ROUND_CLOSEST),
+		0x20c, 8, 0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(CLK_I2S_DIV, "i2s_div", "audio_pll_mux",
-		0x210, 8, CLK_DIVIDER_ROUND_CLOSEST),
+		0x210, 8, 0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(CLK_SPDIF_DIV, "spdif_div", "audio_pll_mux",
-		0x214, 8, CLK_DIVIDER_ROUND_CLOSEST),
+		0x214, 8, 0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(CLK_AUDIO_DAC_DIV, "audio_dac_div", "audio_pll_mux",
-		0x218, 8, CLK_DIVIDER_ROUND_CLOSEST),
+		0x218, 8, 0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV(CLK_RPU_V_DIV, "rpu_v_div", "rpu_v_pll_mux", 0x21c, 2),
 	DIV(CLK_RPU_L_DIV, "rpu_l_div", "rpu_l_mux", 0x220, 2),
 	DIV(CLK_RPU_SLEEP_DIV, "rpu_sleep_div", "xtal", 0x224, 10),
@@ -75,13 +75,13 @@ static struct pistachio_div pistachio_divs[] __initdata = {
 	DIV(CLK_USB_PHY_DIV, "usb_phy_div", "sys_internal_div", 0x22c, 6),
 	DIV(CLK_ENET_DIV, "enet_div", "enet_mux", 0x230, 6),
 	DIV_F(CLK_UART0_INTERNAL_DIV, "uart0_internal_div", "sys_pll_mux",
-	      0x234, 3, CLK_DIVIDER_ROUND_CLOSEST),
+	      0x234, 3, 0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(CLK_UART0_DIV, "uart0_div", "uart0_internal_div", 0x238, 10,
-	      CLK_DIVIDER_ROUND_CLOSEST),
+	      0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(CLK_UART1_INTERNAL_DIV, "uart1_internal_div", "sys_pll_mux",
-	      0x23c, 3, CLK_DIVIDER_ROUND_CLOSEST),
+	      0x23c, 3, 0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(CLK_UART1_DIV, "uart1_div", "uart1_internal_div", 0x240, 10,
-	      CLK_DIVIDER_ROUND_CLOSEST),
+	      0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV(CLK_SYS_INTERNAL_DIV, "sys_internal_div", "sys_pll_mux", 0x244, 3),
 	DIV(CLK_SPI0_INTERNAL_DIV, "spi0_internal_div", "sys_pll_mux",
 	    0x248, 3),
@@ -226,13 +226,13 @@ static struct pistachio_div pistachio_periph_divs[] __initdata = {
 	DIV(PERIPH_CLK_COUNTER_SLOW_DIV, "counter_slow_div",
 	    "counter_slow_pre_div", 0x118, 7),
 	DIV_F(PERIPH_CLK_IR_PRE_DIV, "ir_pre_div", "periph_sys", 0x11c, 7,
-	      CLK_DIVIDER_ROUND_CLOSEST),
+	      0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(PERIPH_CLK_IR_DIV, "ir_div", "ir_pre_div", 0x120, 7,
-	      CLK_DIVIDER_ROUND_CLOSEST),
+	      0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(PERIPH_CLK_WD_PRE_DIV, "wd_pre_div", "periph_sys", 0x124, 7,
-	      CLK_DIVIDER_ROUND_CLOSEST),
+	      0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(PERIPH_CLK_WD_DIV, "wd_div", "wd_pre_div", 0x128, 7,
-	      CLK_DIVIDER_ROUND_CLOSEST),
+	      0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV(PERIPH_CLK_PDM_PRE_DIV, "pdm_pre_div", "periph_sys", 0x12c, 7),
 	DIV(PERIPH_CLK_PDM_DIV, "pdm_div", "pdm_pre_div", 0x130, 7),
 	DIV(PERIPH_CLK_PWM_PRE_DIV, "pwm_pre_div", "periph_sys", 0x134, 7),
diff --git a/drivers/clk/pistachio/clk.c b/drivers/clk/pistachio/clk.c
index 85faa83..380879b 100644
--- a/drivers/clk/pistachio/clk.c
+++ b/drivers/clk/pistachio/clk.c
@@ -99,7 +99,8 @@ void pistachio_clk_register_div(struct pistachio_clk_provider *p,
 
 	for (i = 0; i < num; i++) {
 		clk = clk_register_divider(NULL, div[i].name, div[i].parent,
-					   0, p->base + div[i].reg, 0,
+					   div[i].clk_flags,
+					   p->base + div[i].reg, 0,
 					   div[i].width, div[i].div_flags,
 					   NULL);
 		p->clk_data.clks[div[i].id] = clk;
diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
index ea48d15..1589227 100644
--- a/drivers/clk/pistachio/clk.h
+++ b/drivers/clk/pistachio/clk.h
@@ -54,6 +54,7 @@ struct pistachio_div {
 	unsigned int id;
 	unsigned long reg;
 	unsigned int width;
+	unsigned int clk_flags;
 	unsigned int div_flags;
 	const char *name;
 	const char *parent;
@@ -64,17 +65,19 @@ struct pistachio_div {
 		.id		= _id,				\
 		.reg		= _reg,				\
 		.width		= _width,			\
+		.clk_flags	= 0,				\
 		.div_flags	= 0,				\
 		.name		= _name,			\
 		.parent		= _pname,			\
 	}
 
-#define DIV_F(_id, _name, _pname, _reg, _width, _div_flags)	\
+#define DIV_F(_id, _name, _pname, _reg, _width, _clkf, _divf)	\
 	{							\
 		.id		= _id,				\
 		.reg		= _reg,				\
 		.width		= _width,			\
-		.div_flags	= _div_flags,			\
+		.clk_flags	= _clkf,			\
+		.div_flags	= _divf,			\
 		.name		= _name,			\
 		.parent		= _pname,			\
 	}
-- 
2.3.3

Re: [PATCH 4/9] clk: pistachio: Extend DIV_F to pass clk_flags as well

[Andrew Bresticker]

On Thu, May 21, 2015 at 4:57 PM, Ezequiel Garcia
<ezequiel.garcia@imgtec.com> wrote:
> As preparation work to support MIPS PLL rate change propagation, this
> commit extends the DIV_F macro to pass clk_flags in addition to div_flags.
>
> Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

Reviewed-by: Andrew Bresticker <abrestic@chromium.org>

[PATCH 5/9] clk: pistachio: Add a MUX_F macro to pass clk_flags

[Ezequiel Garcia]

As preparation work to support MIPS PLL rate change propagation, this
commit adds a MUX_F macro to pass clk_flags.

Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk.c |  2 +-
 drivers/clk/pistachio/clk.h | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/pistachio/clk.c b/drivers/clk/pistachio/clk.c
index 380879b..c9228e3 100644
--- a/drivers/clk/pistachio/clk.c
+++ b/drivers/clk/pistachio/clk.c
@@ -82,7 +82,7 @@ void pistachio_clk_register_mux(struct pistachio_clk_provider *p,
 	for (i = 0; i < num; i++) {
 		clk = clk_register_mux(NULL, mux[i].name, mux[i].parents,
 				       mux[i].num_parents,
-				       CLK_SET_RATE_NO_REPARENT,
+				       mux[i].clk_flags,
 				       p->base + mux[i].reg, mux[i].shift,
 				       get_count_order(mux[i].num_parents),
 				       0, NULL);
diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
index 1589227..3bb6bbe 100644
--- a/drivers/clk/pistachio/clk.h
+++ b/drivers/clk/pistachio/clk.h
@@ -32,6 +32,7 @@ struct pistachio_mux {
 	unsigned int id;
 	unsigned long reg;
 	unsigned int shift;
+	unsigned int clk_flags;
 	unsigned int num_parents;
 	const char *name;
 	const char **parents;
@@ -44,11 +45,22 @@ struct pistachio_mux {
 		.id		= _id,				\
 		.reg		= _reg,				\
 		.shift		= _shift,			\
+		.clk_flags      = CLK_SET_RATE_NO_REPARENT,     \
 		.name		= _name,			\
 		.parents	= _pnames,			\
 		.num_parents	= ARRAY_SIZE(_pnames)		\
 	}
 
+#define MUX_F(_id, _name, _pnames, _reg, _shift, _clkf)		\
+{                                                               \
+	.id             = _id,                                  \
+	.reg            = _reg,                                 \
+	.shift          = _shift,                               \
+	.name           = _name,                                \
+	.parents        = _pnames,                              \
+	.num_parents    = ARRAY_SIZE(_pnames),                  \
+	.clk_flags      = _clkf,				\
+}
 
 struct pistachio_div {
 	unsigned int id;
-- 
2.3.3

[PATCH 5/9] clk: pistachio: Add a MUX_F macro to pass clk_flags

[Ezequiel Garcia]

As preparation work to support MIPS PLL rate change propagation, this
commit adds a MUX_F macro to pass clk_flags.

Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk.c |  2 +-
 drivers/clk/pistachio/clk.h | 12 ++++++++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/pistachio/clk.c b/drivers/clk/pistachio/clk.c
index 380879b..c9228e3 100644
--- a/drivers/clk/pistachio/clk.c
+++ b/drivers/clk/pistachio/clk.c
@@ -82,7 +82,7 @@ void pistachio_clk_register_mux(struct pistachio_clk_provider *p,
 	for (i = 0; i < num; i++) {
 		clk = clk_register_mux(NULL, mux[i].name, mux[i].parents,
 				       mux[i].num_parents,
-				       CLK_SET_RATE_NO_REPARENT,
+				       mux[i].clk_flags,
 				       p->base + mux[i].reg, mux[i].shift,
 				       get_count_order(mux[i].num_parents),
 				       0, NULL);
diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
index 1589227..3bb6bbe 100644
--- a/drivers/clk/pistachio/clk.h
+++ b/drivers/clk/pistachio/clk.h
@@ -32,6 +32,7 @@ struct pistachio_mux {
 	unsigned int id;
 	unsigned long reg;
 	unsigned int shift;
+	unsigned int clk_flags;
 	unsigned int num_parents;
 	const char *name;
 	const char **parents;
@@ -44,11 +45,22 @@ struct pistachio_mux {
 		.id		= _id,				\
 		.reg		= _reg,				\
 		.shift		= _shift,			\
+		.clk_flags      = CLK_SET_RATE_NO_REPARENT,     \
 		.name		= _name,			\
 		.parents	= _pnames,			\
 		.num_parents	= ARRAY_SIZE(_pnames)		\
 	}
 
+#define MUX_F(_id, _name, _pnames, _reg, _shift, _clkf)		\
+{                                                               \
+	.id             = _id,                                  \
+	.reg            = _reg,                                 \
+	.shift          = _shift,                               \
+	.name           = _name,                                \
+	.parents        = _pnames,                              \
+	.num_parents    = ARRAY_SIZE(_pnames),                  \
+	.clk_flags      = _clkf,				\
+}
 
 struct pistachio_div {
 	unsigned int id;
-- 
2.3.3

Re: [PATCH 5/9] clk: pistachio: Add a MUX_F macro to pass clk_flags

[Andrew Bresticker]

On Thu, May 21, 2015 at 4:57 PM, Ezequiel Garcia
<ezequiel.garcia@imgtec.com> wrote:
> As preparation work to support MIPS PLL rate change propagation, this
> commit adds a MUX_F macro to pass clk_flags.
>
> Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

> --- a/drivers/clk/pistachio/clk.h
> +++ b/drivers/clk/pistachio/clk.h

> @@ -44,11 +45,22 @@ struct pistachio_mux {
>                 .id             = _id,                          \
>                 .reg            = _reg,                         \
>                 .shift          = _shift,                       \
> +               .clk_flags      = CLK_SET_RATE_NO_REPARENT,     \
>                 .name           = _name,                        \
>                 .parents        = _pnames,                      \
>                 .num_parents    = ARRAY_SIZE(_pnames)           \
>         }
>
> +#define MUX_F(_id, _name, _pnames, _reg, _shift, _clkf)                \
> +{                                                               \
> +       .id             = _id,                                  \
> +       .reg            = _reg,                                 \
> +       .shift          = _shift,                               \
> +       .name           = _name,                                \
> +       .parents        = _pnames,                              \
> +       .num_parents    = ARRAY_SIZE(_pnames),                  \
> +       .clk_flags      = _clkf,                                \
> +}

nit: the indentation here is inconsistent with the other macros in this file.

[PATCH 6/9] clk: pistachio: Propagate rate changes in the MIPS PLL clock sub-tree

[Ezequiel Garcia]

This commit passes CLK_SET_RATE_PARENT to the "mips_div",
"mips_internal_div", and "mips_pll_mux" clocks. This flag is needed for the
"mips" clock to propagate rate changes up to the "mips_pll" root clock.

Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pistachio.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pistachio.c b/drivers/clk/pistachio/clk-pistachio.c
index 47e2fb1..22a7ebd 100644
--- a/drivers/clk/pistachio/clk-pistachio.c
+++ b/drivers/clk/pistachio/clk-pistachio.c
@@ -57,9 +57,10 @@ static struct pistachio_fixed_factor pistachio_ffs[] __initdata = {
 };
 
 static struct pistachio_div pistachio_divs[] __initdata = {
-	DIV(CLK_MIPS_INTERNAL_DIV, "mips_internal_div", "mips_pll_mux",
-	    0x204, 2),
-	DIV(CLK_MIPS_DIV, "mips_div", "mips_internal_div", 0x208, 8),
+	DIV_F(CLK_MIPS_INTERNAL_DIV, "mips_internal_div", "mips_pll_mux",
+		0x204, 2, CLK_SET_RATE_PARENT, 0),
+	DIV_F(CLK_MIPS_DIV, "mips_div", "mips_internal_div",
+		0x208, 8, CLK_SET_RATE_PARENT, 0),
 	DIV_F(CLK_AUDIO_DIV, "audio_div", "audio_mux",
 		0x20c, 8, 0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(CLK_I2S_DIV, "i2s_div", "audio_pll_mux",
@@ -126,7 +127,8 @@ PNAME(mux_xtal_bt) = { "xtal", "bt_pll" };
 static struct pistachio_mux pistachio_muxes[] __initdata = {
 	MUX(CLK_AUDIO_REF_MUX, "audio_refclk_mux", mux_xtal_audio_refclk,
 	    0x200, 0),
-	MUX(CLK_MIPS_PLL_MUX, "mips_pll_mux", mux_xtal_mips, 0x200, 1),
+	MUX_F(CLK_MIPS_PLL_MUX, "mips_pll_mux", mux_xtal_mips,
+	    0x200, 1, CLK_SET_RATE_PARENT),
 	MUX(CLK_AUDIO_PLL_MUX, "audio_pll_mux", mux_xtal_audio, 0x200, 2),
 	MUX(CLK_AUDIO_MUX, "audio_mux", mux_audio_debug, 0x200, 4),
 	MUX(CLK_RPU_V_PLL_MUX, "rpu_v_pll_mux", mux_xtal_rpu_v, 0x200, 5),
-- 
2.3.3

[PATCH 6/9] clk: pistachio: Propagate rate changes in the MIPS PLL clock sub-tree

[Ezequiel Garcia]

This commit passes CLK_SET_RATE_PARENT to the "mips_div",
"mips_internal_div", and "mips_pll_mux" clocks. This flag is needed for the
"mips" clock to propagate rate changes up to the "mips_pll" root clock.

Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pistachio.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pistachio.c b/drivers/clk/pistachio/clk-pistachio.c
index 47e2fb1..22a7ebd 100644
--- a/drivers/clk/pistachio/clk-pistachio.c
+++ b/drivers/clk/pistachio/clk-pistachio.c
@@ -57,9 +57,10 @@ static struct pistachio_fixed_factor pistachio_ffs[] __initdata = {
 };
 
 static struct pistachio_div pistachio_divs[] __initdata = {
-	DIV(CLK_MIPS_INTERNAL_DIV, "mips_internal_div", "mips_pll_mux",
-	    0x204, 2),
-	DIV(CLK_MIPS_DIV, "mips_div", "mips_internal_div", 0x208, 8),
+	DIV_F(CLK_MIPS_INTERNAL_DIV, "mips_internal_div", "mips_pll_mux",
+		0x204, 2, CLK_SET_RATE_PARENT, 0),
+	DIV_F(CLK_MIPS_DIV, "mips_div", "mips_internal_div",
+		0x208, 8, CLK_SET_RATE_PARENT, 0),
 	DIV_F(CLK_AUDIO_DIV, "audio_div", "audio_mux",
 		0x20c, 8, 0, CLK_DIVIDER_ROUND_CLOSEST),
 	DIV_F(CLK_I2S_DIV, "i2s_div", "audio_pll_mux",
@@ -126,7 +127,8 @@ PNAME(mux_xtal_bt) = { "xtal", "bt_pll" };
 static struct pistachio_mux pistachio_muxes[] __initdata = {
 	MUX(CLK_AUDIO_REF_MUX, "audio_refclk_mux", mux_xtal_audio_refclk,
 	    0x200, 0),
-	MUX(CLK_MIPS_PLL_MUX, "mips_pll_mux", mux_xtal_mips, 0x200, 1),
+	MUX_F(CLK_MIPS_PLL_MUX, "mips_pll_mux", mux_xtal_mips,
+	    0x200, 1, CLK_SET_RATE_PARENT),
 	MUX(CLK_AUDIO_PLL_MUX, "audio_pll_mux", mux_xtal_audio, 0x200, 2),
 	MUX(CLK_AUDIO_MUX, "audio_mux", mux_audio_debug, 0x200, 4),
 	MUX(CLK_RPU_V_PLL_MUX, "rpu_v_pll_mux", mux_xtal_rpu_v, 0x200, 5),
-- 
2.3.3

Re: [PATCH 6/9] clk: pistachio: Propagate rate changes in the MIPS PLL clock sub-tree

[Andrew Bresticker]

On Thu, May 21, 2015 at 4:57 PM, Ezequiel Garcia
<ezequiel.garcia@imgtec.com> wrote:
> This commit passes CLK_SET_RATE_PARENT to the "mips_div",
> "mips_internal_div", and "mips_pll_mux" clocks. This flag is needed for the
> "mips" clock to propagate rate changes up to the "mips_pll" root clock.
>
> Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

IIRC the clk core will prefer changing a downstream divider over
propagating the rate change up another level.  So, for example, if
MIPS_PLL is initially 400Mhz and we request a MIPS rate of 200Mhz,
we'll change the first intermediate divider to /2 rather than
propagate the rate change up to MIPS_PLL.  Wouldn't it be more
power-efficient to set the MIPS_PLL directly to the requested rate
rather than using external dividers to divide it down?

Re: [PATCH 6/9] clk: pistachio: Propagate rate changes in the MIPS PLL clock sub-tree

[Ezequiel Garcia]

On 05/22/2015 02:42 PM, Andrew Bresticker wrote:
> On Thu, May 21, 2015 at 4:57 PM, Ezequiel Garcia
> <ezequiel.garcia@imgtec.com> wrote:
>> This commit passes CLK_SET_RATE_PARENT to the "mips_div",
>> "mips_internal_div", and "mips_pll_mux" clocks. This flag is needed for the
>> "mips" clock to propagate rate changes up to the "mips_pll" root clock.
>>
>> Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> 
> IIRC the clk core will prefer changing a downstream divider over
> propagating the rate change up another level.  So, for example, if
> MIPS_PLL is initially 400Mhz and we request a MIPS rate of 200Mhz,
> we'll change the first intermediate divider to /2 rather than
> propagate the rate change up to MIPS_PLL.  Wouldn't it be more
> power-efficient to set the MIPS_PLL directly to the requested rate
> rather than using external dividers to divide it down?
> 

Indeed.

Do you think we still want to be able to change the MIPS clk rate and
propagate the change up to the PLL? Otherwise, I'll drop this patch and
I'll drop the DIV_F and MUX_F macro patches.

-- 
Ezequiel

Re: [PATCH 6/9] clk: pistachio: Propagate rate changes in the MIPS PLL clock sub-tree

[Andrew Bresticker]

On Fri, May 22, 2015 at 10:52 AM, Ezequiel Garcia
<ezequiel.garcia@imgtec.com> wrote:
>
>
> On 05/22/2015 02:42 PM, Andrew Bresticker wrote:
>> On Thu, May 21, 2015 at 4:57 PM, Ezequiel Garcia
>> <ezequiel.garcia@imgtec.com> wrote:
>>> This commit passes CLK_SET_RATE_PARENT to the "mips_div",
>>> "mips_internal_div", and "mips_pll_mux" clocks. This flag is needed for the
>>> "mips" clock to propagate rate changes up to the "mips_pll" root clock.
>>>
>>> Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
>>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
>>
>> IIRC the clk core will prefer changing a downstream divider over
>> propagating the rate change up another level.  So, for example, if
>> MIPS_PLL is initially 400Mhz and we request a MIPS rate of 200Mhz,
>> we'll change the first intermediate divider to /2 rather than
>> propagate the rate change up to MIPS_PLL.  Wouldn't it be more
>> power-efficient to set the MIPS_PLL directly to the requested rate
>> rather than using external dividers to divide it down?
>>
>
> Indeed.
>
> Do you think we still want to be able to change the MIPS clk rate and
> propagate the change up to the PLL? Otherwise, I'll drop this patch and
> I'll drop the DIV_F and MUX_F macro patches.

Well I do think we want to propagate changes from MIPS up to MIPS_PLL,
but we need to work around the behavior of CLK_SET_RATE_PARENT when
applied to divider clocks.  Since there's no reason to ever set the
dividers between MIPS and MIPS_PLL to something besides /1
(James/Govindraj/Damien, correct me if I'm wrong here), then I think
we could just set CLK_DIVIDER_READ_ONLY on those dividers.  It's a bit
of a hack, but it's certainly simpler than writing a separate CPU
clock driver.

[PATCH 7/9] clk: pistachio: Add a rate table for the MIPS PLL

[Ezequiel Garcia]

This commit adds a rate parameter table, which makes it possible for
the MIPS PLL to support rate change.

Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pistachio.c | 12 +++++++++++-
 drivers/clk/pistachio/clk.h           | 12 ++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/pistachio/clk-pistachio.c b/drivers/clk/pistachio/clk-pistachio.c
index 22a7ebd..0ac7429 100644
--- a/drivers/clk/pistachio/clk-pistachio.c
+++ b/drivers/clk/pistachio/clk-pistachio.c
@@ -145,8 +145,18 @@ static struct pistachio_mux pistachio_muxes[] __initdata = {
 	MUX(CLK_BT_PLL_MUX, "bt_pll_mux", mux_xtal_bt, 0x200, 17),
 };
 
+static struct pistachio_pll_rate_table mips_pll_rates[] = {
+	MIPS_PLL_RATES(52000000, 416000000, 1, 16, 2, 1),
+	MIPS_PLL_RATES(52000000, 442000000, 1, 17, 2, 1),
+	MIPS_PLL_RATES(52000000, 468000000, 1, 18, 2, 1),
+	MIPS_PLL_RATES(52000000, 494000000, 1, 19, 2, 1),
+	MIPS_PLL_RATES(52000000, 520000000, 1, 20, 2, 1),
+	MIPS_PLL_RATES(52000000, 546000000, 1, 21, 2, 1),
+};
+
 static struct pistachio_pll pistachio_plls[] __initdata = {
-	PLL_FIXED(CLK_MIPS_PLL, "mips_pll", "xtal", PLL_GF40LP_LAINT, 0x0),
+	PLL(CLK_MIPS_PLL, "mips_pll", "xtal", PLL_GF40LP_LAINT, 0x0,
+	    mips_pll_rates),
 	PLL_FIXED(CLK_AUDIO_PLL, "audio_pll", "audio_refclk_mux",
 		  PLL_GF40LP_FRAC, 0xc),
 	PLL_FIXED(CLK_RPU_V_PLL, "rpu_v_pll", "xtal", PLL_GF40LP_LAINT, 0x20),
diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
index 3bb6bbe..b5d22d6 100644
--- a/drivers/clk/pistachio/clk.h
+++ b/drivers/clk/pistachio/clk.h
@@ -121,6 +121,18 @@ struct pistachio_pll_rate_table {
 	unsigned int frac;
 };
 
+#define MIPS_PLL_RATES(_fref, _fout, _refdiv, _fbdiv,	\
+		       _postdiv1, _postdiv2)		\
+{                                                       \
+	.fref           = _fref,                        \
+	.fout           = _fout,                        \
+	.refdiv         = _refdiv,                      \
+	.fbdiv          = _fbdiv,                       \
+	.postdiv1       = _postdiv1,                    \
+	.postdiv2       = _postdiv2,                    \
+	.frac           = 0,                            \
+}
+
 enum pistachio_pll_type {
 	PLL_GF40LP_LAINT,
 	PLL_GF40LP_FRAC,
-- 
2.3.3

[PATCH 7/9] clk: pistachio: Add a rate table for the MIPS PLL

[Ezequiel Garcia]

This commit adds a rate parameter table, which makes it possible for
the MIPS PLL to support rate change.

Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pistachio.c | 12 +++++++++++-
 drivers/clk/pistachio/clk.h           | 12 ++++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/pistachio/clk-pistachio.c b/drivers/clk/pistachio/clk-pistachio.c
index 22a7ebd..0ac7429 100644
--- a/drivers/clk/pistachio/clk-pistachio.c
+++ b/drivers/clk/pistachio/clk-pistachio.c
@@ -145,8 +145,18 @@ static struct pistachio_mux pistachio_muxes[] __initdata = {
 	MUX(CLK_BT_PLL_MUX, "bt_pll_mux", mux_xtal_bt, 0x200, 17),
 };
 
+static struct pistachio_pll_rate_table mips_pll_rates[] = {
+	MIPS_PLL_RATES(52000000, 416000000, 1, 16, 2, 1),
+	MIPS_PLL_RATES(52000000, 442000000, 1, 17, 2, 1),
+	MIPS_PLL_RATES(52000000, 468000000, 1, 18, 2, 1),
+	MIPS_PLL_RATES(52000000, 494000000, 1, 19, 2, 1),
+	MIPS_PLL_RATES(52000000, 520000000, 1, 20, 2, 1),
+	MIPS_PLL_RATES(52000000, 546000000, 1, 21, 2, 1),
+};
+
 static struct pistachio_pll pistachio_plls[] __initdata = {
-	PLL_FIXED(CLK_MIPS_PLL, "mips_pll", "xtal", PLL_GF40LP_LAINT, 0x0),
+	PLL(CLK_MIPS_PLL, "mips_pll", "xtal", PLL_GF40LP_LAINT, 0x0,
+	    mips_pll_rates),
 	PLL_FIXED(CLK_AUDIO_PLL, "audio_pll", "audio_refclk_mux",
 		  PLL_GF40LP_FRAC, 0xc),
 	PLL_FIXED(CLK_RPU_V_PLL, "rpu_v_pll", "xtal", PLL_GF40LP_LAINT, 0x20),
diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
index 3bb6bbe..b5d22d6 100644
--- a/drivers/clk/pistachio/clk.h
+++ b/drivers/clk/pistachio/clk.h
@@ -121,6 +121,18 @@ struct pistachio_pll_rate_table {
 	unsigned int frac;
 };
 
+#define MIPS_PLL_RATES(_fref, _fout, _refdiv, _fbdiv,	\
+		       _postdiv1, _postdiv2)		\
+{                                                       \
+	.fref           = _fref,                        \
+	.fout           = _fout,                        \
+	.refdiv         = _refdiv,                      \
+	.fbdiv          = _fbdiv,                       \
+	.postdiv1       = _postdiv1,                    \
+	.postdiv2       = _postdiv2,                    \
+	.frac           = 0,                            \
+}
+
 enum pistachio_pll_type {
 	PLL_GF40LP_LAINT,
 	PLL_GF40LP_FRAC,
-- 
2.3.3

Re: [PATCH 7/9] clk: pistachio: Add a rate table for the MIPS PLL

[Andrew Bresticker]

On Thu, May 21, 2015 at 4:57 PM, Ezequiel Garcia
<ezequiel.garcia@imgtec.com> wrote:
> This commit adds a rate parameter table, which makes it possible for
> the MIPS PLL to support rate change.
>
> Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> ---
>  drivers/clk/pistachio/clk-pistachio.c | 12 +++++++++++-
>  drivers/clk/pistachio/clk.h           | 12 ++++++++++++
>  2 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/pistachio/clk-pistachio.c b/drivers/clk/pistachio/clk-pistachio.c
> index 22a7ebd..0ac7429 100644
> --- a/drivers/clk/pistachio/clk-pistachio.c
> +++ b/drivers/clk/pistachio/clk-pistachio.c
> @@ -145,8 +145,18 @@ static struct pistachio_mux pistachio_muxes[] __initdata = {
>         MUX(CLK_BT_PLL_MUX, "bt_pll_mux", mux_xtal_bt, 0x200, 17),
>  };
>
> +static struct pistachio_pll_rate_table mips_pll_rates[] = {
> +       MIPS_PLL_RATES(52000000, 416000000, 1, 16, 2, 1),
> +       MIPS_PLL_RATES(52000000, 442000000, 1, 17, 2, 1),
> +       MIPS_PLL_RATES(52000000, 468000000, 1, 18, 2, 1),
> +       MIPS_PLL_RATES(52000000, 494000000, 1, 19, 2, 1),
> +       MIPS_PLL_RATES(52000000, 520000000, 1, 20, 2, 1),
> +       MIPS_PLL_RATES(52000000, 546000000, 1, 21, 2, 1),
> +};
> +
>  static struct pistachio_pll pistachio_plls[] __initdata = {
> -       PLL_FIXED(CLK_MIPS_PLL, "mips_pll", "xtal", PLL_GF40LP_LAINT, 0x0),
> +       PLL(CLK_MIPS_PLL, "mips_pll", "xtal", PLL_GF40LP_LAINT, 0x0,
> +           mips_pll_rates),
>         PLL_FIXED(CLK_AUDIO_PLL, "audio_pll", "audio_refclk_mux",
>                   PLL_GF40LP_FRAC, 0xc),
>         PLL_FIXED(CLK_RPU_V_PLL, "rpu_v_pll", "xtal", PLL_GF40LP_LAINT, 0x20),
> diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
> index 3bb6bbe..b5d22d6 100644
> --- a/drivers/clk/pistachio/clk.h
> +++ b/drivers/clk/pistachio/clk.h
> @@ -121,6 +121,18 @@ struct pistachio_pll_rate_table {
>         unsigned int frac;
>  };
>
> +#define MIPS_PLL_RATES(_fref, _fout, _refdiv, _fbdiv,  \
> +                      _postdiv1, _postdiv2)            \
> +{                                                       \
> +       .fref           = _fref,                        \
> +       .fout           = _fout,                        \
> +       .refdiv         = _refdiv,                      \
> +       .fbdiv          = _fbdiv,                       \
> +       .postdiv1       = _postdiv1,                    \
> +       .postdiv2       = _postdiv2,                    \
> +       .frac           = 0,                            \
> +}

Wouldn't this be applicable to any integer PLL, not just MIPS_PLL?
Also, maybe we should just populate fout_{min,max} here, setting them
to _fout?  See my comment in patch 3/9.

Re: [PATCH 7/9] clk: pistachio: Add a rate table for the MIPS PLL

[Ezequiel Garcia]

On 05/22/2015 02:45 PM, Andrew Bresticker wrote:
> On Thu, May 21, 2015 at 4:57 PM, Ezequiel Garcia
> <ezequiel.garcia@imgtec.com> wrote:
>> This commit adds a rate parameter table, which makes it possible for
>> the MIPS PLL to support rate change.
>>
>> Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
>> ---
>>  drivers/clk/pistachio/clk-pistachio.c | 12 +++++++++++-
>>  drivers/clk/pistachio/clk.h           | 12 ++++++++++++
>>  2 files changed, 23 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/pistachio/clk-pistachio.c b/drivers/clk/pistachio/clk-pistachio.c
>> index 22a7ebd..0ac7429 100644
>> --- a/drivers/clk/pistachio/clk-pistachio.c
>> +++ b/drivers/clk/pistachio/clk-pistachio.c
>> @@ -145,8 +145,18 @@ static struct pistachio_mux pistachio_muxes[] __initdata = {
>>         MUX(CLK_BT_PLL_MUX, "bt_pll_mux", mux_xtal_bt, 0x200, 17),
>>  };
>>
>> +static struct pistachio_pll_rate_table mips_pll_rates[] = {
>> +       MIPS_PLL_RATES(52000000, 416000000, 1, 16, 2, 1),
>> +       MIPS_PLL_RATES(52000000, 442000000, 1, 17, 2, 1),
>> +       MIPS_PLL_RATES(52000000, 468000000, 1, 18, 2, 1),
>> +       MIPS_PLL_RATES(52000000, 494000000, 1, 19, 2, 1),
>> +       MIPS_PLL_RATES(52000000, 520000000, 1, 20, 2, 1),
>> +       MIPS_PLL_RATES(52000000, 546000000, 1, 21, 2, 1),
>> +};
>> +
>>  static struct pistachio_pll pistachio_plls[] __initdata = {
>> -       PLL_FIXED(CLK_MIPS_PLL, "mips_pll", "xtal", PLL_GF40LP_LAINT, 0x0),
>> +       PLL(CLK_MIPS_PLL, "mips_pll", "xtal", PLL_GF40LP_LAINT, 0x0,
>> +           mips_pll_rates),
>>         PLL_FIXED(CLK_AUDIO_PLL, "audio_pll", "audio_refclk_mux",
>>                   PLL_GF40LP_FRAC, 0xc),
>>         PLL_FIXED(CLK_RPU_V_PLL, "rpu_v_pll", "xtal", PLL_GF40LP_LAINT, 0x20),
>> diff --git a/drivers/clk/pistachio/clk.h b/drivers/clk/pistachio/clk.h
>> index 3bb6bbe..b5d22d6 100644
>> --- a/drivers/clk/pistachio/clk.h
>> +++ b/drivers/clk/pistachio/clk.h
>> @@ -121,6 +121,18 @@ struct pistachio_pll_rate_table {
>>         unsigned int frac;
>>  };
>>
>> +#define MIPS_PLL_RATES(_fref, _fout, _refdiv, _fbdiv,  \
>> +                      _postdiv1, _postdiv2)            \
>> +{                                                       \
>> +       .fref           = _fref,                        \
>> +       .fout           = _fout,                        \
>> +       .refdiv         = _refdiv,                      \
>> +       .fbdiv          = _fbdiv,                       \
>> +       .postdiv1       = _postdiv1,                    \
>> +       .postdiv2       = _postdiv2,                    \
>> +       .frac           = 0,                            \
>> +}
> 
> Wouldn't this be applicable to any integer PLL, not just MIPS_PLL?
> Also, maybe we should just populate fout_{min,max} here, setting them
> to _fout?  See my comment in patch 3/9.
> 

Yes, you are right. An INT_PLL_RATE would be OK, setting frac to 0 and
fout_{min,max} to fout.

I'll respin.
-- 
Ezequiel

[PATCH 8/9] clk: pistachio: Add sanity checks on PLL configuration

[Ezequiel Garcia]

From: Kevin Cernekee <cernekee@chromium.org>

When setting the PLL rates, check that:

 - VCO is within range
 - PFD is within range
 - PLL is disabled when postdiv is changed
 - postdiv2 <= postdiv1

Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pll.c | 83 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 79 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pll.c b/drivers/clk/pistachio/clk-pll.c
index cf000bb..59728bb 100644
--- a/drivers/clk/pistachio/clk-pll.c
+++ b/drivers/clk/pistachio/clk-pll.c
@@ -6,9 +6,12 @@
  * version 2, as published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
 #include <linux/clk-provider.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/printk.h>
 #include <linux/slab.h>
 
 #include "clk.h"
@@ -50,6 +53,18 @@
 #define PLL_CTRL4			0x10
 #define PLL_FRAC_CTRL4_BYPASS		BIT(28)
 
+#define MIN_PFD				9600000UL
+#define MIN_VCO_LA			400000000UL
+#define MAX_VCO_LA			1600000000UL
+#define MIN_VCO_FRAC_INT		600000000UL
+#define MAX_VCO_FRAC_INT		1600000000UL
+#define MIN_VCO_FRAC_FRAC		600000000UL
+#define MAX_VCO_FRAC_FRAC		2400000000UL
+#define MIN_OUTPUT_LA			8000000UL
+#define MAX_OUTPUT_LA			1600000000UL
+#define MIN_OUTPUT_FRAC			12000000UL
+#define MAX_OUTPUT_FRAC			1600000000UL
+
 struct pistachio_clk_pll {
 	struct clk_hw hw;
 	void __iomem *base;
@@ -179,12 +194,29 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 	struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
 	struct pistachio_pll_rate_table *params;
 	int enabled = pll_gf40lp_frac_is_enabled(hw);
-	u32 val, frac;
+	u32 val, frac, vco, old_postdiv1, old_postdiv2;
+	const char *name = __clk_get_name(hw->clk);
+
+	if (rate < MIN_OUTPUT_FRAC || rate > MAX_OUTPUT_FRAC)
+		return -EINVAL;
 
 	params = pll_get_params(pll, parent_rate, rate);
-	if (!params)
+	if (!params || !params->refdiv)
 		return -EINVAL;
 
+	vco = params->fref * params->fbdiv / params->refdiv;
+	if (vco < MIN_VCO_FRAC_FRAC || vco > MAX_VCO_FRAC_FRAC)
+		pr_warn("%s: VCO %u is out of range %lu..%lu\n", name, vco,
+			MIN_VCO_FRAC_FRAC, MAX_VCO_FRAC_FRAC);
+
+	val = params->fref / params->refdiv;
+	if (val < MIN_PFD)
+		pr_warn("%s: PFD %u is too low (min %lu)\n",
+			name, val, MIN_PFD);
+	if (val > vco / 16)
+		pr_warn("%s: PFD %u is too high (max %u)\n",
+			name, val, vco / 16);
+
 	/* Calculate the frac parameter */
 	frac = rate * params->refdiv * params->postdiv1 * params->postdiv2;
 	frac -= (params->fbdiv * parent_rate);
@@ -198,6 +230,19 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 	pll_writel(pll, val, PLL_CTRL1);
 
 	val = pll_readl(pll, PLL_CTRL2);
+
+	old_postdiv1 = (val >> PLL_FRAC_CTRL2_POSTDIV1_SHIFT) &
+		       PLL_FRAC_CTRL2_POSTDIV1_MASK;
+	old_postdiv2 = (val >> PLL_FRAC_CTRL2_POSTDIV2_SHIFT) &
+		       PLL_FRAC_CTRL2_POSTDIV2_MASK;
+	if (enabled &&
+	    (params->postdiv1 != old_postdiv1 ||
+	     params->postdiv2 != old_postdiv2))
+		pr_warn("%s: changing postdiv while PLL is enabled\n", name);
+
+	if (params->postdiv2 > params->postdiv1)
+		pr_warn("%s: postdiv2 should not exceed postdiv1\n", name);
+
 	val &= ~((PLL_FRAC_CTRL2_FRAC_MASK << PLL_FRAC_CTRL2_FRAC_SHIFT) |
 		 (PLL_FRAC_CTRL2_POSTDIV1_MASK <<
 		  PLL_FRAC_CTRL2_POSTDIV1_SHIFT) |
@@ -296,13 +341,43 @@ static int pll_gf40lp_laint_set_rate(struct clk_hw *hw, unsigned long rate,
 	struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
 	struct pistachio_pll_rate_table *params;
 	int enabled = pll_gf40lp_laint_is_enabled(hw);
-	u32 val;
+	u32 val, vco, old_postdiv1, old_postdiv2;
+	const char *name = __clk_get_name(hw->clk);
+
+	if (rate < MIN_OUTPUT_LA || rate > MAX_OUTPUT_LA)
+		return -EINVAL;
 
 	params = pll_get_params(pll, parent_rate, rate);
-	if (!params)
+	if (!params || !params->refdiv)
 		return -EINVAL;
 
+	vco = params->fref * params->fbdiv / params->refdiv;
+	if (vco < MIN_VCO_LA || vco > MAX_VCO_LA)
+		pr_warn("%s: VCO %u is out of range %lu..%lu\n", name, vco,
+			MIN_VCO_LA, MAX_VCO_LA);
+
+	val = params->fref / params->refdiv;
+	if (val < MIN_PFD)
+		pr_warn("%s: PFD %u is too low (min %lu)\n",
+			name, val, MIN_PFD);
+	if (val > vco / 16)
+		pr_warn("%s: PFD %u is too high (max %u)\n",
+			name, val, vco / 16);
+
 	val = pll_readl(pll, PLL_CTRL1);
+
+	old_postdiv1 = (val >> PLL_INT_CTRL1_POSTDIV1_SHIFT) &
+		       PLL_INT_CTRL1_POSTDIV1_MASK;
+	old_postdiv2 = (val >> PLL_INT_CTRL1_POSTDIV2_SHIFT) &
+		       PLL_INT_CTRL1_POSTDIV2_MASK;
+	if (enabled &&
+	    (params->postdiv1 != old_postdiv1 ||
+	     params->postdiv2 != old_postdiv2))
+		pr_warn("%s: changing postdiv while PLL is enabled\n", name);
+
+	if (params->postdiv2 > params->postdiv1)
+		pr_warn("%s: postdiv2 should not exceed postdiv1\n", name);
+
 	val &= ~((PLL_CTRL1_REFDIV_MASK << PLL_CTRL1_REFDIV_SHIFT) |
 		 (PLL_CTRL1_FBDIV_MASK << PLL_CTRL1_FBDIV_SHIFT) |
 		 (PLL_INT_CTRL1_POSTDIV1_MASK << PLL_INT_CTRL1_POSTDIV1_SHIFT) |
-- 
2.3.3

[PATCH 8/9] clk: pistachio: Add sanity checks on PLL configuration

[Ezequiel Garcia]

From: Kevin Cernekee <cernekee@chromium.org>

When setting the PLL rates, check that:

 - VCO is within range
 - PFD is within range
 - PLL is disabled when postdiv is changed
 - postdiv2 <= postdiv1

Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pll.c | 83 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 79 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pll.c b/drivers/clk/pistachio/clk-pll.c
index cf000bb..59728bb 100644
--- a/drivers/clk/pistachio/clk-pll.c
+++ b/drivers/clk/pistachio/clk-pll.c
@@ -6,9 +6,12 @@
  * version 2, as published by the Free Software Foundation.
  */
 
+#define pr_fmt(fmt) "%s: " fmt, __func__
+
 #include <linux/clk-provider.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/printk.h>
 #include <linux/slab.h>
 
 #include "clk.h"
@@ -50,6 +53,18 @@
 #define PLL_CTRL4			0x10
 #define PLL_FRAC_CTRL4_BYPASS		BIT(28)
 
+#define MIN_PFD				9600000UL
+#define MIN_VCO_LA			400000000UL
+#define MAX_VCO_LA			1600000000UL
+#define MIN_VCO_FRAC_INT		600000000UL
+#define MAX_VCO_FRAC_INT		1600000000UL
+#define MIN_VCO_FRAC_FRAC		600000000UL
+#define MAX_VCO_FRAC_FRAC		2400000000UL
+#define MIN_OUTPUT_LA			8000000UL
+#define MAX_OUTPUT_LA			1600000000UL
+#define MIN_OUTPUT_FRAC			12000000UL
+#define MAX_OUTPUT_FRAC			1600000000UL
+
 struct pistachio_clk_pll {
 	struct clk_hw hw;
 	void __iomem *base;
@@ -179,12 +194,29 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 	struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
 	struct pistachio_pll_rate_table *params;
 	int enabled = pll_gf40lp_frac_is_enabled(hw);
-	u32 val, frac;
+	u32 val, frac, vco, old_postdiv1, old_postdiv2;
+	const char *name = __clk_get_name(hw->clk);
+
+	if (rate < MIN_OUTPUT_FRAC || rate > MAX_OUTPUT_FRAC)
+		return -EINVAL;
 
 	params = pll_get_params(pll, parent_rate, rate);
-	if (!params)
+	if (!params || !params->refdiv)
 		return -EINVAL;
 
+	vco = params->fref * params->fbdiv / params->refdiv;
+	if (vco < MIN_VCO_FRAC_FRAC || vco > MAX_VCO_FRAC_FRAC)
+		pr_warn("%s: VCO %u is out of range %lu..%lu\n", name, vco,
+			MIN_VCO_FRAC_FRAC, MAX_VCO_FRAC_FRAC);
+
+	val = params->fref / params->refdiv;
+	if (val < MIN_PFD)
+		pr_warn("%s: PFD %u is too low (min %lu)\n",
+			name, val, MIN_PFD);
+	if (val > vco / 16)
+		pr_warn("%s: PFD %u is too high (max %u)\n",
+			name, val, vco / 16);
+
 	/* Calculate the frac parameter */
 	frac = rate * params->refdiv * params->postdiv1 * params->postdiv2;
 	frac -= (params->fbdiv * parent_rate);
@@ -198,6 +230,19 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 	pll_writel(pll, val, PLL_CTRL1);
 
 	val = pll_readl(pll, PLL_CTRL2);
+
+	old_postdiv1 = (val >> PLL_FRAC_CTRL2_POSTDIV1_SHIFT) &
+		       PLL_FRAC_CTRL2_POSTDIV1_MASK;
+	old_postdiv2 = (val >> PLL_FRAC_CTRL2_POSTDIV2_SHIFT) &
+		       PLL_FRAC_CTRL2_POSTDIV2_MASK;
+	if (enabled &&
+	    (params->postdiv1 != old_postdiv1 ||
+	     params->postdiv2 != old_postdiv2))
+		pr_warn("%s: changing postdiv while PLL is enabled\n", name);
+
+	if (params->postdiv2 > params->postdiv1)
+		pr_warn("%s: postdiv2 should not exceed postdiv1\n", name);
+
 	val &= ~((PLL_FRAC_CTRL2_FRAC_MASK << PLL_FRAC_CTRL2_FRAC_SHIFT) |
 		 (PLL_FRAC_CTRL2_POSTDIV1_MASK <<
 		  PLL_FRAC_CTRL2_POSTDIV1_SHIFT) |
@@ -296,13 +341,43 @@ static int pll_gf40lp_laint_set_rate(struct clk_hw *hw, unsigned long rate,
 	struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
 	struct pistachio_pll_rate_table *params;
 	int enabled = pll_gf40lp_laint_is_enabled(hw);
-	u32 val;
+	u32 val, vco, old_postdiv1, old_postdiv2;
+	const char *name = __clk_get_name(hw->clk);
+
+	if (rate < MIN_OUTPUT_LA || rate > MAX_OUTPUT_LA)
+		return -EINVAL;
 
 	params = pll_get_params(pll, parent_rate, rate);
-	if (!params)
+	if (!params || !params->refdiv)
 		return -EINVAL;
 
+	vco = params->fref * params->fbdiv / params->refdiv;
+	if (vco < MIN_VCO_LA || vco > MAX_VCO_LA)
+		pr_warn("%s: VCO %u is out of range %lu..%lu\n", name, vco,
+			MIN_VCO_LA, MAX_VCO_LA);
+
+	val = params->fref / params->refdiv;
+	if (val < MIN_PFD)
+		pr_warn("%s: PFD %u is too low (min %lu)\n",
+			name, val, MIN_PFD);
+	if (val > vco / 16)
+		pr_warn("%s: PFD %u is too high (max %u)\n",
+			name, val, vco / 16);
+
 	val = pll_readl(pll, PLL_CTRL1);
+
+	old_postdiv1 = (val >> PLL_INT_CTRL1_POSTDIV1_SHIFT) &
+		       PLL_INT_CTRL1_POSTDIV1_MASK;
+	old_postdiv2 = (val >> PLL_INT_CTRL1_POSTDIV2_SHIFT) &
+		       PLL_INT_CTRL1_POSTDIV2_MASK;
+	if (enabled &&
+	    (params->postdiv1 != old_postdiv1 ||
+	     params->postdiv2 != old_postdiv2))
+		pr_warn("%s: changing postdiv while PLL is enabled\n", name);
+
+	if (params->postdiv2 > params->postdiv1)
+		pr_warn("%s: postdiv2 should not exceed postdiv1\n", name);
+
 	val &= ~((PLL_CTRL1_REFDIV_MASK << PLL_CTRL1_REFDIV_SHIFT) |
 		 (PLL_CTRL1_FBDIV_MASK << PLL_CTRL1_FBDIV_SHIFT) |
 		 (PLL_INT_CTRL1_POSTDIV1_MASK << PLL_INT_CTRL1_POSTDIV1_SHIFT) |
-- 
2.3.3

Re: [PATCH 8/9] clk: pistachio: Add sanity checks on PLL configuration

[Andrew Bresticker]

On Thu, May 21, 2015 at 4:57 PM, Ezequiel Garcia
<ezequiel.garcia@imgtec.com> wrote:
> From: Kevin Cernekee <cernekee@chromium.org>
>
> When setting the PLL rates, check that:
>
>  - VCO is within range
>  - PFD is within range
>  - PLL is disabled when postdiv is changed
>  - postdiv2 <= postdiv1
>
> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

Reviewed-by: Andrew Bresticker <abrestic@chromium.org>

[PATCH 9/9] clk: pistachio: Correct critical clock list

[Ezequiel Garcia]

From: Damien Horsley <Damien.Horsley@imgtec.com>

Correct the critical clock list. The current one is wrong, and may
fail under some circumstances.

Signed-off-by: Damien Horsley <Damien.Horsley@imgtec.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pistachio.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pistachio.c b/drivers/clk/pistachio/clk-pistachio.c
index 0ac7429..5fc617b 100644
--- a/drivers/clk/pistachio/clk-pistachio.c
+++ b/drivers/clk/pistachio/clk-pistachio.c
@@ -171,9 +171,15 @@ PNAME(mux_debug) = { "mips_pll_mux", "rpu_v_pll_mux",
 		     "wifi_pll_mux", "bt_pll_mux" };
 static u32 mux_debug_idx[] = { 0x0, 0x1, 0x2, 0x4, 0x8, 0x10 };
 
-static unsigned int pistachio_critical_clks[] __initdata = {
+static unsigned int pistachio_critical_clks_core[] __initdata = {
 	CLK_MIPS,
-	CLK_PERIPH_SYS,
+};
+
+static unsigned int pistachio_critical_clks_sys[] __initdata = {
+	PERIPH_CLK_SYS,
+	PERIPH_CLK_SYS_BUS,
+	PERIPH_CLK_DDR,
+	PERIPH_CLK_ROM,
 };
 
 static void __init pistachio_clk_init(struct device_node *np)
@@ -205,8 +211,8 @@ static void __init pistachio_clk_init(struct device_node *np)
 
 	pistachio_clk_register_provider(p);
 
-	pistachio_clk_force_enable(p, pistachio_critical_clks,
-				   ARRAY_SIZE(pistachio_critical_clks));
+	pistachio_clk_force_enable(p, pistachio_critical_clks_core,
+				   ARRAY_SIZE(pistachio_critical_clks_core));
 }
 CLK_OF_DECLARE(pistachio_clk, "img,pistachio-clk", pistachio_clk_init);
 
@@ -273,6 +279,9 @@ static void __init pistachio_clk_periph_init(struct device_node *np)
 				    ARRAY_SIZE(pistachio_periph_gates));
 
 	pistachio_clk_register_provider(p);
+
+	pistachio_clk_force_enable(p, pistachio_critical_clks_sys,
+				   ARRAY_SIZE(pistachio_critical_clks_sys));
 }
 CLK_OF_DECLARE(pistachio_clk_periph, "img,pistachio-clk-periph",
 	       pistachio_clk_periph_init);
-- 
2.3.3

[PATCH 9/9] clk: pistachio: Correct critical clock list

[Ezequiel Garcia]

From: Damien Horsley <Damien.Horsley@imgtec.com>

Correct the critical clock list. The current one is wrong, and may
fail under some circumstances.

Signed-off-by: Damien Horsley <Damien.Horsley@imgtec.com>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
---
 drivers/clk/pistachio/clk-pistachio.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pistachio.c b/drivers/clk/pistachio/clk-pistachio.c
index 0ac7429..5fc617b 100644
--- a/drivers/clk/pistachio/clk-pistachio.c
+++ b/drivers/clk/pistachio/clk-pistachio.c
@@ -171,9 +171,15 @@ PNAME(mux_debug) = { "mips_pll_mux", "rpu_v_pll_mux",
 		     "wifi_pll_mux", "bt_pll_mux" };
 static u32 mux_debug_idx[] = { 0x0, 0x1, 0x2, 0x4, 0x8, 0x10 };
 
-static unsigned int pistachio_critical_clks[] __initdata = {
+static unsigned int pistachio_critical_clks_core[] __initdata = {
 	CLK_MIPS,
-	CLK_PERIPH_SYS,
+};
+
+static unsigned int pistachio_critical_clks_sys[] __initdata = {
+	PERIPH_CLK_SYS,
+	PERIPH_CLK_SYS_BUS,
+	PERIPH_CLK_DDR,
+	PERIPH_CLK_ROM,
 };
 
 static void __init pistachio_clk_init(struct device_node *np)
@@ -205,8 +211,8 @@ static void __init pistachio_clk_init(struct device_node *np)
 
 	pistachio_clk_register_provider(p);
 
-	pistachio_clk_force_enable(p, pistachio_critical_clks,
-				   ARRAY_SIZE(pistachio_critical_clks));
+	pistachio_clk_force_enable(p, pistachio_critical_clks_core,
+				   ARRAY_SIZE(pistachio_critical_clks_core));
 }
 CLK_OF_DECLARE(pistachio_clk, "img,pistachio-clk", pistachio_clk_init);
 
@@ -273,6 +279,9 @@ static void __init pistachio_clk_periph_init(struct device_node *np)
 				    ARRAY_SIZE(pistachio_periph_gates));
 
 	pistachio_clk_register_provider(p);
+
+	pistachio_clk_force_enable(p, pistachio_critical_clks_sys,
+				   ARRAY_SIZE(pistachio_critical_clks_sys));
 }
 CLK_OF_DECLARE(pistachio_clk_periph, "img,pistachio-clk-periph",
 	       pistachio_clk_periph_init);
-- 
2.3.3

Re: [PATCH 9/9] clk: pistachio: Correct critical clock list

[Andrew Bresticker]

On Thu, May 21, 2015 at 4:57 PM, Ezequiel Garcia
<ezequiel.garcia@imgtec.com> wrote:
> From: Damien Horsley <Damien.Horsley@imgtec.com>
>
> Correct the critical clock list. The current one is wrong, and may
> fail under some circumstances.
>
> Signed-off-by: Damien Horsley <Damien.Horsley@imgtec.com>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>

The commit message could be more descriptive, e.g. explaining which
additional clocks must be enabled at all times and why, especially
since forcing on clocks form the clock driver is very much frowned
upon unless absolutely necessary.  Otherwise,

Reviewed-by: Andrew Bresticker <abrestic@chromium.org>

Re: [PATCH 9/9] clk: pistachio: Correct critical clock list

[Ezequiel Garcia]

On 05/22/2015 02:56 PM, Andrew Bresticker wrote:
> On Thu, May 21, 2015 at 4:57 PM, Ezequiel Garcia
> <ezequiel.garcia@imgtec.com> wrote:
>> From: Damien Horsley <Damien.Horsley@imgtec.com>
>>
>> Correct the critical clock list. The current one is wrong, and may
>> fail under some circumstances.
>>
>> Signed-off-by: Damien Horsley <Damien.Horsley@imgtec.com>
>> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> 
> The commit message could be more descriptive, e.g. explaining which
> additional clocks must be enabled at all times and why, especially
> since forcing on clocks form the clock driver is very much frowned
> upon unless absolutely necessary.  Otherwise,
> 
> Reviewed-by: Andrew Bresticker <abrestic@chromium.org>
> 

OK, I'll explain this in detail.

Thanks for the review,
-- 
Ezequiel