[PATCH] configs: am62x_evm_*: Fix USB DFU configuration

[Siddharth Vadapalli]

On Mon, Dec 09, 2024 at 02:32:37PM +0200, Roger Quadros wrote:

Hello Roger,

> On 06/12/2024 12:07, Siddharth Vadapalli wrote:

[...]

> > Looking at the list of compatibles in dwc3-generic.c and focusing on the
> > compatible "ti,keystone-dwc3", I see that this compatible is present in:
> > dts/upstream/src/arm/ti/keystone/keystone.dtsi
> > among other keystone device-tree files. The hierarchy of the nodes and
> > compatibles for the USB Subsystem and the USB Controller in Keystone is
> > similar to that of AM62*. The difference however is the following:
> > A) For non AM62* devices:
> 
> note there are 2 cases here:
> 
> A1) that enable CONFIG_USB_DWC3_GENERIC
> 
> > 	USB Subsystem [Wrapper] driver is dwc3-generic.c
> Yes.
> 
> > 		USB Controller [DWC3] driver is xhci-dwc3.c
> 
> No.
> 
> Note that none of the platforms that enable CONFIG_USB_DWC3_GENERIC
> enable CONFIG_USB_XHCI_DWC3. They should not because dwc3-generic.c
> takes care of registering the XHCI driver via xhci_register/deregister()

Thank you for pointing this out.

> 
> A2) that don't enable CONFIG_USB_DWC3_GENERIC
> 
> These are usually only interested in host mode, they enable
> CONFIG_USB_XHCI_DWC3 and may also enable CONFIG_USB_XHCI_DWC3_OF_SIMPLE
> 
> So  USB Wrapper/glue driver is DWC3_OF_SIMPLE
> 	USB driver is xhci-dwc3.c
> 
> Some platforms have their own glue
> 	CONFIG_USB_XHCI_STI	dwc3-sti-glue.c
> 	CONFIG_USB_XHCI_OCTEON	dwc3-octeon-glue.c
> 
> > B) For AM62* devices:
> > 	USB Subsystem [Wrapper] driver is dwc3-am62.c
> > 	which exposes its own ".glue_configure" callback similar to what
> > 	is done in dwc3-generic.c for other devices.
> 
> The Subsystem driver is still dwc3-generic.c. Glue driver is dwc3-am62.c
> 
> > 		USB Controller [DWC3] driver is xhci-dwc3.c
> 
> No.
> We don't require CONFIG_USB_XHCI_DWC3 as XHCI registration is done
> by dwc3-generic. USB Host driver is USB_XHCI_HCD.

Ok.

> 
> > So maybe non AM62* devices have:
> > 	dwc3-generic.c for Subsystem + xhci-dwc3.c for USB Controller
> > while AM62* devices have:
> > 	dwc3-am62.c for Subsystem + dwc3-generic.c (neither Subsystem
> > 	nor Controller?) + xhci-dwc3.c for USB Controller
> > We could probably end up with the two driver situation similar to non
> > AM62* devices if we update dwc3-generic.c to handle the configuration
> > performed by dwc3-am62.c. But that still means we have two drivers:
> > dwc3-generic.c and xhci-dwc3.c
> > Maybe viewing the output of "dm tree" on a non AM62* device with both
> > CONFIG_USB_XHCI_DWC3 and CONFIG_USB_DWC3_GENERIC enabled will let us
> > know whether that is a valid configuration or not.
> 
> Enabling both is an invalid configuration and no config does it. AM62 was
> doing it wrong and so we fix it with this patch.

Understood. Thank you for explaining. I agree that the fix in this patch
is correct and your earlier suggestion of Kconfig guards:
"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_mode is USB_DR_MODE_OTG or
USB_DR_MODE_HOST."
could be implemented in a follow up patch.

Regards,
Siddharth.