On Fri, Apr 28, 2023 at 9:44 AM Adam Ford <aford...@gmail.com> wrote: > > On Fri, Apr 28, 2023 at 11:26 AM Fabio Estevam <feste...@gmail.com> wrote: > > > > Hi Tim, > > > > On Fri, Apr 28, 2023 at 12:48 PM Tim Harvey <thar...@gateworks.com> wrote: > > > > > Yes I think that is similar enough to test. In my experience simply > > > enabling otg2 via dt on imx8mm-evk shows the issue I see here but > > > Fabio says he sees a hang on 'usb start' even before this dt sync and > > > I don't know why my results on an imx8mm-evk differ. > > > > I started from scratch today and now our results match. > > > > Applied the following change against U-Boot master: > > > > diff --git a/arch/arm/dts/imx8mm-evk.dtsi b/arch/arm/dts/imx8mm-evk.dtsi > > index 7d6317d95b13..898639e33d5e 100644 > > --- a/arch/arm/dts/imx8mm-evk.dtsi > > +++ b/arch/arm/dts/imx8mm-evk.dtsi > > @@ -417,6 +417,10 @@ > > }; > > }; > > > > +&usbotg2 { > > + status = "okay"; > > +}; > > + > > &usdhc2 { > > assigned-clocks = <&clk IMX8MM_CLK_USDHC2>; > > assigned-clock-rates = <200000000>; > > diff --git a/configs/imx8mm_evk_defconfig b/configs/imx8mm_evk_defconfig > > index ab9ad41b4548..70c7a21f2d9f 100644 > > --- a/configs/imx8mm_evk_defconfig > > +++ b/configs/imx8mm_evk_defconfig > > @@ -119,3 +119,4 @@ CONFIG_CI_UDC=y > > CONFIG_SDP_LOADADDR=0x40400000 > > CONFIG_USB_GADGET_DOWNLOAD=y > > CONFIG_IMX_WATCHDOG=y > > +CONFIG_CMD_USB=y > > -- > > 2.34.1 > > > > Running "usb start" does not hang. > > > > Running "ums 0 mmc 1", CTRL+C and then "ums 0 mmc 1" does not work (SD > > card is not mounted on PC on the second time). > > > > After applying the imx8mm.dtsi sync with kernel 6.3: > > > > Running "ums 0 mmc 1", CTRL+C and then "ums 0 mmc 1" works fine. > > > > "usb start" hangs. > > > > So, yes, I agree we cannot do the imx8mm.dtsi sync with 6.3 right now > > as we need to fix the USB hang first. > > > > If anyone has any ideas as to why syncing the commit below: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/arch/arm64/boot/dts/freescale/imx8mm.dtsi?h=v6.3&id=4585c79ff477f9517b7f384a4fce351417e8fa36 > > > > causes issues in U-Boot, please let us know. > > I am not in a place to test this as I am traveling, but I thought I'd > throw out an idea. The power-domain looks like it moved to the > usbphynop2 driver which has the compatible name of "usb-nop-xceiv" > Is there a a driver for this? Does it get enabled? > If not, maybe we could update the imx8mm-u-u-boot.dtsi to restore the > power-domains to a driver that is present. >
Adam, Ya, I think you were on the right track here. There is a driver (driver/phy/nop-phy.c) which does get enabled but with the dt sync the phy's power domain gets enabled after EHCI registers are accessed. I believe the fix we need is the following which moves phy setup before the register access (where it should have been along with the case for !defined(CONFIG_PHY): diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index 91633f013a55..fae20838c60a 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -703,6 +703,10 @@ static int ehci_usb_probe(struct udevice *dev) usb_internal_phy_clock_gate(priv->phy_addr, 1); usb_phy_enable(ehci, priv->phy_addr); #endif +#else + ret = generic_setup_phy(dev, &priv->phy, 0); + if (ret) + goto err_regulator; #endif #if CONFIG_IS_ENABLED(DM_REGULATOR) @@ -725,12 +729,6 @@ static int ehci_usb_probe(struct udevice *dev) mdelay(10); -#if defined(CONFIG_PHY) - ret = generic_setup_phy(dev, &priv->phy, 0); - if (ret) - goto err_regulator; -#endif - hccr = (struct ehci_hccr *)((uintptr_t)&ehci->caplength); hcor = (struct ehci_hcor *)((uintptr_t)hccr + HC_LENGTH(ehci_readl(&(hccr)->cr_capbase))); If everyone agrees here I'll submit a formal patch which should get applied through Marek via the usb tree before the dt sync. Best Regards, Tim