[PATCH 0/2] usb: dwc3: Add support for standalone DWC3 nodes
Hi all,
I recently submitted a patch to fix the support for the DWC3 controller on the imx8mq which, unlike most DWC3 implementation, doesn't use a top glue node with child DWC3 nodes. Instead it has the DWC3 node directly on the main bus.
Angus Ainslie then asked why this patch was needed as he had submitted the original support for the imx8mq. Looking into the issue it turned out that Angus patch basically let the driver use the `port` subnodes, which are there to define the connection to a type C connector, as DWC3 nodes. As the board I'm working on has no type C connecor, hence no `port` subnodes the driver just did nothing in my case.
This new series replace my previous patch (usb: dwc3-generic: Fix the iMX8MQ support). It starts by reverting Angus patch as it was not following the DT binding and then add support for generic DWC3 without glue node. This fix the imx8mq case and might add support for a few other SoC at the same time.
Alban
Alban Bedel (2): Revert "usb: dwc3: dwc3-generic: check the parent nodes" usb: dwc3: Add support for standalone DWC3 nodes
drivers/usb/dwc3/dwc3-generic.c | 124 +++++++++++++++++--------------- 1 file changed, 68 insertions(+), 56 deletions(-)
This reverts commit c08db05455bcb2259849a096acf2e90cce258849.
All the devices supported by this driver use a top glue node with DWC3 devices a subnodes. The imx8mq has no glue device, the DWC3 is directly on the parent bus. But the DWC3 node might have `port` subnode to define the connection to type C connectors.
The code added by commit c08db05455bc ("usb: dwc3: dwc3-generic: check the parent nodes") would let the driver interpret the `port` subnode as DWC3 nodes, which lead to the need to add code to lookup properties in the parent node. As this code is base on a missunderstanding of the binding let's just revert it, a following commit will add support for standalone DWC3 devices.
Signed-off-by: Alban Bedel alban.bedel@aerq.com --- drivers/usb/dwc3/dwc3-generic.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c index 01bd0ca190e3..8d53ba779024 100644 --- a/drivers/usb/dwc3/dwc3-generic.c +++ b/drivers/usb/dwc3/dwc3-generic.c @@ -110,12 +110,7 @@ static int dwc3_generic_of_to_plat(struct udevice *dev) struct dwc3_generic_plat *plat = dev_get_plat(dev); ofnode node = dev_ofnode(dev);
- if (!strncmp(dev->name, "port", 4) || !strncmp(dev->name, "hub", 3)) { - /* This is a leaf so check the parent */ - plat->base = dev_read_addr(dev->parent); - } else { - plat->base = dev_read_addr(dev); - } + plat->base = dev_read_addr(dev);
plat->maximum_speed = usb_get_maximum_speed(node); if (plat->maximum_speed == USB_SPEED_UNKNOWN) { @@ -125,13 +120,8 @@ static int dwc3_generic_of_to_plat(struct udevice *dev)
plat->dr_mode = usb_get_dr_mode(node); if (plat->dr_mode == USB_DR_MODE_UNKNOWN) { - /* might be a leaf so check the parent for mode */ - node = dev_ofnode(dev->parent); - plat->dr_mode = usb_get_dr_mode(node); - if (plat->dr_mode == USB_DR_MODE_UNKNOWN) { - pr_err("Invalid usb mode setup\n"); - return -ENODEV; - } + pr_err("Invalid usb mode setup\n"); + return -ENODEV; }
return 0; @@ -311,20 +301,16 @@ static int dwc3_glue_bind(struct udevice *parent) { ofnode node; int ret; - enum usb_dr_mode dr_mode; - - dr_mode = usb_get_dr_mode(dev_ofnode(parent));
ofnode_for_each_subnode(node, dev_ofnode(parent)) { const char *name = ofnode_get_name(node); + enum usb_dr_mode dr_mode; struct udevice *dev; const char *driver = NULL;
debug("%s: subnode name: %s\n", __func__, name);
- /* if the parent node doesn't have a mode check the leaf */ - if (!dr_mode) - dr_mode = usb_get_dr_mode(node); + dr_mode = usb_get_dr_mode(node);
switch (dr_mode) { case USB_DR_MODE_PERIPHERAL: @@ -464,7 +450,6 @@ static const struct udevice_id dwc3_glue_ids[] = { { .compatible = "rockchip,rk3328-dwc3" }, { .compatible = "rockchip,rk3399-dwc3" }, { .compatible = "qcom,dwc3" }, - { .compatible = "fsl,imx8mq-dwc3" }, { .compatible = "intel,tangier-dwc3" }, { } };
Most DWC3 implementations use a top glue node with DWC3 subnodes, which is what the dwc3-generic-wrapper driver support. But there is also implementations where the DWC3 node is directly on the main bus, like on the imx8mq.
To support this case split the handling of each DWC3 node out of dwc3_glue_bind() to make it reusable and add a new driver to handle standalone DWC3 nodes.
While at it also rename the dwc3-generic-wrapper to dwc3-glue-wrapper to better reflect what it support and the general naming in the driver.
Signed-off-by: Alban Bedel alban.bedel@aerq.com --- drivers/usb/dwc3/dwc3-generic.c | 103 ++++++++++++++++++++------------ 1 file changed, 65 insertions(+), 38 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c index 8d53ba779024..e554a3d60360 100644 --- a/drivers/usb/dwc3/dwc3-generic.c +++ b/drivers/usb/dwc3/dwc3-generic.c @@ -297,50 +297,64 @@ struct dwc3_glue_ops ti_ops = { .select_dr_mode = dwc3_ti_select_dr_mode, };
+static int dwc3_bind_to_node(struct udevice *parent, ofnode node) +{ + const char *name = ofnode_get_name(node); + enum usb_dr_mode dr_mode; + struct udevice *dev; + const char *driver = NULL; + int ret; + + debug("%s: subnode name: %s\n", __func__, name); + + dr_mode = usb_get_dr_mode(node); + + switch (dr_mode) { + case USB_DR_MODE_PERIPHERAL: + case USB_DR_MODE_OTG: +#if CONFIG_IS_ENABLED(DM_USB_GADGET) + debug("%s: dr_mode: OTG or Peripheral\n", __func__); + driver = "dwc3-generic-peripheral"; +#endif + break; +#if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD) + case USB_DR_MODE_HOST: + debug("%s: dr_mode: HOST\n", __func__); + driver = "dwc3-generic-host"; + break; +#endif + default: + debug("%s: unsupported dr_mode\n", __func__); + return -ENODEV; + }; + + if (!driver) + return -ENOTSUPP; + + ret = device_bind_driver_to_node(parent, driver, name, + node, &dev); + if (ret) + debug("%s: not able to bind usb device mode\n", + __func__); + return ret; +} + +static int dwc3_bind(struct udevice *parent) +{ + return dwc3_bind_to_node(parent, dev_ofnode(parent)); +} + static int dwc3_glue_bind(struct udevice *parent) { ofnode node; int ret;
ofnode_for_each_subnode(node, dev_ofnode(parent)) { - const char *name = ofnode_get_name(node); - enum usb_dr_mode dr_mode; - struct udevice *dev; - const char *driver = NULL; - - debug("%s: subnode name: %s\n", __func__, name); - - dr_mode = usb_get_dr_mode(node); - - switch (dr_mode) { - case USB_DR_MODE_PERIPHERAL: - case USB_DR_MODE_OTG: -#if CONFIG_IS_ENABLED(DM_USB_GADGET) - debug("%s: dr_mode: OTG or Peripheral\n", __func__); - driver = "dwc3-generic-peripheral"; -#endif - break; -#if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD) - case USB_DR_MODE_HOST: - debug("%s: dr_mode: HOST\n", __func__); - driver = "dwc3-generic-host"; - break; -#endif - default: - debug("%s: unsupported dr_mode\n", __func__); - return -ENODEV; - }; - - if (!driver) + ret = dwc3_bind_to_node(parent, node); + if (ret == -ENOTSUPP) continue; - - ret = device_bind_driver_to_node(parent, driver, name, - node, &dev); - if (ret) { - debug("%s: not able to bind usb device mode\n", - __func__); + if (ret) return ret; - } }
return 0; @@ -440,6 +454,19 @@ static int dwc3_glue_remove(struct udevice *dev) return 0; }
+static const struct udevice_id dwc3_ids[] = { + { .compatible = "snps,dwc3" }, + { .compatible = "synopsys,dwc3" }, + { } +}; + +U_BOOT_DRIVER(dwc3_wrapper) = { + .name = "dwc3-wrapper", + .id = UCLASS_NOP, + .of_match = dwc3_ids, + .bind = dwc3_bind, +}; + static const struct udevice_id dwc3_glue_ids[] = { { .compatible = "xlnx,zynqmp-dwc3" }, { .compatible = "xlnx,versal-dwc3" }, @@ -454,8 +481,8 @@ static const struct udevice_id dwc3_glue_ids[] = { { } };
-U_BOOT_DRIVER(dwc3_generic_wrapper) = { - .name = "dwc3-generic-wrapper", +U_BOOT_DRIVER(dwc3_glue_wrapper) = { + .name = "dwc3-glue-wrapper", .id = UCLASS_NOP, .of_match = dwc3_glue_ids, .bind = dwc3_glue_bind,
participants (1)
-
Alban Bedel