| Message ID | 20241016101513.39984-1-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [v3] clk: renesas: rzg2l: Fix FOUTPOSTDIV clk | expand |
Hi Biju, On Wed, Oct 16, 2024 at 12:15 PM Biju Das <biju.das.jz@bp.renesas.com> 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@renesas.com> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > --- > v2->v3: > * Used mul_u32_u32() for 32-bit multiplication. Thanks for the update! > --- a/drivers/clk/renesas/rzg2l-cpg.c > +++ b/drivers/clk/renesas/rzg2l-cpg.c > @@ -557,10 +557,10 @@ 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 = mul_u32_u32(EXTAL_FREQ_IN_MEGA_HZ * MEGA, (params->pl5_intin << 24) + > + params->pl5_fracin) / params->pl5_refdiv >> 24; The division is a 64-by-32 division, so it should use the div_u64() helper. > + foutpostdiv_rate = DIV_ROUND_CLOSEST_ULL(foutvco_rate, > + params->pl5_postdiv1 * params->pl5_postdiv2); > > return foutpostdiv_rate; > } Gr{oetje,eeting}s, Geert
Hi Geert, Thanks for the feedback. > -----Original Message----- > From: Geert Uytterhoeven <geert@linux-m68k.org> > Sent: Thursday, October 24, 2024 11:12 AM > To: Biju Das <biju.das.jz@bp.renesas.com> > Cc: Michael Turquette <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Geert Uytterhoeven > <geert+renesas@glider.be>; linux-renesas-soc@vger.kernel.org; linux-clk@vger.kernel.org; Prabhakar > Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; biju.das.au <biju.das.au@gmail.com>; Hien Huynh > <hien.huynh.px@renesas.com> > Subject: Re: [PATCH v3] clk: renesas: rzg2l: Fix FOUTPOSTDIV clk > > Hi Biju, > > On Wed, Oct 16, 2024 at 12:15 PM Biju Das <biju.das.jz@bp.renesas.com> 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@renesas.com> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > --- > > v2->v3: > > * Used mul_u32_u32() for 32-bit multiplication. > > Thanks for the update! > > > --- a/drivers/clk/renesas/rzg2l-cpg.c > > +++ b/drivers/clk/renesas/rzg2l-cpg.c > > @@ -557,10 +557,10 @@ 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 = mul_u32_u32(EXTAL_FREQ_IN_MEGA_HZ * MEGA, (params->pl5_intin << 24) + > > + params->pl5_fracin) / > > + params->pl5_refdiv >> 24; > > The division is a 64-by-32 division, so it should use the div_u64() helper. You mean, foutvco_rate = div_u64(mul_u32_u32(EXTAL_FREQ_IN_MEGA_HZ * MEGA, (params->pl5_intin << 24) + params->pl5_fracin) , params->pl5_refdiv) >> 24; ?? Cheers, Biju
Hi Biju, On Thu, Oct 24, 2024 at 1:51 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > From: Geert Uytterhoeven <geert@linux-m68k.org> > > On Wed, Oct 16, 2024 at 12:15 PM Biju Das <biju.das.jz@bp.renesas.com> 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@renesas.com> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > > --- > > > v2->v3: > > > * Used mul_u32_u32() for 32-bit multiplication. > > > > Thanks for the update! > > > > > --- a/drivers/clk/renesas/rzg2l-cpg.c > > > +++ b/drivers/clk/renesas/rzg2l-cpg.c > > > @@ -557,10 +557,10 @@ 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 = mul_u32_u32(EXTAL_FREQ_IN_MEGA_HZ * MEGA, (params->pl5_intin << 24) + > > > + params->pl5_fracin) / > > > + params->pl5_refdiv >> 24; > > > > > The division is a 64-by-32 division, so it should use the div_u64() helper. > > You mean, > > foutvco_rate = div_u64(mul_u32_u32(EXTAL_FREQ_IN_MEGA_HZ * MEGA, (params->pl5_intin << 24) + > params->pl5_fracin) , params->pl5_refdiv) >> 24; ?? Exactly. Gr{oetje,eeting}s, Geert
diff --git a/drivers/clk/renesas/rzg2l-cpg.c b/drivers/clk/renesas/rzg2l-cpg.c index 88bf39e8c79c..4449afb57eda 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; params->pl5_intin = rate / MEGA; params->pl5_fracin = div_u64(((u64)rate % MEGA) << 24, MEGA); @@ -557,10 +557,10 @@ 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 = mul_u32_u32(EXTAL_FREQ_IN_MEGA_HZ * MEGA, (params->pl5_intin << 24) + + params->pl5_fracin) / params->pl5_refdiv >> 24; + foutpostdiv_rate = DIV_ROUND_CLOSEST_ULL(foutvco_rate, + params->pl5_postdiv1 * params->pl5_postdiv2); return foutpostdiv_rate; }
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@renesas.com> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> --- v2->v3: * Used mul_u32_u32() for 32-bit multiplication. v1->v2: * Improved the precision by division of params->pl5_refdiv done after all multiplication. --- drivers/clk/renesas/rzg2l-cpg.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)