On Wed, Apr 16, 2025 at 5:34 PM Adam Ford <aford...@gmail.com> wrote:
>
> On Wed, Apr 16, 2025 at 9:19 AM Christoph Niedermaier
> <cniederma...@dh-electronics.com> wrote:
> >
> > From: Francesco Dolcini <france...@dolcini.it>
> > Sent: Wednesday, April 16, 2025 11:27 AM
> > > On Tue, Apr 15, 2025 at 02:13:30PM -0300, Fabio Estevam wrote:
> > >> On Tue, Apr 15, 2025 at 1:55 PM Marek Vasut <ma...@denx.de> wrote:
> > >>> Do you have af9cdd1ccd2d ("Revert "arm64: dts: imx8mn: Include 32kHz
> > >>> oscillator clock in SPL DTs"") in place ? If so, try and revert it, does
> > >>> it help ?
> > >>
> > >> It seems I have not pushed the "Revert "arm64: dts: imx8mn: Include 32kHz
> > >> oscillator clock in SPL DTs" fix. Sorry about that.
> > >>
> > >> As Francesco is using imx8mp, then I guess we also need to do the same
> > >> for imx8mp:
> > >>
> > >> diff --git a/arch/arm/dts/imx8mp-u-boot.dtsi
> > >> b/arch/arm/dts/imx8mp-u-boot.dtsi
> > >> index 6de9ab5d37cf..35af412f01b0 100644
> > >> --- a/arch/arm/dts/imx8mp-u-boot.dtsi
> > >> +++ b/arch/arm/dts/imx8mp-u-boot.dtsi
> > >> @@ -37,11 +37,6 @@
> > >> /delete-property/ assigned-clock-rates;
> > >> };
> > >>
> > >> -&osc_32k {
> > >> - bootph-pre-ram;
> > >> - bootph-all;
> > >> -};
> > >> -
> > >> &osc_24m {
> > >> bootph-pre-ram;
> > >> bootph-all;
> > >>
> > >> Does this fix the boot problem?
> > >
> > > My issue is not the boot. The issue is that USB is not working.
> > >
> > > With this patch applied nothing changes, the issue is not solved.
> > >
> > > Francesco
> >
> > Hello,
> >
> > I don't know if it is related or not, I noticed that Adam Ford's patches
> > commit 6d33ca36e3b ("clk: imx8mm: register UART clocks") and
> > commit 8999b76f238 ("clk: imx8mn: register UART clocks")
> > use the former name "clock-osc-24m". But Marek's patch
> > commit b4734c9c333 ("clk: imx: Convert clock-osc-* back to osc_*")
> > converted clock-osc-* to osc_*. So I think the UART clocks should also
> > use the current name osc_*.
>
> I sent a fix for that, but back to the subject at hand.
>
> I added some debug code to the clk driver to see which clock is failing:
>
> u-boot=> usb start
> starting USB...
> Bus usb@38200000: clk_mux_fetch_parent_index hsio_axi
> clk_mux_fetch_parent_index (parent) sys_pll2_500m
> imx8m_clk_composite_divider_recalc_rate: name hsio_axi prate:
> 500000000 reg: 0000000030388380
> clk_mux_fetch_parent_index hsio_axi
> clk_mux_fetch_parent_index (parent) sys_pll1_800m
> imx8m_clk_composite_divider_recalc_rate: name hsio_axi prate:
> 800000000 reg: 0000000030388380
> clk_mux_fetch_parent_index usb_phy_ref
> clk_mux_fetch_parent_index (parent) clock-osc-24m
> Could not fetch index
> Failed to get PHY0 for usb@38200000
> Port not available.
> u-boot=>
>
> When I change the name back to "clock-osc-24m" in
> imx8mp_usb_phy_ref_sels, the functionality returns.
>
> u-boot=> usb start
> starting USB...
> Bus usb@38200000: clk_mux_fetch_parent_index hsio_axi
> clk_mux_fetch_parent_index (parent) sys_pll2_500m
> imx8m_clk_composite_divider_recalc_rate: name hsio_axi prate:
> 500000000 reg: 0000000030388380
> clk_mux_fetch_parent_index hsio_axi
> clk_mux_fetch_parent_index (parent) sys_pll1_800m
> imx8m_clk_composite_divider_recalc_rate: name hsio_axi prate:
> 800000000 reg: 0000000030388380
> clk_mux_fetch_parent_index usb_phy_ref
> clk_mux_fetch_parent_index (parent) clock-osc-24m
> Register 2000140 NbrPorts 2
> Starting the controller
> USB XHCI 1.10
> scanning bus usb@38200000 for devices... clk_mux_fetch_parent_index hsio_axi
> clk_mux_fetch_parent_index (parent) sys_pll1_800m
> imx8m_clk_composite_divider_recalc_rate: name hsio_axi prate:
> 800000000 reg: 0000000030388380
> 4 USB Device(s) found
> scanning usb for storage devices... 0 Storage Device(s) found
> u-boot=>
>
> DM tree
>
> Show the parent of usb_phy_ref is "clock-osc-24m"
>
> u-boot=> dm tree
> Class Seq Probed Driver Name
> -----------------------------------------------------------
> root 0 [ + ] root_driver root_driver
> clk 0 [ + ] fixed_clock |-- clock-osc-32k
> clk 1 [ + ] fixed_clock |-- clock-osc-24m
> clk 11 [ + ] ccf_clk_mux | |-- dram_pll_ref_sel
> clk 16 [ + ] imx_clk_pll1443x | | `-- dram_pll
> clk 21 [ + ] ccf_clk_mux | | `--
> dram_pll_bypass
> clk 26 [ + ] clk_gate | | `--
> dram_pll_out
> clk 94 [ + ] ccf_clk_mux | |
> `-- dram_core_clk
> clk 95 [ ] imx_clk_gate2 | |
> `-- dram1_root_clk
> <snip>
> clk 126 [ ] imx_clk_gate2 | |-- usb_suspend_clk
> clk 82 [ + ] clk_composite | `-- usb_phy_ref
> clk 127 [ ] imx_clk_gate2 | `-- usb_phy_root_clk
>
> The device tree node lists the 24m clock as:
>
> osc_24m: clock-osc-24m
>
> So it seems to me, we need to somehow address the translation from
> clock-osc-24m back to osc_24m.
>From what I can tell, the name of the clock is osc_24m as defined by
clock-output-names in the node, but we are not looking at that entry.
There is a function, dev_read_string_index, which appears to let us
look for the that string, and it returns so we can sort through the
list. Adding some debug code, a few other functions look for the
index, and they appear to be returning valid numbers, and the match
for osc_24m returns an index of 0 which appear correct.
With the following patch, the 'usb start' command now works for me, so
I would expect the usb peripheral mode would too.
adam
Can you try the following patch and see if it works?
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index d7411f8f282..de669164939 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -31,6 +31,7 @@
#include <dm/device.h>
#include <dm/device_compat.h>
#include <dm/devres.h>
+#include <dm/read.h>
#include <dm/uclass.h>
#include <linux/bitops.h>
#include <linux/clk-provider.h>
@@ -104,7 +105,7 @@ u8 clk_mux_get_parent(struct clk *clk)
int clk_mux_fetch_parent_index(struct clk *clk, struct clk *parent)
{
struct clk_mux *mux = to_clk_mux(clk);
-
+ const char *name;
int i;
if (!parent)
@@ -113,6 +114,9 @@ int clk_mux_fetch_parent_index(struct clk *clk,
struct clk *parent)
for (i = 0; i < mux->num_parents; i++) {
if (!strcmp(parent->dev->name, mux->parent_names[i]))
return i;
+ dev_read_string_index(parent->dev,
"clock-output-names", i, &name);
+ if (!strcmp(name, mux->parent_names[i]))
+ return i;
if (!strcmp(parent->dev->name,
clk_resolve_parent_clk(clk->dev,
mux->parent_names[i])))
>
> adam
>
>
> >
> > Regards
> > Christoph
