Hello Fabio, > -----Original Message----- > From: Fabio Estevam <[email protected]> > Sent: Sunday, May 2, 2021 11:45 PM > To: ZHIZHIKIN Andrey <[email protected]> > Cc: U-Boot-Denx <[email protected]>; Stefano Babic <[email protected]>; > NXP i.MX U-Boot Team <[email protected]>; Peng Fan > <[email protected]>; Simon Glass <[email protected]>; Ye Li <[email protected]> > 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@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 > > <[email protected]> > > Cc: Stefano Babic <[email protected]> > > Cc: Fabio Estevam <[email protected]> > > Cc: "NXP i.MX U-Boot Team" <[email protected]> > > Cc: Peng Fan <[email protected]> > > Cc: Simon Glass <[email protected]> > > Cc: Ye Li <[email protected]> > > 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
