Hi Biju, On Fri, Oct 11, 2024 at 6:20 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote: > While computing foutpostdiv_rate, the value of params->pl5_fracin > is discarded, which results in the wrong refresh rate. Fix the formula > for computing foutpostdiv_rate. > > Fixes: 1561380ee72f ("clk: renesas: rzg2l: Add FOUTPOSTDIV clk support") > Signed-off-by: Hien Huynh <hien.huynh.px@xxxxxxxxxxx> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > --- > v1->v2: > * Improved the precision by division of params->pl5_refdiv > done after all multiplication. > --- > drivers/clk/renesas/rzg2l-cpg.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c > index 88bf39e8c79c..a1e22d353689 100644 > --- a/drivers/clk/renesas/rzg2l-cpg.c > +++ b/drivers/clk/renesas/rzg2l-cpg.c > @@ -548,7 +548,7 @@ static unsigned long > rzg2l_cpg_get_foutpostdiv_rate(struct rzg2l_pll5_param *params, > unsigned long rate) > { > - unsigned long foutpostdiv_rate; > + unsigned long foutpostdiv_rate, foutvco_rate; While the resulting 64-bit value fits in foutvco_rate because unsigned long is 64-bit on the target platform, I'd rather play it safe (reuse!) and use u64 explicitly. > > params->pl5_intin = rate / MEGA; > params->pl5_fracin = div_u64(((u64)rate % MEGA) << 24, MEGA); > @@ -557,10 +557,12 @@ rzg2l_cpg_get_foutpostdiv_rate(struct rzg2l_pll5_param *params, > params->pl5_postdiv2 = 1; > params->pl5_spread = 0x16; > > - foutpostdiv_rate = > - EXTAL_FREQ_IN_MEGA_HZ * MEGA / params->pl5_refdiv * > - ((((params->pl5_intin << 24) + params->pl5_fracin)) >> 24) / > - (params->pl5_postdiv1 * params->pl5_postdiv2); > + foutvco_rate = > + (EXTAL_FREQ_IN_MEGA_HZ * MEGA * > + ((params->pl5_intin << 24) + params->pl5_fracin) / > + params->pl5_refdiv) >> 24; Shouldn't this use mul_u32_u32(EXTAL_FREQ_IN_MEGA_HZ * MEGA, ((params->pl5_intin << 24) + params->pl5_fracin)) instead of a plain multiplication? See also the comment for mul_u32_u32() in <linux/math64.h>. > + foutpostdiv_rate = DIV_ROUND_CLOSEST_ULL(foutvco_rate, > + params->pl5_postdiv1 * params->pl5_postdiv2); Unfortunately we don't have a helper macro yet to round the result of div_u64(), so you will have to open-code that (for now). > > return foutpostdiv_rate; > } Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds