On 06/12/2024 11:17, Roger Quadros wrote:
Hello Siddharth,
On 06/12/2024 09:19, Siddharth Vadapalli wrote:
On Tue, Dec 03, 2024 at 10:40:29PM +0200, Roger Quadros wrote:
Hello Roger,
CONFIG_USB_XHCI_DWC3 is not required for AM62x as the XHCI driver is registered through the dwc3-generic driver.
CONFIG_USB_XHCI_DWC3 causes problems by hijacking the USB controller even if it is not set for Host mode in device tree.
'dm tree' output after 'usb start' is fixed from
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_hub 1 [ + ] usb_hub | | | `-- usb_hub usb 1 [ + ] xhci-dwc3 | | `-- usb@31100000 usb_hub 2 [ + ] usb_hub | | `-- usb_hub
[notice that 'xhci-dwc3' and 'usb_hub' drivers are probed for both USB instances although the first instance is supposed to be 'peripheral' only]
to
simple_bus 5 [ ] dwc3-am62 | |-- dwc3-usb@f900000 usb_gadget 0 [ ] dwc3-generic-periphe | | `-- usb@31000000 simple_bus 6 [ + ] dwc3-am62 | |-- dwc3-usb@f910000 usb 1 [ + ] dwc3-generic-host | | `-- usb@31100000 usb_hub 0 [ + ] usb_hub | | `-- usb_hub
While this fixes the issue, I am wondering if the issue lies elsewhere. In U-Boot, the compatible "snps,dwc3" is associated with: drivers/usb/host/xhci-dwc3.c while in Linux, the compatible "snps,dwc3" is associated with: drivers/usb/dwc3/core.c
So there seem to be two alternatives that I could think of:
- Modify U-Boot to match Linux in the sense that we associate
"snps,dwc3" with drivers/usb/dwc3/core.c in U-Boot.
Many platforms (grep for CONFIG_USB_XHCI_DWC3 in configs/) use xhci-dwc3 to get Host mode working without the need for core.c. Maybe it was more simpler that way? Also see USB_XHCI_DWC3_OF_SIMPLE.
So if we drop "snps,dwc3" from xhci-dwc3, we will have to ensure each and every platform works.
- With the understanding that "dr_mode" doesn't have to be host/otg for
the compatible "snps,dwc3" which is tied to drivers/usb/host/xhci-dwc3.c, do the following to exit probe when "dr_mode" is "peripheral":
diff --git a/drivers/usb/host/xhci-dwc3.c b/drivers/usb/host/xhci-dwc3.c index e3e0ceff43e..edfd7b97a73 100644 --- a/drivers/usb/host/xhci-dwc3.c +++ b/drivers/usb/host/xhci-dwc3.c @@ -208,6 +208,8 @@ static int xhci_dwc3_probe(struct udevice *dev) writel(reg, &dwc3_reg->g_usb2phycfg[0]);
dr_mode = usb_get_dr_mode(dev_ofnode(dev));
if (dr_mode == USB_DR_MODE_PERIPHERAL)return -ENODEV; if (dr_mode == USB_DR_MODE_OTG && dev_read_bool(dev, "usb-role-switch")) { dr_mode = usb_get_role_switch_default_mode(dev_ofnode(dev));
which will still show "xhci-dwc3" in the output of "dm tree" after "usb start", but the driver won't be probed (absence of "+" in the "Probed" column of "dm tree" output).
I realized later that if dr_mode == USB_DR_MODE_OTG/HOST which is the case for am62a, then xhci-dwc3.c will still be probed and we have 2 drivers probed for the same controller?
We do not want that right? So not having CONFIG_USB_XHCI_DWC3 is still required for am62*.
Maybe we still need some Kconfig guards that prevent both CONFIG_USB_XHCI_DWC3 and USB_DWC3_GENERIC to be set together as 2 drivers will claim the controller if dr_mdoe is USB_DR_MODE_OTG or USB_DR_MODE_HOST.
I agree with your second proposal. But it is a separate bug (in xhci-dwc3 driver) which also fixes the issue this patch is fixing.
Regardless, I think both fixes should go in.
Could you please send a patch to fix xhci-dwc3? Thanks!
I just wanted to point this out as a possible fix for other scenarios. Disabling "CONFIG_USB_XHCI_DWC3" seems to be the best choice in the current scenario (i.e. it shouldn't have been set in "USB DFU" fragment to begin with).
Fixes: dfc2dff5a844 ("configs: am62x_evm_*: Enable USB and DFU support") Signed-off-by: Roger Quadros rogerq@kernel.org
Reviewed-by: Siddharth Vadapalli s-vadapalli@ti.com
configs/am62x_a53_usbdfu.config | 1 - 1 file changed, 1 deletion(-)
diff --git a/configs/am62x_a53_usbdfu.config b/configs/am62x_a53_usbdfu.config index 3a19cf23287..0d3c6df1e73 100644 --- a/configs/am62x_a53_usbdfu.config +++ b/configs/am62x_a53_usbdfu.config @@ -16,7 +16,6 @@ CONFIG_USB=y CONFIG_DM_USB_GADGET=y CONFIG_SPL_DM_USB_GADGET=y CONFIG_USB_XHCI_HCD=y -CONFIG_USB_XHCI_DWC3=y CONFIG_USB_DWC3=y CONFIG_USB_DWC3_GENERIC=y CONFIG_SPL_USB_DWC3_GENERIC=y
base-commit: 3073246d1be682071d8b3d07d06c2484907aed60 change-id: 20241203-am62-usb-xhci-be62bc9584c9
Best regards,
Roger Quadros rogerq@kernel.org