On Fri, Apr 23, 2021 at 8:21 AM Marek Vasut marex@denx.de wrote:
On 4/23/21 5:04 PM, Tim Harvey wrote:
On Thu, Apr 22, 2021 at 12:16 PM Marek Vasut marex@denx.de wrote:
On 4/20/21 2:33 AM, Tim Harvey wrote:
[...]
+/* USB Configs */ +#define CONFIG_EHCI_HCD_INIT_AFTER_RESET +#define CONFIG_MXC_USB_PORTSC (PORT_PTS_UTMI | PORT_PTS_PTW) +#define CONFIG_USB_MAX_CONTROLLER_COUNT 2
- #endif /*_VERDIN_IMX8MM_H */
-- 2.30.2
Marek,
Thanks for your work on USB support for IMX8M!
I'm attempting to add USB support to the venice board following this example but I think there are still some things missing from the dt to make this work. I find that mx6_parse_dt_adds failes; Looks like it is required to have an alias that points to the phy but then it fails because the phy doesn't have a reg. Also, it would see the CONFIG_MXC_USB_PORTSC is no longer needed as that is now the default.
Have a look at this patch:
https://source.denx.de/u-boot/custodians/u-boot-usb/-/commit/8203ee09275c307...
That should fix it for you.
It would be good however if you could take the mx6 ventana and test the driver there, and possibly submit fixes in case the USB is still broken there. What would be even better is if you could implement the MXS PHY driver, so all the !CONFIG_PHY stuff can be removed for MX6 too. That shouldn't be much effort.
Marek,
That does resolve the issue with mx6_parse_dt_adds failing and works for IMX8MM host mode (and likely peripheral mode) but not for otg. For otg we also need something like: @@ -523,7 +568,7 @@ static int ehci_usb_phy_mode(struct udevice *dev) plat->init_type = USB_INIT_DEVICE; else plat->init_type = USB_INIT_HOST;
} else if (is_mx7()) {
} else if (is_mx7() || is_imx8mm()) { phy_status = (void __iomem *)(addr + USBNC_PHY_STATUS_OFFSET); val = readl(phy_status);but then the issue is we hang due to the otg power domain not being enabled when ehci_usb_of_to_plat is called and I'm not quite clear why that is.
ehci_usb_phy_mode() is called from ehci_usb_of_to_plat(), which happens before probe(), so the power domain at that point is off. You would have to move that entire code to probe() to get OTG working properly, without the current workaround.
Why would the power domain get probed/enabled for the usbotg2 bus but not the usbotg1 bus? Here is some debugging: u-boot=> usb start starting USB... Bus usb@32e40000: ehci_usb_phy_mode usb@32e40000 usb@32e40000 probe ret=-22 probe failed, error -22 ^^^ probe fails here because ehci_usb_phy_mode returns EINVAL for dr_mode=otg but if we try to read the phy_status reg we will hang b/c power domain is not enabled yet Bus usb@32e50000: imx8m_power_domain_probe gpc@303a0000 imx8m_power_domain_probe pgc ^^^ why did power domain get probed on the 2nd bus and not the first?
I don't know, can you have a look ?
Marek,
The reg domain does not get enabled for usbotg1 because device_of_to_plat gets called 'before' dev_power_domain_on in device_probe.
The following will get imx8mm USB otg working:
For OTG defer setting type until probe after clock and power have been brought up. index 06be9deaaa..2183ae4f9d 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -523,7 +523,7 @@ static int ehci_usb_phy_mode(struct udevice *dev) plat->init_type = USB_INIT_DEVICE; else plat->init_type = USB_INIT_HOST; - } else if (is_mx7()) { + } else if (is_mx7() || is_imx8mm()) { phy_status = (void __iomem *)(addr + USBNC_PHY_STATUS_OFFSET); val = readl(phy_status); @@ -555,7 +555,10 @@ static int ehci_usb_of_to_plat(struct udevice *dev) break; case USB_DR_MODE_OTG: case USB_DR_MODE_UNKNOWN: - return ehci_usb_phy_mode(dev); + if (is_imx8mm()) + plat->init_type = USB_INIT_HOST; + else + return ehci_usb_phy_mode(dev); };
return 0; @@ -657,6 +660,13 @@ static int ehci_usb_probe(struct udevice *dev) mdelay(1); #endif
+ if (is_imx8mm() && (usb_get_dr_mode(dev_ofnode(dev)) == USB_DR_MODE_OTG)) { + ret = ehci_usb_phy_mode(dev); + if (ret) + return ret; + priv->init_type = plat->init_type; + }; + #if CONFIG_IS_ENABLED(DM_REGULATOR) ret = device_get_supply_regulator(dev, "vbus-supply", &priv->vbus_supply);
imx8m_power_domain_probe power-domain@0 imx8m_power_domain_probe power-domain@3 imx8m_power_domain_on power-domain@3 imx8m_power_domain_on power-domain@0 ehci_usb_probe usb@32e50000 USB EHCI 1.00 usb@32e50000 probe ret=0 scanning bus usb@32e50000 for devices... imx8m_power_domain_on power-domain@3 imx8m_power_domain_on power-domain@0 3 USB Device(s) found scanning usb for storage devices... 0 Storage Device(s) found u-boot=>
I also verified your patch does not break USB for IMX6Q/DL ventana as well.
That's good, thank you
I am still a bit concerned that the power-domain patches you merged in this series are introducing bindings that are not approved upstream: d78f7d8199 ARM: dts: imx8mn: Add power domain nodes f0e10e33e5 ARM: dts: imx8mm: Add power domain nodes
Are you under the impression that is what will go upstream?
Pretty much, yes. And if there are some changes further down the line, we can pick those later when they hit upstream. I think this is better than having no working USB at all.
I'm still waiting to see what this 'virtual power-domain' patch looks like that I believe Abel or Jacky was going to work on. Perhaps you, Adam, or Frieder took part in that discussion?
That is still going on, yes.