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:
> 1. 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.
> 2. 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 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 <rog...@kernel.org>
>
> Reviewed-by: Siddharth Vadapalli <s-vadapa...@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 <rog...@kernel.org>
>>
--
cheers,
-roger
