[PATCH] usb: cdns3: use VBUS Valid to determine role for dr_mode OTG

[Siddharth Vadapalli]

On 04/02/26 7:39 PM, Marek Vasut wrote:
> On 2/4/26 1:12 PM, Siddharth Vadapalli wrote:
>> The cdns3_bind() function is responsible for identifying the appropriate
>> driver to bind to the USB Controller's device-tree node. If the 
>> device-tree
>> node has the 'dr_mode' property set to 'otg', the existing approach fails
>> to bind a driver, leading to loss of functionality.
>>
>> To address this, use the VBUS Valid field of the OTG Status register to
>> determine the role as follows:
>> - If VBUS Valid field is set, it indicates that a USB Host is supplying
>>    power and the Controller should assume the Peripheral role.
>> - If VBUS Valid field is clear, it indicates the absence of a USB Host 
>> and
>>    the Controller should assume the Host role.
>>
>> Additionally, when 'dr_mode' happens to be 'otg' and the STRAP settings
>> are not specified, use VBUS Valid to determine the role in 
>> cdns3_drd_init()
>> and assign it to cdns->dr_mode.
>>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli at ti.com>
>> ---
>>
>> Hello,
>>
>> This patch is based on commit
>> a8d982e1f17 x86: cpu: Fix crash on FTRACE enabled builds
>> of the master branch of U-Boot.
>>
>> Patch has been tested in the context of USB DFU boot on the J784S4 SoC
>> with a Cadence USB 3.0 Controller. Since the Linux device-tree for the
>> J784S4-EVM specifies the 'dr_mode' as OTG, due to reuse of Linux
>> device-tree by U-Boot, the 'dr_mode' of 'otg' causes USB DFU Boot to 
>> fail.
>> While one approach to fix it is to update 'dr_mode' to 'peripheral' in
>> the 'k3-j784s4-u-boot.dtsi' file as a HACK, it doesn't scale to other
>> SoCs and all of their 'u-boot.dtsi' files will have to be udpated as 
>> well.
>> While I am providing Texas Instruments SoCs as an example, I presume that
>> SoCs from other vendors are also affected by the 'dr_mode' being 'otg'.
>> This patch addresses the issue by using the VBUS Valid bit of the OTG
>> status register to override 'dr_mode' from 'otg' to a deterministic role
>> to enable functionality.
>>
>> The USB DFU Boot Logs for the R5 SPL stage on J784S4 SoC with the current
>> U-Boot master as of commit
>> a8d982e1f17 x86: cpu: Fix crash on FTRACE enabled builds
>> are:
>> https://gist.github.com/Siddharth-Vadapalli-at-TI/ 
>> b910f326b6c9fad760190bd299ae3ff3
>> and the logs indicate that USB DFU Boot fails. The failure is 
>> specifically
>> due to 'dr_mode' being 'otg' and the cdns3_bind() function failing to 
>> bind
>> the device-tree node to the peripheral driver required for DFU Boot.
>>
>> The USB DFU Boot Logs for the R5 SPL stage with just this patch applied
>> while leaving 'dr_mode' at its existing value of 'otg' are:
>> https://gist.github.com/Siddharth-Vadapalli-at- 
>> TI/033b9772b6ab105b96806adf890848fe
>>
>> Regards,
>> Siddharth.
>>
>>   drivers/usb/cdns3/core.c | 53 ++++++++++++++++++++++++++++++++++++++++
>>   drivers/usb/cdns3/drd.c  | 11 +++++++++
>>   2 files changed, 64 insertions(+)
>>
>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>> index 4434dc15bec..b7be5c48fc9 100644
>> --- a/drivers/usb/cdns3/core.c
>> +++ b/drivers/usb/cdns3/core.c
>> @@ -392,6 +392,55 @@ static const struct udevice_id cdns3_ids[] = {
>>       { },
>>   };
>> +/*
>> + * The VBUS Valid Bit in the OTG Status register can be used to 
>> determine
>> + * the role. When VBUS Valid is set, it indicates that a USB Host is 
>> supplying
>> + * power, so the Controller should assume the PERIPHERAL role. If it 
>> isn't set,
>> + * it indicates the absence of a USB Host, so the Controller should 
>> assume the
>> + * HOST role. If the OTG Status register is inaccessible, return OTG 
>> role.
>> + */
>> +static enum usb_dr_mode cdns3_get_otg_mode(ofnode node)
>> +{
>> +    struct cdns3 cdns, *cdnsp;
>> +    void __iomem *otg_regs;
>> +    fdt_addr_t otg_addr;
>> +    int otg_reg_index;
>> +    int vbus;
>> +
>> +    otg_reg_index = ofnode_stringlist_search(node, "reg-names", "otg");
>> +    if (otg_reg_index < 0)
>> +        return USB_DR_MODE_OTG;
> This is wrong, the function should return error and possibly mode via 
> function parameter , do not conflate return value and mode please .

Regarding returning an error, the patch maintains status quo for the 
case where we are unable to read the USB OTG Status register. Please 
refer to the following diff in the patch:


	@@ -413,6 +462,10 @@ int cdns3_bind(struct udevice *parent)
  		if (dr_mode == USB_DR_MODE_UNKNOWN)
  			dr_mode = usb_get_dr_mode(dev_ofnode(parent));

	+	/* Use VBUS Valid to determine role */
	+	if (dr_mode == USB_DR_MODE_OTG)
	+		dr_mode = cdns3_get_otg_mode(node);
	+
  		switch (dr_mode) {
	 #if defined(CONFIG_SPL_USB_HOST) || \
  		(!defined(CONFIG_XPL_BUILD) && defined(CONFIG_USB_HOST))

Only if the 'dr_mode' is already 'OTG', the newly added function 
cdns3_get_otg_mode() is invoked. In case cdns3_get_otg_mode() cannot 
determine the mode (otg status register cannot be read), we return otg 
mode to maintain existing behavior, following which, the code will use 
the CONFIGS to determine the role.

Returning an error will alter existing behavior rather than extending it 
for enhancing functionality.

Regarding passing the mode via a function parameter, I could do so, but 
I followed the implementation of other functions such as
	usb_get_dr_mode()
which 'returns' the mode rather than using a function parameter to fill 
in the mode.

Please let me know what you think.

Regards,
Siddharth.