[PATCH V2 24/24] ARM: imx8m: verdin-imx8mm: Enable USB Host support
Tim Harvey
tharvey at gateworks.com
Tue Apr 27 02:01:48 CEST 2021
On Fri, Apr 23, 2021 at 8:21 AM Marek Vasut <marex at 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 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.
>
> 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 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())
+ 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 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.
>
> 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.
More information about the U-Boot
mailing list