| Message ID | 20210427170857.21273-2-tharvey@gateworks.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | [1/3] usb: ehci-mx6: move mode set/detect to probe | expand |
On 4/27/21 7:08 PM, Tim Harvey wrote: > Add support for determining host vs peripheral mode for IMX8MM > configured as OTG. > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > --- > drivers/usb/host/ehci-mx6.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c > index c2dfe49012..d055d2b1fe 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()) { This likely also applies to 8mq/mm/mn/mp , i.e. all of them.
On Tue, Apr 27, 2021 at 10:44 AM Marek Vasut <marex@denx.de> wrote: > > On 4/27/21 7:08 PM, Tim Harvey wrote: > > Add support for determining host vs peripheral mode for IMX8MM > > configured as OTG. > > > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > > --- > > drivers/usb/host/ehci-mx6.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c > > index c2dfe49012..d055d2b1fe 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()) { > > This likely also applies to 8mq/mm/mn/mp , i.e. all of them. Agreed. Perhaps Adam, Frieder, or Fabio have something to test with? I only have IMX8M Mini at the moment. Tim
Am Tue, Apr 27, 2021 at 10:50:34AM -0700 schrieb Tim Harvey: > On Tue, Apr 27, 2021 at 10:44 AM Marek Vasut <marex@denx.de> wrote: > > > > On 4/27/21 7:08 PM, Tim Harvey wrote: > > > Add support for determining host vs peripheral mode for IMX8MM > > > configured as OTG. > > > > > > Signed-off-by: Tim Harvey <tharvey@gateworks.com> > > > --- > > > drivers/usb/host/ehci-mx6.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c > > > index c2dfe49012..d055d2b1fe 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()) { > > > > This likely also applies to 8mq/mm/mn/mp , i.e. all of them. > > Agreed. Perhaps Adam, Frieder, or Fabio have something to test with? I > only have IMX8M Mini at the moment. No, I don't think this applies to all of them, 8MM and 8MN should be correct. So from a historic point of view the first one that showed up is the 8MQ, which has a DesignWare xHCI controller. There's no eHCI, so this driver doesn't attach and there is neither the fsl,imx27-usb nor the fsl,imx7d-usb compatible. Hence not having imx8mq should be correct. When the i.MX8MM showed up the biggest difference obviously was the silicon technology, reducing temperature. But it also replaced the DesignWare xHCI with ... what was it, the cadence OTG controller? This is basically a carbon copy of the fsl,imx7d-usb (which has the i.MX6 as heritage). Hence imx8mm is correct here. Then the i.MX8MP showed up, which is builds on that, but it finally gets the DesignWare xHCI back. So in that regard it's more a better version of i.MX8MQ. The fsl,imx7d-usb compatible is not part of the dts, hence not having imx8mp should be correct. The i.MX8MN is new to me, but by grepping you can see that the MN also has the fsl,imx7d-usb compatible. Hence I think only 8MM and 8MN should be added, not 8MQ or 8MP. Patrick
On 4/27/21 9:24 PM, Patrick Wildt wrote: > Am Tue, Apr 27, 2021 at 10:50:34AM -0700 schrieb Tim Harvey: >> On Tue, Apr 27, 2021 at 10:44 AM Marek Vasut <marex@denx.de> wrote: >>> >>> On 4/27/21 7:08 PM, Tim Harvey wrote: >>>> Add support for determining host vs peripheral mode for IMX8MM >>>> configured as OTG. >>>> >>>> Signed-off-by: Tim Harvey <tharvey@gateworks.com> >>>> --- >>>> drivers/usb/host/ehci-mx6.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c >>>> index c2dfe49012..d055d2b1fe 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()) { >>> >>> This likely also applies to 8mq/mm/mn/mp , i.e. all of them. >> >> Agreed. Perhaps Adam, Frieder, or Fabio have something to test with? I >> only have IMX8M Mini at the moment. > > No, I don't think this applies to all of them, 8MM and 8MN should be > correct. > > So from a historic point of view the first one that showed up is the > 8MQ, which has a DesignWare xHCI controller. There's no eHCI, so this > driver doesn't attach and there is neither the fsl,imx27-usb nor the > fsl,imx7d-usb compatible. Hence not having imx8mq should be correct. > > When the i.MX8MM showed up the biggest difference obviously was the > silicon technology, reducing temperature. But it also replaced the > DesignWare xHCI with ... what was it, the cadence OTG controller? chipidea > This is basically a carbon copy of the fsl,imx7d-usb (which has the > i.MX6 as heritage). Hence imx8mm is correct here. Not quite, the mx8mm/mn variant is a bit more complex due to the extra power domain stuff. And the PHY/MISC bits changed from mx7 too. > Then the i.MX8MP showed up, which is builds on that, but it finally > gets the DesignWare xHCI back. So in that regard it's more a better > version of i.MX8MQ. The fsl,imx7d-usb compatible is not part of the > dts, hence not having imx8mp should be correct. > > The i.MX8MN is new to me, but by grepping you can see that the MN also > has the fsl,imx7d-usb compatible. Seems there is always some sort of overlap between the Ms in terms of IPs that are reused. > Hence I think only 8MM and 8MN should be added, not 8MQ or 8MP. I agree, 8MP also only lists xhci (likely dwc3), like original M(Q). MN should work, I use these patches on a board with it, in host mode only.
On Tue, Apr 27, 2021 at 12:52 PM Marek Vasut <marex@denx.de> wrote: > > On 4/27/21 9:24 PM, Patrick Wildt wrote: > > Am Tue, Apr 27, 2021 at 10:50:34AM -0700 schrieb Tim Harvey: > >> On Tue, Apr 27, 2021 at 10:44 AM Marek Vasut <marex@denx.de> wrote: > >>> > >>> On 4/27/21 7:08 PM, Tim Harvey wrote: > >>>> Add support for determining host vs peripheral mode for IMX8MM > >>>> configured as OTG. > >>>> > >>>> Signed-off-by: Tim Harvey <tharvey@gateworks.com> > >>>> --- > >>>> drivers/usb/host/ehci-mx6.c | 2 +- > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c > >>>> index c2dfe49012..d055d2b1fe 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()) { > >>> > >>> This likely also applies to 8mq/mm/mn/mp , i.e. all of them. > >> > >> Agreed. Perhaps Adam, Frieder, or Fabio have something to test with? I > >> only have IMX8M Mini at the moment. > > > > No, I don't think this applies to all of them, 8MM and 8MN should be > > correct. > > > > So from a historic point of view the first one that showed up is the > > 8MQ, which has a DesignWare xHCI controller. There's no eHCI, so this > > driver doesn't attach and there is neither the fsl,imx27-usb nor the > > fsl,imx7d-usb compatible. Hence not having imx8mq should be correct. > > > > When the i.MX8MM showed up the biggest difference obviously was the > > silicon technology, reducing temperature. But it also replaced the > > DesignWare xHCI with ... what was it, the cadence OTG controller? > > chipidea > > > This is basically a carbon copy of the fsl,imx7d-usb (which has the > > i.MX6 as heritage). Hence imx8mm is correct here. > > Not quite, the mx8mm/mn variant is a bit more complex due to the extra > power domain stuff. And the PHY/MISC bits changed from mx7 too. > > > Then the i.MX8MP showed up, which is builds on that, but it finally > > gets the DesignWare xHCI back. So in that regard it's more a better > > version of i.MX8MQ. The fsl,imx7d-usb compatible is not part of the > > dts, hence not having imx8mp should be correct. > > > > The i.MX8MN is new to me, but by grepping you can see that the MN also > > has the fsl,imx7d-usb compatible. > > Seems there is always some sort of overlap between the Ms in terms of > IPs that are reused. > > > Hence I think only 8MM and 8MN should be added, not 8MQ or 8MP. > > I agree, 8MP also only lists xhci (likely dwc3), like original M(Q). > MN should work, I use these patches on a board with it, in host mode only. I will re-submit and add is_imx8mn().... that's what the NXP downstream u-boot does as well. Tim
On 4/28/21 3:55 AM, Tim Harvey wrote: > On Tue, Apr 27, 2021 at 12:52 PM Marek Vasut <marex@denx.de> wrote: >> >> On 4/27/21 9:24 PM, Patrick Wildt wrote: >>> Am Tue, Apr 27, 2021 at 10:50:34AM -0700 schrieb Tim Harvey: >>>> On Tue, Apr 27, 2021 at 10:44 AM Marek Vasut <marex@denx.de> wrote: >>>>> >>>>> On 4/27/21 7:08 PM, Tim Harvey wrote: >>>>>> Add support for determining host vs peripheral mode for IMX8MM >>>>>> configured as OTG. >>>>>> >>>>>> Signed-off-by: Tim Harvey <tharvey@gateworks.com> >>>>>> --- >>>>>> drivers/usb/host/ehci-mx6.c | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c >>>>>> index c2dfe49012..d055d2b1fe 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()) { >>>>> >>>>> This likely also applies to 8mq/mm/mn/mp , i.e. all of them. >>>> >>>> Agreed. Perhaps Adam, Frieder, or Fabio have something to test with? I >>>> only have IMX8M Mini at the moment. >>> >>> No, I don't think this applies to all of them, 8MM and 8MN should be >>> correct. >>> >>> So from a historic point of view the first one that showed up is the >>> 8MQ, which has a DesignWare xHCI controller. There's no eHCI, so this >>> driver doesn't attach and there is neither the fsl,imx27-usb nor the >>> fsl,imx7d-usb compatible. Hence not having imx8mq should be correct. >>> >>> When the i.MX8MM showed up the biggest difference obviously was the >>> silicon technology, reducing temperature. But it also replaced the >>> DesignWare xHCI with ... what was it, the cadence OTG controller? >> >> chipidea >> >>> This is basically a carbon copy of the fsl,imx7d-usb (which has the >>> i.MX6 as heritage). Hence imx8mm is correct here. >> >> Not quite, the mx8mm/mn variant is a bit more complex due to the extra >> power domain stuff. And the PHY/MISC bits changed from mx7 too. >> >>> Then the i.MX8MP showed up, which is builds on that, but it finally >>> gets the DesignWare xHCI back. So in that regard it's more a better >>> version of i.MX8MQ. The fsl,imx7d-usb compatible is not part of the >>> dts, hence not having imx8mp should be correct. >>> >>> The i.MX8MN is new to me, but by grepping you can see that the MN also >>> has the fsl,imx7d-usb compatible. >> >> Seems there is always some sort of overlap between the Ms in terms of >> IPs that are reused. >> >>> Hence I think only 8MM and 8MN should be added, not 8MQ or 8MP. >> >> I agree, 8MP also only lists xhci (likely dwc3), like original M(Q). >> MN should work, I use these patches on a board with it, in host mode only. > > I will re-submit and add is_imx8mn().... that's what the NXP > downstream u-boot does as well. Thanks
Hi Tim, On Tue, Apr 27, 2021 at 10:55 PM Tim Harvey <tharvey@gateworks.com> wrote: > I will re-submit and add is_imx8mn().... that's what the NXP > downstream u-boot does as well. When you re-submit this, please add: Tested-by: Fabio Estevam <festevam@gmail.com> USB gadget works now on imx8mm-evk. Thanks
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index c2dfe49012..d055d2b1fe 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);
Add support for determining host vs peripheral mode for IMX8MM configured as OTG. Signed-off-by: Tim Harvey <tharvey@gateworks.com> --- drivers/usb/host/ehci-mx6.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)