Hi Siddharth,
On 03/12/2024 11:37, Siddharth Vadapalli wrote:
There are only two callers of "dwc3_generic_probe()", namely:
- dwc3_generic_peripheral_probe()
- dwc3_generic_host_probe()
Currently, the "mode" is set based on the device-tree node of the platform device. Also, the DWC3 core doesn't support updating the "mode" dynamically at runtime if it is set to "OTG", i.e. "OTG" is treated as a separate mode in itself, rather than being treated as a mode which should eventually lead to "host"/"peripheral".
Actually this is not entirely true. Sorry for not catching this earlier. I did a deep dive today to debug why XHCI is being registered for both ports on am62.
I see that configs/am62x_a53_usbdfu.config is setting CONFIG_USB_XHCI_DWC3. This is wrong. It should not be used for AM62x platform.
Also.
Please see dwc3_glue_bind_common().
if (CONFIG_IS_ENABLED(DM_USB_GADGET) && (dr_mode == USB_DR_MODE_PERIPHERAL || dr_mode == USB_DR_MODE_OTG)) { printf("%s: dr_mode: OTG or Peripheral\n", __func__); driver = "dwc3-generic-peripheral"; } else if (CONFIG_IS_ENABLED(USB_HOST) && dr_mode == USB_DR_MODE_HOST) { printf("%s: dr_mode: HOST\n", __func__); driver = "dwc3-generic-host"; } else { printf("%s: unsupported dr_mode %d\n", __func__, dr_mode); return -ENODEV; }
ret = device_bind_driver_to_node(parent, driver, name, node, &dev); if (ret) {
So if dr_mode is "otg" it will be bound to dwc3-generic-peripheral driver only. I verified that even if I change dr_mode from "peripheral" to "otg" DFU mode does work from u-boot shell.
Which makes my suggestions for checking dr_mode during and your patch seem unnecessary.
What is not clear though is, why is DFU mode not working for you. At which stage DFU is failing? R5 SPL/A53 SPL?
Can you please try disabling CONFIG_USB_XHCI_DWC3 in all configs as it will hijack the controller exclusively for host mode.
Given that the callers of "dwc3_generic_probe()" clarify the expected "mode" of the USB Controller, use that "mode" instead of the one specified in the device-tree. This shall allow the USB Controller to function both as a "Host" and as a "Peripheral" when the "mode" is "otg" in the device-tree, based on the caller of "dwc3_generic_probe()".
Ideally, the USB ID pin should be used to determine whether or not the requested role can be enabled. However, that can be implemented in the future as an incremental feature over the current implementation.
Signed-off-by: Siddharth Vadapalli s-vadapalli@ti.com
v1: https://patchwork.ozlabs.org/project/uboot/list/?series=434253&state=%2A... Changes since v1:
- Based on the feedback received on the v1 series, the device-tree specified role is also taken into account in dwc3_generic_probe(), in addition to the caller of dwc3_generic_probe().
drivers/usb/dwc3/dwc3-generic.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c index 2ab41cbae45..fe98b50c42c 100644 --- a/drivers/usb/dwc3/dwc3-generic.c +++ b/drivers/usb/dwc3/dwc3-generic.c @@ -51,7 +51,8 @@ struct dwc3_generic_host_priv { };
static int dwc3_generic_probe(struct udevice *dev,
struct dwc3_generic_priv *priv)
struct dwc3_generic_priv *priv,enum usb_dr_mode mode)
This is not necessary as dwc3_generic_host_probe will only be registered if dr_mode == "host" and dwc3_generic_peripheral_probe will only be registered if dr_mode == "host" or "peripheral"
{ int rc; struct dwc3_generic_plat *plat = dev_get_plat(dev); @@ -62,7 +63,19 @@ static int dwc3_generic_probe(struct udevice *dev,
dwc3->dev = dev; dwc3->maximum_speed = plat->maximum_speed;
- dwc3->dr_mode = plat->dr_mode;
- /*
* If the controller supports OTG as indicated by plat->dr_mode,* then either Host or Peripheral mode is acceptable.* Otherwise, error out since the platform cannot support the mode* being requested by the caller of dwc3_generic_probe().*/- if (plat->dr_mode != mode && plat->dr_mode != USB_DR_MODE_OTG) {
pr_err("Requested usb mode is not supported by platform\n");
This is actually not necessary. Sorry for suggesting this approach before.
return -EINVAL;- }
- dwc3->dr_mode = mode;
#if CONFIG_IS_ENABLED(OF_CONTROL) dwc3_of_parse(dwc3);
@@ -197,7 +210,7 @@ static int dwc3_generic_peripheral_probe(struct udevice *dev) { struct dwc3_generic_priv *priv = dev_get_priv(dev);
- return dwc3_generic_probe(dev, priv);
- return dwc3_generic_probe(dev, priv, USB_DR_MODE_PERIPHERAL);
}
static int dwc3_generic_peripheral_remove(struct udevice *dev) @@ -241,7 +254,7 @@ static int dwc3_generic_host_probe(struct udevice *dev) struct dwc3_generic_host_priv *priv = dev_get_priv(dev); int rc;
- rc = dwc3_generic_probe(dev, &priv->gen_priv);
- rc = dwc3_generic_probe(dev, &priv->gen_priv, USB_DR_MODE_HOST); if (rc) return rc;