Hi, Aisheng > -----Original Message----- > From: Aisheng Dong > Sent: Monday, April 29, 2019 7:36 PM > To: Anson Huang <[email protected]>; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected]; [email protected]; > [email protected]; [email protected] > Cc: dl-linux-imx <[email protected]> > 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 <[email protected]> > > > --- > > > 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 <[email protected]> > > > > 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
