| Message ID | 20240707082001.20746-2-michael@amarulasolutions.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi Michael, On Sun, Jul 7, 2024 at 5:20 AM Michael Trimarchi <michael@amarulasolutions.com> wrote: > -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", The kernel uses osc_24m. Why does U-Boot need to be different? To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
Hi On Wed, Jul 10, 2024 at 3:34 PM Fabio Estevam <festevam@gmail.com> wrote: > > Hi Michael, > > On Sun, Jul 7, 2024 at 5:20 AM Michael Trimarchi > <michael@amarulasolutions.com> wrote: > > > -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", > > The kernel uses osc_24m. Why does U-Boot need to be different? clk dump It's how uboot is registering the clk in dts. My idea now was not to change it to not impact other architectures. I have a lot of changes but I don't want to address regression and fix and test before send more patches MIchael
On Wed, Jul 10, 2024 at 10:39 AM Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote: > clk dump > > It's how uboot is registering the clk in dts. My idea now was not to > change it to not impact other architectures. > I have a lot of changes but I don't want to address regression and fix > and test before send more patches Ok, I see that 'clock-osc-24m' is already used in U-Boot by the other peripherals. Your change looks correct. Ideally, we should use 'osc_24m' in the future to align with the kernel, but that's a different task. To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
HI Fabio On Wed, Jul 10, 2024 at 3:43 PM Fabio Estevam <festevam@gmail.com> wrote: > > On Wed, Jul 10, 2024 at 10:39 AM Michael Nazzareno Trimarchi > <michael@amarulasolutions.com> wrote: > > > clk dump > > > > It's how uboot is registering the clk in dts. My idea now was not to > > change it to not impact other architectures. > > I have a lot of changes but I don't want to address regression and fix > > and test before send more patches > > Ok, I see that 'clock-osc-24m' is already used in U-Boot by the other > peripherals. > > Your change looks correct. > > Ideally, we should use 'osc_24m' in the future to align with the > kernel, but that's a different task. I will rephrase the commit message Michael
On Thu, Jul 11, 2024 at 2:56 AM Michael Nazzareno Trimarchi
<michael@amarulasolutions.com> wrote:
> I will rephrase the commit message
It looks good as is.
Applied all, thanks.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
On Sun, Jul 7, 2024 at 3:45 AM Michael Trimarchi <michael@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@denx.de> > Signed-off-by: Michael Trimarchi <michael@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? adam > base = dev_read_addr_ptr(dev); > if (!base) > return -EINVAL; > -- > 2.43.0 > To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
Hi Adam On Mon, Nov 4, 2024 at 5:11 PM Adam Ford <aford173@gmail.com> wrote: > > On Sun, Jul 7, 2024 at 3:45 AM Michael Trimarchi > <michael@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@denx.de> > > Signed-off-by: Michael Trimarchi <michael@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? If (ret) put here a print Michael > adam > > > base = dev_read_addr_ptr(dev); > > if (!base) > > return -EINVAL; > > -- > > 2.43.0 > >
On Mon, Nov 4, 2024 at 10:29 AM Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote: > > Hi Adam > > On Mon, Nov 4, 2024 at 5:11 PM Adam Ford <aford173@gmail.com> wrote: > > > > On Sun, Jul 7, 2024 at 3:45 AM Michael Trimarchi > > <michael@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@denx.de> > > > Signed-off-by: Michael Trimarchi <michael@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@amarulasolutions.com > __________________________________ > > Amarula Solutions BV > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > T. +31 (0)85 111 9172 > info@amarulasolutions.com > www.amarulasolutions.com To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
Hi Adam On Mon, Nov 4, 2024 at 6:01 PM Adam Ford <aford173@gmail.com> wrote: > > On Mon, Nov 4, 2024 at 10:29 AM Michael Nazzareno Trimarchi > <michael@amarulasolutions.com> wrote: > > > > Hi Adam > > > > On Mon, Nov 4, 2024 at 5:11 PM Adam Ford <aford173@gmail.com> wrote: > > > > > > On Sun, Jul 7, 2024 at 3:45 AM Michael Trimarchi > > > <michael@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@denx.de> > > > > Signed-off-by: Michael Trimarchi <michael@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)); > Seems that is not in the image spl so it can not find it. Are you sure that you can inside the SPL Michael > 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@amarulasolutions.com > > __________________________________ > > > > Amarula Solutions BV > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > > T. +31 (0)85 111 9172 > > info@amarulasolutions.com > > www.amarulasolutions.com
On Mon, Nov 4, 2024 at 11:04 AM Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote: > > Hi Adam > > On Mon, Nov 4, 2024 at 6:01 PM Adam Ford <aford173@gmail.com> wrote: > > > > On Mon, Nov 4, 2024 at 10:29 AM Michael Nazzareno Trimarchi > > <michael@amarulasolutions.com> wrote: > > > > > > Hi Adam > > > > > > On Mon, Nov 4, 2024 at 5:11 PM Adam Ford <aford173@gmail.com> wrote: > > > > > > > > On Sun, Jul 7, 2024 at 3:45 AM Michael Trimarchi > > > > <michael@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@denx.de> > > > > > Signed-off-by: Michael Trimarchi <michael@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)); > > > > Seems that is not in the image spl so it can not find it. Are you sure > that you can inside > the SPL I am not sure I understand what you are asking, but the clock node appears in spl/u-boot-spl.dtb: clock-osc-24m { compatible = "fixed-clock"; #clock-cells = <0x00>; clock-frequency = <0x16e3600>; clock-output-names = "osc_24m"; phandle = <0x18>; }; Since the clock-output-names is "osc_24m" I wonder if clk_get_by_name is the right function. Looking at other uses of this function, it looks like it's looking for a 'clock-names' parameter and not clock-output-names. adam > > Michael > > > 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@amarulasolutions.com > > > __________________________________ > > > > > > Amarula Solutions BV > > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > > > T. +31 (0)85 111 9172 > > > info@amarulasolutions.com > > > www.amarulasolutions.com > > > > -- > Michael Nazzareno Trimarchi > Co-Founder & Chief Executive Officer > M. +39 347 913 2170 > michael@amarulasolutions.com > __________________________________ > > Amarula Solutions BV > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > T. +31 (0)85 111 9172 > info@amarulasolutions.com > www.amarulasolutions.com To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
Hi On Mon, Nov 4, 2024 at 6:17 PM Adam Ford <aford173@gmail.com> wrote: > > On Mon, Nov 4, 2024 at 11:04 AM Michael Nazzareno Trimarchi > <michael@amarulasolutions.com> wrote: > > > > Hi Adam > > > > On Mon, Nov 4, 2024 at 6:01 PM Adam Ford <aford173@gmail.com> wrote: > > > > > > On Mon, Nov 4, 2024 at 10:29 AM Michael Nazzareno Trimarchi > > > <michael@amarulasolutions.com> wrote: > > > > > > > > Hi Adam > > > > > > > > On Mon, Nov 4, 2024 at 5:11 PM Adam Ford <aford173@gmail.com> wrote: > > > > > > > > > > On Sun, Jul 7, 2024 at 3:45 AM Michael Trimarchi > > > > > <michael@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@denx.de> > > > > > > Signed-off-by: Michael Trimarchi <michael@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)); > > > > > > > Seems that is not in the image spl so it can not find it. Are you sure > > that you can inside > > the SPL > > I am not sure I understand what you are asking, but the clock node > appears in spl/u-boot-spl.dtb: > > clock-osc-24m { > compatible = "fixed-clock"; > #clock-cells = <0x00>; > clock-frequency = <0x16e3600>; > clock-output-names = "osc_24m"; > phandle = <0x18>; > }; > > Since the clock-output-names is "osc_24m" I wonder if clk_get_by_name > is the right function. Looking at other uses of this function, it > looks like it's looking for a 'clock-names' parameter and not > clock-output-names. Yes, it's the right answer. That is strange because it works in uboot image. Is the node the same in uboot-dtb? Michael > > adam > > > > Michael > > > > > 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@amarulasolutions.com > > > > __________________________________ > > > > > > > > Amarula Solutions BV > > > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > > > > T. +31 (0)85 111 9172 > > > > info@amarulasolutions.com > > > > www.amarulasolutions.com > > > > > > > > -- > > Michael Nazzareno Trimarchi > > Co-Founder & Chief Executive Officer > > M. +39 347 913 2170 > > michael@amarulasolutions.com > > __________________________________ > > > > Amarula Solutions BV > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > > T. +31 (0)85 111 9172 > > info@amarulasolutions.com > > www.amarulasolutions.com
On Mon, Nov 4, 2024 at 11:22 AM Michael Nazzareno Trimarchi <michael@amarulasolutions.com> wrote: > > Hi > > On Mon, Nov 4, 2024 at 6:17 PM Adam Ford <aford173@gmail.com> wrote: > > > > On Mon, Nov 4, 2024 at 11:04 AM Michael Nazzareno Trimarchi > > <michael@amarulasolutions.com> wrote: > > > > > > Hi Adam > > > > > > On Mon, Nov 4, 2024 at 6:01 PM Adam Ford <aford173@gmail.com> wrote: > > > > > > > > On Mon, Nov 4, 2024 at 10:29 AM Michael Nazzareno Trimarchi > > > > <michael@amarulasolutions.com> wrote: > > > > > > > > > > Hi Adam > > > > > > > > > > On Mon, Nov 4, 2024 at 5:11 PM Adam Ford <aford173@gmail.com> wrote: > > > > > > > > > > > > On Sun, Jul 7, 2024 at 3:45 AM Michael Trimarchi > > > > > > <michael@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@denx.de> > > > > > > > Signed-off-by: Michael Trimarchi <michael@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)); > > > > > > > > > > Seems that is not in the image spl so it can not find it. Are you sure > > > that you can inside > > > the SPL > > > > I am not sure I understand what you are asking, but the clock node > > appears in spl/u-boot-spl.dtb: > > > > clock-osc-24m { > > compatible = "fixed-clock"; > > #clock-cells = <0x00>; > > clock-frequency = <0x16e3600>; > > clock-output-names = "osc_24m"; > > phandle = <0x18>; > > }; > > > > Since the clock-output-names is "osc_24m" I wonder if clk_get_by_name > > is the right function. Looking at other uses of this function, it > > looks like it's looking for a 'clock-names' parameter and not > > clock-output-names. > > Yes, it's the right answer. That is strange because it works in uboot > image. Is the node the same in uboot-dtb? The U-Boot phase device tree looks like this: clock-osc-24m { compatible = "fixed-clock"; #clock-cells = <0x00>; clock-frequency = <0x16e3600>; clock-output-names = "osc_24m"; bootph-pre-ram; bootph-all; phandle = <0x18>; }; > > Michael > > > > > adam > > > > > > Michael > > > > > > > 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@amarulasolutions.com > > > > > __________________________________ > > > > > > > > > > Amarula Solutions BV > > > > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > > > > > T. +31 (0)85 111 9172 > > > > > info@amarulasolutions.com > > > > > www.amarulasolutions.com > > > > > > > > > > > > -- > > > Michael Nazzareno Trimarchi > > > Co-Founder & Chief Executive Officer > > > M. +39 347 913 2170 > > > michael@amarulasolutions.com > > > __________________________________ > > > > > > Amarula Solutions BV > > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > > > T. +31 (0)85 111 9172 > > > info@amarulasolutions.com > > > www.amarulasolutions.com > > > > -- > Michael Nazzareno Trimarchi > Co-Founder & Chief Executive Officer > M. +39 347 913 2170 > michael@amarulasolutions.com > __________________________________ > > Amarula Solutions BV > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > T. +31 (0)85 111 9172 > info@amarulasolutions.com > www.amarulasolutions.com To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
On Mon, Nov 4, 2024 at 11:24 AM Adam Ford <aford173@gmail.com> wrote: > > On Mon, Nov 4, 2024 at 11:22 AM Michael Nazzareno Trimarchi > <michael@amarulasolutions.com> wrote: > > > > Hi > > > > On Mon, Nov 4, 2024 at 6:17 PM Adam Ford <aford173@gmail.com> wrote: > > > > > > On Mon, Nov 4, 2024 at 11:04 AM Michael Nazzareno Trimarchi > > > <michael@amarulasolutions.com> wrote: > > > > > > > > Hi Adam > > > > > > > > On Mon, Nov 4, 2024 at 6:01 PM Adam Ford <aford173@gmail.com> wrote: > > > > > > > > > > On Mon, Nov 4, 2024 at 10:29 AM Michael Nazzareno Trimarchi > > > > > <michael@amarulasolutions.com> wrote: > > > > > > > > > > > > Hi Adam > > > > > > > > > > > > On Mon, Nov 4, 2024 at 5:11 PM Adam Ford <aford173@gmail.com> wrote: > > > > > > > > > > > > > > On Sun, Jul 7, 2024 at 3:45 AM Michael Trimarchi > > > > > > > <michael@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@denx.de> > > > > > > > > Signed-off-by: Michael Trimarchi <michael@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)); > > > > > > > > > > > > > Seems that is not in the image spl so it can not find it. Are you sure > > > > that you can inside > > > > the SPL > > > > > > I am not sure I understand what you are asking, but the clock node > > > appears in spl/u-boot-spl.dtb: > > > > > > clock-osc-24m { > > > compatible = "fixed-clock"; > > > #clock-cells = <0x00>; > > > clock-frequency = <0x16e3600>; > > > clock-output-names = "osc_24m"; > > > phandle = <0x18>; > > > }; > > > > > > Since the clock-output-names is "osc_24m" I wonder if clk_get_by_name > > > is the right function. Looking at other uses of this function, it > > > looks like it's looking for a 'clock-names' parameter and not > > > clock-output-names. > > > > Yes, it's the right answer. That is strange because it works in uboot > > image. Is the node the same in uboot-dtb? > > The U-Boot phase device tree looks like this: > > clock-osc-24m { > compatible = "fixed-clock"; > #clock-cells = <0x00>; > clock-frequency = <0x16e3600>; > clock-output-names = "osc_24m"; > bootph-pre-ram; > bootph-all; > phandle = <0x18>; > }; > I did some more investigating, and with the device tree debugging turned on, I get the following: U-Boot SPL 2025.01-rc1-00172-g14c2ad85629c-dirty (Nov 09 2024 - 17:10:26 -0600) clk_set_defaults(i2c1grp) clk_set_default_parents: could not read assigned-clock-parents for 970690 clk_set_defaults(i2c@30a20000) clk_set_default_parents: could not read assigned-clock-parents for 970f30 clk_set_defaults(clock-controller@30380000) clk_set_default_parents: could not read assigned-clock-parents for 970ab8 clk_set_defaults(clock-osc-24m) clk_set_default_parents: could not read assigned-clock-parents for 9700f8 clk_set_defaults(clock-osc-24m) clk_set_default_parents: could not read assigned-clock-parents for 9700f8 clk_get_by_name_nodev(node=92a30c, name=osc_24m, clk=96fca8) clk_get_by_index_tail: Node 'clock-controller@30380000', property 'clocks', failed to request CLK index 1: -22 clk_get_by_index_tail: uclass_get_device_by_of_offset failed: err=-22 Failed to get i2c clk clk_set_defaults(wdoggrp) clk_set_default_parents: could not read assigned-clock-parents for 970858 clk_set_defaults(watchdog@30280000) clk_set_default_parents: could not read assigned-clock-parents for 970500 WDT: Not starting watchdog@30280000 clk_set_defaults(clock-controller@30380000) clk_set_default_parents: could not read assigned-clock-parents for 970ab8 clk_get_by_name_nodev(node=92a30c, name=osc_24m, clk=96fd38) clk_get_by_index_tail: Node 'clock-controller@30380000', property 'clocks', failed to request CLK index 1: -22 Failed to find clock node. Check device tree Trying to boot from BOOTROM Boot Stage: Recovery boot ------- If I modify the code so that it doesn't return early and just skips the clock registration phase like: 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)); ------ The above work-around appears to work, and the i2c appears to enable the clock-osc-24m clock.: U-Boot SPL 2025.01-rc1-00172-g14c2ad85629c-dirty (Nov 09 2024 - 17:16:06 -0600) clk_set_defaults(i2c1grp) clk_set_default_parents: could not read assigned-clock-parents for 970690 clk_set_defaults(i2c@30a20000) clk_set_default_parents: could not read assigned-clock-parents for 970f30 clk_set_defaults(clock-controller@30380000) clk_set_default_parents: could not read assigned-clock-parents for 970ab8 clk_set_defaults(clock-osc-24m) clk_set_default_parents: could not read assigned-clock-parents for 9700f8 clk_set_defaults(clock-osc-24m) clk_set_default_parents: could not read assigned-clock-parents for 9700f8 clk_get_by_name_nodev(node=92a2fc, name=osc_24m, clk=96fca8) clk_get_by_index_tail: Node 'clock-controller@30380000', property 'clocks', failed to request CLK index 1: -22 clk_set_defaults(clock-controller@30380000) clk_set_default_parents: could not read assigned-clock-parents for 970ab8 clk_of_xlate_default(clk=9712a8) clk_request(dev=970ab8, clk=9712a8) clk_enable(clk=9712a8 name=clock-controller@30380000) clk_enable(clk=975180 name=i2c1) clk_enable(clk=970190 name=clock-osc-24m) I am not sure what the difference is between osc_24m and clock-osc-24m. clk_get_rate(clk=9712a8) clk_get_rate(clk=976980) clk_get_parent_rate(clk=976980) clk_get_parent(clk=976980) clk_get_rate(clk=975180) clk_get_parent_rate(clk=975180) clk_get_parent(clk=975180) clk_get_rate(clk=970190) clk_set_defaults(pmicirqgrp) clk_set_default_parents: could not read assigned-clock-parents for 970728 clk_set_defaults(pmic@4b) clk_set_default_parents: could not read assigned-clock-parents for 970ff8 clk_set_defaults(wdoggrp) clk_set_default_parents: could not read assigned-clock-parents for 970858 clk_set_defaults(watchdog@30280000) clk_set_default_parents: could not read assigned-clock-parents for 970500 WDT: Not starting watchdog@30280000 Trying to boot from BOOTROM I am not really sure why, and / I was hoping someone might have an idea. If not, I might send a patch for this because without it, the imx8mn_beacon board doesn't run at the proper voltage for the LPDDR4 controller. adam > > > > Michael > > > > > > > > adam > > > > > > > > Michael > > > > > > > > > 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@amarulasolutions.com > > > > > > __________________________________ > > > > > > > > > > > > Amarula Solutions BV > > > > > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > > > > > > T. +31 (0)85 111 9172 > > > > > > info@amarulasolutions.com > > > > > > www.amarulasolutions.com > > > > > > > > > > > > > > > > -- > > > > Michael Nazzareno Trimarchi > > > > Co-Founder & Chief Executive Officer > > > > M. +39 347 913 2170 > > > > michael@amarulasolutions.com > > > > __________________________________ > > > > > > > > Amarula Solutions BV > > > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > > > > T. +31 (0)85 111 9172 > > > > info@amarulasolutions.com > > > > www.amarulasolutions.com > > > > > > > > -- > > Michael Nazzareno Trimarchi > > Co-Founder & Chief Executive Officer > > M. +39 347 913 2170 > > michael@amarulasolutions.com > > __________________________________ > > > > Amarula Solutions BV > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL > > T. +31 (0)85 111 9172 > > info@amarulasolutions.com > > www.amarulasolutions.com To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
Hi Adam, On Sat, Nov 9, 2024 at 8:23 PM Adam Ford <aford173@gmail.com> wrote: > I did some more investigating, and with the device tree debugging > turned on, I get the following: ... > clk_set_defaults(clock-controller@30380000) > clk_set_default_parents: could not read assigned-clock-parents for 970ab8 > clk_get_by_name_nodev(node=92a30c, name=osc_24m, clk=96fd38) > clk_get_by_index_tail: Node 'clock-controller@30380000', property > 'clocks', failed to request CLK index 1: -22 > Failed to find clock node. Check device tree > Trying to boot from BOOTROM > Boot Stage: Recovery boot Could you please test Marek's patch? https://patchwork.ozlabs.org/project/uboot/patch/20241122015858.557561-1-marex@denx.de/ I plan to apply it for 2025.01 to fix this regression. To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
On Fri, Nov 22, 2024 at 6:38 AM Fabio Estevam <festevam@gmail.com> wrote: > > Hi Adam, > > On Sat, Nov 9, 2024 at 8:23 PM Adam Ford <aford173@gmail.com> wrote: > > > I did some more investigating, and with the device tree debugging > > turned on, I get the following: > ... > > clk_set_defaults(clock-controller@30380000) > > clk_set_default_parents: could not read assigned-clock-parents for 970ab8 > > clk_get_by_name_nodev(node=92a30c, name=osc_24m, clk=96fd38) > > clk_get_by_index_tail: Node 'clock-controller@30380000', property > > 'clocks', failed to request CLK index 1: -22 > > Failed to find clock node. Check device tree > > Trying to boot from BOOTROM > > Boot Stage: Recovery boot > > Could you please test Marek's patch? > > https://patchwork.ozlabs.org/project/uboot/patch/20241122015858.557561-1-marex@denx.de/ Sure. Do you want me to test it against a specific branch or the master mainline branch? adam > > I plan to apply it for 2025.01 to fix this regression. To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
On Fri, Nov 22, 2024 at 10:11 AM Adam Ford <aford173@gmail.com> wrote: > Sure. Do you want me to test it against a specific branch or the > master mainline branch? Master branch, please. To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
On Fri, Nov 22, 2024 at 7:17 AM Fabio Estevam <festevam@gmail.com> wrote: > > On Fri, Nov 22, 2024 at 10:11 AM Adam Ford <aford173@gmail.com> wrote: > > > Sure. Do you want me to test it against a specific branch or the > > master mainline branch? > > Master branch, please. I should have a response for you by Monday. adam To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.
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)); + 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)); + base = dev_read_addr_ptr(dev); if (!base) return -EINVAL;
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@denx.de> Signed-off-by: Michael Trimarchi <michael@amarulasolutions.com> --- drivers/clk/imx/clk-imx8mn.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)