* [PATCH] clk: imx: add fractional-N pll support to pllv4
@ 2019-04-29 3:18 Anson Huang
2019-04-29 11:28 ` Aisheng Dong
0 siblings, 1 reply; 5+ messages in thread
From: Anson Huang @ 2019-04-29 3:18 UTC (permalink / raw)
To: mturquette, sboyd, shawnguo, s.hauer, kernel, festevam,
Aisheng Dong, linux-clk, linux-arm-kernel, linux-kernel
Cc: dl-linux-imx
The pllv4 supports fractional-N function, the formula is:
PLL output freq = input * (mult + num/denom),
This patch adds fractional-N function support, including
clock round rate, calculate rate and set rate, with this
patch, the clock rate of APLL in clock tree is more accurate
than before:
Without fraction:
apll_pre_sel 1 1 1 24000000 0 0 50000
apll_pre_div 1 1 2 24000000 0 0 50000
apll 1 1 2 528000000 0 0 50000
apll_pfd3 0 0 0 792000000 0 0 50000
apll_pfd2 0 0 0 339428571 0 0 50000
apll_pfd1 0 0 0 352000000 0 0 50000
usdhc0 0 0 0 352000000 0 0 50000
apll_pfd0 1 1 1 352000000 0 0 50000
With fraction:
apll_pre_sel 1 1 1 24000000 0 0 50000
apll_pre_div 1 1 2 24000000 0 0 50000
apll 1 1 2 529200000 0 0 50000
apll_pfd3 0 0 0 793800000 0 0 50000
apll_pfd2 0 0 0 340200000 0 0 50000
apll_pfd1 0 0 0 352800000 0 0 50000
usdhc0 0 0 0 352800000 0 0 50000
apll_pfd0 1 1 1 352800000 0 0 50000
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
drivers/clk/imx/clk-pllv4.c | 68 +++++++++++++++++++++++++++++++++++++++------
1 file changed, 60 insertions(+), 8 deletions(-)
diff --git a/drivers/clk/imx/clk-pllv4.c b/drivers/clk/imx/clk-pllv4.c
index d38bc9f..4ced5ca 100644
--- a/drivers/clk/imx/clk-pllv4.c
+++ b/drivers/clk/imx/clk-pllv4.c
@@ -64,13 +64,18 @@ static unsigned long clk_pllv4_recalc_rate(struct clk_hw *hw,
unsigned long parent_rate)
{
struct clk_pllv4 *pll = to_clk_pllv4(hw);
- u32 div;
+ u32 mult = readl_relaxed(pll->base + PLL_CFG_OFFSET);
+ u32 mfn = readl_relaxed(pll->base + PLL_NUM_OFFSET);
+ u32 mfd = readl_relaxed(pll->base + PLL_DENOM_OFFSET);
+ u64 temp64 = parent_rate;
- div = readl_relaxed(pll->base + PLL_CFG_OFFSET);
- div &= BM_PLL_MULT;
- div >>= BP_PLL_MULT;
+ mult &= BM_PLL_MULT;
+ mult >>= BP_PLL_MULT;
- return parent_rate * div;
+ temp64 *= mfn;
+ do_div(temp64, mfd);
+
+ return (parent_rate * mult) + (u32)temp64;
}
static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned long rate,
@@ -78,14 +83,47 @@ static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned long rate,
{
unsigned long parent_rate = *prate;
unsigned long round_rate, i;
+ bool found = false;
+ u32 mfn, mfd = 1000000;
+ u32 max_mfd = 0x3FFFFFFF;
+ u64 temp64;
for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
round_rate = parent_rate * pllv4_mult_table[i];
- if (rate >= round_rate)
- return round_rate;
+ if (rate >= round_rate) {
+ found = true;
+ break;
+ }
+ }
+
+ if (!found) {
+ pr_warn("%s: unable to round rate %lu, parent rate %lu\n",
+ clk_hw_get_name(hw), rate, parent_rate);
+ return 0;
}
- return round_rate;
+ if (parent_rate <= max_mfd)
+ mfd = parent_rate;
+
+ temp64 = (u64)(rate - round_rate);
+ temp64 *= mfd;
+ do_div(temp64, parent_rate);
+ mfn = temp64;
+
+ /*
+ * NOTE: The value of numerator must always be configured to be
+ * less than the value of the denominator. If we can't get a proper
+ * pair of mfn/mfd, we simply return the round_rate without using
+ * the frac part.
+ */
+ if (mfn >= mfd)
+ return round_rate;
+
+ temp64 = (u64)parent_rate;
+ temp64 *= mfn;
+ do_div(temp64, mfd);
+
+ return round_rate + (u32)temp64;
}
static bool clk_pllv4_is_valid_mult(unsigned int mult)
@@ -106,17 +144,31 @@ static int clk_pllv4_set_rate(struct clk_hw *hw, unsigned long rate,
{
struct clk_pllv4 *pll = to_clk_pllv4(hw);
u32 val, mult;
+ u32 mfn, mfd = 1000000;
+ u32 max_mfd = 0x3FFFFFFF;
+ u64 temp64;
mult = rate / parent_rate;
if (!clk_pllv4_is_valid_mult(mult))
return -EINVAL;
+ if (parent_rate <= max_mfd)
+ mfd = parent_rate;
+
+ temp64 = (u64)(rate - mult * parent_rate);
+ temp64 *= mfd;
+ do_div(temp64, parent_rate);
+ mfn = temp64;
+
val = readl_relaxed(pll->base + PLL_CFG_OFFSET);
val &= ~BM_PLL_MULT;
val |= mult << BP_PLL_MULT;
writel_relaxed(val, pll->base + PLL_CFG_OFFSET);
+ writel_relaxed(mfn, pll->base + PLL_NUM_OFFSET);
+ writel_relaxed(mfd, pll->base + PLL_DENOM_OFFSET);
+
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* RE: [PATCH] clk: imx: add fractional-N pll support to pllv4
2019-04-29 3:18 [PATCH] clk: imx: add fractional-N pll support to pllv4 Anson Huang
@ 2019-04-29 11:28 ` Aisheng Dong
2019-04-29 11:35 ` Aisheng Dong
2019-04-29 12:34 ` Anson Huang
0 siblings, 2 replies; 5+ messages in thread
From: Aisheng Dong @ 2019-04-29 11:28 UTC (permalink / raw)
To: Anson Huang, mturquette, sboyd, shawnguo, s.hauer, kernel,
festevam, linux-clk, linux-arm-kernel, linux-kernel
Cc: dl-linux-imx
> From: Anson Huang
> Sent: Monday, April 29, 2019 11:19 AM
>
clk: imx: pllv4: add fractional-N pll support
> The pllv4 supports fractional-N function, the formula is:
>
> PLL output freq = input * (mult + num/denom),
>
> This patch adds fractional-N function support, including clock round rate,
> calculate rate and set rate, with this patch, the clock rate of APLL in clock tree
> is more accurate than before:
>
> Without fraction:
> apll_pre_sel 1 1 1 24000000
> 0 0 50000
> apll_pre_div 1 1 2 24000000
> 0 0 50000
> apll 1 1 2 528000000
> 0 0 50000
> apll_pfd3 0 0 0 792000000
> 0 0 50000
> apll_pfd2 0 0 0 339428571
> 0 0 50000
> apll_pfd1 0 0 0 352000000
> 0 0 50000
> usdhc0 0 0 0 352000000
> 0 0 50000
> apll_pfd0 1 1 1 352000000
> 0 0 50000
>
> With fraction:
> apll_pre_sel 1 1 1 24000000
> 0 0 50000
> apll_pre_div 1 1 2 24000000
> 0 0 50000
> apll 1 1 2 529200000
> 0 0 50000
> apll_pfd3 0 0 0 793800000
> 0 0 50000
> apll_pfd2 0 0 0 340200000
> 0 0 50000
> apll_pfd1 0 0 0 352800000
> 0 0 50000
> usdhc0 0 0 0 352800000
> 0 0 50000
> apll_pfd0 1 1 1 352800000
> 0 0 50000
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> drivers/clk/imx/clk-pllv4.c | 68
> +++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 60 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/clk/imx/clk-pllv4.c b/drivers/clk/imx/clk-pllv4.c index
> d38bc9f..4ced5ca 100644
> --- a/drivers/clk/imx/clk-pllv4.c
> +++ b/drivers/clk/imx/clk-pllv4.c
> @@ -64,13 +64,18 @@ static unsigned long clk_pllv4_recalc_rate(struct
> clk_hw *hw,
> unsigned long parent_rate)
> {
> struct clk_pllv4 *pll = to_clk_pllv4(hw);
> - u32 div;
> + u32 mult = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> + u32 mfn = readl_relaxed(pll->base + PLL_NUM_OFFSET);
> + u32 mfd = readl_relaxed(pll->base + PLL_DENOM_OFFSET);
Nitpick:
We usually don't write code like this.
How about separate the assignment from declaration?
> + u64 temp64 = parent_rate;
>
> - div = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> - div &= BM_PLL_MULT;
> - div >>= BP_PLL_MULT;
> + mult &= BM_PLL_MULT;
> + mult >>= BP_PLL_MULT;
>
> - return parent_rate * div;
> + temp64 *= mfn;
> + do_div(temp64, mfd);
> +
> + return (parent_rate * mult) + (u32)temp64;
> }
>
> static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned long rate, @@
> -78,14 +83,47 @@ static long clk_pllv4_round_rate(struct clk_hw *hw,
> unsigned long rate, {
> unsigned long parent_rate = *prate;
> unsigned long round_rate, i;
> + bool found = false;
> + u32 mfn, mfd = 1000000;
> + u32 max_mfd = 0x3FFFFFFF;
Please keep sort from long to short.
And the multi Max_mfd definitions could be move out the function and
Defined use macro.
> + u64 temp64;
>
> for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> round_rate = parent_rate * pllv4_mult_table[i];
> - if (rate >= round_rate)
> - return round_rate;
> + if (rate >= round_rate) {
> + found = true;
> + break;
> + }
> + }
> +
> + if (!found) {
> + pr_warn("%s: unable to round rate %lu, parent rate %lu\n",
> + clk_hw_get_name(hw), rate, parent_rate);
> + return 0;
> }
>
> - return round_rate;
> + if (parent_rate <= max_mfd)
> + mfd = parent_rate;
> +
> + temp64 = (u64)(rate - round_rate);
> + temp64 *= mfd;
> + do_div(temp64, parent_rate);
> + mfn = temp64;
> +
> + /*
> + * NOTE: The value of numerator must always be configured to be
> + * less than the value of the denominator. If we can't get a proper
> + * pair of mfn/mfd, we simply return the round_rate without using
> + * the frac part.
> + */
> + if (mfn >= mfd)
> + return round_rate;
> +
> + temp64 = (u64)parent_rate;
> + temp64 *= mfn;
> + do_div(temp64, mfd);
> +
> + return round_rate + (u32)temp64;
> }
>
> static bool clk_pllv4_is_valid_mult(unsigned int mult) @@ -106,17 +144,31
> @@ static int clk_pllv4_set_rate(struct clk_hw *hw, unsigned long rate, {
> struct clk_pllv4 *pll = to_clk_pllv4(hw);
> u32 val, mult;
> + u32 mfn, mfd = 1000000;
> + u32 max_mfd = 0x3FFFFFFF;
Ditto
Otherwise:
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
Regards
Dong Aisheng
> + u64 temp64;
>
> mult = rate / parent_rate;
>
> if (!clk_pllv4_is_valid_mult(mult))
> return -EINVAL;
>
> + if (parent_rate <= max_mfd)
> + mfd = parent_rate;
> +
> + temp64 = (u64)(rate - mult * parent_rate);
> + temp64 *= mfd;
> + do_div(temp64, parent_rate);
> + mfn = temp64;
> +
> val = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> val &= ~BM_PLL_MULT;
> val |= mult << BP_PLL_MULT;
> writel_relaxed(val, pll->base + PLL_CFG_OFFSET);
>
> + writel_relaxed(mfn, pll->base + PLL_NUM_OFFSET);
> + writel_relaxed(mfd, pll->base + PLL_DENOM_OFFSET);
> +
> return 0;
> }
>
> --
> 2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH] clk: imx: add fractional-N pll support to pllv4
2019-04-29 11:28 ` Aisheng Dong
@ 2019-04-29 11:35 ` Aisheng Dong
2019-04-29 12:33 ` Anson Huang
2019-04-29 12:34 ` Anson Huang
1 sibling, 1 reply; 5+ messages in thread
From: Aisheng Dong @ 2019-04-29 11:35 UTC (permalink / raw)
To: Anson Huang, mturquette, sboyd, shawnguo, s.hauer, kernel,
festevam, linux-clk, linux-arm-kernel, linux-kernel
Cc: dl-linux-imx
> From: Aisheng Dong
> Sent: Monday, April 29, 2019 7:28 PM
> Subject: RE: [PATCH] clk: imx: add fractional-N pll support to pllv4
> > The pllv4 supports fractional-N function, the formula is:
> >
> > PLL output freq = input * (mult + num/denom),
> >
> > This patch adds fractional-N function support, including clock round
> > rate, calculate rate and set rate, with this patch, the clock rate of
> > APLL in clock tree is more accurate than before:
> >
BTW, one more question:
Does B0 chip support fractional for SPLL as this patch affects both APLL and SPLL?
I did not see NUM&DENOM register for SPLL in my doc, not sure if it's latest version.
Regards
Dong Aisheng
> > Without fraction:
> > apll_pre_sel 1 1 1 24000000
> > 0 0 50000
> > apll_pre_div 1 1 2 24000000
> > 0 0 50000
> > apll 1 1 2 528000000
> > 0 0 50000
> > apll_pfd3 0 0 0 792000000
> > 0 0 50000
> > apll_pfd2 0 0 0 339428571
> > 0 0 50000
> > apll_pfd1 0 0 0 352000000
> > 0 0 50000
> > usdhc0 0 0 0
> 352000000
> > 0 0 50000
> > apll_pfd0 1 1 1 352000000
> > 0 0 50000
> >
> > With fraction:
> > apll_pre_sel 1 1 1 24000000
> > 0 0 50000
> > apll_pre_div 1 1 2 24000000
> > 0 0 50000
> > apll 1 1 2 529200000
> > 0 0 50000
> > apll_pfd3 0 0 0 793800000
> > 0 0 50000
> > apll_pfd2 0 0 0 340200000
> > 0 0 50000
> > apll_pfd1 0 0 0 352800000
> > 0 0 50000
> > usdhc0 0 0 0
> 352800000
> > 0 0 50000
> > apll_pfd0 1 1 1 352800000
> > 0 0 50000
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > drivers/clk/imx/clk-pllv4.c | 68
> > +++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 60 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/clk/imx/clk-pllv4.c b/drivers/clk/imx/clk-pllv4.c
> > index d38bc9f..4ced5ca 100644
> > --- a/drivers/clk/imx/clk-pllv4.c
> > +++ b/drivers/clk/imx/clk-pllv4.c
> > @@ -64,13 +64,18 @@ static unsigned long clk_pllv4_recalc_rate(struct
> > clk_hw *hw,
> > unsigned long parent_rate)
> > {
> > struct clk_pllv4 *pll = to_clk_pllv4(hw);
> > - u32 div;
> > + u32 mult = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> > + u32 mfn = readl_relaxed(pll->base + PLL_NUM_OFFSET);
> > + u32 mfd = readl_relaxed(pll->base + PLL_DENOM_OFFSET);
>
> Nitpick:
> We usually don't write code like this.
> How about separate the assignment from declaration?
>
> > + u64 temp64 = parent_rate;
> >
> > - div = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> > - div &= BM_PLL_MULT;
> > - div >>= BP_PLL_MULT;
> > + mult &= BM_PLL_MULT;
> > + mult >>= BP_PLL_MULT;
> >
> > - return parent_rate * div;
> > + temp64 *= mfn;
> > + do_div(temp64, mfd);
> > +
> > + return (parent_rate * mult) + (u32)temp64;
> > }
> >
> > static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned long
> > rate, @@
> > -78,14 +83,47 @@ static long clk_pllv4_round_rate(struct clk_hw *hw,
> > unsigned long rate, {
> > unsigned long parent_rate = *prate;
> > unsigned long round_rate, i;
> > + bool found = false;
> > + u32 mfn, mfd = 1000000;
> > + u32 max_mfd = 0x3FFFFFFF;
>
> Please keep sort from long to short.
> And the multi Max_mfd definitions could be move out the function and
> Defined use macro.
>
> > + u64 temp64;
> >
> > for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> > round_rate = parent_rate * pllv4_mult_table[i];
> > - if (rate >= round_rate)
> > - return round_rate;
> > + if (rate >= round_rate) {
> > + found = true;
> > + break;
> > + }
> > + }
> > +
> > + if (!found) {
> > + pr_warn("%s: unable to round rate %lu, parent rate %lu\n",
> > + clk_hw_get_name(hw), rate, parent_rate);
> > + return 0;
> > }
> >
> > - return round_rate;
> > + if (parent_rate <= max_mfd)
> > + mfd = parent_rate;
> > +
> > + temp64 = (u64)(rate - round_rate);
> > + temp64 *= mfd;
> > + do_div(temp64, parent_rate);
> > + mfn = temp64;
> > +
> > + /*
> > + * NOTE: The value of numerator must always be configured to be
> > + * less than the value of the denominator. If we can't get a proper
> > + * pair of mfn/mfd, we simply return the round_rate without using
> > + * the frac part.
> > + */
> > + if (mfn >= mfd)
> > + return round_rate;
> > +
> > + temp64 = (u64)parent_rate;
> > + temp64 *= mfn;
> > + do_div(temp64, mfd);
> > +
> > + return round_rate + (u32)temp64;
> > }
> >
> > static bool clk_pllv4_is_valid_mult(unsigned int mult) @@ -106,17
> > +144,31 @@ static int clk_pllv4_set_rate(struct clk_hw *hw, unsigned long
> rate, {
> > struct clk_pllv4 *pll = to_clk_pllv4(hw);
> > u32 val, mult;
> > + u32 mfn, mfd = 1000000;
> > + u32 max_mfd = 0x3FFFFFFF;
>
> Ditto
>
> Otherwise:
> Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
>
> Regards
> Dong Aisheng
>
> > + u64 temp64;
> >
> > mult = rate / parent_rate;
> >
> > if (!clk_pllv4_is_valid_mult(mult))
> > return -EINVAL;
> >
> > + if (parent_rate <= max_mfd)
> > + mfd = parent_rate;
> > +
> > + temp64 = (u64)(rate - mult * parent_rate);
> > + temp64 *= mfd;
> > + do_div(temp64, parent_rate);
> > + mfn = temp64;
> > +
> > val = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> > val &= ~BM_PLL_MULT;
> > val |= mult << BP_PLL_MULT;
> > writel_relaxed(val, pll->base + PLL_CFG_OFFSET);
> >
> > + writel_relaxed(mfn, pll->base + PLL_NUM_OFFSET);
> > + writel_relaxed(mfd, pll->base + PLL_DENOM_OFFSET);
> > +
> > return 0;
> > }
> >
> > --
> > 2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH] clk: imx: add fractional-N pll support to pllv4
2019-04-29 11:35 ` Aisheng Dong
@ 2019-04-29 12:33 ` Anson Huang
0 siblings, 0 replies; 5+ messages in thread
From: Anson Huang @ 2019-04-29 12:33 UTC (permalink / raw)
To: Aisheng Dong, mturquette, sboyd, shawnguo, s.hauer, kernel,
festevam, linux-clk, linux-arm-kernel, linux-kernel
Cc: dl-linux-imx
Hi, Aisheng
> -----Original Message-----
> From: Aisheng Dong
> Sent: Monday, April 29, 2019 7:36 PM
> To: Anson Huang <anson.huang@nxp.com>; mturquette@baylibre.com;
> sboyd@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-clk@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>
> Subject: RE: [PATCH] clk: imx: add fractional-N pll support to pllv4
>
> > From: Aisheng Dong
> > Sent: Monday, April 29, 2019 7:28 PM
> > Subject: RE: [PATCH] clk: imx: add fractional-N pll support to pllv4
> > > The pllv4 supports fractional-N function, the formula is:
> > >
> > > PLL output freq = input * (mult + num/denom),
> > >
> > > This patch adds fractional-N function support, including clock round
> > > rate, calculate rate and set rate, with this patch, the clock rate
> > > of APLL in clock tree is more accurate than before:
> > >
>
> BTW, one more question:
> Does B0 chip support fractional for SPLL as this patch affects both APLL and
> SPLL?
> I did not see NUM&DENOM register for SPLL in my doc, not sure if it's latest
> version.
There are NUM&DENOM for both SPLL and APLL in reference manual, my RM is
V2(2.0), published on 12/2017.
Anson.
>
> Regards
> Dong Aisheng
>
> > > Without fraction:
> > > apll_pre_sel 1 1 1 24000000
> > > 0 0 50000
> > > apll_pre_div 1 1 2 24000000
> > > 0 0 50000
> > > apll 1 1 2 528000000
> > > 0 0 50000
> > > apll_pfd3 0 0 0 792000000
> > > 0 0 50000
> > > apll_pfd2 0 0 0 339428571
> > > 0 0 50000
> > > apll_pfd1 0 0 0 352000000
> > > 0 0 50000
> > > usdhc0 0 0 0
> > 352000000
> > > 0 0 50000
> > > apll_pfd0 1 1 1 352000000
> > > 0 0 50000
> > >
> > > With fraction:
> > > apll_pre_sel 1 1 1 24000000
> > > 0 0 50000
> > > apll_pre_div 1 1 2 24000000
> > > 0 0 50000
> > > apll 1 1 2 529200000
> > > 0 0 50000
> > > apll_pfd3 0 0 0 793800000
> > > 0 0 50000
> > > apll_pfd2 0 0 0 340200000
> > > 0 0 50000
> > > apll_pfd1 0 0 0 352800000
> > > 0 0 50000
> > > usdhc0 0 0 0
> > 352800000
> > > 0 0 50000
> > > apll_pfd0 1 1 1 352800000
> > > 0 0 50000
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > ---
> > > drivers/clk/imx/clk-pllv4.c | 68
> > > +++++++++++++++++++++++++++++++++++++++------
> > > 1 file changed, 60 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/clk/imx/clk-pllv4.c
> > > b/drivers/clk/imx/clk-pllv4.c index d38bc9f..4ced5ca 100644
> > > --- a/drivers/clk/imx/clk-pllv4.c
> > > +++ b/drivers/clk/imx/clk-pllv4.c
> > > @@ -64,13 +64,18 @@ static unsigned long
> > > clk_pllv4_recalc_rate(struct clk_hw *hw,
> > > unsigned long parent_rate)
> > > {
> > > struct clk_pllv4 *pll = to_clk_pllv4(hw);
> > > - u32 div;
> > > + u32 mult = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> > > + u32 mfn = readl_relaxed(pll->base + PLL_NUM_OFFSET);
> > > + u32 mfd = readl_relaxed(pll->base + PLL_DENOM_OFFSET);
> >
> > Nitpick:
> > We usually don't write code like this.
> > How about separate the assignment from declaration?
> >
> > > + u64 temp64 = parent_rate;
> > >
> > > - div = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> > > - div &= BM_PLL_MULT;
> > > - div >>= BP_PLL_MULT;
> > > + mult &= BM_PLL_MULT;
> > > + mult >>= BP_PLL_MULT;
> > >
> > > - return parent_rate * div;
> > > + temp64 *= mfn;
> > > + do_div(temp64, mfd);
> > > +
> > > + return (parent_rate * mult) + (u32)temp64;
> > > }
> > >
> > > static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned long
> > > rate, @@
> > > -78,14 +83,47 @@ static long clk_pllv4_round_rate(struct clk_hw *hw,
> > > unsigned long rate, {
> > > unsigned long parent_rate = *prate;
> > > unsigned long round_rate, i;
> > > + bool found = false;
> > > + u32 mfn, mfd = 1000000;
> > > + u32 max_mfd = 0x3FFFFFFF;
> >
> > Please keep sort from long to short.
> > And the multi Max_mfd definitions could be move out the function and
> > Defined use macro.
> >
> > > + u64 temp64;
> > >
> > > for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> > > round_rate = parent_rate * pllv4_mult_table[i];
> > > - if (rate >= round_rate)
> > > - return round_rate;
> > > + if (rate >= round_rate) {
> > > + found = true;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + if (!found) {
> > > + pr_warn("%s: unable to round rate %lu, parent rate %lu\n",
> > > + clk_hw_get_name(hw), rate, parent_rate);
> > > + return 0;
> > > }
> > >
> > > - return round_rate;
> > > + if (parent_rate <= max_mfd)
> > > + mfd = parent_rate;
> > > +
> > > + temp64 = (u64)(rate - round_rate);
> > > + temp64 *= mfd;
> > > + do_div(temp64, parent_rate);
> > > + mfn = temp64;
> > > +
> > > + /*
> > > + * NOTE: The value of numerator must always be configured to be
> > > + * less than the value of the denominator. If we can't get a proper
> > > + * pair of mfn/mfd, we simply return the round_rate without using
> > > + * the frac part.
> > > + */
> > > + if (mfn >= mfd)
> > > + return round_rate;
> > > +
> > > + temp64 = (u64)parent_rate;
> > > + temp64 *= mfn;
> > > + do_div(temp64, mfd);
> > > +
> > > + return round_rate + (u32)temp64;
> > > }
> > >
> > > static bool clk_pllv4_is_valid_mult(unsigned int mult) @@ -106,17
> > > +144,31 @@ static int clk_pllv4_set_rate(struct clk_hw *hw, unsigned
> > > +long
> > rate, {
> > > struct clk_pllv4 *pll = to_clk_pllv4(hw);
> > > u32 val, mult;
> > > + u32 mfn, mfd = 1000000;
> > > + u32 max_mfd = 0x3FFFFFFF;
> >
> > Ditto
> >
> > Otherwise:
> > Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
> >
> > Regards
> > Dong Aisheng
> >
> > > + u64 temp64;
> > >
> > > mult = rate / parent_rate;
> > >
> > > if (!clk_pllv4_is_valid_mult(mult))
> > > return -EINVAL;
> > >
> > > + if (parent_rate <= max_mfd)
> > > + mfd = parent_rate;
> > > +
> > > + temp64 = (u64)(rate - mult * parent_rate);
> > > + temp64 *= mfd;
> > > + do_div(temp64, parent_rate);
> > > + mfn = temp64;
> > > +
> > > val = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> > > val &= ~BM_PLL_MULT;
> > > val |= mult << BP_PLL_MULT;
> > > writel_relaxed(val, pll->base + PLL_CFG_OFFSET);
> > >
> > > + writel_relaxed(mfn, pll->base + PLL_NUM_OFFSET);
> > > + writel_relaxed(mfd, pll->base + PLL_DENOM_OFFSET);
> > > +
> > > return 0;
> > > }
> > >
> > > --
> > > 2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] clk: imx: add fractional-N pll support to pllv4
2019-04-29 11:28 ` Aisheng Dong
2019-04-29 11:35 ` Aisheng Dong
@ 2019-04-29 12:34 ` Anson Huang
1 sibling, 0 replies; 5+ messages in thread
From: Anson Huang @ 2019-04-29 12:34 UTC (permalink / raw)
To: Aisheng Dong, mturquette, sboyd, shawnguo, s.hauer, kernel,
festevam, linux-clk, linux-arm-kernel, linux-kernel
Cc: dl-linux-imx
Hi, Aisheng
> -----Original Message-----
> From: Aisheng Dong
> Sent: Monday, April 29, 2019 7:28 PM
> To: Anson Huang <anson.huang@nxp.com>; mturquette@baylibre.com;
> sboyd@kernel.org; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; festevam@gmail.com; linux-clk@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
> Cc: dl-linux-imx <linux-imx@nxp.com>
> Subject: RE: [PATCH] clk: imx: add fractional-N pll support to pllv4
>
> > From: Anson Huang
> > Sent: Monday, April 29, 2019 11:19 AM
> >
>
> clk: imx: pllv4: add fractional-N pll support
Will improve it in V2.
> > The pllv4 supports fractional-N function, the formula is:
> >
> > PLL output freq = input * (mult + num/denom),
> >
> > This patch adds fractional-N function support, including clock round
> > rate, calculate rate and set rate, with this patch, the clock rate of
> > APLL in clock tree is more accurate than before:
> >
> > Without fraction:
> > apll_pre_sel 1 1 1 24000000
> > 0 0 50000
> > apll_pre_div 1 1 2 24000000
> > 0 0 50000
> > apll 1 1 2 528000000
> > 0 0 50000
> > apll_pfd3 0 0 0 792000000
> > 0 0 50000
> > apll_pfd2 0 0 0 339428571
> > 0 0 50000
> > apll_pfd1 0 0 0 352000000
> > 0 0 50000
> > usdhc0 0 0 0 352000000
> > 0 0 50000
> > apll_pfd0 1 1 1 352000000
> > 0 0 50000
> >
> > With fraction:
> > apll_pre_sel 1 1 1 24000000
> > 0 0 50000
> > apll_pre_div 1 1 2 24000000
> > 0 0 50000
> > apll 1 1 2 529200000
> > 0 0 50000
> > apll_pfd3 0 0 0 793800000
> > 0 0 50000
> > apll_pfd2 0 0 0 340200000
> > 0 0 50000
> > apll_pfd1 0 0 0 352800000
> > 0 0 50000
> > usdhc0 0 0 0 352800000
> > 0 0 50000
> > apll_pfd0 1 1 1 352800000
> > 0 0 50000
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > drivers/clk/imx/clk-pllv4.c | 68
> > +++++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 60 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/clk/imx/clk-pllv4.c b/drivers/clk/imx/clk-pllv4.c
> > index d38bc9f..4ced5ca 100644
> > --- a/drivers/clk/imx/clk-pllv4.c
> > +++ b/drivers/clk/imx/clk-pllv4.c
> > @@ -64,13 +64,18 @@ static unsigned long clk_pllv4_recalc_rate(struct
> > clk_hw *hw,
> > unsigned long parent_rate)
> > {
> > struct clk_pllv4 *pll = to_clk_pllv4(hw);
> > - u32 div;
> > + u32 mult = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> > + u32 mfn = readl_relaxed(pll->base + PLL_NUM_OFFSET);
> > + u32 mfd = readl_relaxed(pll->base + PLL_DENOM_OFFSET);
>
> Nitpick:
> We usually don't write code like this.
> How about separate the assignment from declaration?
I will improve them in V2.
>
> > + u64 temp64 = parent_rate;
> >
> > - div = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> > - div &= BM_PLL_MULT;
> > - div >>= BP_PLL_MULT;
> > + mult &= BM_PLL_MULT;
> > + mult >>= BP_PLL_MULT;
> >
> > - return parent_rate * div;
> > + temp64 *= mfn;
> > + do_div(temp64, mfd);
> > +
> > + return (parent_rate * mult) + (u32)temp64;
> > }
> >
> > static long clk_pllv4_round_rate(struct clk_hw *hw, unsigned long
> > rate, @@
> > -78,14 +83,47 @@ static long clk_pllv4_round_rate(struct clk_hw *hw,
> > unsigned long rate, {
> > unsigned long parent_rate = *prate;
> > unsigned long round_rate, i;
> > + bool found = false;
> > + u32 mfn, mfd = 1000000;
> > + u32 max_mfd = 0x3FFFFFFF;
>
> Please keep sort from long to short.
> And the multi Max_mfd definitions could be move out the function and
> Defined use macro.
OK, will improve them in V2.
>
> > + u64 temp64;
> >
> > for (i = 0; i < ARRAY_SIZE(pllv4_mult_table); i++) {
> > round_rate = parent_rate * pllv4_mult_table[i];
> > - if (rate >= round_rate)
> > - return round_rate;
> > + if (rate >= round_rate) {
> > + found = true;
> > + break;
> > + }
> > + }
> > +
> > + if (!found) {
> > + pr_warn("%s: unable to round rate %lu, parent rate %lu\n",
> > + clk_hw_get_name(hw), rate, parent_rate);
> > + return 0;
> > }
> >
> > - return round_rate;
> > + if (parent_rate <= max_mfd)
> > + mfd = parent_rate;
> > +
> > + temp64 = (u64)(rate - round_rate);
> > + temp64 *= mfd;
> > + do_div(temp64, parent_rate);
> > + mfn = temp64;
> > +
> > + /*
> > + * NOTE: The value of numerator must always be configured to be
> > + * less than the value of the denominator. If we can't get a proper
> > + * pair of mfn/mfd, we simply return the round_rate without using
> > + * the frac part.
> > + */
> > + if (mfn >= mfd)
> > + return round_rate;
> > +
> > + temp64 = (u64)parent_rate;
> > + temp64 *= mfn;
> > + do_div(temp64, mfd);
> > +
> > + return round_rate + (u32)temp64;
> > }
> >
> > static bool clk_pllv4_is_valid_mult(unsigned int mult) @@ -106,17
> > +144,31 @@ static int clk_pllv4_set_rate(struct clk_hw *hw, unsigned long
> rate, {
> > struct clk_pllv4 *pll = to_clk_pllv4(hw);
> > u32 val, mult;
> > + u32 mfn, mfd = 1000000;
> > + u32 max_mfd = 0x3FFFFFFF;
>
> Ditto
OK.
Thanks,
Anson.
>
> Otherwise:
> Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>
>
> Regards
> Dong Aisheng
>
> > + u64 temp64;
> >
> > mult = rate / parent_rate;
> >
> > if (!clk_pllv4_is_valid_mult(mult))
> > return -EINVAL;
> >
> > + if (parent_rate <= max_mfd)
> > + mfd = parent_rate;
> > +
> > + temp64 = (u64)(rate - mult * parent_rate);
> > + temp64 *= mfd;
> > + do_div(temp64, parent_rate);
> > + mfn = temp64;
> > +
> > val = readl_relaxed(pll->base + PLL_CFG_OFFSET);
> > val &= ~BM_PLL_MULT;
> > val |= mult << BP_PLL_MULT;
> > writel_relaxed(val, pll->base + PLL_CFG_OFFSET);
> >
> > + writel_relaxed(mfn, pll->base + PLL_NUM_OFFSET);
> > + writel_relaxed(mfd, pll->base + PLL_DENOM_OFFSET);
> > +
> > return 0;
> > }
> >
> > --
> > 2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-29 12:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-29 3:18 [PATCH] clk: imx: add fractional-N pll support to pllv4 Anson Huang
2019-04-29 11:28 ` Aisheng Dong
2019-04-29 11:35 ` Aisheng Dong
2019-04-29 12:33 ` Anson Huang
2019-04-29 12:34 ` Anson Huang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).