[PATCH] arm: imx: imx8mm: correct unrecognized fracpll frequency

[ZHIZHIKIN Andrey]

Hello Fabio,

> -----Original Message-----
> From: Fabio Estevam <festevam at gmail.com>
> Sent: Sunday, May 2, 2021 11:45 PM
> To: ZHIZHIKIN Andrey <andrey.zhizhikin at leica-geosystems.com>
> Cc: U-Boot-Denx <u-boot at lists.denx.de>; Stefano Babic <sbabic at denx.de>;
> NXP i.MX U-Boot Team <uboot-imx at nxp.com>; Peng Fan
> <peng.fan at nxp.com>; Simon Glass <sjg at chromium.org>; Ye Li <ye.li at nxp.com>
> Subject: Re: [PATCH] arm: imx: imx8mm: correct unrecognized fracpll frequency
> 
> 
> Hi Andrey,
> 
> After re-reading the patch I have some comments:
> 
> On Sat, May 1, 2021 at 5:13 PM Andrey Zhizhikin <andrey.zhizhikin at leica-
> geosystems.com> wrote:
> >
> > Frequency requested by ddrphy_init_set_dfi_clk from fracpll uses MHZ()
> > macro, which expands the value provided to the Hz range without taking
> > into account the precise Hz setting. This causes the frequency of 266
> > MHz not ot be found in the imx8mm_fracpll_tbl, since it is entered
> > there with a precise Hz value. This in turn causes the boot hang in
> > SPL, as proper DDR fracpll frequency cannot be determined.
> >
> > Correct the value in imx8mm_fracpll_tbl to match the one expanded by
> > MHZ(266) macro, rounding it down to MHz range only.
> >
> > Signed-off-by: Andrey Zhizhikin
> > <andrey.zhizhikin at leica-geosystems.com>
> > Cc: Stefano Babic <sbabic at denx.de>
> > Cc: Fabio Estevam <festevam at gmail.com>
> > Cc: "NXP i.MX U-Boot Team" <uboot-imx at nxp.com>
> > Cc: Peng Fan <peng.fan at nxp.com>
> > Cc: Simon Glass <sjg at chromium.org>
> > Cc: Ye Li <ye.li at nxp.com>
> > Fixes: 825ab6b406 ("driver: ddr: Refine the ddr init driver on imx8m")
> > ---
> >  arch/arm/mach-imx/imx8m/clock_imx8mm.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/mach-imx/imx8m/clock_imx8mm.c
> > b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
> > index 029d06f27f..86ff2b9cc9 100644
> > --- a/arch/arm/mach-imx/imx8m/clock_imx8mm.c
> > +++ b/arch/arm/mach-imx/imx8m/clock_imx8mm.c
> > @@ -54,7 +54,7 @@ static struct imx_int_pll_rate_table imx8mm_fracpll_tbl[]
> = {
> >         PLL_1443X_RATE(600000000U, 300, 3, 2, 0),
> >         PLL_1443X_RATE(594000000U, 99, 1, 2, 0),
> >         PLL_1443X_RATE(400000000U, 300, 9, 1, 0),
> > -       PLL_1443X_RATE(266666667U, 400, 9, 2, 0),
> > +       PLL_1443X_RATE(266000000U, 400, 9, 2, 0),
> 
> This change looks good.
> 
> >         PLL_1443X_RATE(167000000U, 334, 3, 4, 0),
> >         PLL_1443X_RATE(100000000U, 300, 9, 3, 0),  }; @@ -72,7 +72,7
> > @@ static int fracpll_configure(enum pll_clocks pll, u32 freq)
> >         }
> >
> >         if (i == ARRAY_SIZE(imx8mm_fracpll_tbl)) {
> > -               printf("No matched freq table %u\n", freq);
> > +               printf("%s: No matched freq table %u\n", __func__,
> > + freq);
> >                 return -EINVAL;
> >         }
> >
> > @@ -148,7 +148,7 @@ void dram_enable_bypass(ulong clk_val)
> >         }
> >
> >         if (i == ARRAY_SIZE(imx8mm_dram_bypass_tbl)) {
> > -               printf("No matched freq table %lu\n", clk_val);
> > +               printf("%s: No matched freq table %lu\n", __func__,
> > + clk_val);
> 
> , but these two I would put them on a separate patch.

Fair enough, thanks for pointing this out!

Even though this helped me originally to pinpoint the issue, looking at this change now I see it does indeed belong to the separate patch as it does not relate to the PLL fix per se.

I would split this into a separate patch, send it separate, and send a V2 for the PLL fix.

> 
> Thanks

Cheers,
Andrey