[PATCH v2 0/2] usb: dwc3: Add support for standalone DWC3 nodes

newer
[GIT PULL] Please pull u-boot-pmic...

Alban Bedel

16 Jun 2022 16 Jun '22

10:47 a.m.

Hi all,

Sometimes ago I 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

-- v2: - Rebased onto current master - Fixed a typo the log message

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(-)

-- 2.34.1

Attachments:

smime.p7s (application/x-pkcs7-signature — 3.5 KB)

Show replies by date

Alban Bedel

16 Jun 16 Jun

10:47 a.m.

New subject: [PATCH v2 1/2] Revert "usb: dwc3: dwc3-generic: check the parent nodes"

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 based 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 c5310e465cbf..38367ea0f116 100644 --- a/drivers/usb/dwc3/dwc3-generic.c +++ b/drivers/usb/dwc3/dwc3-generic.c @@ -111,12 +111,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) { @@ -126,13 +121,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; @@ -363,20 +353,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: @@ -536,7 +522,6 @@ static const struct udevice_id dwc3_glue_ids[] = { { .compatible = "rockchip,rk3399-dwc3" }, { .compatible = "qcom,dwc3" }, { .compatible = "fsl,imx8mp-dwc3", .data = (ulong)&imx8mp_ops }, - { .compatible = "fsl,imx8mq-dwc3" }, { .compatible = "intel,tangier-dwc3" }, { } };

-- 2.34.1

Alban Bedel

10:47 a.m.

New subject: [PATCH v2 2/2] usb: dwc3: Add support for standalone DWC3 nodes

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 38367ea0f116..31858ba4c81b 100644 --- a/drivers/usb/dwc3/dwc3-generic.c +++ b/drivers/usb/dwc3/dwc3-generic.c @@ -349,50 +349,64 @@ struct dwc3_glue_ops ti_ops = { .glue_configure = dwc3_ti_glue_configure, };

+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; @@ -511,6 +525,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" }, @@ -526,8 +553,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,

-- 2.34.1

Angus Ainslie

6:19 p.m.

Hi Alban,

On 2022-06-16 01:47, Alban Bedel wrote:

...

Hi all,

Sometimes ago I 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.

I tested this on my imx8mq target and it works for SDP and the USB works in Linux.

If I try and use host mode in u-boot that no longer works.

This is a snip from running "dm tree" and the dwc3-generic-host gets identified but the driver isn't bound.

The host controller also doesn't get found when using "usb start"

u-boot=> usb start starting USB... Bus usb@38200000: dwc3-generic-host usb@38200000: this is not a DesignWare USB3 DRD Core dwc3-generic-host usb@38200000: failed to initialize core Port not available.

Thanks Angus

...

Alban

-- v2: - Rebased onto current master

...

- Fixed a typo the log message
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(-)

1060

Age (days ago)

1060

Last active (days ago)

List overview

Download

3 comments

2 participants

tags (0)

participants (2)

Alban Bedel
Angus Ainslie