From mboxrd@z Thu Jan 1 00:00:00 1970 From: Adam Ford Date: Wed, 28 Apr 2021 16:56:06 -0500 Subject: [PATCH V2 24/24] ARM: imx8m: verdin-imx8mm: Enable USB Host support In-Reply-To: References: <20210411162901.7238-1-marex@denx.de> <20210411162901.7238-24-marex@denx.de> <1bb9da38-e7b6-d5f1-8a3d-dbf18d3863b4@denx.de> <6aa1578e-cd79-5369-742b-d89a746d1bac@denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Tue, Apr 27, 2021 at 10:50 AM Tim Harvey wrote: > > On Mon, Apr 26, 2021, 5:35 PM Marek Vasut 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? If you want me to test it on either a Mini or Nano, feel free to CC me as well. I was out of town the last week, so I wasn't in a place to do any work. I am a little behind, so I might need some pointers to prerequisite patches if they're necessary. thanks, adam > > Best regards, > > Tim