[PATCH 1/4] clk: imx: clk-imx8mn Fix nand and spi clock parent
Adam Ford
aford173 at gmail.com
Mon Nov 4 18:01:19 CET 2024
On Mon, Nov 4, 2024 at 10:29 AM Michael Nazzareno Trimarchi
<michael at amarulasolutions.com> wrote:
>
> Hi Adam
>
> On Mon, Nov 4, 2024 at 5:11 PM Adam Ford <aford173 at gmail.com> wrote:
> >
> > On Sun, Jul 7, 2024 at 3:45 AM Michael Trimarchi
> > <michael at amarulasolutions.com> wrote:
> > >
> > > The osc_24m is the clock-output-name and not the one that
> > > is used as internal name reference from the strcmp. The clock
> > > that use osc_24m, will not be able to reparent it as they should.
> > > We need anyway register the osc_24m clock fixed factor in the clock
> > > tree.
> > >
> > > Fixes: 710c4ffb890 ("clk: imx: clk-imx8mn add gpmi nand clocks")
> > > Fixes: 85b1c11989c ("clk: imx: Add ECSPI to iMX8MN")
> > > Cc: Marek Vasut <marex at denx.de>
> > > Signed-off-by: Michael Trimarchi <michael at amarulasolutions.com>
> > > ---
> > > drivers/clk/imx/clk-imx8mn.c | 15 +++++++++++----
> > > 1 file changed, 11 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c
> > > index ed9e16d7c1..bfd1677520 100644
> > > --- a/drivers/clk/imx/clk-imx8mn.c
> > > +++ b/drivers/clk/imx/clk-imx8mn.c
> > > @@ -57,15 +57,15 @@ static const char *imx8mn_usdhc2_sels[] = {"clock-osc-24m", "sys_pll1_400m", "sy
> > > "sys_pll3_out", "sys_pll1_266m", "audio_pll2_out", "sys_pll1_100m", };
> > >
> > > #if CONFIG_IS_ENABLED(DM_SPI)
> > > -static const char *imx8mn_ecspi1_sels[] = {"osc_24m", "sys_pll2_200m", "sys_pll1_40m",
> > > +static const char *imx8mn_ecspi1_sels[] = {"clock-osc-24m", "sys_pll2_200m", "sys_pll1_40m",
> > > "sys_pll1_160m", "sys_pll1_800m", "sys_pll3_out",
> > > "sys_pll2_250m", "audio_pll2_out", };
> > >
> > > -static const char *imx8mn_ecspi2_sels[] = {"osc_24m", "sys_pll2_200m", "sys_pll1_40m",
> > > +static const char *imx8mn_ecspi2_sels[] = {"clock-osc-24m", "sys_pll2_200m", "sys_pll1_40m",
> > > "sys_pll1_160m", "sys_pll1_800m", "sys_pll3_out",
> > > "sys_pll2_250m", "audio_pll2_out", };
> > >
> > > -static const char *imx8mn_ecspi3_sels[] = {"osc_24m", "sys_pll2_200m", "sys_pll1_40m",
> > > +static const char *imx8mn_ecspi3_sels[] = {"clock-osc-24m", "sys_pll2_200m", "sys_pll1_40m",
> > > "sys_pll1_160m", "sys_pll1_800m", "sys_pll3_out",
> > > "sys_pll2_250m", "audio_pll2_out", };
> > > #endif
> > > @@ -105,7 +105,7 @@ static const char *imx8mn_usdhc3_sels[] = {"clock-osc-24m", "sys_pll1_400m", "sy
> > > static const char *imx8mn_qspi_sels[] = {"clock-osc-24m", "sys_pll1_400m", "sys_pll2_333m", "sys_pll2_500m",
> > > "audio_pll2_out", "sys_pll1_266m", "sys_pll3_out", "sys_pll1_100m", };
> > >
> > > -static const char * const imx8mn_nand_sels[] = {"osc_24m", "sys_pll2_500m", "audio_pll1_out",
> > > +static const char * const imx8mn_nand_sels[] = {"clock-osc-24m", "sys_pll2_500m", "audio_pll1_out",
> > > "sys_pll1_400m", "audio_pll2_out", "sys_pll3_out",
> > > "sys_pll2_250m", "video_pll_out", };
> > >
> > > @@ -119,7 +119,9 @@ static const char * const imx8mn_usb_phy_sels[] = {"clock-osc-24m", "sys_pll1_10
> > >
> > > static int imx8mn_clk_probe(struct udevice *dev)
> > > {
> > > + struct clk osc_24m_clk;
> > > void __iomem *base;
> > > + int ret;
> > >
> > > base = (void *)ANATOP_BASE_ADDR;
> > >
> > > @@ -238,6 +240,11 @@ static int imx8mn_clk_probe(struct udevice *dev)
> > > clk_dm(IMX8MN_SYS_PLL2_1000M,
> > > imx_clk_fixed_factor("sys_pll2_1000m", "sys_pll2_out", 1, 1));
> > >
> >
> > I know it's late, but I didn't get around to testing my Nano board for
> > a while. Sorry for the delayed feedback...
> >
> > > + ret = clk_get_by_name(dev, "osc_24m", &osc_24m_clk);
> > > + if (ret)
> > > + return ret;
> > > + clk_dm(IMX8MN_CLK_24M, dev_get_clk_ptr(osc_24m_clk.dev));
> > > +
> >
> > These four lines appear to have introduced a regression on the
> > imx8mn-beacon board. In the SPL phase, I get an error message
> > indicating it cannot find the i2c clk, then access to the PMIC fails.
> > If I remove these four lines, the error message disappears, and the
> > PMIC is happy again.
> >
> > I have confirmed that CLK, and SPL_CLK are both defined. I checked
> > the u-boot-spl.dtb and it shows clock-osc-24m with clock-output-names
> > = "osc_24m"
> >
> > I have also tried to mark the 24m clock as with bootph-pre-ram;:
> > &{/clock-osc-24m} {
> > bootph-pre-ram;
> > };
> >
> > Unfortunately, nothing is helping, and I am not sure what else to try.
> > clk_fixed_rate.o is built into my SPL, so I don't think it's a config
> > issue. I need to the PMIC to increase the voltage since we run the
> > Nano in overdrive mode to run the LPDDR4 at the highest speed.
> >
> > Might anyone have any suggestions?
> >
>
> Not yet. I have a similar setup but I don't set pmic, so I think that
> the symbol is not find
> in SPL and it just fail clock registration. Is that possible?
It does appear that the clock registration failed. Since the system
does an early return, the I2C clocks among others are also not
registered which appears to cause the failure.
I changed the registration around a little:
ret = clk_get_by_name(dev, "osc_24m", &osc_24m_clk);
if (!ret)
clk_dm(IMX8MN_CLK_24M, dev_get_clk_ptr(osc_24m_clk.dev));
This method won't return prematurely, so the I2C and other clocks can
be registered, and it appears to address the problem.
Another alternative might be to move this clock registration until
after all the rest of the clocks are done.
adam
>
> If (ret)
>
> put here a print
>
> Michael
> > adam
> >
> > > base = dev_read_addr_ptr(dev);
> > > if (!base)
> > > return -EINVAL;
> > > --
> > > 2.43.0
> > >
>
>
>
> --
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> michael at amarulasolutions.com
> __________________________________
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info at amarulasolutions.com
> www.amarulasolutions.com
More information about the U-Boot
mailing list