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

[Marek Vasut]

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 ?

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

Also, could you submit a regular patch ?