On Fri, Apr 28, 2023 at 9:44 AM Adam Ford aford173@gmail.com wrote:
On Fri, Apr 28, 2023 at 11:26 AM Fabio Estevam festevam@gmail.com wrote:
Hi Tim,
On Fri, Apr 28, 2023 at 12:48 PM Tim Harvey tharvey@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/ar...
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