| Message ID | 20220118002103.112748-1-aford173@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | [V3] usb: ehci-mx6: Enable OTG detection on imx8mm and imx8mn | expand |
On Mon, Jan 17, 2022 at 4:21 PM Adam Ford <aford173@gmail.com> wrote: > > The imx8mm and imx8mn appear compatible with imx7d-usb > flags in the OTG driver. If the dr_mode is defined as > host or peripheral, the device appears to operate correctly, > however the auto host/peripheral detection results in an error. > > The solution isn't just adding checks for imx8mm and imx8mn to > the check for imx7, because the USB clock needs to be running > to read from the USBNC_PHY_STATUS_OFFSET register or it will hang. > > Marek requested that I not enable the clocks in ehci_usb_of_to_plat, > so I modified that function to return an unknown state if the > device tree does not explicitly state whether it is a host > or a peripheral. > > When the driver probes, it looks to see if it's in the unknown > state, and only then will it read the register to auto-detect. > > Signed-off-by: Adam Ford <aford173@gmail.com> > --- > V3: Keep ehci_usb_of_to_plat but add the ability to return > and unknown state instead of reading the register. > If the probe determines the states is unknown, it will > query the register after the clocks have been enabled. > Because of the slight behavior change, I removed any > review or tested tags. > > V2: Rename ehci_usb_of_to_plat to ehci_usb_dr_mode and call it > from the probe after the clocks are enabled, but before > the data is needed. > > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c > index 1bd6147c76..cf44e53ff7 100644 > --- a/drivers/usb/host/ehci-mx6.c > +++ b/drivers/usb/host/ehci-mx6.c > @@ -543,7 +543,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() || is_imx8mn()) { > phy_status = (void __iomem *)(addr + > USBNC_PHY_STATUS_OFFSET); > val = readl(phy_status); > @@ -573,9 +573,8 @@ static int ehci_usb_of_to_plat(struct udevice *dev) > case USB_DR_MODE_PERIPHERAL: > plat->init_type = USB_INIT_DEVICE; > break; > - case USB_DR_MODE_OTG: > - case USB_DR_MODE_UNKNOWN: > - return ehci_usb_phy_mode(dev); > + default: > + plat->init_type = USB_INIT_UNKNOWN; > }; > > return 0; > @@ -677,6 +676,20 @@ static int ehci_usb_probe(struct udevice *dev) > mdelay(1); > #endif > > + /* > + * If the device tree didn't specify host or device, > + * the default is USB_INIT_UNKNOWN, so we need to check > + * the register. For imx8mm and imx8mn, the clocks need to be > + * running first, so we defer the check until they are. > + */ > + if (priv->init_type == USB_INIT_UNKNOWN) { > + ret = ehci_usb_phy_mode(dev); > + if (ret) > + goto err_clk; > + else > + priv->init_type = plat->init_type; > + } > + > #if CONFIG_IS_ENABLED(DM_REGULATOR) > ret = device_get_supply_regulator(dev, "vbus-supply", > &priv->vbus_supply); > diff --git a/include/usb.h b/include/usb.h > index b3851fdb4f..47d738a786 100644 > --- a/include/usb.h > +++ b/include/usb.h > @@ -163,7 +163,8 @@ struct int_queue; > */ > enum usb_init_type { > USB_INIT_HOST, > - USB_INIT_DEVICE > + USB_INIT_DEVICE, > + USB_INIT_UNKNOWN, > }; > > /********************************************************************** > -- > 2.32.0 > Adam, For v3: tested on imx8mm_venice_defconfig as USB host with USB_ETHER_ASIX and as gadget with CMD_USB_MASS_STORAGE (ums 0 mmc 2). Tested-By: Tim Harvey <tharvey@gateworks.com> Best Regards, Tim
Hi Adam, On Mon, Jan 17, 2022 at 9:21 PM Adam Ford <aford173@gmail.com> wrote: > > The imx8mm and imx8mn appear compatible with imx7d-usb > flags in the OTG driver. If the dr_mode is defined as > host or peripheral, the device appears to operate correctly, > however the auto host/peripheral detection results in an error. > > The solution isn't just adding checks for imx8mm and imx8mn to > the check for imx7, because the USB clock needs to be running > to read from the USBNC_PHY_STATUS_OFFSET register or it will hang. > > Marek requested that I not enable the clocks in ehci_usb_of_to_plat, > so I modified that function to return an unknown state if the > device tree does not explicitly state whether it is a host > or a peripheral. > > When the driver probes, it looks to see if it's in the unknown > state, and only then will it read the register to auto-detect. > > Signed-off-by: Adam Ford <aford173@gmail.com> > --- > V3: Keep ehci_usb_of_to_plat but add the ability to return > and unknown state instead of reading the register. > If the probe determines the states is unknown, it will > query the register after the clocks have been enabled. > Because of the slight behavior change, I removed any > review or tested tags. Unfortunately, v3 breaks 'ums 0 mmc 0' on the imx7s-warp board. The eMMC is no longer mounted and the board hangs.
On Mon, Jan 17, 2022 at 7:46 PM Fabio Estevam <festevam@gmail.com> wrote: > > Hi Adam, > > On Mon, Jan 17, 2022 at 9:21 PM Adam Ford <aford173@gmail.com> wrote: > > > > The imx8mm and imx8mn appear compatible with imx7d-usb > > flags in the OTG driver. If the dr_mode is defined as > > host or peripheral, the device appears to operate correctly, > > however the auto host/peripheral detection results in an error. > > > > The solution isn't just adding checks for imx8mm and imx8mn to > > the check for imx7, because the USB clock needs to be running > > to read from the USBNC_PHY_STATUS_OFFSET register or it will hang. > > > > Marek requested that I not enable the clocks in ehci_usb_of_to_plat, > > so I modified that function to return an unknown state if the > > device tree does not explicitly state whether it is a host > > or a peripheral. > > > > When the driver probes, it looks to see if it's in the unknown > > state, and only then will it read the register to auto-detect. > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > --- > > V3: Keep ehci_usb_of_to_plat but add the ability to return > > and unknown state instead of reading the register. > > If the probe determines the states is unknown, it will > > query the register after the clocks have been enabled. > > Because of the slight behavior change, I removed any > > review or tested tags. > > Unfortunately, v3 breaks 'ums 0 mmc 0' on the imx7s-warp board. Thanks for testing it. I am not really sure what's significantly different between them. Do you get any errors when you run UMS? > > The eMMC is no longer mounted and the board hangs.
Hi Adam, On Mon, Jan 17, 2022 at 10:58 PM Adam Ford <aford173@gmail.com> wrote: > Thanks for testing it. > > I am not really sure what's significantly different between them. Do > you get any errors when you run UMS? Just realized the problem I am seeing is unrelated to your patch. I will need to debug it.
On Mon, 2022-01-17 at 18:21 -0600, Adam Ford wrote: > The imx8mm and imx8mn appear compatible with imx7d-usb > flags in the OTG driver. If the dr_mode is defined as > host or peripheral, the device appears to operate correctly, > however the auto host/peripheral detection results in an error. > > The solution isn't just adding checks for imx8mm and imx8mn to > the check for imx7, because the USB clock needs to be running > to read from the USBNC_PHY_STATUS_OFFSET register or it will hang. > > Marek requested that I not enable the clocks in ehci_usb_of_to_plat, > so I modified that function to return an unknown state if the > device tree does not explicitly state whether it is a host > or a peripheral. > > When the driver probes, it looks to see if it's in the unknown > state, and only then will it read the register to auto-detect. > > Signed-off-by: Adam Ford <aford173@gmail.com> > --- > V3: Keep ehci_usb_of_to_plat but add the ability to return > and unknown state instead of reading the register. > If the probe determines the states is unknown, it will > query the register after the clocks have been enabled. > Because of the slight behavior change, I removed any > review or tested tags. > > V2: Rename ehci_usb_of_to_plat to ehci_usb_dr_mode and call it > from the probe after the clocks are enabled, but before > the data is needed. > > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c > index 1bd6147c76..cf44e53ff7 100644 > --- a/drivers/usb/host/ehci-mx6.c > +++ b/drivers/usb/host/ehci-mx6.c > @@ -543,7 +543,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() || is_imx8mn()) { > phy_status = (void __iomem *)(addr + > USBNC_PHY_STATUS_OFFSET); > val = readl(phy_status); > @@ -573,9 +573,8 @@ static int ehci_usb_of_to_plat(struct udevice *dev) > case USB_DR_MODE_PERIPHERAL: > plat->init_type = USB_INIT_DEVICE; > break; > - case USB_DR_MODE_OTG: > - case USB_DR_MODE_UNKNOWN: > - return ehci_usb_phy_mode(dev); > + default: > + plat->init_type = USB_INIT_UNKNOWN; > }; > > return 0; > @@ -677,6 +676,20 @@ static int ehci_usb_probe(struct udevice *dev) > mdelay(1); > #endif > > + /* > + * If the device tree didn't specify host or device, > + * the default is USB_INIT_UNKNOWN, so we need to check > + * the register. For imx8mm and imx8mn, the clocks need to be > + * running first, so we defer the check until they are. > + */ > + if (priv->init_type == USB_INIT_UNKNOWN) { > + ret = ehci_usb_phy_mode(dev); > + if (ret) > + goto err_clk; Hm, unfortunately, that label is gated by an ifdef leading to the following on colibri_imx7: drivers/usb/host/ehci-mx6.c: In function ‘ehci_usb_probe’: drivers/usb/host/ehci-mx6.c:688:4: error: label ‘err_clk’ used but not defined 688 | goto err_clk; | ^~~~ make[1]: *** [scripts/Makefile.build:254: drivers/usb/host/ehci-mx6.o] Error 1 I have to admit that maybe we should indeed run it with DM_REGULATOR but that won't change anything on this issue. I guess one should really not have any gotos to a label err_clk being ifdefed from somewhere not ifdefed. > + else > + priv->init_type = plat->init_type; > + } > + > #if CONFIG_IS_ENABLED(DM_REGULATOR) > ret = device_get_supply_regulator(dev, "vbus-supply", > &priv->vbus_supply); > diff --git a/include/usb.h b/include/usb.h > index b3851fdb4f..47d738a786 100644 > --- a/include/usb.h > +++ b/include/usb.h > @@ -163,7 +163,8 @@ struct int_queue; > */ > enum usb_init_type { > USB_INIT_HOST, > - USB_INIT_DEVICE > + USB_INIT_DEVICE, > + USB_INIT_UNKNOWN, > }; > > /**********************************************************************
On Mon, 2022-01-17 at 18:21 -0600, Adam Ford wrote: > The imx8mm and imx8mn appear compatible with imx7d-usb > flags in the OTG driver. If the dr_mode is defined as > host or peripheral, the device appears to operate correctly, > however the auto host/peripheral detection results in an error. > > The solution isn't just adding checks for imx8mm and imx8mn to > the check for imx7, because the USB clock needs to be running > to read from the USBNC_PHY_STATUS_OFFSET register or it will hang. > > Marek requested that I not enable the clocks in ehci_usb_of_to_plat, > so I modified that function to return an unknown state if the > device tree does not explicitly state whether it is a host > or a peripheral. > > When the driver probes, it looks to see if it's in the unknown > state, and only then will it read the register to auto-detect. > > Signed-off-by: Adam Ford <aford173@gmail.com> > --- > V3: Keep ehci_usb_of_to_plat but add the ability to return > and unknown state instead of reading the register. > If the probe determines the states is unknown, it will > query the register after the clocks have been enabled. > Because of the slight behavior change, I removed any > review or tested tags. > > V2: Rename ehci_usb_of_to_plat to ehci_usb_dr_mode and call it > from the probe after the clocks are enabled, but before > the data is needed. > > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c > index 1bd6147c76..cf44e53ff7 100644 > --- a/drivers/usb/host/ehci-mx6.c > +++ b/drivers/usb/host/ehci-mx6.c > @@ -543,7 +543,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() || is_imx8mn()) { > phy_status = (void __iomem *)(addr + > USBNC_PHY_STATUS_OFFSET); > val = readl(phy_status); > @@ -573,9 +573,8 @@ static int ehci_usb_of_to_plat(struct udevice *dev) > case USB_DR_MODE_PERIPHERAL: > plat->init_type = USB_INIT_DEVICE; > break; > - case USB_DR_MODE_OTG: > - case USB_DR_MODE_UNKNOWN: > - return ehci_usb_phy_mode(dev); > + default: > + plat->init_type = USB_INIT_UNKNOWN; > }; > > return 0; > @@ -677,6 +676,20 @@ static int ehci_usb_probe(struct udevice *dev) > mdelay(1); > #endif > > + /* > + * If the device tree didn't specify host or device, > + * the default is USB_INIT_UNKNOWN, so we need to check > + * the register. For imx8mm and imx8mn, the clocks need to be > + * running first, so we defer the check until they are. > + */ > + if (priv->init_type == USB_INIT_UNKNOWN) { > + ret = ehci_usb_phy_mode(dev); > + if (ret) > + goto err_clk; Hm, unfortunately, that label is gated by an ifdef leading to the following on colibri_imx7: drivers/usb/host/ehci-mx6.c: In function ‘ehci_usb_probe’: drivers/usb/host/ehci-mx6.c:688:4: error: label ‘err_clk’ used but not defined 688 | goto err_clk; | ^~~~ make[1]: *** [scripts/Makefile.build:254: drivers/usb/host/ehci-mx6.o] Error 1 I have to admit that maybe we should indeed run it with DM_REGULATOR but that won't change anything on this issue. I guess one should really not have any gotos to a label err_clk being ifdefed from somewhere not ifdefed. > + else > + priv->init_type = plat->init_type; > + } > + > #if CONFIG_IS_ENABLED(DM_REGULATOR) > ret = device_get_supply_regulator(dev, "vbus-supply", > &priv->vbus_supply); > diff --git a/include/usb.h b/include/usb.h > index b3851fdb4f..47d738a786 100644 > --- a/include/usb.h > +++ b/include/usb.h > @@ -163,7 +163,8 @@ struct int_queue; > */ > enum usb_init_type { > USB_INIT_HOST, > - USB_INIT_DEVICE > + USB_INIT_DEVICE, > + USB_INIT_UNKNOWN, > }; > > /**********************************************************************
On Tue, Jan 25, 2022 at 12:44 AM Marcel Ziswiler <marcel.ziswiler@toradex.com> wrote: > > On Mon, 2022-01-17 at 18:21 -0600, Adam Ford wrote: > > The imx8mm and imx8mn appear compatible with imx7d-usb > > flags in the OTG driver. If the dr_mode is defined as > > host or peripheral, the device appears to operate correctly, > > however the auto host/peripheral detection results in an error. > > > > The solution isn't just adding checks for imx8mm and imx8mn to > > the check for imx7, because the USB clock needs to be running > > to read from the USBNC_PHY_STATUS_OFFSET register or it will hang. > > > > Marek requested that I not enable the clocks in ehci_usb_of_to_plat, > > so I modified that function to return an unknown state if the > > device tree does not explicitly state whether it is a host > > or a peripheral. > > > > When the driver probes, it looks to see if it's in the unknown > > state, and only then will it read the register to auto-detect. > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > --- > > V3: Keep ehci_usb_of_to_plat but add the ability to return > > and unknown state instead of reading the register. > > If the probe determines the states is unknown, it will > > query the register after the clocks have been enabled. > > Because of the slight behavior change, I removed any > > review or tested tags. > > > > V2: Rename ehci_usb_of_to_plat to ehci_usb_dr_mode and call it > > from the probe after the clocks are enabled, but before > > the data is needed. > > > > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c > > index 1bd6147c76..cf44e53ff7 100644 > > --- a/drivers/usb/host/ehci-mx6.c > > +++ b/drivers/usb/host/ehci-mx6.c > > @@ -543,7 +543,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() || is_imx8mn()) { > > phy_status = (void __iomem *)(addr + > > USBNC_PHY_STATUS_OFFSET); > > val = readl(phy_status); > > @@ -573,9 +573,8 @@ static int ehci_usb_of_to_plat(struct udevice *dev) > > case USB_DR_MODE_PERIPHERAL: > > plat->init_type = USB_INIT_DEVICE; > > break; > > - case USB_DR_MODE_OTG: > > - case USB_DR_MODE_UNKNOWN: > > - return ehci_usb_phy_mode(dev); > > + default: > > + plat->init_type = USB_INIT_UNKNOWN; > > }; > > > > return 0; > > @@ -677,6 +676,20 @@ static int ehci_usb_probe(struct udevice *dev) > > mdelay(1); > > #endif > > > > + /* > > + * If the device tree didn't specify host or device, > > + * the default is USB_INIT_UNKNOWN, so we need to check > > + * the register. For imx8mm and imx8mn, the clocks need to be > > + * running first, so we defer the check until they are. > > + */ > > + if (priv->init_type == USB_INIT_UNKNOWN) { > > + ret = ehci_usb_phy_mode(dev); > > + if (ret) > > + goto err_clk; > > Hm, unfortunately, that label is gated by an ifdef leading to the following on colibri_imx7: > > drivers/usb/host/ehci-mx6.c: In function ‘ehci_usb_probe’: > drivers/usb/host/ehci-mx6.c:688:4: error: label ‘err_clk’ used but not defined > 688 | goto err_clk; > | ^~~~ > make[1]: *** [scripts/Makefile.build:254: drivers/usb/host/ehci-mx6.o] Error 1 > > I have to admit that maybe we should indeed run it with DM_REGULATOR but that won't change anything on this > issue. I guess one should really not have any gotos to a label err_clk being ifdefed from somewhere not > ifdefed. > Adam, Can you re-submit this with an ifdef to handle the correct goto when DM_REGULATOR is undefined? It looks like Fabio figured out his issue with his board and it was unrelated to this (fixed by 'mmc: fsl_esdhc_imx: fix watermark level in dma') so I think Marcel's point is the only blocker on this patch unless there is more discussion needed with Michael or some feedback needed from Marek? Best regards, Tim
On Thu, Feb 3, 2022 at 2:59 PM Tim Harvey <tharvey@gateworks.com> wrote: > > On Tue, Jan 25, 2022 at 12:44 AM Marcel Ziswiler > <marcel.ziswiler@toradex.com> wrote: > > > > On Mon, 2022-01-17 at 18:21 -0600, Adam Ford wrote: > > > The imx8mm and imx8mn appear compatible with imx7d-usb > > > flags in the OTG driver. If the dr_mode is defined as > > > host or peripheral, the device appears to operate correctly, > > > however the auto host/peripheral detection results in an error. > > > > > > The solution isn't just adding checks for imx8mm and imx8mn to > > > the check for imx7, because the USB clock needs to be running > > > to read from the USBNC_PHY_STATUS_OFFSET register or it will hang. > > > > > > Marek requested that I not enable the clocks in ehci_usb_of_to_plat, > > > so I modified that function to return an unknown state if the > > > device tree does not explicitly state whether it is a host > > > or a peripheral. > > > > > > When the driver probes, it looks to see if it's in the unknown > > > state, and only then will it read the register to auto-detect. > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > --- > > > V3: Keep ehci_usb_of_to_plat but add the ability to return > > > and unknown state instead of reading the register. > > > If the probe determines the states is unknown, it will > > > query the register after the clocks have been enabled. > > > Because of the slight behavior change, I removed any > > > review or tested tags. > > > > > > V2: Rename ehci_usb_of_to_plat to ehci_usb_dr_mode and call it > > > from the probe after the clocks are enabled, but before > > > the data is needed. > > > > > > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c > > > index 1bd6147c76..cf44e53ff7 100644 > > > --- a/drivers/usb/host/ehci-mx6.c > > > +++ b/drivers/usb/host/ehci-mx6.c > > > @@ -543,7 +543,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() || is_imx8mn()) { > > > phy_status = (void __iomem *)(addr + > > > USBNC_PHY_STATUS_OFFSET); > > > val = readl(phy_status); > > > @@ -573,9 +573,8 @@ static int ehci_usb_of_to_plat(struct udevice *dev) > > > case USB_DR_MODE_PERIPHERAL: > > > plat->init_type = USB_INIT_DEVICE; > > > break; > > > - case USB_DR_MODE_OTG: > > > - case USB_DR_MODE_UNKNOWN: > > > - return ehci_usb_phy_mode(dev); > > > + default: > > > + plat->init_type = USB_INIT_UNKNOWN; > > > }; > > > > > > return 0; > > > @@ -677,6 +676,20 @@ static int ehci_usb_probe(struct udevice *dev) > > > mdelay(1); > > > #endif > > > > > > + /* > > > + * If the device tree didn't specify host or device, > > > + * the default is USB_INIT_UNKNOWN, so we need to check > > > + * the register. For imx8mm and imx8mn, the clocks need to be > > > + * running first, so we defer the check until they are. > > > + */ > > > + if (priv->init_type == USB_INIT_UNKNOWN) { > > > + ret = ehci_usb_phy_mode(dev); > > > + if (ret) > > > + goto err_clk; > > > > Hm, unfortunately, that label is gated by an ifdef leading to the following on colibri_imx7: > > > > drivers/usb/host/ehci-mx6.c: In function ‘ehci_usb_probe’: > > drivers/usb/host/ehci-mx6.c:688:4: error: label ‘err_clk’ used but not defined > > 688 | goto err_clk; > > | ^~~~ > > make[1]: *** [scripts/Makefile.build:254: drivers/usb/host/ehci-mx6.o] Error 1 > > > > I have to admit that maybe we should indeed run it with DM_REGULATOR but that won't change anything on this > > issue. I guess one should really not have any gotos to a label err_clk being ifdefed from somewhere not > > ifdefed. > > > > Adam, > > Can you re-submit this with an ifdef to handle the correct goto when > DM_REGULATOR is undefined? > > It looks like Fabio figured out his issue with his board and it was > unrelated to this (fixed by 'mmc: fsl_esdhc_imx: fix watermark level > in dma') so I think Marcel's point is the only blocker on this patch > unless there is more discussion needed with Michael or some feedback > needed from Marek? Tim, Not a problem. I was waiting for feedback from Marek in case there were more issues, but I can fix the goto reference and resubmit. I'm build-testing the colibri_imx7 now. I'm just moving the 'err_clk' reference out of the #ifdef so the same reference can be used. It doesn't look like it should be inside the #ifdef anyway. You should see something shortly, and I'll add you to the CC list. adam > > Best regards, > > Tim
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index 1bd6147c76..cf44e53ff7 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -543,7 +543,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() || is_imx8mn()) { phy_status = (void __iomem *)(addr + USBNC_PHY_STATUS_OFFSET); val = readl(phy_status); @@ -573,9 +573,8 @@ static int ehci_usb_of_to_plat(struct udevice *dev) case USB_DR_MODE_PERIPHERAL: plat->init_type = USB_INIT_DEVICE; break; - case USB_DR_MODE_OTG: - case USB_DR_MODE_UNKNOWN: - return ehci_usb_phy_mode(dev); + default: + plat->init_type = USB_INIT_UNKNOWN; }; return 0; @@ -677,6 +676,20 @@ static int ehci_usb_probe(struct udevice *dev) mdelay(1); #endif + /* + * If the device tree didn't specify host or device, + * the default is USB_INIT_UNKNOWN, so we need to check + * the register. For imx8mm and imx8mn, the clocks need to be + * running first, so we defer the check until they are. + */ + if (priv->init_type == USB_INIT_UNKNOWN) { + ret = ehci_usb_phy_mode(dev); + if (ret) + goto err_clk; + else + priv->init_type = plat->init_type; + } + #if CONFIG_IS_ENABLED(DM_REGULATOR) ret = device_get_supply_regulator(dev, "vbus-supply", &priv->vbus_supply); diff --git a/include/usb.h b/include/usb.h index b3851fdb4f..47d738a786 100644 --- a/include/usb.h +++ b/include/usb.h @@ -163,7 +163,8 @@ struct int_queue; */ enum usb_init_type { USB_INIT_HOST, - USB_INIT_DEVICE + USB_INIT_DEVICE, + USB_INIT_UNKNOWN, }; /**********************************************************************
The imx8mm and imx8mn appear compatible with imx7d-usb flags in the OTG driver. If the dr_mode is defined as host or peripheral, the device appears to operate correctly, however the auto host/peripheral detection results in an error. The solution isn't just adding checks for imx8mm and imx8mn to the check for imx7, because the USB clock needs to be running to read from the USBNC_PHY_STATUS_OFFSET register or it will hang. Marek requested that I not enable the clocks in ehci_usb_of_to_plat, so I modified that function to return an unknown state if the device tree does not explicitly state whether it is a host or a peripheral. When the driver probes, it looks to see if it's in the unknown state, and only then will it read the register to auto-detect. Signed-off-by: Adam Ford <aford173@gmail.com> --- V3: Keep ehci_usb_of_to_plat but add the ability to return and unknown state instead of reading the register. If the probe determines the states is unknown, it will query the register after the clocks have been enabled. Because of the slight behavior change, I removed any review or tested tags. V2: Rename ehci_usb_of_to_plat to ehci_usb_dr_mode and call it from the probe after the clocks are enabled, but before the data is needed.