Re: [PATCH v2] clk: renesas: rzg2l: Fix FOUTPOSTDIV clk

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux