[PATCH v2 0/4] clk: pistachio: Fixes for pll calculations.

[PATCH v2 0/4] clk: pistachio: Fixes for pll calculations.

[Govindraj Raja]

Patch 1/4 to 3/4 are pll calculation fixes for clock settings.

Patch 4/4: Is a reword and repost based on earlier thread:
http://patchwork.linux-mips.org/patch/10108/

Series Based on 4.2-rc5
Tested with Pistachio Bring-up-Board.

Changes from v1:
---------------
Fix comments from Andrew.
Topics:

* http://patchwork.linux-mips.org/patch/10888/
  (Remove long conversion for pll_rate_table variables)
	
* http://patchwork.linux-mips.org/patch/10889/
  (reword pll to PLL)

* http://patchwork.linux-mips.org/patch/10891/
  (squash 4/6 to 3/6)

* http://patchwork.linux-mips.org/patch/10892/
  (squash 5/6 to 3/6) 

* http://patchwork.linux-mips.org/patch/10893/
  (Add missing signed-off)


Damien.Horsley (1):
  clk: pistachio: correct critical clock list

Zdenko Pulitika (3):
  clk: pistachio: Fix 32bit integer overflows
  clk: pistachio: Fix override of clk-pll settings from boot loader
  clk: pistachio: Fix PLL rate calculation in integer mode

 drivers/clk/pistachio/clk-pistachio.c | 19 ++++++---
 drivers/clk/pistachio/clk-pll.c       | 75 ++++++++++++++++++++++++++---------
 2 files changed, 71 insertions(+), 23 deletions(-)

-- 
1.9.1

[PATCH v2 1/4] clk: pistachio: Fix 32bit integer overflows

[Govindraj Raja]

From: Zdenko Pulitika <zdenko.pulitika@imgtec.com>

This commit fixes 32bit integer overflows throughout the pll driver
(i.e. wherever the result of integer multiplication may exceed the
range of u32).

One of the functions affected by this problem is .recalc_rate. It
returns incorrect rate for some pll settings (not for all though)
which in turn results in the incorrect rate setup of pll's child
clocks.

Signed-off-by: Zdenko Pulitika <zdenko.pulitika@imgtec.com>
Signed-off-by: Govindraj Raja <govindraj.raja@imgtec.com>
---
 drivers/clk/pistachio/clk-pll.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pll.c b/drivers/clk/pistachio/clk-pll.c
index e17dada..68066ef 100644
--- a/drivers/clk/pistachio/clk-pll.c
+++ b/drivers/clk/pistachio/clk-pll.c
@@ -88,12 +88,10 @@ static inline void pll_lock(struct pistachio_clk_pll *pll)
 		cpu_relax();
 }
 
-static inline u32 do_div_round_closest(u64 dividend, u32 divisor)
+static inline u64 do_div_round_closest(u64 dividend, u64 divisor)
 {
 	dividend += divisor / 2;
-	do_div(dividend, divisor);
-
-	return dividend;
+	return div64_u64(dividend, divisor);
 }
 
 static inline struct pistachio_clk_pll *to_pistachio_pll(struct clk_hw *hw)
@@ -173,7 +171,7 @@ 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, vco, old_postdiv1, old_postdiv2;
+	u64 val, vco, old_postdiv1, old_postdiv2;
 	const char *name = __clk_get_name(hw->clk);
 
 	if (rate < MIN_OUTPUT_FRAC || rate > MAX_OUTPUT_FRAC)
@@ -183,17 +181,17 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 	if (!params || !params->refdiv)
 		return -EINVAL;
 
-	vco = params->fref * params->fbdiv / params->refdiv;
+	vco = div64_u64(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,
+		pr_warn("%s: VCO %llu is out of range %lu..%lu\n", name, vco,
 			MIN_VCO_FRAC_FRAC, MAX_VCO_FRAC_FRAC);
 
-	val = params->fref / params->refdiv;
+	val = div64_u64(params->fref, params->refdiv);
 	if (val < MIN_PFD)
-		pr_warn("%s: PFD %u is too low (min %lu)\n",
+		pr_warn("%s: PFD %llu 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",
+		pr_warn("%s: PFD %llu is too high (max %llu)\n",
 			name, val, vco / 16);
 
 	val = pll_readl(pll, PLL_CTRL1);
@@ -237,8 +235,7 @@ static unsigned long pll_gf40lp_frac_recalc_rate(struct clk_hw *hw,
 						 unsigned long parent_rate)
 {
 	struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
-	u32 val, prediv, fbdiv, frac, postdiv1, postdiv2;
-	u64 rate = parent_rate;
+	u64 val, prediv, fbdiv, frac, postdiv1, postdiv2, rate;
 
 	val = pll_readl(pll, PLL_CTRL1);
 	prediv = (val >> PLL_CTRL1_REFDIV_SHIFT) & PLL_CTRL1_REFDIV_MASK;
@@ -251,6 +248,7 @@ static unsigned long pll_gf40lp_frac_recalc_rate(struct clk_hw *hw,
 		PLL_FRAC_CTRL2_POSTDIV2_MASK;
 	frac = (val >> PLL_FRAC_CTRL2_FRAC_SHIFT) & PLL_FRAC_CTRL2_FRAC_MASK;
 
+	rate = parent_rate;
 	rate *= (fbdiv << 24) + frac;
 	rate = do_div_round_closest(rate, (prediv * postdiv1 * postdiv2) << 24);
 
@@ -325,12 +323,12 @@ static int pll_gf40lp_laint_set_rate(struct clk_hw *hw, unsigned long rate,
 	if (!params || !params->refdiv)
 		return -EINVAL;
 
-	vco = params->fref * params->fbdiv / params->refdiv;
+	vco = div_u64(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;
+	val = div_u64(params->fref, params->refdiv);
 	if (val < MIN_PFD)
 		pr_warn("%s: PFD %u is too low (min %lu)\n",
 			name, val, MIN_PFD);
-- 
1.9.1

[PATCH v2 2/4] clk: pistachio: Fix override of clk-pll settings from boot loader

[Govindraj Raja]

From: Zdenko Pulitika <zdenko.pulitika@imgtec.com>

PLL enable callbacks are overriding PLL mode (int/frac) and
Noise reduction (on/off) settings set by the boot loader which
results in the incorrect clock rate.

PLL mode and noise reduction are defined by the DSMPD and DACPD bits
of the PLL control register. PLL .enable() callbacks enable PLL
by deasserting all power-down bits of the PLL control register,
including DSMPD and DACPD bits, which is not necessary since
these bits don't actually enable/disable PLL.

This commit fixes the problem by removing DSMPD and DACPD bits
from the "PLL enable" mask.

Reviewed-by: Andrew Bresitcker <abrestic@chromium.org>
Signed-off-by: Zdenko Pulitika <zdenko.pulitika@imgtec.com>
Signed-off-by: Govindraj Raja <govindraj.raja@imgtec.com>
---
 drivers/clk/pistachio/clk-pll.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pll.c b/drivers/clk/pistachio/clk-pll.c
index 68066ef..9a38a2b 100644
--- a/drivers/clk/pistachio/clk-pll.c
+++ b/drivers/clk/pistachio/clk-pll.c
@@ -134,8 +134,7 @@ static int pll_gf40lp_frac_enable(struct clk_hw *hw)
 	u32 val;
 
 	val = pll_readl(pll, PLL_CTRL3);
-	val &= ~(PLL_FRAC_CTRL3_PD | PLL_FRAC_CTRL3_DACPD |
-		 PLL_FRAC_CTRL3_DSMPD | PLL_FRAC_CTRL3_FOUTPOSTDIVPD |
+	val &= ~(PLL_FRAC_CTRL3_PD | PLL_FRAC_CTRL3_FOUTPOSTDIVPD |
 		 PLL_FRAC_CTRL3_FOUT4PHASEPD | PLL_FRAC_CTRL3_FOUTVCOPD);
 	pll_writel(pll, val, PLL_CTRL3);
 
@@ -277,7 +276,7 @@ static int pll_gf40lp_laint_enable(struct clk_hw *hw)
 	u32 val;
 
 	val = pll_readl(pll, PLL_CTRL1);
-	val &= ~(PLL_INT_CTRL1_PD | PLL_INT_CTRL1_DSMPD |
+	val &= ~(PLL_INT_CTRL1_PD |
 		 PLL_INT_CTRL1_FOUTPOSTDIVPD | PLL_INT_CTRL1_FOUTVCOPD);
 	pll_writel(pll, val, PLL_CTRL1);
 
-- 
1.9.1

[PATCH v2 3/4] clk: pistachio: Fix PLL rate calculation in integer mode

[Govindraj Raja]

From: Zdenko Pulitika <zdenko.pulitika@imgtec.com>

.recalc_rate callback for the fractional PLL doesn't take operating
mode into account when calculating PLL rate. This results in
the incorrect PLL rates when PLL is operating in integer mode.

Operating mode of fractional PLL is based on the value of the
fractional divider. Currently it assumes that the PLL will always
be configured in fractional mode which may not be
the case. This may result in the wrong output frequency.

Also vco was calculated based on the current operating mode which
makes no sense because .set_rate is setting operating mode. Instead,
vco should be calculated using PLL settings that are about to be set.

Signed-off-by: Zdenko Pulitika <zdenko.pulitika@imgtec.com>
Signed-off-by: Govindraj Raja <govindraj.raja@imgtec.com>
---
 drivers/clk/pistachio/clk-pll.c | 46 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pll.c b/drivers/clk/pistachio/clk-pll.c
index 9a38a2b..5554fa4 100644
--- a/drivers/clk/pistachio/clk-pll.c
+++ b/drivers/clk/pistachio/clk-pll.c
@@ -65,6 +65,10 @@
 #define MIN_OUTPUT_FRAC			12000000UL
 #define MAX_OUTPUT_FRAC			1600000000UL
 
+/* Fractional PLL operating modes */
+#define PLL_MODE_INT			1
+#define PLL_MODE_FRAC			0
+
 struct pistachio_clk_pll {
 	struct clk_hw hw;
 	void __iomem *base;
@@ -99,6 +103,29 @@ static inline struct pistachio_clk_pll *to_pistachio_pll(struct clk_hw *hw)
 	return container_of(hw, struct pistachio_clk_pll, hw);
 }
 
+static inline u32 pll_frac_get_mode(struct clk_hw *hw)
+{
+	struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
+	u32 val;
+
+	val = pll_readl(pll, PLL_CTRL3) & PLL_FRAC_CTRL3_DSMPD;
+	return val ? PLL_MODE_INT : PLL_MODE_FRAC;
+}
+
+static inline void pll_frac_set_mode(struct clk_hw *hw, u32 mode)
+{
+	struct pistachio_clk_pll *pll = to_pistachio_pll(hw);
+	u32 val;
+
+	val = pll_readl(pll, PLL_CTRL3);
+	if (mode == PLL_MODE_INT)
+		val |= PLL_FRAC_CTRL3_DSMPD | PLL_FRAC_CTRL3_DACPD;
+	else
+		val &= ~(PLL_FRAC_CTRL3_DSMPD | PLL_FRAC_CTRL3_DACPD);
+
+	pll_writel(pll, val, PLL_CTRL3);
+}
+
 static struct pistachio_pll_rate_table *
 pll_get_params(struct pistachio_clk_pll *pll, unsigned long fref,
 	       unsigned long fout)
@@ -180,7 +207,11 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw, unsigned long rate,
 	if (!params || !params->refdiv)
 		return -EINVAL;
 
-	vco = div64_u64(params->fref * params->fbdiv, params->refdiv);
+	/* calculate vco */
+	vco = params->fref;
+	vco *= (params->fbdiv << 24) + params->frac;
+	vco = div64_u64(vco, params->refdiv << 24);
+
 	if (vco < MIN_VCO_FRAC_FRAC || vco > MAX_VCO_FRAC_FRAC)
 		pr_warn("%s: VCO %llu is out of range %lu..%lu\n", name, vco,
 			MIN_VCO_FRAC_FRAC, MAX_VCO_FRAC_FRAC);
@@ -224,6 +255,12 @@ 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);
 
+	/* set operating mode */
+	if (params->frac)
+		pll_frac_set_mode(hw, PLL_MODE_FRAC);
+	else
+		pll_frac_set_mode(hw, PLL_MODE_INT);
+
 	if (enabled)
 		pll_lock(pll);
 
@@ -247,8 +284,13 @@ static unsigned long pll_gf40lp_frac_recalc_rate(struct clk_hw *hw,
 		PLL_FRAC_CTRL2_POSTDIV2_MASK;
 	frac = (val >> PLL_FRAC_CTRL2_FRAC_SHIFT) & PLL_FRAC_CTRL2_FRAC_MASK;
 
+	/* get operating mode (int/frac) and calculate rate accordingly */
 	rate = parent_rate;
-	rate *= (fbdiv << 24) + frac;
+	if (pll_frac_get_mode(hw) == PLL_MODE_FRAC)
+		rate *= (fbdiv << 24) + frac;
+	else
+		rate *= (fbdiv << 24);
+
 	rate = do_div_round_closest(rate, (prediv * postdiv1 * postdiv2) << 24);
 
 	return rate;
-- 
1.9.1

[PATCH v2 4/4] clk: pistachio: correct critical clock list

[Govindraj Raja]

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

Current critical clock list for pistachio enables
only mips and sys clocks by default but there also
other clocks that are not claimed by anyone and
needs to be enabled by default.

This patch updates the critical clocks that needs
to enabled by default.

Add a separate struct to distinguish the critical clocks
one is core clock(mips) and others are from periph_clk_*

Reviewed-by: Andrew Bresticker <abrestic@chromium.org>
Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
Signed-off-by: Damien.Horsley <Damien.Horsley@imgtec.com>
Signed-off-by: Govindraj Raja <govindraj.raja@imgtec.com>
---
 drivers/clk/pistachio/clk-pistachio.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pistachio.c b/drivers/clk/pistachio/clk-pistachio.c
index 8c0fe88..c4ceb5e 100644
--- a/drivers/clk/pistachio/clk-pistachio.c
+++ b/drivers/clk/pistachio/clk-pistachio.c
@@ -159,9 +159,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 = {
-	CLK_MIPS,
-	CLK_PERIPH_SYS,
+static unsigned int pistachio_critical_clks_core[] __initdata = {
+	CLK_MIPS
+};
+
+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)
@@ -193,8 +199,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);
 
@@ -261,6 +267,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);
-- 
1.9.1

Re: [PATCH v2 4/4] clk: pistachio: correct critical clock list

[Sergei Shtylyov]

Hello.

On 8/7/2015 7:20 PM, Govindraj Raja wrote:

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

> Current critical clock list for pistachio enables
> only mips and sys clocks by default but there also
                                                ^ are

> other clocks that are not claimed by anyone and
> needs to be enabled by default.

> This patch updates the critical clocks that needs
> to enabled by default.
     ^ be

> Add a separate struct to distinguish the critical clocks

    Need colon here.

> one is core clock(mips) and others are from periph_clk_*

> Reviewed-by: Andrew Bresticker <abrestic@chromium.org>
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@imgtec.com>
> Signed-off-by: Damien.Horsley <Damien.Horsley@imgtec.com>
> Signed-off-by: Govindraj Raja <govindraj.raja@imgtec.com>

MBR, Sergei

RE: [PATCH v2 1/4] clk: pistachio: Fix 32bit integer overflows

[Zdenko Pulitika]

Govindraj,

> -----Original Message-----
> From: Govindraj Raja
> Sent: 07 August 2015 17:20
> To: linux-mips@linux-mips.org; linux-clk@vger.kernel.org; Stephen Boyd;
> Michael Turquette
> Cc: Zdenko Pulitika; Kevin Cernekee; Ralf Baechle; Andrew Bresticker; Jam=
es
> Hartley; Govindraj Raja; Damien Horsley; James Hogan; Ezequiel Garcia;
> Govindraj Raja
> Subject: [PATCH v2 1/4] clk: pistachio: Fix 32bit integer overflows
>=20
> From: Zdenko Pulitika <zdenko.pulitika@imgtec.com>
>=20
> This commit fixes 32bit integer overflows throughout the pll driver (i.e.
> wherever the result of integer multiplication may exceed the range of u32=
).
>=20
> One of the functions affected by this problem is .recalc_rate. It returns
> incorrect rate for some pll settings (not for all though) which in turn r=
esults in
> the incorrect rate setup of pll's child clocks.
>=20
> Signed-off-by: Zdenko Pulitika <zdenko.pulitika@imgtec.com>
> Signed-off-by: Govindraj Raja <govindraj.raja@imgtec.com>
> ---
>  drivers/clk/pistachio/clk-pll.c | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
>=20
> diff --git a/drivers/clk/pistachio/clk-pll.c b/drivers/clk/pistachio/clk-=
pll.c index
> e17dada..68066ef 100644
> --- a/drivers/clk/pistachio/clk-pll.c
> +++ b/drivers/clk/pistachio/clk-pll.c
> @@ -88,12 +88,10 @@ static inline void pll_lock(struct pistachio_clk_pll =
*pll)
>  		cpu_relax();
>  }
>=20
> -static inline u32 do_div_round_closest(u64 dividend, u32 divisor)
> +static inline u64 do_div_round_closest(u64 dividend, u64 divisor)
>  {
>  	dividend +=3D divisor / 2;
> -	do_div(dividend, divisor);
> -
> -	return dividend;
> +	return div64_u64(dividend, divisor);
>  }
>=20
>  static inline struct pistachio_clk_pll *to_pistachio_pll(struct clk_hw *=
hw) @@ -
> 173,7 +171,7 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw,
> unsigned long rate,
>  	struct pistachio_clk_pll *pll =3D to_pistachio_pll(hw);
>  	struct pistachio_pll_rate_table *params;
>  	int enabled =3D pll_gf40lp_frac_is_enabled(hw);
> -	u32 val, vco, old_postdiv1, old_postdiv2;
> +	u64 val, vco, old_postdiv1, old_postdiv2;
>  	const char *name =3D __clk_get_name(hw->clk);
>=20
>  	if (rate < MIN_OUTPUT_FRAC || rate > MAX_OUTPUT_FRAC) @@ -
> 183,17 +181,17 @@ static int pll_gf40lp_frac_set_rate(struct clk_hw *hw,
> unsigned long rate,
>  	if (!params || !params->refdiv)
>  		return -EINVAL;
>=20
> -	vco =3D params->fref * params->fbdiv / params->refdiv;
> +	vco =3D div64_u64(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,
> +		pr_warn("%s: VCO %llu is out of range %lu..%lu\n", name, vco,
>  			MIN_VCO_FRAC_FRAC, MAX_VCO_FRAC_FRAC);
>=20
> -	val =3D params->fref / params->refdiv;
> +	val =3D div64_u64(params->fref, params->refdiv);
>  	if (val < MIN_PFD)
> -		pr_warn("%s: PFD %u is too low (min %lu)\n",
> +		pr_warn("%s: PFD %llu 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",
> +		pr_warn("%s: PFD %llu is too high (max %llu)\n",
>  			name, val, vco / 16);
>=20
>  	val =3D pll_readl(pll, PLL_CTRL1);
> @@ -237,8 +235,7 @@ static unsigned long pll_gf40lp_frac_recalc_rate(stru=
ct
> clk_hw *hw,
>  						 unsigned long parent_rate)
>  {
>  	struct pistachio_clk_pll *pll =3D to_pistachio_pll(hw);
> -	u32 val, prediv, fbdiv, frac, postdiv1, postdiv2;
> -	u64 rate =3D parent_rate;
> +	u64 val, prediv, fbdiv, frac, postdiv1, postdiv2, rate;
>=20
>  	val =3D pll_readl(pll, PLL_CTRL1);
>  	prediv =3D (val >> PLL_CTRL1_REFDIV_SHIFT) &
> PLL_CTRL1_REFDIV_MASK; @@ -251,6 +248,7 @@ static unsigned long
> pll_gf40lp_frac_recalc_rate(struct clk_hw *hw,
>  		PLL_FRAC_CTRL2_POSTDIV2_MASK;
>  	frac =3D (val >> PLL_FRAC_CTRL2_FRAC_SHIFT) &
> PLL_FRAC_CTRL2_FRAC_MASK;
>=20
> +	rate =3D parent_rate;
>  	rate *=3D (fbdiv << 24) + frac;
>  	rate =3D do_div_round_closest(rate, (prediv * postdiv1 * postdiv2) << 2=
4);
>=20
> @@ -325,12 +323,12 @@ static int pll_gf40lp_laint_set_rate(struct clk_hw
> *hw, unsigned long rate,
>  	if (!params || !params->refdiv)
>  		return -EINVAL;
>=20
> -	vco =3D params->fref * params->fbdiv / params->refdiv;
> +	vco =3D div_u64(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);
>=20
> -	val =3D params->fref / params->refdiv;
> +	val =3D div_u64(params->fref, params->refdiv);
>  	if (val < MIN_PFD)
>  		pr_warn("%s: PFD %u is too low (min %lu)\n",
>  			name, val, MIN_PFD);
> --
> 1.9.1

[Zdenko Pulitika] Reverting pll_rate_table members from 64 to 32 bit re-int=
roduces multiplication overflow issue.=20
We can either 1) keep 64bit members in pll_rate_table and forget about over=
flow or 2) have 32 bit members but
then we need to type cast them to u64 in every multiplication expression wh=
ich may overflow. In my opinion, first=20
solution is safer and nicer, 2nd will result in ugly typecasts throughout t=
he code and, more importantly, there's a=20
risk of overflow bug being repeated if somebody wishes to modify/upgrade th=
e existing code.=20

RE: [PATCH v2 1/4] clk: pistachio: Fix 32bit integer overflows

[Govindraj Raja]

Hi Andrew,

> -----Original Message-----
> From: Zdenko Pulitika
> Sent: 10 August 2015 04:56 PM
> To: Govindraj Raja; linux-mips@linux-mips.org; linux-clk@vger.kernel.org;
> Stephen Boyd; Michael Turquette
> Cc: Kevin Cernekee; Ralf Baechle; Andrew Bresticker; James Hartley; Damie=
n
> Horsley; James Hogan; Ezequiel Garcia
> Subject: RE: [PATCH v2 1/4] clk: pistachio: Fix 32bit integer overflows
>=20
> Govindraj,
>=20
> > -----Original Message-----
> > From: Govindraj Raja
> > Sent: 07 August 2015 17:20
> > To: linux-mips@linux-mips.org; linux-clk@vger.kernel.org; Stephen
> > Boyd; Michael Turquette
> > Cc: Zdenko Pulitika; Kevin Cernekee; Ralf Baechle; Andrew Bresticker;
> > James Hartley; Govindraj Raja; Damien Horsley; James Hogan; Ezequiel
> > Garcia; Govindraj Raja
> > Subject: [PATCH v2 1/4] clk: pistachio: Fix 32bit integer overflows
> >
> > From: Zdenko Pulitika <zdenko.pulitika@imgtec.com>
> >
> > This commit fixes 32bit integer overflows throughout the pll driver (i.=
e.
> > wherever the result of integer multiplication may exceed the range of u=
32).
> >
> > One of the functions affected by this problem is .recalc_rate. It
> > returns incorrect rate for some pll settings (not for all though)
> > which in turn results in the incorrect rate setup of pll's child clocks=
.
> >
> > Signed-off-by: Zdenko Pulitika <zdenko.pulitika@imgtec.com>
> > Signed-off-by: Govindraj Raja <govindraj.raja@imgtec.com>
> > ---
> >  drivers/clk/pistachio/clk-pll.c | 26 ++++++++++++--------------
> >  1 file changed, 12 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/clk/pistachio/clk-pll.c
> > b/drivers/clk/pistachio/clk-pll.c index e17dada..68066ef 100644
> > --- a/drivers/clk/pistachio/clk-pll.c
> > +++ b/drivers/clk/pistachio/clk-pll.c
> > @@ -88,12 +88,10 @@ static inline void pll_lock(struct pistachio_clk_pl=
l *pll)
> >  		cpu_relax();
> >  }
> >
> > -static inline u32 do_div_round_closest(u64 dividend, u32 divisor)

[...]


> > 1.9.1
>=20
> [Zdenko Pulitika] Reverting pll_rate_table members from 64 to 32 bit re-
> introduces multiplication overflow issue.
> We can either 1) keep 64bit members in pll_rate_table and forget about ov=
erflow
> or 2) have 32 bit members but then we need to type cast them to u64 in ev=
ery
> multiplication expression which may overflow. In my opinion, first soluti=
on is
> safer and nicer, 2nd will result in ugly typecasts throughout the code an=
d, more
> importantly, there's a risk of overflow bug being repeated if somebody wi=
shes to
> modify/upgrade the existing code.

Like Zdenko pointed out some operations in pll calculation may overflow wit=
hout=20
converting the struct pistachio_pll_rate_table members to long=20

For example in below code snippet operation:

[..]

params->fref * params->fbdiv

[..]

So to be on safer side it's better to have the original code [1].

--
Thanks,
Govindraj.R

[1]:
http://patchwork.linux-mips.org/patch/10888/