[PATCH 0/8] clk: i.MX: PLL14xx: Support dynamic rates

[PATCH 0/8] clk: i.MX: PLL14xx: Support dynamic rates

[Sascha Hauer]

This series has some fixes and cleanups to the i.MX PLL14xx driver. The
PLL is found on the various i.MX8 SoCs. Finally the series aims to add
dynamic rates support to the PLL which we need for setting suitable
rates for Audio applications, but there are several cleanups worth adding
even without adding the final patch.

Sascha

Sascha Hauer (8):
  clk: imx: pll14xx: Use register defines consistently
  clk: imx: pll14xx: Fix masking
  clk: imx: pll14xx: Use FIELD_GET/FIELD_PREP
  clk: imx: pll14xx: consolidate rate calculation
  clk: imx: pll14xx: name variables after usage
  clk: imx: pll14xx: explicitly return lowest rate
  clk: imx: pll14xx: Add pr_fmt
  clk: imx: pll14xx: Support dynamic rates

 drivers/clk/imx/clk-pll14xx.c | 284 +++++++++++++++++++++++-----------
 1 file changed, 192 insertions(+), 92 deletions(-)

-- 
2.30.2

[PATCH 1/8] clk: imx: pll14xx: Use register defines consistently

[Sascha Hauer]

The driver has defines for the registers, but they are mostly unused.
Use the defines consistently throughout the driver. While at it rename
DIV_CTL to DIV_CTL0 because that's the name in the reference manual.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/clk/imx/clk-pll14xx.c | 49 ++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index 2b5ed86b9dbbb..cae64d750672e 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -15,7 +15,8 @@
 #include "clk.h"
 
 #define GNRL_CTL	0x0
-#define DIV_CTL		0x4
+#define DIV_CTL0	0x4
+#define DIV_CTL1	0x8
 #define LOCK_STATUS	BIT(31)
 #define LOCK_SEL_MASK	BIT(29)
 #define CLKE_MASK	BIT(11)
@@ -122,7 +123,7 @@ static unsigned long clk_pll1416x_recalc_rate(struct clk_hw *hw,
 	u32 mdiv, pdiv, sdiv, pll_div;
 	u64 fvco = parent_rate;
 
-	pll_div = readl_relaxed(pll->base + 4);
+	pll_div = readl_relaxed(pll->base + DIV_CTL0);
 	mdiv = (pll_div & MDIV_MASK) >> MDIV_SHIFT;
 	pdiv = (pll_div & PDIV_MASK) >> PDIV_SHIFT;
 	sdiv = (pll_div & SDIV_MASK) >> SDIV_SHIFT;
@@ -141,8 +142,8 @@ static unsigned long clk_pll1443x_recalc_rate(struct clk_hw *hw,
 	short int kdiv;
 	u64 fvco = parent_rate;
 
-	pll_div_ctl0 = readl_relaxed(pll->base + 4);
-	pll_div_ctl1 = readl_relaxed(pll->base + 8);
+	pll_div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
+	pll_div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
 	mdiv = (pll_div_ctl0 & MDIV_MASK) >> MDIV_SHIFT;
 	pdiv = (pll_div_ctl0 & PDIV_MASK) >> PDIV_SHIFT;
 	sdiv = (pll_div_ctl0 & SDIV_MASK) >> SDIV_SHIFT;
@@ -172,7 +173,7 @@ static int clk_pll14xx_wait_lock(struct clk_pll14xx *pll)
 {
 	u32 val;
 
-	return readl_poll_timeout(pll->base, val, val & LOCK_STATUS, 0,
+	return readl_poll_timeout(pll->base + GNRL_CTL, val, val & LOCK_STATUS, 0,
 			LOCK_TIMEOUT_US);
 }
 
@@ -191,32 +192,32 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
 		return -EINVAL;
 	}
 
-	tmp = readl_relaxed(pll->base + 4);
+	tmp = readl_relaxed(pll->base + DIV_CTL0);
 
 	if (!clk_pll14xx_mp_change(rate, tmp)) {
 		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
 		tmp |= rate->sdiv << SDIV_SHIFT;
-		writel_relaxed(tmp, pll->base + 4);
+		writel_relaxed(tmp, pll->base + DIV_CTL0);
 
 		return 0;
 	}
 
 	/* Bypass clock and set lock to pll output lock */
-	tmp = readl_relaxed(pll->base);
+	tmp = readl_relaxed(pll->base + GNRL_CTL);
 	tmp |= LOCK_SEL_MASK;
-	writel_relaxed(tmp, pll->base);
+	writel_relaxed(tmp, pll->base + GNRL_CTL);
 
 	/* Enable RST */
 	tmp &= ~RST_MASK;
-	writel_relaxed(tmp, pll->base);
+	writel_relaxed(tmp, pll->base + GNRL_CTL);
 
 	/* Enable BYPASS */
 	tmp |= BYPASS_MASK;
-	writel(tmp, pll->base);
+	writel(tmp, pll->base + GNRL_CTL);
 
 	div_val = (rate->mdiv << MDIV_SHIFT) | (rate->pdiv << PDIV_SHIFT) |
 		(rate->sdiv << SDIV_SHIFT);
-	writel_relaxed(div_val, pll->base + 0x4);
+	writel_relaxed(div_val, pll->base + DIV_CTL0);
 
 	/*
 	 * According to SPEC, t3 - t2 need to be greater than
@@ -228,7 +229,7 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	/* Disable RST */
 	tmp |= RST_MASK;
-	writel_relaxed(tmp, pll->base);
+	writel_relaxed(tmp, pll->base + GNRL_CTL);
 
 	/* Wait Lock */
 	ret = clk_pll14xx_wait_lock(pll);
@@ -237,7 +238,7 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	/* Bypass */
 	tmp &= ~BYPASS_MASK;
-	writel_relaxed(tmp, pll->base);
+	writel_relaxed(tmp, pll->base + GNRL_CTL);
 
 	return 0;
 }
@@ -257,32 +258,32 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 		return -EINVAL;
 	}
 
-	tmp = readl_relaxed(pll->base + 4);
+	tmp = readl_relaxed(pll->base + DIV_CTL0);
 
 	if (!clk_pll14xx_mp_change(rate, tmp)) {
 		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
 		tmp |= rate->sdiv << SDIV_SHIFT;
-		writel_relaxed(tmp, pll->base + 4);
+		writel_relaxed(tmp, pll->base + DIV_CTL0);
 
 		tmp = rate->kdiv << KDIV_SHIFT;
-		writel_relaxed(tmp, pll->base + 8);
+		writel_relaxed(tmp, pll->base + DIV_CTL1);
 
 		return 0;
 	}
 
 	/* Enable RST */
-	tmp = readl_relaxed(pll->base);
+	tmp = readl_relaxed(pll->base + GNRL_CTL);
 	tmp &= ~RST_MASK;
-	writel_relaxed(tmp, pll->base);
+	writel_relaxed(tmp, pll->base + GNRL_CTL);
 
 	/* Enable BYPASS */
 	tmp |= BYPASS_MASK;
-	writel_relaxed(tmp, pll->base);
+	writel_relaxed(tmp, pll->base + GNRL_CTL);
 
 	div_val = (rate->mdiv << MDIV_SHIFT) | (rate->pdiv << PDIV_SHIFT) |
 		(rate->sdiv << SDIV_SHIFT);
-	writel_relaxed(div_val, pll->base + 0x4);
-	writel_relaxed(rate->kdiv << KDIV_SHIFT, pll->base + 0x8);
+	writel_relaxed(div_val, pll->base + DIV_CTL0);
+	writel_relaxed(rate->kdiv << KDIV_SHIFT, pll->base + DIV_CTL1);
 
 	/*
 	 * According to SPEC, t3 - t2 need to be greater than
@@ -294,7 +295,7 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	/* Disable RST */
 	tmp |= RST_MASK;
-	writel_relaxed(tmp, pll->base);
+	writel_relaxed(tmp, pll->base + GNRL_CTL);
 
 	/* Wait Lock*/
 	ret = clk_pll14xx_wait_lock(pll);
@@ -303,7 +304,7 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	/* Bypass */
 	tmp &= ~BYPASS_MASK;
-	writel_relaxed(tmp, pll->base);
+	writel_relaxed(tmp, pll->base + GNRL_CTL);
 
 	return 0;
 }
-- 
2.30.2

Re: [PATCH 1/8] clk: imx: pll14xx: Use register defines consistently

[Abel Vesa]

On 22-02-23 08:55:54, Sascha Hauer wrote:
> The driver has defines for the registers, but they are mostly unused.
> Use the defines consistently throughout the driver. While at it rename
> DIV_CTL to DIV_CTL0 because that's the name in the reference manual.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Pretty straightforward.

Reviewed-by: Abel Vesa <abel.vesa@nxp.com>

> ---
>  drivers/clk/imx/clk-pll14xx.c | 49 ++++++++++++++++++-----------------
>  1 file changed, 25 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
> index 2b5ed86b9dbbb..cae64d750672e 100644
> --- a/drivers/clk/imx/clk-pll14xx.c
> +++ b/drivers/clk/imx/clk-pll14xx.c
> @@ -15,7 +15,8 @@
>  #include "clk.h"
>  
>  #define GNRL_CTL	0x0
> -#define DIV_CTL		0x4
> +#define DIV_CTL0	0x4
> +#define DIV_CTL1	0x8
>  #define LOCK_STATUS	BIT(31)
>  #define LOCK_SEL_MASK	BIT(29)
>  #define CLKE_MASK	BIT(11)
> @@ -122,7 +123,7 @@ static unsigned long clk_pll1416x_recalc_rate(struct clk_hw *hw,
>  	u32 mdiv, pdiv, sdiv, pll_div;
>  	u64 fvco = parent_rate;
>  
> -	pll_div = readl_relaxed(pll->base + 4);
> +	pll_div = readl_relaxed(pll->base + DIV_CTL0);
>  	mdiv = (pll_div & MDIV_MASK) >> MDIV_SHIFT;
>  	pdiv = (pll_div & PDIV_MASK) >> PDIV_SHIFT;
>  	sdiv = (pll_div & SDIV_MASK) >> SDIV_SHIFT;
> @@ -141,8 +142,8 @@ static unsigned long clk_pll1443x_recalc_rate(struct clk_hw *hw,
>  	short int kdiv;
>  	u64 fvco = parent_rate;
>  
> -	pll_div_ctl0 = readl_relaxed(pll->base + 4);
> -	pll_div_ctl1 = readl_relaxed(pll->base + 8);
> +	pll_div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
> +	pll_div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
>  	mdiv = (pll_div_ctl0 & MDIV_MASK) >> MDIV_SHIFT;
>  	pdiv = (pll_div_ctl0 & PDIV_MASK) >> PDIV_SHIFT;
>  	sdiv = (pll_div_ctl0 & SDIV_MASK) >> SDIV_SHIFT;
> @@ -172,7 +173,7 @@ static int clk_pll14xx_wait_lock(struct clk_pll14xx *pll)
>  {
>  	u32 val;
>  
> -	return readl_poll_timeout(pll->base, val, val & LOCK_STATUS, 0,
> +	return readl_poll_timeout(pll->base + GNRL_CTL, val, val & LOCK_STATUS, 0,
>  			LOCK_TIMEOUT_US);
>  }
>  
> @@ -191,32 +192,32 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
>  		return -EINVAL;
>  	}
>  
> -	tmp = readl_relaxed(pll->base + 4);
> +	tmp = readl_relaxed(pll->base + DIV_CTL0);
>  
>  	if (!clk_pll14xx_mp_change(rate, tmp)) {
>  		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
>  		tmp |= rate->sdiv << SDIV_SHIFT;
> -		writel_relaxed(tmp, pll->base + 4);
> +		writel_relaxed(tmp, pll->base + DIV_CTL0);
>  
>  		return 0;
>  	}
>  
>  	/* Bypass clock and set lock to pll output lock */
> -	tmp = readl_relaxed(pll->base);
> +	tmp = readl_relaxed(pll->base + GNRL_CTL);
>  	tmp |= LOCK_SEL_MASK;
> -	writel_relaxed(tmp, pll->base);
> +	writel_relaxed(tmp, pll->base + GNRL_CTL);
>  
>  	/* Enable RST */
>  	tmp &= ~RST_MASK;
> -	writel_relaxed(tmp, pll->base);
> +	writel_relaxed(tmp, pll->base + GNRL_CTL);
>  
>  	/* Enable BYPASS */
>  	tmp |= BYPASS_MASK;
> -	writel(tmp, pll->base);
> +	writel(tmp, pll->base + GNRL_CTL);
>  
>  	div_val = (rate->mdiv << MDIV_SHIFT) | (rate->pdiv << PDIV_SHIFT) |
>  		(rate->sdiv << SDIV_SHIFT);
> -	writel_relaxed(div_val, pll->base + 0x4);
> +	writel_relaxed(div_val, pll->base + DIV_CTL0);
>  
>  	/*
>  	 * According to SPEC, t3 - t2 need to be greater than
> @@ -228,7 +229,7 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
>  
>  	/* Disable RST */
>  	tmp |= RST_MASK;
> -	writel_relaxed(tmp, pll->base);
> +	writel_relaxed(tmp, pll->base + GNRL_CTL);
>  
>  	/* Wait Lock */
>  	ret = clk_pll14xx_wait_lock(pll);
> @@ -237,7 +238,7 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
>  
>  	/* Bypass */
>  	tmp &= ~BYPASS_MASK;
> -	writel_relaxed(tmp, pll->base);
> +	writel_relaxed(tmp, pll->base + GNRL_CTL);
>  
>  	return 0;
>  }
> @@ -257,32 +258,32 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
>  		return -EINVAL;
>  	}
>  
> -	tmp = readl_relaxed(pll->base + 4);
> +	tmp = readl_relaxed(pll->base + DIV_CTL0);
>  
>  	if (!clk_pll14xx_mp_change(rate, tmp)) {
>  		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
>  		tmp |= rate->sdiv << SDIV_SHIFT;
> -		writel_relaxed(tmp, pll->base + 4);
> +		writel_relaxed(tmp, pll->base + DIV_CTL0);
>  
>  		tmp = rate->kdiv << KDIV_SHIFT;
> -		writel_relaxed(tmp, pll->base + 8);
> +		writel_relaxed(tmp, pll->base + DIV_CTL1);
>  
>  		return 0;
>  	}
>  
>  	/* Enable RST */
> -	tmp = readl_relaxed(pll->base);
> +	tmp = readl_relaxed(pll->base + GNRL_CTL);
>  	tmp &= ~RST_MASK;
> -	writel_relaxed(tmp, pll->base);
> +	writel_relaxed(tmp, pll->base + GNRL_CTL);
>  
>  	/* Enable BYPASS */
>  	tmp |= BYPASS_MASK;
> -	writel_relaxed(tmp, pll->base);
> +	writel_relaxed(tmp, pll->base + GNRL_CTL);
>  
>  	div_val = (rate->mdiv << MDIV_SHIFT) | (rate->pdiv << PDIV_SHIFT) |
>  		(rate->sdiv << SDIV_SHIFT);
> -	writel_relaxed(div_val, pll->base + 0x4);
> -	writel_relaxed(rate->kdiv << KDIV_SHIFT, pll->base + 0x8);
> +	writel_relaxed(div_val, pll->base + DIV_CTL0);
> +	writel_relaxed(rate->kdiv << KDIV_SHIFT, pll->base + DIV_CTL1);
>  
>  	/*
>  	 * According to SPEC, t3 - t2 need to be greater than
> @@ -294,7 +295,7 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
>  
>  	/* Disable RST */
>  	tmp |= RST_MASK;
> -	writel_relaxed(tmp, pll->base);
> +	writel_relaxed(tmp, pll->base + GNRL_CTL);
>  
>  	/* Wait Lock*/
>  	ret = clk_pll14xx_wait_lock(pll);
> @@ -303,7 +304,7 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
>  
>  	/* Bypass */
>  	tmp &= ~BYPASS_MASK;
> -	writel_relaxed(tmp, pll->base);
> +	writel_relaxed(tmp, pll->base + GNRL_CTL);
>  
>  	return 0;
>  }
> -- 
> 2.30.2
>

[PATCH 2/8] clk: imx: pll14xx: Fix masking

[Sascha Hauer]

The code tries to mask the bits in SDIV_MASK from 'tmp'. SDIV_MASK
already contains the shifted value, so shifting it again is wrong.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/clk/imx/clk-pll14xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index cae64d750672e..b295d8a049009 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -195,7 +195,7 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
 	tmp = readl_relaxed(pll->base + DIV_CTL0);
 
 	if (!clk_pll14xx_mp_change(rate, tmp)) {
-		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
+		tmp &= ~SDIV_MASK;
 		tmp |= rate->sdiv << SDIV_SHIFT;
 		writel_relaxed(tmp, pll->base + DIV_CTL0);
 
@@ -261,7 +261,7 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 	tmp = readl_relaxed(pll->base + DIV_CTL0);
 
 	if (!clk_pll14xx_mp_change(rate, tmp)) {
-		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
+		tmp &= ~SDIV_MASK;
 		tmp |= rate->sdiv << SDIV_SHIFT;
 		writel_relaxed(tmp, pll->base + DIV_CTL0);
 
-- 
2.30.2

Re: [PATCH 2/8] clk: imx: pll14xx: Fix masking

[Abel Vesa]

On 22-02-23 08:55:55, Sascha Hauer wrote:
> The code tries to mask the bits in SDIV_MASK from 'tmp'. SDIV_MASK
> already contains the shifted value, so shifting it again is wrong.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Do we need some fixes tag, maybe?

Reviewed-by: Abel Vesa <abel.vesa@nxp.com>

> ---
>  drivers/clk/imx/clk-pll14xx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
> index cae64d750672e..b295d8a049009 100644
> --- a/drivers/clk/imx/clk-pll14xx.c
> +++ b/drivers/clk/imx/clk-pll14xx.c
> @@ -195,7 +195,7 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
>  	tmp = readl_relaxed(pll->base + DIV_CTL0);
>  
>  	if (!clk_pll14xx_mp_change(rate, tmp)) {
> -		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
> +		tmp &= ~SDIV_MASK;
>  		tmp |= rate->sdiv << SDIV_SHIFT;
>  		writel_relaxed(tmp, pll->base + DIV_CTL0);
>  
> @@ -261,7 +261,7 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
>  	tmp = readl_relaxed(pll->base + DIV_CTL0);
>  
>  	if (!clk_pll14xx_mp_change(rate, tmp)) {
> -		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
> +		tmp &= ~SDIV_MASK;
>  		tmp |= rate->sdiv << SDIV_SHIFT;
>  		writel_relaxed(tmp, pll->base + DIV_CTL0);
>  
> -- 
> 2.30.2
>

Re: [PATCH 2/8] clk: imx: pll14xx: Fix masking

[Sascha Hauer]

On Wed, Feb 23, 2022 at 01:31:09PM +0200, Abel Vesa wrote:
> On 22-02-23 08:55:55, Sascha Hauer wrote:
> > The code tries to mask the bits in SDIV_MASK from 'tmp'. SDIV_MASK
> > already contains the shifted value, so shifting it again is wrong.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> 
> Do we need some fixes tag, maybe?

Not really. I just realized that SDIV_SHIFT is 0, so this is really only
a cosmetic change.

Sascha

> 
> Reviewed-by: Abel Vesa <abel.vesa@nxp.com>
> 
> > ---
> >  drivers/clk/imx/clk-pll14xx.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
> > index cae64d750672e..b295d8a049009 100644
> > --- a/drivers/clk/imx/clk-pll14xx.c
> > +++ b/drivers/clk/imx/clk-pll14xx.c
> > @@ -195,7 +195,7 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
> >  	tmp = readl_relaxed(pll->base + DIV_CTL0);
> >  
> >  	if (!clk_pll14xx_mp_change(rate, tmp)) {
> > -		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
> > +		tmp &= ~SDIV_MASK;
> >  		tmp |= rate->sdiv << SDIV_SHIFT;
> >  		writel_relaxed(tmp, pll->base + DIV_CTL0);
> >  
> > @@ -261,7 +261,7 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
> >  	tmp = readl_relaxed(pll->base + DIV_CTL0);
> >  
> >  	if (!clk_pll14xx_mp_change(rate, tmp)) {
> > -		tmp &= ~(SDIV_MASK) << SDIV_SHIFT;
> > +		tmp &= ~SDIV_MASK;
> >  		tmp |= rate->sdiv << SDIV_SHIFT;
> >  		writel_relaxed(tmp, pll->base + DIV_CTL0);
> >  
> > -- 
> > 2.30.2
> >
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 3/8] clk: imx: pll14xx: Use FIELD_GET/FIELD_PREP

[Sascha Hauer]

Linux has these marvelous FIELD_GET/FIELD_PREP macros for easy access
to bitfields in registers. Use them and remove the now unused *_SHIFT
defines.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/clk/imx/clk-pll14xx.c | 39 ++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index b295d8a049009..3852a42b539e9 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -22,13 +22,9 @@
 #define CLKE_MASK	BIT(11)
 #define RST_MASK	BIT(9)
 #define BYPASS_MASK	BIT(4)
-#define MDIV_SHIFT	12
 #define MDIV_MASK	GENMASK(21, 12)
-#define PDIV_SHIFT	4
 #define PDIV_MASK	GENMASK(9, 4)
-#define SDIV_SHIFT	0
 #define SDIV_MASK	GENMASK(2, 0)
-#define KDIV_SHIFT	0
 #define KDIV_MASK	GENMASK(15, 0)
 
 #define LOCK_TIMEOUT_US		10000
@@ -124,9 +120,9 @@ static unsigned long clk_pll1416x_recalc_rate(struct clk_hw *hw,
 	u64 fvco = parent_rate;
 
 	pll_div = readl_relaxed(pll->base + DIV_CTL0);
-	mdiv = (pll_div & MDIV_MASK) >> MDIV_SHIFT;
-	pdiv = (pll_div & PDIV_MASK) >> PDIV_SHIFT;
-	sdiv = (pll_div & SDIV_MASK) >> SDIV_SHIFT;
+	mdiv = FIELD_GET(MDIV_MASK, pll_div);
+	pdiv = FIELD_GET(PDIV_MASK, pll_div);
+	sdiv = FIELD_GET(SDIV_MASK, pll_div);
 
 	fvco *= mdiv;
 	do_div(fvco, pdiv << sdiv);
@@ -144,10 +140,10 @@ static unsigned long clk_pll1443x_recalc_rate(struct clk_hw *hw,
 
 	pll_div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
 	pll_div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
-	mdiv = (pll_div_ctl0 & MDIV_MASK) >> MDIV_SHIFT;
-	pdiv = (pll_div_ctl0 & PDIV_MASK) >> PDIV_SHIFT;
-	sdiv = (pll_div_ctl0 & SDIV_MASK) >> SDIV_SHIFT;
-	kdiv = pll_div_ctl1 & KDIV_MASK;
+	mdiv = FIELD_GET(MDIV_MASK, pll_div_ctl0);
+	pdiv = FIELD_GET(PDIV_MASK, pll_div_ctl0);
+	sdiv = FIELD_GET(SDIV_MASK, pll_div_ctl0);
+	kdiv = FIELD_GET(KDIV_MASK, pll_div_ctl1);
 
 	/* fvco = (m * 65536 + k) * Fin / (p * 65536) */
 	fvco *= (mdiv * 65536 + kdiv);
@@ -163,8 +159,8 @@ static inline bool clk_pll14xx_mp_change(const struct imx_pll14xx_rate_table *ra
 {
 	u32 old_mdiv, old_pdiv;
 
-	old_mdiv = (pll_div & MDIV_MASK) >> MDIV_SHIFT;
-	old_pdiv = (pll_div & PDIV_MASK) >> PDIV_SHIFT;
+	old_mdiv = FIELD_GET(MDIV_MASK, pll_div);
+	old_pdiv = FIELD_GET(PDIV_MASK, pll_div);
 
 	return rate->mdiv != old_mdiv || rate->pdiv != old_pdiv;
 }
@@ -196,7 +192,7 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	if (!clk_pll14xx_mp_change(rate, tmp)) {
 		tmp &= ~SDIV_MASK;
-		tmp |= rate->sdiv << SDIV_SHIFT;
+		tmp |= FIELD_PREP(SDIV_MASK, rate->sdiv);
 		writel_relaxed(tmp, pll->base + DIV_CTL0);
 
 		return 0;
@@ -215,8 +211,8 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
 	tmp |= BYPASS_MASK;
 	writel(tmp, pll->base + GNRL_CTL);
 
-	div_val = (rate->mdiv << MDIV_SHIFT) | (rate->pdiv << PDIV_SHIFT) |
-		(rate->sdiv << SDIV_SHIFT);
+	div_val = FIELD_PREP(MDIV_MASK, rate->mdiv) | FIELD_PREP(PDIV_MASK, rate->pdiv) |
+		FIELD_PREP(SDIV_MASK, rate->sdiv);
 	writel_relaxed(div_val, pll->base + DIV_CTL0);
 
 	/*
@@ -262,10 +258,10 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	if (!clk_pll14xx_mp_change(rate, tmp)) {
 		tmp &= ~SDIV_MASK;
-		tmp |= rate->sdiv << SDIV_SHIFT;
+		tmp |= FIELD_PREP(SDIV_MASK, rate->sdiv);
 		writel_relaxed(tmp, pll->base + DIV_CTL0);
 
-		tmp = rate->kdiv << KDIV_SHIFT;
+		tmp = FIELD_PREP(KDIV_MASK, rate->kdiv);
 		writel_relaxed(tmp, pll->base + DIV_CTL1);
 
 		return 0;
@@ -280,10 +276,11 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 	tmp |= BYPASS_MASK;
 	writel_relaxed(tmp, pll->base + GNRL_CTL);
 
-	div_val = (rate->mdiv << MDIV_SHIFT) | (rate->pdiv << PDIV_SHIFT) |
-		(rate->sdiv << SDIV_SHIFT);
+	div_val = FIELD_PREP(MDIV_MASK, rate->mdiv) |
+		  FIELD_PREP(PDIV_MASK, rate->pdiv) |
+		  FIELD_PREP(SDIV_MASK, rate->sdiv);
 	writel_relaxed(div_val, pll->base + DIV_CTL0);
-	writel_relaxed(rate->kdiv << KDIV_SHIFT, pll->base + DIV_CTL1);
+	writel_relaxed(FIELD_PREP(KDIV_MASK, rate->kdiv), pll->base + DIV_CTL1);
 
 	/*
 	 * According to SPEC, t3 - t2 need to be greater than
-- 
2.30.2

Re: [PATCH 3/8] clk: imx: pll14xx: Use FIELD_GET/FIELD_PREP

[Abel Vesa]

On 22-02-23 08:55:56, Sascha Hauer wrote:
> Linux has these marvelous FIELD_GET/FIELD_PREP macros for easy access
> to bitfields in registers. Use them and remove the now unused *_SHIFT
> defines.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Abel Vesa <abel.vesa@nxp.com>

> ---
>  drivers/clk/imx/clk-pll14xx.c | 39 ++++++++++++++++-------------------
>  1 file changed, 18 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
> index b295d8a049009..3852a42b539e9 100644
> --- a/drivers/clk/imx/clk-pll14xx.c
> +++ b/drivers/clk/imx/clk-pll14xx.c
> @@ -22,13 +22,9 @@
>  #define CLKE_MASK	BIT(11)
>  #define RST_MASK	BIT(9)
>  #define BYPASS_MASK	BIT(4)
> -#define MDIV_SHIFT	12
>  #define MDIV_MASK	GENMASK(21, 12)
> -#define PDIV_SHIFT	4
>  #define PDIV_MASK	GENMASK(9, 4)
> -#define SDIV_SHIFT	0
>  #define SDIV_MASK	GENMASK(2, 0)
> -#define KDIV_SHIFT	0
>  #define KDIV_MASK	GENMASK(15, 0)
>  
>  #define LOCK_TIMEOUT_US		10000
> @@ -124,9 +120,9 @@ static unsigned long clk_pll1416x_recalc_rate(struct clk_hw *hw,
>  	u64 fvco = parent_rate;
>  
>  	pll_div = readl_relaxed(pll->base + DIV_CTL0);
> -	mdiv = (pll_div & MDIV_MASK) >> MDIV_SHIFT;
> -	pdiv = (pll_div & PDIV_MASK) >> PDIV_SHIFT;
> -	sdiv = (pll_div & SDIV_MASK) >> SDIV_SHIFT;
> +	mdiv = FIELD_GET(MDIV_MASK, pll_div);
> +	pdiv = FIELD_GET(PDIV_MASK, pll_div);
> +	sdiv = FIELD_GET(SDIV_MASK, pll_div);
>  
>  	fvco *= mdiv;
>  	do_div(fvco, pdiv << sdiv);
> @@ -144,10 +140,10 @@ static unsigned long clk_pll1443x_recalc_rate(struct clk_hw *hw,
>  
>  	pll_div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
>  	pll_div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
> -	mdiv = (pll_div_ctl0 & MDIV_MASK) >> MDIV_SHIFT;
> -	pdiv = (pll_div_ctl0 & PDIV_MASK) >> PDIV_SHIFT;
> -	sdiv = (pll_div_ctl0 & SDIV_MASK) >> SDIV_SHIFT;
> -	kdiv = pll_div_ctl1 & KDIV_MASK;
> +	mdiv = FIELD_GET(MDIV_MASK, pll_div_ctl0);
> +	pdiv = FIELD_GET(PDIV_MASK, pll_div_ctl0);
> +	sdiv = FIELD_GET(SDIV_MASK, pll_div_ctl0);
> +	kdiv = FIELD_GET(KDIV_MASK, pll_div_ctl1);
>  
>  	/* fvco = (m * 65536 + k) * Fin / (p * 65536) */
>  	fvco *= (mdiv * 65536 + kdiv);
> @@ -163,8 +159,8 @@ static inline bool clk_pll14xx_mp_change(const struct imx_pll14xx_rate_table *ra
>  {
>  	u32 old_mdiv, old_pdiv;
>  
> -	old_mdiv = (pll_div & MDIV_MASK) >> MDIV_SHIFT;
> -	old_pdiv = (pll_div & PDIV_MASK) >> PDIV_SHIFT;
> +	old_mdiv = FIELD_GET(MDIV_MASK, pll_div);
> +	old_pdiv = FIELD_GET(PDIV_MASK, pll_div);
>  
>  	return rate->mdiv != old_mdiv || rate->pdiv != old_pdiv;
>  }
> @@ -196,7 +192,7 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
>  
>  	if (!clk_pll14xx_mp_change(rate, tmp)) {
>  		tmp &= ~SDIV_MASK;
> -		tmp |= rate->sdiv << SDIV_SHIFT;
> +		tmp |= FIELD_PREP(SDIV_MASK, rate->sdiv);
>  		writel_relaxed(tmp, pll->base + DIV_CTL0);
>  
>  		return 0;
> @@ -215,8 +211,8 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
>  	tmp |= BYPASS_MASK;
>  	writel(tmp, pll->base + GNRL_CTL);
>  
> -	div_val = (rate->mdiv << MDIV_SHIFT) | (rate->pdiv << PDIV_SHIFT) |
> -		(rate->sdiv << SDIV_SHIFT);
> +	div_val = FIELD_PREP(MDIV_MASK, rate->mdiv) | FIELD_PREP(PDIV_MASK, rate->pdiv) |
> +		FIELD_PREP(SDIV_MASK, rate->sdiv);
>  	writel_relaxed(div_val, pll->base + DIV_CTL0);
>  
>  	/*
> @@ -262,10 +258,10 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
>  
>  	if (!clk_pll14xx_mp_change(rate, tmp)) {
>  		tmp &= ~SDIV_MASK;
> -		tmp |= rate->sdiv << SDIV_SHIFT;
> +		tmp |= FIELD_PREP(SDIV_MASK, rate->sdiv);
>  		writel_relaxed(tmp, pll->base + DIV_CTL0);
>  
> -		tmp = rate->kdiv << KDIV_SHIFT;
> +		tmp = FIELD_PREP(KDIV_MASK, rate->kdiv);
>  		writel_relaxed(tmp, pll->base + DIV_CTL1);
>  
>  		return 0;
> @@ -280,10 +276,11 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
>  	tmp |= BYPASS_MASK;
>  	writel_relaxed(tmp, pll->base + GNRL_CTL);
>  
> -	div_val = (rate->mdiv << MDIV_SHIFT) | (rate->pdiv << PDIV_SHIFT) |
> -		(rate->sdiv << SDIV_SHIFT);
> +	div_val = FIELD_PREP(MDIV_MASK, rate->mdiv) |
> +		  FIELD_PREP(PDIV_MASK, rate->pdiv) |
> +		  FIELD_PREP(SDIV_MASK, rate->sdiv);
>  	writel_relaxed(div_val, pll->base + DIV_CTL0);
> -	writel_relaxed(rate->kdiv << KDIV_SHIFT, pll->base + DIV_CTL1);
> +	writel_relaxed(FIELD_PREP(KDIV_MASK, rate->kdiv), pll->base + DIV_CTL1);
>  
>  	/*
>  	 * According to SPEC, t3 - t2 need to be greater than
> -- 
> 2.30.2
>

Re: [PATCH 3/8] clk: imx: pll14xx: Use FIELD_GET/FIELD_PREP

[kernel test robot]

Hi Sascha,

I love your patch! Yet something to improve:

[auto build test ERROR on shawnguo/for-next]
[also build test ERROR on clk/clk-next v5.17-rc5 next-20220222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sascha-Hauer/clk-i-MX-PLL14xx-Support-dynamic-rates/20220223-155846
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: hexagon-randconfig-r015-20220221 (https://download.01.org/0day-ci/archive/20220223/202202232030.DHNZfsc4-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d271fc04d5b97b12e6b797c6067d3c96a8d7470e)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/c12e6c700842e937d181c80ce6ada39017ed6268
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sascha-Hauer/clk-i-MX-PLL14xx-Support-dynamic-rates/20220223-155846
        git checkout c12e6c700842e937d181c80ce6ada39017ed6268
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/clk/imx/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/clk/imx/clk-pll14xx.c:123:9: error: implicit declaration of function 'FIELD_GET' [-Werror,-Wimplicit-function-declaration]
           mdiv = FIELD_GET(MDIV_MASK, pll_div);
                  ^
   drivers/clk/imx/clk-pll14xx.c:143:9: error: implicit declaration of function 'FIELD_GET' [-Werror,-Wimplicit-function-declaration]
           mdiv = FIELD_GET(MDIV_MASK, pll_div_ctl0);
                  ^
   drivers/clk/imx/clk-pll14xx.c:162:13: error: implicit declaration of function 'FIELD_GET' [-Werror,-Wimplicit-function-declaration]
           old_mdiv = FIELD_GET(MDIV_MASK, pll_div);
                      ^
>> drivers/clk/imx/clk-pll14xx.c:195:10: error: implicit declaration of function 'FIELD_PREP' [-Werror,-Wimplicit-function-declaration]
                   tmp |= FIELD_PREP(SDIV_MASK, rate->sdiv);
                          ^
   drivers/clk/imx/clk-pll14xx.c:214:12: error: implicit declaration of function 'FIELD_PREP' [-Werror,-Wimplicit-function-declaration]
           div_val = FIELD_PREP(MDIV_MASK, rate->mdiv) | FIELD_PREP(PDIV_MASK, rate->pdiv) |
                     ^
   drivers/clk/imx/clk-pll14xx.c:261:10: error: implicit declaration of function 'FIELD_PREP' [-Werror,-Wimplicit-function-declaration]
                   tmp |= FIELD_PREP(SDIV_MASK, rate->sdiv);
                          ^
   drivers/clk/imx/clk-pll14xx.c:279:12: error: implicit declaration of function 'FIELD_PREP' [-Werror,-Wimplicit-function-declaration]
           div_val = FIELD_PREP(MDIV_MASK, rate->mdiv) |
                     ^
   7 errors generated.


vim +/FIELD_GET +123 drivers/clk/imx/clk-pll14xx.c

   114	
   115	static unsigned long clk_pll1416x_recalc_rate(struct clk_hw *hw,
   116							  unsigned long parent_rate)
   117	{
   118		struct clk_pll14xx *pll = to_clk_pll14xx(hw);
   119		u32 mdiv, pdiv, sdiv, pll_div;
   120		u64 fvco = parent_rate;
   121	
   122		pll_div = readl_relaxed(pll->base + DIV_CTL0);
 > 123		mdiv = FIELD_GET(MDIV_MASK, pll_div);
   124		pdiv = FIELD_GET(PDIV_MASK, pll_div);
   125		sdiv = FIELD_GET(SDIV_MASK, pll_div);
   126	
   127		fvco *= mdiv;
   128		do_div(fvco, pdiv << sdiv);
   129	
   130		return fvco;
   131	}
   132	
   133	static unsigned long clk_pll1443x_recalc_rate(struct clk_hw *hw,
   134							  unsigned long parent_rate)
   135	{
   136		struct clk_pll14xx *pll = to_clk_pll14xx(hw);
   137		u32 mdiv, pdiv, sdiv, pll_div_ctl0, pll_div_ctl1;
   138		short int kdiv;
   139		u64 fvco = parent_rate;
   140	
   141		pll_div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
   142		pll_div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
   143		mdiv = FIELD_GET(MDIV_MASK, pll_div_ctl0);
   144		pdiv = FIELD_GET(PDIV_MASK, pll_div_ctl0);
   145		sdiv = FIELD_GET(SDIV_MASK, pll_div_ctl0);
   146		kdiv = FIELD_GET(KDIV_MASK, pll_div_ctl1);
   147	
   148		/* fvco = (m * 65536 + k) * Fin / (p * 65536) */
   149		fvco *= (mdiv * 65536 + kdiv);
   150		pdiv *= 65536;
   151	
   152		do_div(fvco, pdiv << sdiv);
   153	
   154		return fvco;
   155	}
   156	
   157	static inline bool clk_pll14xx_mp_change(const struct imx_pll14xx_rate_table *rate,
   158						  u32 pll_div)
   159	{
   160		u32 old_mdiv, old_pdiv;
   161	
   162		old_mdiv = FIELD_GET(MDIV_MASK, pll_div);
   163		old_pdiv = FIELD_GET(PDIV_MASK, pll_div);
   164	
   165		return rate->mdiv != old_mdiv || rate->pdiv != old_pdiv;
   166	}
   167	
   168	static int clk_pll14xx_wait_lock(struct clk_pll14xx *pll)
   169	{
   170		u32 val;
   171	
   172		return readl_poll_timeout(pll->base + GNRL_CTL, val, val & LOCK_STATUS, 0,
   173				LOCK_TIMEOUT_US);
   174	}
   175	
   176	static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
   177					 unsigned long prate)
   178	{
   179		struct clk_pll14xx *pll = to_clk_pll14xx(hw);
   180		const struct imx_pll14xx_rate_table *rate;
   181		u32 tmp, div_val;
   182		int ret;
   183	
   184		rate = imx_get_pll_settings(pll, drate);
   185		if (!rate) {
   186			pr_err("%s: Invalid rate : %lu for pll clk %s\n", __func__,
   187			       drate, clk_hw_get_name(hw));
   188			return -EINVAL;
   189		}
   190	
   191		tmp = readl_relaxed(pll->base + DIV_CTL0);
   192	
   193		if (!clk_pll14xx_mp_change(rate, tmp)) {
   194			tmp &= ~SDIV_MASK;
 > 195			tmp |= FIELD_PREP(SDIV_MASK, rate->sdiv);
   196			writel_relaxed(tmp, pll->base + DIV_CTL0);
   197	
   198			return 0;
   199		}
   200	
   201		/* Bypass clock and set lock to pll output lock */
   202		tmp = readl_relaxed(pll->base + GNRL_CTL);
   203		tmp |= LOCK_SEL_MASK;
   204		writel_relaxed(tmp, pll->base + GNRL_CTL);
   205	
   206		/* Enable RST */
   207		tmp &= ~RST_MASK;
   208		writel_relaxed(tmp, pll->base + GNRL_CTL);
   209	
   210		/* Enable BYPASS */
   211		tmp |= BYPASS_MASK;
   212		writel(tmp, pll->base + GNRL_CTL);
   213	
   214		div_val = FIELD_PREP(MDIV_MASK, rate->mdiv) | FIELD_PREP(PDIV_MASK, rate->pdiv) |
   215			FIELD_PREP(SDIV_MASK, rate->sdiv);
   216		writel_relaxed(div_val, pll->base + DIV_CTL0);
   217	
   218		/*
   219		 * According to SPEC, t3 - t2 need to be greater than
   220		 * 1us and 1/FREF, respectively.
   221		 * FREF is FIN / Prediv, the prediv is [1, 63], so choose
   222		 * 3us.
   223		 */
   224		udelay(3);
   225	
   226		/* Disable RST */
   227		tmp |= RST_MASK;
   228		writel_relaxed(tmp, pll->base + GNRL_CTL);
   229	
   230		/* Wait Lock */
   231		ret = clk_pll14xx_wait_lock(pll);
   232		if (ret)
   233			return ret;
   234	
   235		/* Bypass */
   236		tmp &= ~BYPASS_MASK;
   237		writel_relaxed(tmp, pll->base + GNRL_CTL);
   238	
   239		return 0;
   240	}
   241	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[PATCH 4/8] clk: imx: pll14xx: consolidate rate calculation

[Sascha Hauer]

The PLL driver has support for two different PLLs: The pll1416x and
the pll1443x. The latter has support for an additional kdiv value.
recalc_rate can be the same calculation when kdiv is assumed to be zero
for the PLL which doesn't support that value.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/clk/imx/clk-pll14xx.c | 59 +++++++++++++++--------------------
 1 file changed, 26 insertions(+), 33 deletions(-)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index 3852a42b539e9..7d15c51cf3a41 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -96,6 +96,20 @@ static const struct imx_pll14xx_rate_table *imx_get_pll_settings(
 	return NULL;
 }
 
+static long pll14xx_calc_rate(struct clk_pll14xx *pll, int mdiv, int pdiv,
+			      int sdiv, int kdiv, unsigned long prate)
+{
+	u64 fvco = prate;
+
+	/* fvco = (m * 65536 + k) * Fin / (p * 65536) */
+	fvco *= (mdiv * 65536 + kdiv);
+	pdiv *= 65536;
+
+	do_div(fvco, pdiv << sdiv);
+
+	return fvco;
+}
+
 static long clk_pll14xx_round_rate(struct clk_hw *hw, unsigned long rate,
 			unsigned long *prate)
 {
@@ -112,46 +126,25 @@ static long clk_pll14xx_round_rate(struct clk_hw *hw, unsigned long rate,
 	return rate_table[i - 1].rate;
 }
 
-static unsigned long clk_pll1416x_recalc_rate(struct clk_hw *hw,
-						  unsigned long parent_rate)
-{
-	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
-	u32 mdiv, pdiv, sdiv, pll_div;
-	u64 fvco = parent_rate;
-
-	pll_div = readl_relaxed(pll->base + DIV_CTL0);
-	mdiv = FIELD_GET(MDIV_MASK, pll_div);
-	pdiv = FIELD_GET(PDIV_MASK, pll_div);
-	sdiv = FIELD_GET(SDIV_MASK, pll_div);
-
-	fvco *= mdiv;
-	do_div(fvco, pdiv << sdiv);
-
-	return fvco;
-}
-
-static unsigned long clk_pll1443x_recalc_rate(struct clk_hw *hw,
+static unsigned long clk_pll14xx_recalc_rate(struct clk_hw *hw,
 						  unsigned long parent_rate)
 {
 	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
-	u32 mdiv, pdiv, sdiv, pll_div_ctl0, pll_div_ctl1;
-	short int kdiv;
-	u64 fvco = parent_rate;
+	u32 mdiv, pdiv, sdiv, kdiv, pll_div_ctl0, pll_div_ctl1;
 
 	pll_div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
-	pll_div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
 	mdiv = FIELD_GET(MDIV_MASK, pll_div_ctl0);
 	pdiv = FIELD_GET(PDIV_MASK, pll_div_ctl0);
 	sdiv = FIELD_GET(SDIV_MASK, pll_div_ctl0);
-	kdiv = FIELD_GET(KDIV_MASK, pll_div_ctl1);
 
-	/* fvco = (m * 65536 + k) * Fin / (p * 65536) */
-	fvco *= (mdiv * 65536 + kdiv);
-	pdiv *= 65536;
-
-	do_div(fvco, pdiv << sdiv);
+	if (pll->type == PLL_1443X) {
+		pll_div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
+		kdiv = FIELD_GET(KDIV_MASK, pll_div_ctl1);
+	} else {
+		kdiv = 0;
+	}
 
-	return fvco;
+	return pll14xx_calc_rate(pll, mdiv, pdiv, sdiv, kdiv, parent_rate);
 }
 
 static inline bool clk_pll14xx_mp_change(const struct imx_pll14xx_rate_table *rate,
@@ -362,20 +355,20 @@ static const struct clk_ops clk_pll1416x_ops = {
 	.prepare	= clk_pll14xx_prepare,
 	.unprepare	= clk_pll14xx_unprepare,
 	.is_prepared	= clk_pll14xx_is_prepared,
-	.recalc_rate	= clk_pll1416x_recalc_rate,
+	.recalc_rate	= clk_pll14xx_recalc_rate,
 	.round_rate	= clk_pll14xx_round_rate,
 	.set_rate	= clk_pll1416x_set_rate,
 };
 
 static const struct clk_ops clk_pll1416x_min_ops = {
-	.recalc_rate	= clk_pll1416x_recalc_rate,
+	.recalc_rate	= clk_pll14xx_recalc_rate,
 };
 
 static const struct clk_ops clk_pll1443x_ops = {
 	.prepare	= clk_pll14xx_prepare,
 	.unprepare	= clk_pll14xx_unprepare,
 	.is_prepared	= clk_pll14xx_is_prepared,
-	.recalc_rate	= clk_pll1443x_recalc_rate,
+	.recalc_rate	= clk_pll14xx_recalc_rate,
 	.round_rate	= clk_pll14xx_round_rate,
 	.set_rate	= clk_pll1443x_set_rate,
 };
-- 
2.30.2

[PATCH 5/8] clk: imx: pll14xx: name variables after usage

[Sascha Hauer]

In clk_pll1443x_set_rate() 'tmp' is used for the content of different
registers which makes it a bit hard to follow. Use different variables
named after the registers to make it clearer.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/clk/imx/clk-pll14xx.c | 42 +++++++++++++++++------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index 7d15c51cf3a41..c56abd3c47460 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -237,7 +237,7 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 {
 	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
 	const struct imx_pll14xx_rate_table *rate;
-	u32 tmp, div_val;
+	u32 gnrl_ctl, div_ctl0;
 	int ret;
 
 	rate = imx_get_pll_settings(pll, drate);
@@ -247,32 +247,32 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 		return -EINVAL;
 	}
 
-	tmp = readl_relaxed(pll->base + DIV_CTL0);
+	div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
 
-	if (!clk_pll14xx_mp_change(rate, tmp)) {
-		tmp &= ~SDIV_MASK;
-		tmp |= FIELD_PREP(SDIV_MASK, rate->sdiv);
-		writel_relaxed(tmp, pll->base + DIV_CTL0);
+	if (!clk_pll14xx_mp_change(rate, div_ctl0)) {
+		div_ctl0 &= ~SDIV_MASK;
+		div_ctl0 |= FIELD_PREP(SDIV_MASK, rate->sdiv);
+		writel_relaxed(div_ctl0, pll->base + DIV_CTL0);
 
-		tmp = FIELD_PREP(KDIV_MASK, rate->kdiv);
-		writel_relaxed(tmp, pll->base + DIV_CTL1);
+		writel_relaxed(FIELD_PREP(KDIV_MASK, rate->kdiv),
+			       pll->base + DIV_CTL1);
 
 		return 0;
 	}
 
 	/* Enable RST */
-	tmp = readl_relaxed(pll->base + GNRL_CTL);
-	tmp &= ~RST_MASK;
-	writel_relaxed(tmp, pll->base + GNRL_CTL);
+	gnrl_ctl = readl_relaxed(pll->base + GNRL_CTL);
+	gnrl_ctl &= ~RST_MASK;
+	writel_relaxed(gnrl_ctl, pll->base + GNRL_CTL);
 
 	/* Enable BYPASS */
-	tmp |= BYPASS_MASK;
-	writel_relaxed(tmp, pll->base + GNRL_CTL);
+	gnrl_ctl |= BYPASS_MASK;
+	writel_relaxed(gnrl_ctl, pll->base + GNRL_CTL);
 
-	div_val = FIELD_PREP(MDIV_MASK, rate->mdiv) |
-		  FIELD_PREP(PDIV_MASK, rate->pdiv) |
-		  FIELD_PREP(SDIV_MASK, rate->sdiv);
-	writel_relaxed(div_val, pll->base + DIV_CTL0);
+	div_ctl0 = FIELD_PREP(MDIV_MASK, rate->mdiv) |
+		   FIELD_PREP(PDIV_MASK, rate->pdiv) |
+		   FIELD_PREP(SDIV_MASK, rate->sdiv);
+	writel_relaxed(div_ctl0, pll->base + DIV_CTL0);
 	writel_relaxed(FIELD_PREP(KDIV_MASK, rate->kdiv), pll->base + DIV_CTL1);
 
 	/*
@@ -284,8 +284,8 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 	udelay(3);
 
 	/* Disable RST */
-	tmp |= RST_MASK;
-	writel_relaxed(tmp, pll->base + GNRL_CTL);
+	gnrl_ctl |= RST_MASK;
+	writel_relaxed(gnrl_ctl, pll->base + GNRL_CTL);
 
 	/* Wait Lock*/
 	ret = clk_pll14xx_wait_lock(pll);
@@ -293,8 +293,8 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 		return ret;
 
 	/* Bypass */
-	tmp &= ~BYPASS_MASK;
-	writel_relaxed(tmp, pll->base + GNRL_CTL);
+	gnrl_ctl &= ~BYPASS_MASK;
+	writel_relaxed(gnrl_ctl, pll->base + GNRL_CTL);
 
 	return 0;
 }
-- 
2.30.2

[PATCH 6/8] clk: imx: pll14xx: explicitly return lowest rate

[Sascha Hauer]

clk_pll14xx_round_rate() returns the lowest rate by indexing into
the rate table with the variable i. i is actually pll->rate_count
as this is the value we come out of the loop with. Use pll->rate_count
explicitly to make it a bit more clear what is being done. While at
it fix a typo in the comment. No functional change.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/clk/imx/clk-pll14xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index c56abd3c47460..469e0cbb24487 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -117,13 +117,13 @@ static long clk_pll14xx_round_rate(struct clk_hw *hw, unsigned long rate,
 	const struct imx_pll14xx_rate_table *rate_table = pll->rate_table;
 	int i;
 
-	/* Assumming rate_table is in descending order */
+	/* Assuming rate_table is in descending order */
 	for (i = 0; i < pll->rate_count; i++)
 		if (rate >= rate_table[i].rate)
 			return rate_table[i].rate;
 
 	/* return minimum supported value */
-	return rate_table[i - 1].rate;
+	return rate_table[pll->rate_count - 1].rate;
 }
 
 static unsigned long clk_pll14xx_recalc_rate(struct clk_hw *hw,
-- 
2.30.2

[PATCH 7/8] clk: imx: pll14xx: Add pr_fmt

[Sascha Hauer]

Print all messages from within the pll14xx driver with a common
prefix using pr_fmt. No need to print function names anymore, so
drop them from the messages.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/clk/imx/clk-pll14xx.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index 469e0cbb24487..28c75963a80bd 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -3,6 +3,8 @@
  * Copyright 2017-2018 NXP.
  */
 
+#define pr_fmt(fmt) "pll14xx: " fmt
+
 #include <linux/bits.h>
 #include <linux/clk-provider.h>
 #include <linux/err.h>
@@ -176,7 +178,7 @@ static int clk_pll1416x_set_rate(struct clk_hw *hw, unsigned long drate,
 
 	rate = imx_get_pll_settings(pll, drate);
 	if (!rate) {
-		pr_err("%s: Invalid rate : %lu for pll clk %s\n", __func__,
+		pr_err("Invalid rate %lu for pll clk %s\n", __func__,
 		       drate, clk_hw_get_name(hw));
 		return -EINVAL;
 	}
@@ -403,8 +405,7 @@ struct clk_hw *imx_dev_clk_hw_pll14xx(struct device *dev, const char *name,
 		init.ops = &clk_pll1443x_ops;
 		break;
 	default:
-		pr_err("%s: Unknown pll type for pll clk %s\n",
-		       __func__, name);
+		pr_err("Unknown pll type for pll clk %s\n", name);
 		kfree(pll);
 		return ERR_PTR(-EINVAL);
 	}
@@ -423,8 +424,7 @@ struct clk_hw *imx_dev_clk_hw_pll14xx(struct device *dev, const char *name,
 
 	ret = clk_hw_register(dev, hw);
 	if (ret) {
-		pr_err("%s: failed to register pll %s %d\n",
-			__func__, name, ret);
+		pr_err("failed to register pll %s %d\n", name, ret);
 		kfree(pll);
 		return ERR_PTR(ret);
 	}
-- 
2.30.2

Re: [PATCH 7/8] clk: imx: pll14xx: Add pr_fmt

[kernel test robot]

Hi Sascha,

I love your patch! Perhaps something to improve:

[auto build test WARNING on shawnguo/for-next]
[also build test WARNING on clk/clk-next v5.17-rc5 next-20220222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sascha-Hauer/clk-i-MX-PLL14xx-Support-dynamic-rates/20220223-155846
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220223/202202231803.Xvmq9bEq-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/0c0c0d808f682f365239da62a5670bbd74275b5f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sascha-Hauer/clk-i-MX-PLL14xx-Support-dynamic-rates/20220223-155846
        git checkout 0c0c0d808f682f365239da62a5670bbd74275b5f
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash drivers/clk/imx/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/clk/imx/clk-pll14xx.c: In function 'clk_pll14xx_recalc_rate':
   drivers/clk/imx/clk-pll14xx.c:138:16: error: implicit declaration of function 'FIELD_GET'; did you mean 'FOLL_GET'? [-Werror=implicit-function-declaration]
     138 |         mdiv = FIELD_GET(MDIV_MASK, pll_div_ctl0);
         |                ^~~~~~~~~
         |                FOLL_GET
   In file included from include/asm-generic/bug.h:22,
                    from arch/alpha/include/asm/bug.h:23,
                    from include/linux/bug.h:5,
                    from include/linux/thread_info.h:13,
                    from include/asm-generic/current.h:5,
                    from ./arch/alpha/include/generated/asm/current.h:1,
                    from include/linux/mutex.h:14,
                    from include/linux/kernfs.h:11,
                    from include/linux/sysfs.h:16,
                    from include/linux/kobject.h:20,
                    from include/linux/of.h:17,
                    from include/linux/clk-provider.h:9,
                    from drivers/clk/imx/clk-pll14xx.c:9:
   drivers/clk/imx/clk-pll14xx.c: In function 'clk_pll1416x_set_rate':
>> include/linux/kern_levels.h:5:25: warning: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'const char *' [-Wformat=]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:418:25: note: in definition of macro 'printk_index_wrap'
     418 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:489:9: note: in expansion of macro 'printk'
     489 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~
   include/linux/kern_levels.h:11:25: note: in expansion of macro 'KERN_SOH'
      11 | #define KERN_ERR        KERN_SOH "3"    /* error conditions */
         |                         ^~~~~~~~
   include/linux/printk.h:489:16: note: in expansion of macro 'KERN_ERR'
     489 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |                ^~~~~~~~
   drivers/clk/imx/clk-pll14xx.c:181:17: note: in expansion of macro 'pr_err'
     181 |                 pr_err("Invalid rate %lu for pll clk %s\n", __func__,
         |                 ^~~~~~
>> include/linux/kern_levels.h:5:25: warning: format '%s' expects argument of type 'char *', but argument 3 has type 'long unsigned int' [-Wformat=]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:418:25: note: in definition of macro 'printk_index_wrap'
     418 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:489:9: note: in expansion of macro 'printk'
     489 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~
   include/linux/kern_levels.h:11:25: note: in expansion of macro 'KERN_SOH'
      11 | #define KERN_ERR        KERN_SOH "3"    /* error conditions */
         |                         ^~~~~~~~
   include/linux/printk.h:489:16: note: in expansion of macro 'KERN_ERR'
     489 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |                ^~~~~~~~
   drivers/clk/imx/clk-pll14xx.c:181:17: note: in expansion of macro 'pr_err'
     181 |                 pr_err("Invalid rate %lu for pll clk %s\n", __func__,
         |                 ^~~~~~
>> include/linux/kern_levels.h:5:25: warning: too many arguments for format [-Wformat-extra-args]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:418:25: note: in definition of macro 'printk_index_wrap'
     418 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:489:9: note: in expansion of macro 'printk'
     489 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~
   include/linux/kern_levels.h:11:25: note: in expansion of macro 'KERN_SOH'
      11 | #define KERN_ERR        KERN_SOH "3"    /* error conditions */
         |                         ^~~~~~~~
   include/linux/printk.h:489:16: note: in expansion of macro 'KERN_ERR'
     489 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |                ^~~~~~~~
   drivers/clk/imx/clk-pll14xx.c:181:17: note: in expansion of macro 'pr_err'
     181 |                 pr_err("Invalid rate %lu for pll clk %s\n", __func__,
         |                 ^~~~~~
   drivers/clk/imx/clk-pll14xx.c:190:24: error: implicit declaration of function 'FIELD_PREP' [-Werror=implicit-function-declaration]
     190 |                 tmp |= FIELD_PREP(SDIV_MASK, rate->sdiv);
         |                        ^~~~~~~~~~
   cc1: some warnings being treated as errors


vim +5 include/linux/kern_levels.h

314ba3520e513a Joe Perches 2012-07-30   4  
04d2c8c83d0e3a Joe Perches 2012-07-30  @5  #define KERN_SOH	"\001"		/* ASCII Start Of Header */
04d2c8c83d0e3a Joe Perches 2012-07-30   6  #define KERN_SOH_ASCII	'\001'
04d2c8c83d0e3a Joe Perches 2012-07-30   7  
04d2c8c83d0e3a Joe Perches 2012-07-30   8  #define KERN_EMERG	KERN_SOH "0"	/* system is unusable */
04d2c8c83d0e3a Joe Perches 2012-07-30   9  #define KERN_ALERT	KERN_SOH "1"	/* action must be taken immediately */
04d2c8c83d0e3a Joe Perches 2012-07-30  10  #define KERN_CRIT	KERN_SOH "2"	/* critical conditions */
04d2c8c83d0e3a Joe Perches 2012-07-30 @11  #define KERN_ERR	KERN_SOH "3"	/* error conditions */
04d2c8c83d0e3a Joe Perches 2012-07-30  12  #define KERN_WARNING	KERN_SOH "4"	/* warning conditions */
04d2c8c83d0e3a Joe Perches 2012-07-30  13  #define KERN_NOTICE	KERN_SOH "5"	/* normal but significant condition */
04d2c8c83d0e3a Joe Perches 2012-07-30  14  #define KERN_INFO	KERN_SOH "6"	/* informational */
04d2c8c83d0e3a Joe Perches 2012-07-30  15  #define KERN_DEBUG	KERN_SOH "7"	/* debug-level messages */
04d2c8c83d0e3a Joe Perches 2012-07-30  16  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[PATCH 8/8] clk: imx: pll14xx: Support dynamic rates

[Sascha Hauer]

The pll1443x PLL so far only supports rates from a rate table passed
during initialization. Calculating PLL settings dynamically helps audio
applications to get their desired rates, so support for this is added
in this patch.

The strategy to get to the PLL setting for a rate is:

- First try to only adjust kdiv which specifies the fractional part of the PLL.
  This setting can be changed without glitches on the output and is therefore
  preferred
- When that isn't possible then the rate table is searched for suitable rates,
  so for standard rates the same settings are used as without this patch
- As a last resort the best settings are calculated dynamically

The code in this patch is based on patches from Adrian Alonso <adrian.alonso@nxp.com>
and Mads Bligaard Nielsen <bli@bang-olufsen.dk>

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/clk/imx/clk-pll14xx.c | 143 ++++++++++++++++++++++++++++++----
 1 file changed, 126 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/imx/clk-pll14xx.c b/drivers/clk/imx/clk-pll14xx.c
index 28c75963a80bd..d2e2c742ce8f2 100644
--- a/drivers/clk/imx/clk-pll14xx.c
+++ b/drivers/clk/imx/clk-pll14xx.c
@@ -28,6 +28,8 @@
 #define PDIV_MASK	GENMASK(9, 4)
 #define SDIV_MASK	GENMASK(2, 0)
 #define KDIV_MASK	GENMASK(15, 0)
+#define KDIV_MIN	SHRT_MIN
+#define KDIV_MAX	SHRT_MAX
 
 #define LOCK_TIMEOUT_US		10000
 
@@ -112,7 +114,106 @@ static long pll14xx_calc_rate(struct clk_pll14xx *pll, int mdiv, int pdiv,
 	return fvco;
 }
 
-static long clk_pll14xx_round_rate(struct clk_hw *hw, unsigned long rate,
+static long pll1443x_calc_kdiv(int mdiv, int pdiv, int sdiv,
+		unsigned long rate, unsigned long prate)
+{
+	long kdiv;
+
+	/* calc kdiv = round(rate * pdiv * 65536 * 2^sdiv / prate) - (mdiv * 65536) */
+	kdiv = ((rate * ((pdiv * 65536) << sdiv) + prate / 2) / prate) - (mdiv * 65536);
+
+	return clamp_t(short, kdiv, KDIV_MIN, KDIV_MAX);
+}
+
+static void imx_pll14xx_calc_settings(struct clk_pll14xx *pll, unsigned long rate,
+				      unsigned long prate, struct imx_pll14xx_rate_table *t)
+{
+	u32 pll_div_ctl0, pll_div_ctl1;
+	int mdiv, pdiv, sdiv, kdiv;
+	long fvco, rate_min, rate_max, dist, best = LONG_MAX;
+	const struct imx_pll14xx_rate_table *tt;
+
+	/*
+	 * Fractional PLL constrains:
+	 *
+	 * a) 6MHz <= prate <= 25MHz
+	 * b) 1 <= p <= 63 (1 <= p <= 4 prate = 24MHz)
+	 * c) 64 <= m <= 1023
+	 * d) 0 <= s <= 6
+	 * e) -32768 <= k <= 32767
+	 *
+	 * fvco = (m * 65536 + k) * prate / (p * 65536)
+	 */
+
+	pll_div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
+	mdiv = FIELD_GET(MDIV_MASK, pll_div_ctl0);
+	pdiv = FIELD_GET(PDIV_MASK, pll_div_ctl0);
+	sdiv = FIELD_GET(SDIV_MASK, pll_div_ctl0);
+	pll_div_ctl1 = readl_relaxed(pll->base + DIV_CTL1);
+
+	/* First see if we can get the desired rate by only adjusting kdiv (glitch free) */
+	rate_min = pll14xx_calc_rate(pll, mdiv, pdiv, sdiv, KDIV_MIN, prate);
+	rate_max = pll14xx_calc_rate(pll, mdiv, pdiv, sdiv, KDIV_MAX, prate);
+
+	if (rate >= rate_min && rate <= rate_max) {
+		kdiv = pll1443x_calc_kdiv(mdiv, pdiv, sdiv, rate, prate);
+		pr_debug("%s: in=%ld, want=%ld Only adjust kdiv %ld -> %d\n",
+			 clk_hw_get_name(&pll->hw), prate, rate,
+			 FIELD_GET(KDIV_MASK, pll_div_ctl1), kdiv);
+		fvco = pll14xx_calc_rate(pll, mdiv, pdiv, sdiv, kdiv, prate);
+		t->rate = (unsigned int)fvco;
+		t->mdiv = mdiv;
+		t->pdiv = pdiv;
+		t->sdiv = sdiv;
+		t->kdiv = kdiv;
+		return;
+	}
+
+	/* Then try if we can get the desired rate from one of the static entries */
+	tt = imx_get_pll_settings(pll, rate);
+	if (tt) {
+		pr_debug("%s: in=%ld, want=%ld, Using PLL setting from table\n",
+			 clk_hw_get_name(&pll->hw), prate, rate);
+		t->rate = tt->rate;
+		t->mdiv = tt->mdiv;
+		t->pdiv = tt->pdiv;
+		t->sdiv = tt->sdiv;
+		t->kdiv = tt->kdiv;
+		return;
+	}
+
+	/* Finally calculate best values */
+	for (pdiv = 1; pdiv <= 7; pdiv++) {
+		for (sdiv = 0; sdiv <= 6; sdiv++) {
+			/* calc mdiv = round(rate * pdiv * 2^sdiv) / prate) */
+			mdiv = DIV_ROUND_CLOSEST(rate * (pdiv << sdiv), prate);
+			mdiv = clamp(mdiv, 64, 1023);
+
+			kdiv = pll1443x_calc_kdiv(mdiv, pdiv, sdiv, rate, prate);
+			fvco = pll14xx_calc_rate(pll, mdiv, pdiv, sdiv, kdiv, prate);
+
+			/* best match */
+			dist = abs((long)rate - (long)fvco);
+			if (dist < best) {
+				best = dist;
+				t->rate = (unsigned int)fvco;
+				t->mdiv = mdiv;
+				t->pdiv = pdiv;
+				t->sdiv = sdiv;
+				t->kdiv = kdiv;
+
+				if (!dist)
+					goto found;
+			}
+		}
+	}
+found:
+	pr_debug("%s: in=%ld, want=%ld got=%d (pdiv=%d sdiv=%d mdiv=%d kdiv=%d)\n",
+		 clk_hw_get_name(&pll->hw), prate, rate, t->rate, t->pdiv, t->sdiv,
+		 t->mdiv, t->kdiv);
+}
+
+static long clk_pll1416x_round_rate(struct clk_hw *hw, unsigned long rate,
 			unsigned long *prate)
 {
 	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
@@ -128,6 +229,17 @@ static long clk_pll14xx_round_rate(struct clk_hw *hw, unsigned long rate,
 	return rate_table[pll->rate_count - 1].rate;
 }
 
+static long clk_pll1443x_round_rate(struct clk_hw *hw, unsigned long rate,
+			unsigned long *prate)
+{
+	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
+	struct imx_pll14xx_rate_table t;
+
+	imx_pll14xx_calc_settings(pll, rate, *prate, &t);
+
+	return t.rate;
+}
+
 static unsigned long clk_pll14xx_recalc_rate(struct clk_hw *hw,
 						  unsigned long parent_rate)
 {
@@ -238,25 +350,21 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 				 unsigned long prate)
 {
 	struct clk_pll14xx *pll = to_clk_pll14xx(hw);
-	const struct imx_pll14xx_rate_table *rate;
+	struct imx_pll14xx_rate_table rate;
 	u32 gnrl_ctl, div_ctl0;
 	int ret;
 
-	rate = imx_get_pll_settings(pll, drate);
-	if (!rate) {
-		pr_err("%s: Invalid rate : %lu for pll clk %s\n", __func__,
-			drate, clk_hw_get_name(hw));
-		return -EINVAL;
-	}
+	imx_pll14xx_calc_settings(pll, drate, prate, &rate);
 
 	div_ctl0 = readl_relaxed(pll->base + DIV_CTL0);
 
-	if (!clk_pll14xx_mp_change(rate, div_ctl0)) {
+	if (!clk_pll14xx_mp_change(&rate, div_ctl0)) {
+		/* only sdiv and/or kdiv changed - no need to RESET PLL */
 		div_ctl0 &= ~SDIV_MASK;
-		div_ctl0 |= FIELD_PREP(SDIV_MASK, rate->sdiv);
+		div_ctl0 |= FIELD_PREP(SDIV_MASK, rate.sdiv);
 		writel_relaxed(div_ctl0, pll->base + DIV_CTL0);
 
-		writel_relaxed(FIELD_PREP(KDIV_MASK, rate->kdiv),
+		writel_relaxed(FIELD_PREP(KDIV_MASK, rate.kdiv),
 			       pll->base + DIV_CTL1);
 
 		return 0;
@@ -271,11 +379,12 @@ static int clk_pll1443x_set_rate(struct clk_hw *hw, unsigned long drate,
 	gnrl_ctl |= BYPASS_MASK;
 	writel_relaxed(gnrl_ctl, pll->base + GNRL_CTL);
 
-	div_ctl0 = FIELD_PREP(MDIV_MASK, rate->mdiv) |
-		   FIELD_PREP(PDIV_MASK, rate->pdiv) |
-		   FIELD_PREP(SDIV_MASK, rate->sdiv);
+	div_ctl0 = FIELD_PREP(MDIV_MASK, rate.mdiv) |
+		   FIELD_PREP(PDIV_MASK, rate.pdiv) |
+		   FIELD_PREP(SDIV_MASK, rate.sdiv);
 	writel_relaxed(div_ctl0, pll->base + DIV_CTL0);
-	writel_relaxed(FIELD_PREP(KDIV_MASK, rate->kdiv), pll->base + DIV_CTL1);
+
+	writel_relaxed(FIELD_PREP(KDIV_MASK, rate.kdiv), pll->base + DIV_CTL1);
 
 	/*
 	 * According to SPEC, t3 - t2 need to be greater than
@@ -358,7 +467,7 @@ static const struct clk_ops clk_pll1416x_ops = {
 	.unprepare	= clk_pll14xx_unprepare,
 	.is_prepared	= clk_pll14xx_is_prepared,
 	.recalc_rate	= clk_pll14xx_recalc_rate,
-	.round_rate	= clk_pll14xx_round_rate,
+	.round_rate	= clk_pll1416x_round_rate,
 	.set_rate	= clk_pll1416x_set_rate,
 };
 
@@ -371,7 +480,7 @@ static const struct clk_ops clk_pll1443x_ops = {
 	.unprepare	= clk_pll14xx_unprepare,
 	.is_prepared	= clk_pll14xx_is_prepared,
 	.recalc_rate	= clk_pll14xx_recalc_rate,
-	.round_rate	= clk_pll14xx_round_rate,
+	.round_rate	= clk_pll1443x_round_rate,
 	.set_rate	= clk_pll1443x_set_rate,
 };
 
-- 
2.30.2

Re: [PATCH 8/8] clk: imx: pll14xx: Support dynamic rates

[kernel test robot]

Hi Sascha,

I love your patch! Perhaps something to improve:

[auto build test WARNING on shawnguo/for-next]
[also build test WARNING on clk/clk-next v5.17-rc5 next-20220222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sascha-Hauer/clk-i-MX-PLL14xx-Support-dynamic-rates/20220223-155846
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20220223/202202232005.F6GoajML-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/c2601acc01166ae3f20b60817e44d3e94a023c6f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sascha-Hauer/clk-i-MX-PLL14xx-Support-dynamic-rates/20220223-155846
        git checkout c2601acc01166ae3f20b60817e44d3e94a023c6f
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=alpha SHELL=/bin/bash drivers/clk/imx/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   drivers/clk/imx/clk-pll14xx.c: In function 'imx_pll14xx_calc_settings':
   drivers/clk/imx/clk-pll14xx.c:149:16: error: implicit declaration of function 'FIELD_GET'; did you mean 'FOLL_GET'? [-Werror=implicit-function-declaration]
     149 |         mdiv = FIELD_GET(MDIV_MASK, pll_div_ctl0);
         |                ^~~~~~~~~
         |                FOLL_GET
>> drivers/clk/imx/clk-pll14xx.c:6:21: warning: format '%ld' expects argument of type 'long int', but argument 6 has type 'int' [-Wformat=]
       6 | #define pr_fmt(fmt) "pll14xx: " fmt
         |                     ^~~~~~~~~~~
   include/linux/dynamic_debug.h:134:29: note: in expansion of macro 'pr_fmt'
     134 |                 func(&id, ##__VA_ARGS__);               \
         |                             ^~~~~~~~~~~
   include/linux/dynamic_debug.h:152:9: note: in expansion of macro '__dynamic_func_call'
     152 |         __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:162:9: note: in expansion of macro '_dynamic_func_call'
     162 |         _dynamic_func_call(fmt, __dynamic_pr_debug,             \
         |         ^~~~~~~~~~~~~~~~~~
   include/linux/printk.h:570:9: note: in expansion of macro 'dynamic_pr_debug'
     570 |         dynamic_pr_debug(fmt, ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~
   drivers/clk/imx/clk-pll14xx.c:160:17: note: in expansion of macro 'pr_debug'
     160 |                 pr_debug("%s: in=%ld, want=%ld Only adjust kdiv %ld -> %d\n",
         |                 ^~~~~~~~
   In file included from include/asm-generic/bug.h:22,
                    from arch/alpha/include/asm/bug.h:23,
                    from include/linux/bug.h:5,
                    from include/linux/thread_info.h:13,
                    from include/asm-generic/current.h:5,
                    from ./arch/alpha/include/generated/asm/current.h:1,
                    from include/linux/mutex.h:14,
                    from include/linux/kernfs.h:11,
                    from include/linux/sysfs.h:16,
                    from include/linux/kobject.h:20,
                    from include/linux/of.h:17,
                    from include/linux/clk-provider.h:9,
                    from drivers/clk/imx/clk-pll14xx.c:9:
   drivers/clk/imx/clk-pll14xx.c: In function 'clk_pll1416x_set_rate':
   include/linux/kern_levels.h:5:25: warning: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'const char *' [-Wformat=]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:418:25: note: in definition of macro 'printk_index_wrap'
     418 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:489:9: note: in expansion of macro 'printk'
     489 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~
   include/linux/kern_levels.h:11:25: note: in expansion of macro 'KERN_SOH'
      11 | #define KERN_ERR        KERN_SOH "3"    /* error conditions */
         |                         ^~~~~~~~
   include/linux/printk.h:489:16: note: in expansion of macro 'KERN_ERR'
     489 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |                ^~~~~~~~
   drivers/clk/imx/clk-pll14xx.c:293:17: note: in expansion of macro 'pr_err'
     293 |                 pr_err("Invalid rate %lu for pll clk %s\n", __func__,
         |                 ^~~~~~
   include/linux/kern_levels.h:5:25: warning: format '%s' expects argument of type 'char *', but argument 3 has type 'long unsigned int' [-Wformat=]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:418:25: note: in definition of macro 'printk_index_wrap'
     418 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:489:9: note: in expansion of macro 'printk'
     489 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~
   include/linux/kern_levels.h:11:25: note: in expansion of macro 'KERN_SOH'
      11 | #define KERN_ERR        KERN_SOH "3"    /* error conditions */
         |                         ^~~~~~~~
   include/linux/printk.h:489:16: note: in expansion of macro 'KERN_ERR'
     489 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |                ^~~~~~~~
   drivers/clk/imx/clk-pll14xx.c:293:17: note: in expansion of macro 'pr_err'
     293 |                 pr_err("Invalid rate %lu for pll clk %s\n", __func__,
         |                 ^~~~~~
   include/linux/kern_levels.h:5:25: warning: too many arguments for format [-Wformat-extra-args]
       5 | #define KERN_SOH        "\001"          /* ASCII Start Of Header */
         |                         ^~~~~~
   include/linux/printk.h:418:25: note: in definition of macro 'printk_index_wrap'
     418 |                 _p_func(_fmt, ##__VA_ARGS__);                           \
         |                         ^~~~
   include/linux/printk.h:489:9: note: in expansion of macro 'printk'
     489 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |         ^~~~~~
   include/linux/kern_levels.h:11:25: note: in expansion of macro 'KERN_SOH'
      11 | #define KERN_ERR        KERN_SOH "3"    /* error conditions */
         |                         ^~~~~~~~
   include/linux/printk.h:489:16: note: in expansion of macro 'KERN_ERR'
     489 |         printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
         |                ^~~~~~~~
   drivers/clk/imx/clk-pll14xx.c:293:17: note: in expansion of macro 'pr_err'
     293 |                 pr_err("Invalid rate %lu for pll clk %s\n", __func__,
         |                 ^~~~~~
   drivers/clk/imx/clk-pll14xx.c:302:24: error: implicit declaration of function 'FIELD_PREP' [-Werror=implicit-function-declaration]
     302 |                 tmp |= FIELD_PREP(SDIV_MASK, rate->sdiv);
         |                        ^~~~~~~~~~
   cc1: some warnings being treated as errors


vim +6 drivers/clk/imx/clk-pll14xx.c

0c0c0d808f682f Sascha Hauer 2022-02-23 @6  #define pr_fmt(fmt) "pll14xx: " fmt
0c0c0d808f682f Sascha Hauer 2022-02-23  7  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org