[PATCH V2 24/24] ARM: imx8m: verdin-imx8mm: Enable USB Host support

[Tim Harvey]

On Thu, Apr 22, 2021 at 12:16 PM Marek Vasut <marex at 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/8203ee09275c30766a1fd1cf19c60a904ee72f8c
>
> 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. 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 at 32e40000: ehci_usb_phy_mode usb at 32e40000
usb at 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 at 32e50000: imx8m_power_domain_probe gpc at 303a0000
imx8m_power_domain_probe pgc
^^^ why did power domain get probed on the 2nd bus and not the first?
imx8m_power_domain_probe power-domain at 0
imx8m_power_domain_probe power-domain at 3
imx8m_power_domain_on power-domain at 3
imx8m_power_domain_on power-domain at 0
ehci_usb_probe usb at 32e50000
USB EHCI 1.00
usb at 32e50000 probe ret=0
scanning bus usb at 32e50000 for devices... imx8m_power_domain_on power-domain at 3
imx8m_power_domain_on power-domain at 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.

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? 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?

Best regards,

Tim