On Tue, 2022-04-19 at 07:34 -0700, Angus Ainslie wrote:

Hi Alban,

On 2022-04-19 01:07, Alban Bedel wrote:

...

The binding of iMX8MQ USB controller doesn't use child nodes like the other devices supported in this driver. To support it split the child nodes parsing to its own function and add a field to the platform data to indicate that we should just use the top node.

I'm not clear on what this is fixing. Doesn't the original code deal with a devicetree stanza that has the information in either the node or the parent.

Which case does this fix ?

It fix the case where there is no top glue node and only a standalone node. The driver currently do nothing in this case as it only act on the sub nodes.

Now looking at the binding[1] we could as well just replace "fsl,imx8mq-dwc3" with "snps,dwc3" in the OF match list as nothing specific to the iMX8MQ is done in the code.

Regards, Alban

1: https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindi...

Thanks Angus

...

Signed-off-by: Alban Bedel alban.bedel@aerq.com

drivers/usb/dwc3/dwc3-generic.c | 95 +++++++++++++++++++------------

1 file changed, 56 insertions(+), 39 deletions(-)

diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c index 01bd0ca190e3..defef43ff503 100644 --- a/drivers/usb/dwc3/dwc3-generic.c +++ b/drivers/usb/dwc3/dwc3-generic.c @@ -221,6 +221,7 @@ U_BOOT_DRIVER(dwc3_generic_host) = {  struct dwc3_glue_ops {         void (*select_dr_mode)(struct udevice *dev, int index,                                enum usb_dr_mode mode); +       int single_node;  };

void dwc3_ti_select_dr_mode(struct udevice *dev, int index, @@ -307,54 +308,66 @@ struct dwc3_glue_ops ti_ops = {         .select_dr_mode = dwc3_ti_select_dr_mode,  };

+static int dwc3_glue_bind_node(struct udevice *parent, ofnode node, +                              enum usb_dr_mode dr_mode) +{ +       const char *name = ofnode_get_name(node); +       const char *driver = NULL; +       struct udevice *dev; +       int ret;

+       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);

+       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 0;

+       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_glue_bind(struct udevice *parent)  { +       struct dwc3_glue_ops *ops = (struct dwc3_glue_ops *)dev_get_driver_data(parent);         ofnode node;         int ret;         enum usb_dr_mode dr_mode;

+       if (ops->single_node) +               return dwc3_glue_bind_node(parent, dev_ofnode(parent), 0);

dr_mode = usb_get_dr_mode(dev_ofnode(parent));

ofnode_for_each_subnode(node, dev_ofnode(parent)) { -               const char *name = ofnode_get_name(node); -               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);

-               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) -                       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__); +               ret = dwc3_glue_bind_node(parent, node, dr_mode); +               if (ret)                         return ret; -               }         }

return 0; @@ -454,6 +467,10 @@ static int dwc3_glue_remove(struct udevice *dev)         return 0;  }

+struct dwc3_glue_ops single_node_ops = { +       .single_node = 1, +};

static const struct udevice_id dwc3_glue_ids[] = {         { .compatible = "xlnx,zynqmp-dwc3" },         { .compatible = "xlnx,versal-dwc3" }, @@ -464,7 +481,7 @@ static const struct udevice_id dwc3_glue_ids[] = {         { .compatible = "rockchip,rk3328-dwc3" },         { .compatible = "rockchip,rk3399-dwc3" },         { .compatible = "qcom,dwc3" }, -       { .compatible = "fsl,imx8mq-dwc3" }, +       { .compatible = "fsl,imx8mq-dwc3", .data = (ulong)&single_node_ops },         { .compatible = "intel,tangier-dwc3" },         { }  };