Hi Geert,
Thanks for the feedback.
> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: Friday, October 11, 2024 3:07 PM
> Subject: Re: [PATCH] clk: renesas: rzg2l: Fix FOUTPOSTDIV clk
>
> Hi Biju,
>
> On Wed, Oct 9, 2024 at 4:03 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>
>
> Thanks for your patch!
>
> > --- 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;
> >
> > 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_refdiv;
> > + foutvco_rate = mul_u64_u32_shr(foutvco_rate,
> > + (params->pl5_intin << 24) + params->pl5_fracin,
> > + 24);
>
> The first parameter is not u64, but unsigned long, and its value always fits in u32, so
> "mul_u32_u32(..., ...) >> 24" should do?
>
> However, if you care about precision, the division by params->pl5_refdiv should be done after all
> multiplication, too.
I do care about precision. I willlike below in next version.
+ foutvco_rate =
+ (EXTAL_FREQ_IN_MEGA_HZ * MEGA *
+ ((params->pl5_intin << 24) + params->pl5_fracin) /
+ params->pl5_refdiv) >> 24;
Cheers,
Biju
>
> > + foutpostdiv_rate = DIV_ROUND_CLOSEST_ULL(foutvco_rate,
> > + params->pl5_postdiv1
> > + * params->pl5_postdiv2);
> >
> > 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
