Hi, Aisheng > -----Original Message----- > From: Aisheng Dong > Sent: Monday, April 29, 2019 7:28 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: 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 <[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? 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 <[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
