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

[Tim Harvey]

On Mon, Apr 26, 2021, 5:35 PM Marek Vasut <marex at denx.de> wrote:
>
> On 4/27/21 2:01 AM, Tim Harvey wrote:
> [...]
> >>> 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?
> >>
> >> 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())
>
> Does this mean OTG doesn't work on the 8MM then ?

IMX8MM USB in general still doesn't work without your:
usb: ehci-mx6: Limit PHY address parsing to !CONFIG_PHY

With your patch, IMX8MM 'host' works but 'otg' will fail probe with
-22 (due ehci_usb_phy_mode called from of_to_plat and it not having a
case for 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;
> > +       };
>
> I have to wonder, why not move the whole OTG/Host/Device detection to
> probe then ?

Yes, I think that is the right thing to do.

>
> Also, could you submit a regular patch ?

Yes, I will post patches to fix IMX8MM OTG. Can you submit your 'usb:
ehci-mx6: Limit PHY address parsing to !CONFIG_PHY' patch so I can go
on top of that or can I just pull that into my series?

Best regards,

Tim