[PATCH 1/3] usb: ehci-mx6: move mode set/detect to probe
There is no need to set and/or detect mode in of_to_plat and accessing phy registers at that point before device power domain and clock are enabled will cause hangs on platforms such as IMX8M Mini.
Move the mode set/detect from of_to_plat into the probe and remove the unnecessary of_to_plat.
Signed-off-by: Tim Harvey tharvey@gateworks.com --- drivers/usb/host/ehci-mx6.c | 42 ++++++++++++++----------------------- 1 file changed, 16 insertions(+), 26 deletions(-)
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index 06be9deaaa..c2dfe49012 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -539,28 +539,6 @@ static int ehci_usb_phy_mode(struct udevice *dev) return 0; }
-static int ehci_usb_of_to_plat(struct udevice *dev) -{ - struct usb_plat *plat = dev_get_plat(dev); - enum usb_dr_mode dr_mode; - - dr_mode = usb_get_dr_mode(dev_ofnode(dev)); - - switch (dr_mode) { - case USB_DR_MODE_HOST: - plat->init_type = USB_INIT_HOST; - break; - 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); - }; - - return 0; -} - static int mx6_parse_dt_addrs(struct udevice *dev) { #if !defined(CONFIG_PHY) @@ -623,7 +601,6 @@ static int ehci_usb_probe(struct udevice *dev) struct usb_plat *plat = dev_get_plat(dev); struct usb_ehci *ehci = dev_read_addr_ptr(dev); struct ehci_mx6_priv_data *priv = dev_get_priv(dev); - enum usb_init_type type = plat->init_type; struct ehci_hccr *hccr; struct ehci_hcor *hcor; int ret; @@ -641,7 +618,6 @@ static int ehci_usb_probe(struct udevice *dev) return ret;
priv->ehci = ehci; - priv->init_type = type;
#if CONFIG_IS_ENABLED(CLK) ret = clk_get_by_index(dev, 0, &priv->clk); @@ -657,6 +633,21 @@ static int ehci_usb_probe(struct udevice *dev) mdelay(1); #endif
+ switch (usb_get_dr_mode(dev_ofnode(dev))) { + case USB_DR_MODE_HOST: + plat->init_type = USB_INIT_HOST; + break; + case USB_DR_MODE_PERIPHERAL: + plat->init_type = USB_INIT_DEVICE; + break; + case USB_DR_MODE_OTG: + case USB_DR_MODE_UNKNOWN: + 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); @@ -680,7 +671,7 @@ static int ehci_usb_probe(struct udevice *dev) #if CONFIG_IS_ENABLED(DM_REGULATOR) if (priv->vbus_supply) { ret = regulator_set_enable(priv->vbus_supply, - (type == USB_INIT_DEVICE) ? + (priv->init_type == USB_INIT_DEVICE) ? false : true); if (ret && ret != -ENOSYS) { printf("Error enabling VBUS supply (ret=%i)\n", ret); @@ -764,7 +755,6 @@ U_BOOT_DRIVER(usb_mx6) = { .name = "ehci_mx6", .id = UCLASS_USB, .of_match = mx6_usb_ids, - .of_to_plat = ehci_usb_of_to_plat, .probe = ehci_usb_probe, .remove = ehci_usb_remove, .ops = &ehci_usb_ops,
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()) { phy_status = (void __iomem *)(addr + USBNC_PHY_STATUS_OFFSET); val = readl(phy_status);
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
Enable USB support for host controller and various USB ethernet devices.
Signed-off-by: Tim Harvey tharvey@gateworks.com --- configs/imx8mm_venice_defconfig | 15 +++++++++++++++ include/configs/imx8mm_venice.h | 4 ++++ 2 files changed, 19 insertions(+)
diff --git a/configs/imx8mm_venice_defconfig b/configs/imx8mm_venice_defconfig index 01bf2f362a..0d16f8d1c0 100644 --- a/configs/imx8mm_venice_defconfig +++ b/configs/imx8mm_venice_defconfig @@ -48,6 +48,7 @@ CONFIG_CMD_FUSE=y CONFIG_CMD_GPIO=y CONFIG_CMD_I2C=y CONFIG_CMD_MMC=y +CONFIG_CMD_USB=y CONFIG_CMD_CACHE=y CONFIG_CMD_TIME=y CONFIG_CMD_UUID=y @@ -92,6 +93,8 @@ CONFIG_MII=y CONFIG_PINCTRL=y CONFIG_SPL_PINCTRL=y CONFIG_PINCTRL_IMX8M=y +CONFIG_POWER_DOMAIN=y +CONFIG_IMX8M_POWER_DOMAIN=y CONFIG_DM_PMIC=y CONFIG_DM_PMIC_BD71837=y CONFIG_SPL_DM_PMIC_BD71837=y @@ -107,5 +110,17 @@ CONFIG_SYSRESET_PSCI=y CONFIG_SYSRESET_WATCHDOG=y CONFIG_DM_THERMAL=y CONFIG_IMX_TMU=y +CONFIG_USB=y +CONFIG_DM_USB=y +# CONFIG_SPL_DM_USB is not set +CONFIG_USB_EHCI_HCD=y +CONFIG_USB_HOST_ETHER=y +CONFIG_USB_ETHER_ASIX=y +CONFIG_USB_ETHER_ASIX88179=y +CONFIG_USB_ETHER_LAN75XX=y +CONFIG_USB_ETHER_LAN78XX=y +CONFIG_USB_ETHER_MCS7830=y +CONFIG_USB_ETHER_RTL8152=y +CONFIG_USB_ETHER_SMSC95XX=y CONFIG_IMX_WATCHDOG=y CONFIG_HEXDUMP=y diff --git a/include/configs/imx8mm_venice.h b/include/configs/imx8mm_venice.h index 91669255e1..5e9f4c89d1 100644 --- a/include/configs/imx8mm_venice.h +++ b/include/configs/imx8mm_venice.h @@ -99,6 +99,10 @@ /* UART */ #define CONFIG_MXC_UART_BASE UART2_BASE_ADDR
+/* USB */ +#define CONFIG_EHCI_HCD_INIT_AFTER_RESET +#define CONFIG_USB_MAX_CONTROLLER_COUNT 2 + /* Monitor Command Prompt */ #define CONFIG_SYS_PROMPT_HUSH_PS2 "> " #define CONFIG_SYS_CBSIZE SZ_2K
On 4/27/21 7:08 PM, Tim Harvey wrote:
There is no need to set and/or detect mode in of_to_plat and accessing phy registers at that point before device power domain and clock are enabled will cause hangs on platforms such as IMX8M Mini.
Move the mode set/detect from of_to_plat into the probe and remove the unnecessary of_to_plat.
Signed-off-by: Tim Harvey tharvey@gateworks.com
drivers/usb/host/ehci-mx6.c | 42 ++++++++++++++----------------------- 1 file changed, 16 insertions(+), 26 deletions(-)
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index 06be9deaaa..c2dfe49012 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -539,28 +539,6 @@ static int ehci_usb_phy_mode(struct udevice *dev) return 0; }
-static int ehci_usb_of_to_plat(struct udevice *dev) -{
- struct usb_plat *plat = dev_get_plat(dev);
- enum usb_dr_mode dr_mode;
- dr_mode = usb_get_dr_mode(dev_ofnode(dev));
- switch (dr_mode) {
- case USB_DR_MODE_HOST:
plat->init_type = USB_INIT_HOST;break;- 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);- };
- return 0;
-}
[...]
@@ -764,7 +755,6 @@ U_BOOT_DRIVER(usb_mx6) = { .name = "ehci_mx6", .id = UCLASS_USB, .of_match = mx6_usb_ids,
- .of_to_plat = ehci_usb_of_to_plat,
I wonder why it was implemented in of_to_plat originally , maybe there is some reason for that ?
On Tue, Apr 27, 2021 at 10:45 AM Marek Vasut marex@denx.de wrote:
On 4/27/21 7:08 PM, Tim Harvey wrote:
There is no need to set and/or detect mode in of_to_plat and accessing phy registers at that point before device power domain and clock are enabled will cause hangs on platforms such as IMX8M Mini.
Move the mode set/detect from of_to_plat into the probe and remove the unnecessary of_to_plat.
Signed-off-by: Tim Harvey tharvey@gateworks.com
drivers/usb/host/ehci-mx6.c | 42 ++++++++++++++----------------------- 1 file changed, 16 insertions(+), 26 deletions(-)
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index 06be9deaaa..c2dfe49012 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -539,28 +539,6 @@ static int ehci_usb_phy_mode(struct udevice *dev) return 0; }
-static int ehci_usb_of_to_plat(struct udevice *dev) -{
struct usb_plat *plat = dev_get_plat(dev);enum usb_dr_mode dr_mode;dr_mode = usb_get_dr_mode(dev_ofnode(dev));switch (dr_mode) {case USB_DR_MODE_HOST:plat->init_type = USB_INIT_HOST;break;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);};return 0;-}
[...]
@@ -764,7 +755,6 @@ U_BOOT_DRIVER(usb_mx6) = { .name = "ehci_mx6", .id = UCLASS_USB, .of_match = mx6_usb_ids,
.of_to_plat = ehci_usb_of_to_plat,I wonder why it was implemented in of_to_plat originally , maybe there is some reason for that ?
Marek,
Looking back the commit that added the ehci_usb_ofdata_to_platdata was: cccbddc38c43 ("usb: ehci-mx6: implement ofdata_to_platdata")
Before that there was a board-specific function that would set the usb_plat->init_type.
The only reason to set usb_plat->init_type in of_to_plat would be so that drivers/usb/host/usb-uclass.c would have knowledge of it but I only see that it is set there in usb_setup_ehci_gadget.
I added Peng, Stefano, and Simon to the thread to see if they see an issue with doing away with of_to_plat setting the usb_plat->init_type prior to probe.
Tim
On 4/28/21 3:51 AM, Tim Harvey wrote:
On Tue, Apr 27, 2021 at 10:45 AM Marek Vasut marex@denx.de wrote:
On 4/27/21 7:08 PM, Tim Harvey wrote:
There is no need to set and/or detect mode in of_to_plat and accessing phy registers at that point before device power domain and clock are enabled will cause hangs on platforms such as IMX8M Mini.
Move the mode set/detect from of_to_plat into the probe and remove the unnecessary of_to_plat.
Signed-off-by: Tim Harvey tharvey@gateworks.com
drivers/usb/host/ehci-mx6.c | 42 ++++++++++++++----------------------- 1 file changed, 16 insertions(+), 26 deletions(-)
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index 06be9deaaa..c2dfe49012 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -539,28 +539,6 @@ static int ehci_usb_phy_mode(struct udevice *dev) return 0; }
-static int ehci_usb_of_to_plat(struct udevice *dev) -{
struct usb_plat *plat = dev_get_plat(dev);enum usb_dr_mode dr_mode;dr_mode = usb_get_dr_mode(dev_ofnode(dev));switch (dr_mode) {case USB_DR_MODE_HOST:plat->init_type = USB_INIT_HOST;break;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);};return 0;-}
[...]
@@ -764,7 +755,6 @@ U_BOOT_DRIVER(usb_mx6) = { .name = "ehci_mx6", .id = UCLASS_USB, .of_match = mx6_usb_ids,
.of_to_plat = ehci_usb_of_to_plat,I wonder why it was implemented in of_to_plat originally , maybe there is some reason for that ?
Marek,
Looking back the commit that added the ehci_usb_ofdata_to_platdata was: cccbddc38c43 ("usb: ehci-mx6: implement ofdata_to_platdata")
Before that there was a board-specific function that would set the usb_plat->init_type.
The only reason to set usb_plat->init_type in of_to_plat would be so that drivers/usb/host/usb-uclass.c would have knowledge of it but I only see that it is set there in usb_setup_ehci_gadget.
So interaction with Gadget, that's what I was afraid of.
I added Peng, Stefano, and Simon to the thread to see if they see an issue with doing away with of_to_plat setting the usb_plat->init_type prior to probe.
I added Lukasz too.
On Tue, Apr 27, 2021 at 6:59 PM Marek Vasut marex@denx.de wrote:
On 4/28/21 3:51 AM, Tim Harvey wrote:
On Tue, Apr 27, 2021 at 10:45 AM Marek Vasut marex@denx.de wrote:
On 4/27/21 7:08 PM, Tim Harvey wrote:
There is no need to set and/or detect mode in of_to_plat and accessing phy registers at that point before device power domain and clock are enabled will cause hangs on platforms such as IMX8M Mini.
Move the mode set/detect from of_to_plat into the probe and remove the unnecessary of_to_plat.
Signed-off-by: Tim Harvey tharvey@gateworks.com
drivers/usb/host/ehci-mx6.c | 42 ++++++++++++++----------------------- 1 file changed, 16 insertions(+), 26 deletions(-)
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index 06be9deaaa..c2dfe49012 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -539,28 +539,6 @@ static int ehci_usb_phy_mode(struct udevice *dev) return 0; }
-static int ehci_usb_of_to_plat(struct udevice *dev) -{
struct usb_plat *plat = dev_get_plat(dev);enum usb_dr_mode dr_mode;dr_mode = usb_get_dr_mode(dev_ofnode(dev));switch (dr_mode) {case USB_DR_MODE_HOST:plat->init_type = USB_INIT_HOST;break;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);};return 0;-}
[...]
@@ -764,7 +755,6 @@ U_BOOT_DRIVER(usb_mx6) = { .name = "ehci_mx6", .id = UCLASS_USB, .of_match = mx6_usb_ids,
.of_to_plat = ehci_usb_of_to_plat,I wonder why it was implemented in of_to_plat originally , maybe there is some reason for that ?
Marek,
Looking back the commit that added the ehci_usb_ofdata_to_platdata was: cccbddc38c43 ("usb: ehci-mx6: implement ofdata_to_platdata")
Before that there was a board-specific function that would set the usb_plat->init_type.
The only reason to set usb_plat->init_type in of_to_plat would be so that drivers/usb/host/usb-uclass.c would have knowledge of it but I only see that it is set there in usb_setup_ehci_gadget.
So interaction with Gadget, that's what I was afraid of.
I added Peng, Stefano, and Simon to the thread to see if they see an issue with doing away with of_to_plat setting the usb_plat->init_type prior to probe.
I added Lukasz too.
Marek,
Is there something you want me to change here?
I am happy to test gadget support as my board has an OTG connector. Could you give me some pointers on how to configure a gadget in U-Boot in order to test?
Best regards,
Tim
Hi Tim.
On Thu, Jun 3, 2021 at 12:50 PM Tim Harvey tharvey@gateworks.com wrote:
Marek,
Is there something you want me to change here?
I am happy to test gadget support as my board has an OTG connector. Could you give me some pointers on how to configure a gadget in U-Boot in order to test?
One quick way to test gadget support is via the "ums 0 mmc 0" command.
This will mount the eMMC content via USB mass storage device on the PC.
You can check configs/warp7_defconfig for an example on how to select the USB gadget options.
I applied this one and tested on a imx7s-warp board via "ums 0 mmc 0".
Tested-by: Fabio Estevam festevam@gmail.com
I am interested in getting USB gadget support in imx8mm too.
Thanks
On Thu, Jul 1, 2021 at 5:41 PM Fabio Estevam festevam@gmail.com wrote:
Hi Tim.
On Thu, Jun 3, 2021 at 12:50 PM Tim Harvey tharvey@gateworks.com wrote:
Marek,
Is there something you want me to change here?
I am happy to test gadget support as my board has an OTG connector. Could you give me some pointers on how to configure a gadget in U-Boot in order to test?
One quick way to test gadget support is via the "ums 0 mmc 0" command.
This will mount the eMMC content via USB mass storage device on the PC.
You can check configs/warp7_defconfig for an example on how to select the USB gadget options.
I applied this one and tested on a imx7s-warp board via "ums 0 mmc 0".
Tested-by: Fabio Estevam festevam@gmail.com
I am interested in getting USB gadget support in imx8mm too.
Fabio,
Right - I finally remembered that earlier today and just sent a v2 of this series showing that I indeed tested that on both imx6q and imx8mm. When you get a chance please show your Tested-by on that thread.
Tim
participants (4)
-
Fabio Estevam -
Marek Vasut -
Patrick Wildt -
Tim Harvey