[PATCH 0/2] AM62A DWC3: Add support for USB DFU boot in OTG mode
Hello,
This series adds support for USB DFU boot on TI's AM62A SoC which has two instances of DWC3 USB Controllers namely USB0 and USB1. The USB0 instance of the USB Controller supports USB DFU boot: ROM => tiboot3.bin => tispl.bin => u-boot.img
USB DFU Boot requires the USB Controller to be configured for Gadget mode of operation. Since the USB0 instance of the DWC3 USB Controller supports both Host and Gadget modes of operation via the Type-C interface on the AM62A7-SK board, the device-tree specifies the "dr_mode" as "OTG". However, there is currently no support for dynamically switching the "mode" from Host to Gadget and vice-versa with the help of a state-machine. The OTG mode is treated as a separate mode in itself rather than being treated as an intermediate stage before assuming the Host/Gadget mode. Due to this, USB DFU boot via the Type-C interface doesn't work as the USB Controller hasn't been appropriately configured for Device/Gadget mode of operation. One option is to change the device-tree to specify "dr_mode" as "peripheral" and force the controller to assume the Device role. This will imply that the U-Boot device-tree for AM62A diverges from its Linux counterpart. Therefore, with the intent of keeping the device-tree uniform across Linux and U-Boot, and at the same time, in order to enable USB DFU boot in "OTG" mode with the DWC3 Controller, the first patch in this series sets the "mode" on the basis of the caller function, rather than using the "dr_mode" property in the device-tree. There are only two callers of "dwc3_generic_probe()", each of which clearly specify the expected mode of configuration. This will enable both Host and Device mode of operation based on the command executed by the user, thereby truly supporting "OTG" functionality when the USB Controller supports it.
The second patch in this series adds USB DFU environment for AM62A, enabling USB DFU Boot and USB DFU flash on AM62A.
In addition to the patches in this series, the following device-tree changes will be required to test USB DFU on AM62A (bootph-all property to be added to ensure that USB Controller is present at all stages for DFU Boot): https://gist.github.com/Siddharth-Vadapalli-at-TI/53ba02cb0ff4a09c47e920d082... The above device-tree changes will be made to the Linux device-tree, which shall ensure that the same shall be a part of U-Boot device-tree eventually.
The USB DFU config fragments for AM62x have been used for enabling USB DFU boot on AM62a as follows: R5 => am62ax_evm_r5_defconfig + am62x_r5_usbdfu.config A53 => am62ax_evm_a53_defconfig + am62x_a53_usbdfu.config
Logs validating USB DFU boot with this series: https://gist.github.com/Siddharth-Vadapalli-at-TI/daa71da1b0e478a51afea42605...
Series is based on commit 3073246d1be Prepare v2025.01-rc3 of the master branch of U-Boot.
Regards, Siddharth.
Siddharth Vadapalli (2): usb: dwc3-generic: set "mode" based on caller of dwc3_generic_probe() board: ti: am62ax: env: include environment for DFU
board/ti/am62ax/am62ax.env | 1 + drivers/usb/dwc3/dwc3-generic.c | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-)
There are only two callers of "dwc3_generic_probe()", namely: 1. dwc3_generic_peripheral_probe() 2. 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".
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()".
Signed-off-by: Siddharth Vadapalli s-vadapalli@ti.com --- drivers/usb/dwc3/dwc3-generic.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c index 2ab41cbae45..55e62b35c61 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) { int rc; struct dwc3_generic_plat *plat = dev_get_plat(dev); @@ -62,7 +63,7 @@ static int dwc3_generic_probe(struct udevice *dev,
dwc3->dev = dev; dwc3->maximum_speed = plat->maximum_speed; - dwc3->dr_mode = plat->dr_mode; + dwc3->dr_mode = mode; #if CONFIG_IS_ENABLED(OF_CONTROL) dwc3_of_parse(dwc3);
@@ -197,7 +198,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 +242,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;
On 26/11/2024 14:03, 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".
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()".
Signed-off-by: Siddharth Vadapalli s-vadapalli@ti.com
drivers/usb/dwc3/dwc3-generic.c | 9 +++++----
TI AM62a is not even using this driver so how did you test this? Did I miss a DT patch?
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c index 2ab41cbae45..55e62b35c61 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){ int rc; struct dwc3_generic_plat *plat = dev_get_plat(dev); @@ -62,7 +63,7 @@ static int dwc3_generic_probe(struct udevice *dev,
dwc3->dev = dev; dwc3->maximum_speed = plat->maximum_speed;
- dwc3->dr_mode = plat->dr_mode;
- dwc3->dr_mode = mode;
if platform supported mode was "host" only or "peripheral" only then you shouldn't we prevent from using the other role?
In u-boot shell, if user starts Host driver first and then starts peripheral driver how does it work? Did you test this case?
#if CONFIG_IS_ENABLED(OF_CONTROL) dwc3_of_parse(dwc3);
@@ -197,7 +198,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 +242,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;
Hello Roger,
On 28-11-2024 18:40, Roger Quadros wrote:
On 26/11/2024 14:03, 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".
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()".
Signed-off-by: Siddharth Vadapalli s-vadapalli@ti.com
drivers/usb/dwc3/dwc3-generic.c | 9 +++++----
TI AM62a is not even using this driver so how did you test this? Did I miss a DT patch?
USB DFU boot utilizes this driver when the "dr_mode" is set to "otg" in the device-tree for AM62A. This driver is invoked both for the host and device commands corresponding to USB. I came up with this patch while trying to enable USB DFU boot on AM62A with "dr_mode" as "otg" in the device-tree. Additionally, USB DFU boot works on AM62A with the change made by this patch and it doesn't work without this change. I have traced the call sequence to see that the driver is probed and have come up with the change performed in this patch on the basis of this observation.
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c index 2ab41cbae45..55e62b35c61 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){ int rc; struct dwc3_generic_plat *plat = dev_get_plat(dev); @@ -62,7 +63,7 @@ static int dwc3_generic_probe(struct udevice *dev,
dwc3->dev = dev; dwc3->maximum_speed = plat->maximum_speed;
- dwc3->dr_mode = plat->dr_mode;
- dwc3->dr_mode = mode;
if platform supported mode was "host" only or "peripheral" only then you shouldn't we prevent from using the other role?
Yes, but that check doesn't exist prior to this patch either. Platform could support only "host" mode as specified by "dr_mode", but if the user runs a command for device mode it will lead to "dwc3_generic_peripheral_probe" in this driver. On the other hand, if the check does exist outside this driver, then it doesn't have to be checked within "dwc3_generic_probe". There are only two callers of "dwc3_generic_probe", both of which are in this driver, and both of which unconditionally invoke "dwc3_generic_probe".
However, I agree that having a check here will help. I will update this patch to add the check to confirm if there is a conflict with what the platform supports, though this doesn't seem to be the case with the existing code.
In u-boot shell, if user starts Host driver first and then starts peripheral driver how does it work? Did you test this case?
I didn't test the case where both are used. I validated that peripheral mode works across various stages of boot with "otg" mode via USB DFU boot. This isn't the case without this patch.
If we forget the reason that this patch is introduced and take a moment to analyze the existing code prior to this patch, we see that: 1. Both dwc3_generic_peripheral_probe() and dwc3_generic_host_probe() invoke dwc3_generic_probe(). 2. dwc3_generic_probe() sets the role based on the role specified in the device-tree and not what the caller wants the role to be. This seems to be strange to me considering that dwc3_generic_probe() simply proceeds to configure without comparing the role supported by the platform with the role requested by the caller.
So even if we entirely forget the "otg" fix being attempted by this patch, the same patch is fixing something else i.e. ensuring that dwc3_generic_probe() does what the caller wants it to do by using the role that the caller expects. Isn't it incorrect that the existing code might end up with: a) Caller being dwc3_generic_host_probe() with dr_mode being set to "peripheral" within dwc3_generic_probe() based on the device-tree and proceeding to configure the controller. b) Caller being dwc3_generic_peripheral_probe() with dr_mode being set to "host" within dwc3_generic_probe() based on the device-tree and proceeding to configure the controller. Shouldn't this inconsistency be fixed? This patch fixes that by setting dr_mode to what the caller expects. However, based on your suggestion, I agree that there should have been a check within dwc3_generic_probe() to ensure that the platform supports what the caller is requesting. I will update this patch by adding the check.
Please let me know what you think.
[...]
On 28/11/2024 19:20, Siddharth Vadapalli wrote:
Hello Roger,
On 28-11-2024 18:40, Roger Quadros wrote:
On 26/11/2024 14:03, 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".
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()".
Signed-off-by: Siddharth Vadapalli s-vadapalli@ti.com
drivers/usb/dwc3/dwc3-generic.c | 9 +++++----
TI AM62a is not even using this driver so how did you test this? Did I miss a DT patch?
USB DFU boot utilizes this driver when the "dr_mode" is set to "otg" in the device-tree for AM62A. This driver is invoked both for the host and device commands corresponding to USB. I came up with this patch while trying to enable USB DFU boot on AM62A with "dr_mode" as "otg" in the device-tree. Additionally, USB DFU boot works on AM62A with the change made by this patch and it doesn't work without this change. I have traced the call sequence to see that the driver is probed and have come up with the change performed in this patch on the basis of this observation.
Although the glue driver portion of the dwc3-generic driver is not used by AM62 it still uses the generic_probe() portion via U_BOOT_DRIVER(dwc3_generic_host) and U_BOOT_DRIVER(dwc3_generic_peripheral)
here is the dm tree output on am62-sk after executing "usb start" command.
simple_bus 5 [ + ] dwc3-am62 | |-- dwc3-usb@f900000 usb_gadget 0 [ ] dwc3-generic-periphe | | |-- usb@31000000 usb 0 [ + ] xhci-dwc3 | | `-- usb@31000000 usb_hub 0 [ + ] usb_hub | | `-- usb_hub simple_bus 6 [ + ] dwc3-am62 | |-- dwc3-usb@f910000 usb 1 [ ] dwc3-generic-host | | |-- usb@31100000 usb 1 [ + ] xhci-dwc3 | | `-- usb@31100000 usb_hub 1 [ + ] usb_hub | | `-- usb_hub
what is strange is that even though usb@31000000 is configured as "peripheral" (in k3-am625-sk-u-boot.dtsi), xhci-dwc3 and usb_hub drivers are still initialized for it.
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c index 2ab41cbae45..55e62b35c61 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){ int rc; struct dwc3_generic_plat *plat = dev_get_plat(dev); @@ -62,7 +63,7 @@ static int dwc3_generic_probe(struct udevice *dev,
dwc3->dev = dev; dwc3->maximum_speed = plat->maximum_speed;
- dwc3->dr_mode = plat->dr_mode;
- dwc3->dr_mode = mode;
if platform supported mode was "host" only or "peripheral" only then you shouldn't we prevent from using the other role?
Yes, but that check doesn't exist prior to this patch either. Platform could support only "host" mode as specified by "dr_mode", but if the user runs a command for device mode it will lead to "dwc3_generic_peripheral_probe" in this driver. On the other hand, if the check does exist outside this driver, then it doesn't have to be checked within "dwc3_generic_probe". There are only two callers of "dwc3_generic_probe", both of which are in this driver, and both of which unconditionally invoke "dwc3_generic_probe".
However, I agree that having a check here will help. I will update this patch to add the check to confirm if there is a conflict with what the platform supports, though this doesn't seem to be the case with the existing code.
In u-boot shell, if user starts Host driver first and then starts peripheral driver how does it work? Did you test this case?
I didn't test the case where both are used. I validated that peripheral mode works across various stages of boot with "otg" mode via USB DFU boot. This isn't the case without this patch.
If we forget the reason that this patch is introduced and take a moment to analyze the existing code prior to this patch, we see that:
- Both dwc3_generic_peripheral_probe() and dwc3_generic_host_probe()
invoke dwc3_generic_probe(). 2. dwc3_generic_probe() sets the role based on the role specified in the/ device-tree and not what the caller wants the role to be. This seems to be strange to me considering that dwc3_generic_probe() simply proceeds to configure without comparing the role supported by the platform with the role requested by the caller.
So even if we entirely forget the "otg" fix being attempted by this patch, the same patch is fixing something else i.e. ensuring that dwc3_generic_probe() does what the caller wants it to do by using the role that the caller expects. Isn't it incorrect that the existing code might end up with: a) Caller being dwc3_generic_host_probe() with dr_mode being set to "peripheral" within dwc3_generic_probe() based on the device-tree and proceeding to configure the controller
Maye this is the reason why host controller is initialized for peripheral only port in the dm tree output I listed above. This needs to be fixed.
b) Caller being dwc3_generic_peripheral_probe() with dr_mode being set to "host" within dwc3_generic_probe() based on the device-tree and proceeding to configure the controller.
This too needs to be fixed.
Shouldn't this inconsistency be fixed? This patch fixes that by setting dr_mode to what the caller expects. However, based on your suggestion, I agree that there should have been a check within dwc3_generic_probe() to ensure that the platform supports what the caller is requesting. I will update this patch by adding the check.
Apart from the platform supported mode check when not "otg", if platform supports "otg" we need to check and prevent role setting if hardware is not in the correct state based on connected USB cable. i.e. if host mode cable is used then prevent it from running as a peripheral. if peripheral mode cable is used then prevent it from running as a host. (based on ID status?)
Please let me know what you think.
[...]
On Fri, Nov 29, 2024 at 02:28:36PM +0200, Roger Quadros wrote:
On 28/11/2024 19:20, Siddharth Vadapalli wrote:
[...]
USB DFU boot utilizes this driver when the "dr_mode" is set to "otg" in the device-tree for AM62A. This driver is invoked both for the host and device commands corresponding to USB. I came up with this patch while trying to enable USB DFU boot on AM62A with "dr_mode" as "otg" in the device-tree. Additionally, USB DFU boot works on AM62A with the change made by this patch and it doesn't work without this change. I have traced the call sequence to see that the driver is probed and have come up with the change performed in this patch on the basis of this observation.
Although the glue driver portion of the dwc3-generic driver is not used by AM62 it still uses the generic_probe() portion via U_BOOT_DRIVER(dwc3_generic_host) and U_BOOT_DRIVER(dwc3_generic_peripheral)
here is the dm tree output on am62-sk after executing "usb start" command.
simple_bus 5 [ + ] dwc3-am62 | |-- dwc3-usb@f900000 usb_gadget 0 [ ] dwc3-generic-periphe | | |-- usb@31000000 usb 0 [ + ] xhci-dwc3 | | `-- usb@31000000 usb_hub 0 [ + ] usb_hub | | `-- usb_hub simple_bus 6 [ + ] dwc3-am62 | |-- dwc3-usb@f910000 usb 1 [ ] dwc3-generic-host | | |-- usb@31100000 usb 1 [ + ] xhci-dwc3 | | `-- usb@31100000 usb_hub 1 [ + ] usb_hub | | `-- usb_hub
what is strange is that even though usb@31000000 is configured as "peripheral" (in k3-am625-sk-u-boot.dtsi), xhci-dwc3 and usb_hub drivers are still initialized for it.
CONFIG_OF_UPSTREAM is enabled in am62x_evm_a53_defconfig. So dr_mode might be "otg" as specified in: dts/upstream/src/arm64/ti/k3-am62-main.dtsi I'm not sure about this however. Additionally, this series isn't required for AM62. DFU Boot works on AM62 without this series as well. This series is required for AM62A.
[...]
I didn't test the case where both are used. I validated that peripheral mode works across various stages of boot with "otg" mode via USB DFU boot. This isn't the case without this patch.
If we forget the reason that this patch is introduced and take a moment to analyze the existing code prior to this patch, we see that:
- Both dwc3_generic_peripheral_probe() and dwc3_generic_host_probe()
invoke dwc3_generic_probe(). 2. dwc3_generic_probe() sets the role based on the role specified in the/ device-tree and not what the caller wants the role to be. This seems to be strange to me considering that dwc3_generic_probe() simply proceeds to configure without comparing the role supported by the platform with the role requested by the caller.
So even if we entirely forget the "otg" fix being attempted by this patch, the same patch is fixing something else i.e. ensuring that dwc3_generic_probe() does what the caller wants it to do by using the role that the caller expects. Isn't it incorrect that the existing code might end up with: a) Caller being dwc3_generic_host_probe() with dr_mode being set to "peripheral" within dwc3_generic_probe() based on the device-tree and proceeding to configure the controller
Maye this is the reason why host controller is initialized for peripheral only port in the dm tree output I listed above. This needs to be fixed.
Sure, I will post a patch to do the following: If dr_mode specified in device-tree is "host" then return -EINVAL within dwc3_generic_probe() if the caller is "dwc3_generic_peripheral_probe()". Similarly, if dr_mode specified in the device-tree is "peripheral" then return -EINVAL within dwc3_generic_probe() if the caller is "dwc3_generic_host_probe()". When dr_mode is specified as "otg" in the device-tree, then configure the mode based on the caller of dwc3_generic_probe().
Please let me know if the above implementation is acceptable.
b) Caller being dwc3_generic_peripheral_probe() with dr_mode being set to "host" within dwc3_generic_probe() based on the device-tree and proceeding to configure the controller.
This too needs to be fixed.
Shouldn't this inconsistency be fixed? This patch fixes that by setting dr_mode to what the caller expects. However, based on your suggestion, I agree that there should have been a check within dwc3_generic_probe() to ensure that the platform supports what the caller is requesting. I will update this patch by adding the check.
Apart from the platform supported mode check when not "otg", if platform supports "otg" we need to check and prevent role setting if hardware is not in the correct state based on connected USB cable. i.e. if host mode cable is used then prevent it from running as a peripheral. if peripheral mode cable is used then prevent it from running as a host. (based on ID status?)
Yes, I agree that using the ID status to determine the role is accurate. At the same time, I think that in addition to this, if the user runs a command corresponding to "peripheral" mode of operation with the ID status indicating "host", then it will be better to take this inconsistency into account and error out rather than simply proceeding to configure on the basis of the ID status. If you have any feedback, kindly let me know.
Regards, Siddharth.
On 02/12/2024 07:42, Siddharth Vadapalli wrote:
On Fri, Nov 29, 2024 at 02:28:36PM +0200, Roger Quadros wrote:
On 28/11/2024 19:20, Siddharth Vadapalli wrote:
[...]
USB DFU boot utilizes this driver when the "dr_mode" is set to "otg" in the device-tree for AM62A. This driver is invoked both for the host and device commands corresponding to USB. I came up with this patch while trying to enable USB DFU boot on AM62A with "dr_mode" as "otg" in the device-tree. Additionally, USB DFU boot works on AM62A with the change made by this patch and it doesn't work without this change. I have traced the call sequence to see that the driver is probed and have come up with the change performed in this patch on the basis of this observation.
Although the glue driver portion of the dwc3-generic driver is not used by AM62 it still uses the generic_probe() portion via U_BOOT_DRIVER(dwc3_generic_host) and U_BOOT_DRIVER(dwc3_generic_peripheral)
here is the dm tree output on am62-sk after executing "usb start" command.
simple_bus 5 [ + ] dwc3-am62 | |-- dwc3-usb@f900000 usb_gadget 0 [ ] dwc3-generic-periphe | | |-- usb@31000000 usb 0 [ + ] xhci-dwc3 | | `-- usb@31000000 usb_hub 0 [ + ] usb_hub | | `-- usb_hub simple_bus 6 [ + ] dwc3-am62 | |-- dwc3-usb@f910000 usb 1 [ ] dwc3-generic-host | | |-- usb@31100000 usb 1 [ + ] xhci-dwc3 | | `-- usb@31100000 usb_hub 1 [ + ] usb_hub | | `-- usb_hub
what is strange is that even though usb@31000000 is configured as "peripheral" (in k3-am625-sk-u-boot.dtsi), xhci-dwc3 and usb_hub drivers are still initialized for it.
CONFIG_OF_UPSTREAM is enabled in am62x_evm_a53_defconfig. So dr_mode might be "otg" as specified in: dts/upstream/src/arm64/ti/k3-am62-main.dtsi
Not it is not "otg". it is "peripheral". I verified from the generated u-boot.dtb
Isn't k3-am625-sk-u-boot.dtsi still used even if it is CONFIG_OF_UPSTREAM?
I'm not sure about this however. Additionally, this series isn't required for AM62. DFU Boot works on AM62 without this series as well. This series is required for AM62A.
It works on am62-sk because USB mode is forced to "peripheral" in k3-am625-sk-u-boot.dtsi.
That hack can be removed once your series fixes it the right way.
[...]
I didn't test the case where both are used. I validated that peripheral mode works across various stages of boot with "otg" mode via USB DFU boot. This isn't the case without this patch.
If we forget the reason that this patch is introduced and take a moment to analyze the existing code prior to this patch, we see that:
- Both dwc3_generic_peripheral_probe() and dwc3_generic_host_probe()
invoke dwc3_generic_probe(). 2. dwc3_generic_probe() sets the role based on the role specified in the/ device-tree and not what the caller wants the role to be. This seems to be strange to me considering that dwc3_generic_probe() simply proceeds to configure without comparing the role supported by the platform with the role requested by the caller.
So even if we entirely forget the "otg" fix being attempted by this patch, the same patch is fixing something else i.e. ensuring that dwc3_generic_probe() does what the caller wants it to do by using the role that the caller expects. Isn't it incorrect that the existing code might end up with: a) Caller being dwc3_generic_host_probe() with dr_mode being set to "peripheral" within dwc3_generic_probe() based on the device-tree and proceeding to configure the controller
Maye this is the reason why host controller is initialized for peripheral only port in the dm tree output I listed above. This needs to be fixed.
Sure, I will post a patch to do the following: If dr_mode specified in device-tree is "host" then return -EINVAL within dwc3_generic_probe() if the caller is "dwc3_generic_peripheral_probe()". Similarly, if dr_mode specified in the device-tree is "peripheral" then return -EINVAL within dwc3_generic_probe() if the caller is "dwc3_generic_host_probe()". When dr_mode is specified as "otg" in the device-tree, then configure the mode based on the caller of dwc3_generic_probe().
Please let me know if the above implementation is acceptable.
Looks OK to me.
b) Caller being dwc3_generic_peripheral_probe() with dr_mode being set to "host" within dwc3_generic_probe() based on the device-tree and proceeding to configure the controller.
This too needs to be fixed.
Shouldn't this inconsistency be fixed? This patch fixes that by setting dr_mode to what the caller expects. However, based on your suggestion, I agree that there should have been a check within dwc3_generic_probe() to ensure that the platform supports what the caller is requesting. I will update this patch by adding the check.
Apart from the platform supported mode check when not "otg", if platform supports "otg" we need to check and prevent role setting if hardware is not in the correct state based on connected USB cable. i.e. if host mode cable is used then prevent it from running as a peripheral. if peripheral mode cable is used then prevent it from running as a host. (based on ID status?)
Yes, I agree that using the ID status to determine the role is accurate. At the same time, I think that in addition to this, if the user runs a command corresponding to "peripheral" mode of operation with the ID status indicating "host", then it will be better to take this inconsistency into account and error out rather than simply proceeding to configure on the basis of the ID status. If you have any feedback, kindly let me know.
The approach looks OK to me.
On 26/11/2024 14:03, 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".
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()".
Signed-off-by: Siddharth Vadapalli s-vadapalli@ti.com
Reviewed-by: Roger Quadros rogerq@kernel.org
On 11/26/24 1:03 PM, 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".
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()".
Signed-off-by: Siddharth Vadapalli s-vadapalli@ti.com
Reviewed-by: Marek Vasut marex@denx.de
for next
Include the TI K3 DFU environment to support DFU Boot and DFU Flash.
Signed-off-by: Siddharth Vadapalli s-vadapalli@ti.com --- board/ti/am62ax/am62ax.env | 1 + 1 file changed, 1 insertion(+)
diff --git a/board/ti/am62ax/am62ax.env b/board/ti/am62ax/am62ax.env index 97122fb57ba..96d9e1e2797 100644 --- a/board/ti/am62ax/am62ax.env +++ b/board/ti/am62ax/am62ax.env @@ -1,5 +1,6 @@ #include <env/ti/ti_common.env> #include <env/ti/mmc.env> +#include <env/ti/k3_dfu.env> #if CONFIG_CMD_REMOTEPROC #include <env/ti/k3_rproc.env> #endif
On 26/11/2024 14:03, Siddharth Vadapalli wrote:
Include the TI K3 DFU environment to support DFU Boot and DFU Flash.
Signed-off-by: Siddharth Vadapalli s-vadapalli@ti.com
Reviewed-by: Roger Quadros rogerq@kernel.org
On 26/11/2024 14:03, Siddharth Vadapalli wrote:
Hello,
This series adds support for USB DFU boot on TI's AM62A SoC which has two instances of DWC3 USB Controllers namely USB0 and USB1. The USB0 instance of the USB Controller supports USB DFU boot: ROM => tiboot3.bin => tispl.bin => u-boot.img
USB DFU Boot requires the USB Controller to be configured for Gadget mode of operation. Since the USB0 instance of the DWC3 USB Controller supports both Host and Gadget modes of operation via the Type-C interface on the AM62A7-SK board, the device-tree specifies the "dr_mode" as "OTG". However, there is currently no support for dynamically switching the "mode" from Host to Gadget and vice-versa with the help of a state-machine. The OTG mode is treated as a separate mode in itself rather than being treated as an intermediate stage before assuming the Host/Gadget mode. Due to this, USB DFU boot via the Type-C interface doesn't work as the USB Controller hasn't been appropriately configured for Device/Gadget mode of operation. One option is to change the device-tree to specify "dr_mode" as "peripheral" and force the controller to assume the Device role. This will imply that the U-Boot device-tree for AM62A diverges from its Linux counterpart. Therefore, with the intent of keeping the device-tree uniform across Linux and U-Boot, and at the same time, in order to enable USB DFU boot in "OTG" mode with the DWC3 Controller, the first patch in this series sets the "mode" on the basis of the caller function, rather than using the "dr_mode" property in the device-tree. There are only two callers of "dwc3_generic_probe()", each of which clearly specify the expected mode of configuration. This will enable both Host and Device mode of operation based on the command executed by the user, thereby truly supporting "OTG" functionality when the USB Controller supports it.
We don't really support OTG state machine. All you are supporting is user initiated role change.
The second patch in this series adds USB DFU environment for AM62A, enabling USB DFU Boot and USB DFU flash on AM62A.
In addition to the patches in this series, the following device-tree changes will be required to test USB DFU on AM62A (bootph-all property to be added to ensure that USB Controller is present at all stages for DFU Boot): https://gist.github.com/Siddharth-Vadapalli-at-TI/53ba02cb0ff4a09c47e920d082... The above device-tree changes will be made to the Linux device-tree, which shall ensure that the same shall be a part of U-Boot device-tree eventually.
The USB DFU config fragments for AM62x have been used for enabling USB DFU boot on AM62a as follows: R5 => am62ax_evm_r5_defconfig + am62x_r5_usbdfu.config A53 => am62ax_evm_a53_defconfig + am62x_a53_usbdfu.config
Logs validating USB DFU boot with this series: https://gist.github.com/Siddharth-Vadapalli-at-TI/daa71da1b0e478a51afea42605...
Series is based on commit 3073246d1be Prepare v2025.01-rc3 of the master branch of U-Boot.
Regards, Siddharth.
Siddharth Vadapalli (2): usb: dwc3-generic: set "mode" based on caller of dwc3_generic_probe() board: ti: am62ax: env: include environment for DFU
board/ti/am62ax/am62ax.env | 1 + drivers/usb/dwc3/dwc3-generic.c | 9 +++++---- 2 files changed, 6 insertions(+), 4 deletions(-)
Hello Roger,
On 28-11-2024 18:15, Roger Quadros wrote:
On 26/11/2024 14:03, Siddharth Vadapalli wrote:
Hello,
This series adds support for USB DFU boot on TI's AM62A SoC which has two instances of DWC3 USB Controllers namely USB0 and USB1. The USB0 instance of the USB Controller supports USB DFU boot: ROM => tiboot3.bin => tispl.bin => u-boot.img
USB DFU Boot requires the USB Controller to be configured for Gadget mode of operation. Since the USB0 instance of the DWC3 USB Controller supports both Host and Gadget modes of operation via the Type-C interface on the AM62A7-SK board, the device-tree specifies the "dr_mode" as "OTG". However, there is currently no support for dynamically switching the "mode" from Host to Gadget and vice-versa with the help of a state-machine. The OTG mode is treated as a separate mode in itself rather than being treated as an intermediate stage before assuming the Host/Gadget mode. Due to this, USB DFU boot via the Type-C interface doesn't work as the USB Controller hasn't been appropriately configured for Device/Gadget mode of operation. One option is to change the device-tree to specify "dr_mode" as "peripheral" and force the controller to assume the Device role. This will imply that the U-Boot device-tree for AM62A diverges from its Linux counterpart. Therefore, with the intent of keeping the device-tree uniform across Linux and U-Boot, and at the same time, in order to enable USB DFU boot in "OTG" mode with the DWC3 Controller, the first patch in this series sets the "mode" on the basis of the caller function, rather than using the "dr_mode" property in the device-tree. There are only two callers of "dwc3_generic_probe()", each of which clearly specify the expected mode of configuration. This will enable both Host and Device mode of operation based on the command executed by the user, thereby truly supporting "OTG" functionality when the USB Controller supports it.
We don't really support OTG state machine. All you are supporting is user initiated role change.
I should have probably used the term "Dual Role" instead of "OTG". I meant to say that the controller can be used both as Host and Device based on the command executed by the user, if the controller's dr_mode is "otg". I am aware that we don't support the OTG state machine in U-Boot unlike Linux, which is why I have posted this series as a workaround to utilize both Host and Device modes in the interim.
[...]
On Tue, 26 Nov 2024 17:33:17 +0530, Siddharth Vadapalli wrote:
This series adds support for USB DFU boot on TI's AM62A SoC which has two instances of DWC3 USB Controllers namely USB0 and USB1. The USB0 instance of the USB Controller supports USB DFU boot: ROM => tiboot3.bin => tispl.bin => u-boot.img
USB DFU Boot requires the USB Controller to be configured for Gadget mode of operation. Since the USB0 instance of the DWC3 USB Controller supports both Host and Gadget modes of operation via the Type-C interface on the AM62A7-SK board, the device-tree specifies the "dr_mode" as "OTG". However, there is currently no support for dynamically switching the "mode" from Host to Gadget and vice-versa with the help of a state-machine. The OTG mode is treated as a separate mode in itself rather than being treated as an intermediate stage before assuming the Host/Gadget mode. Due to this, USB DFU boot via the Type-C interface doesn't work as the USB Controller hasn't been appropriately configured for Device/Gadget mode of operation. One option is to change the device-tree to specify "dr_mode" as "peripheral" and force the controller to assume the Device role. This will imply that the U-Boot device-tree for AM62A diverges from its Linux counterpart. Therefore, with the intent of keeping the device-tree uniform across Linux and U-Boot, and at the same time, in order to enable USB DFU boot in "OTG" mode with the DWC3 Controller, the first patch in this series sets the "mode" on the basis of the caller function, rather than using the "dr_mode" property in the device-tree. There are only two callers of "dwc3_generic_probe()", each of which clearly specify the expected mode of configuration. This will enable both Host and Device mode of operation based on the command executed by the user, thereby truly supporting "OTG" functionality when the USB Controller supports it.
[...]
Applied to u-boot/next, thanks!
participants (4)
-
Marek Vasut -
Roger Quadros -
Siddharth Vadapalli -
Tom Rini