[PATCH] usb: cdns3: fix cdns3_bind() to avoid returning error and probe parent

[Siddharth Vadapalli]

On 04/04/26 04:05, Marek Vasut wrote:
> On 3/4/26 7:46 AM, Siddharth Vadapalli wrote:
>> On 03/03/26 19:37, Marek Vasut wrote:
>>> On 3/1/26 6:25 AM, Siddharth Vadapalli wrote:
>>>
>>>>>> There are two users of cdns3_bind() which have registered their 
>>>>>> own .bind() callback as cdns3_bind():
>>>>>> 1. cdns3-starfive.c driver
>>>>>> 2. cdns3-ti.c driver
>>>>>> Since .bind() is called before .probe()
>>>>>
>>>>> .bind callback is meant to BIND the driver instance (to a DT node), it 
>>>>> is not meant to initialize the hardware. .probe callback is meant to 
>>>>
>>>> I agree. But that's exactly where the issue lies. When 'dr_mode' is set 
>>>> to OTG, cdns3_bind() is unable to determine if the driver to bind to 
>>>> the DT node is "cdns-usb3-host" or "cdns-usb3-peripheral".
>>>>
>>>> Since [0], we attempt to identify if OTG should imply 'peripheral' or 
>>>> 'host'. But to do that, [0] needs to read the VBUS Valid field of the 
>>>> OTG Status register. And register access requires the USB Controller to 
>>>> be powered ON.
>>>
>>> Correct, so bind is the wrong place to do any HW access .
>>>
>>>> [0]: bfb530e06ca6 ("usb: cdns3: use VBUS Valid to determine role for 
>>>> dr_mode OTG")
>>>>
>>>>> initialize the hardware. This is fundamental part of the U-Boot DM 
>>>>> lazy initialization scheme. If something does initialize hardware 
>>>>> in .bind, then it is very likely wrong.
>>>>>
>>>>>> , and, since cdns3_bind() is also the .bind() callback of the above 
>>>>>> drivers, device_probe() has to be added here itself. Moreover, it is 
>>>>>> only due to the register accesses being performed by the recently 
>>>>>> introduced cdns3_get_otg_mode() function that we need the USB 
>>>>>> Controller to be powered ON at this stage.
>>>>> So, can we fix this so the HW initialization won't be happening in 
>>>>> bind, but instead in .probe ?
>>>>
>>>> That isn't possible
>>>
>>> Can you investigate how to make that possible ? Maybe by having some 
>>> common .probe implementation that would determine the OTG state, or by 
>>> having some stub driver which handles the OTG selection ?
>> A common .probe will not work because the .id has to be either
>> UCLASS_USB_GADGET_GENERIC OR UCLASS_USB
>> for the new stub driver (say cdns-usb3-otg). By fixing .id beforehand, we 
>> will always fail to probe the driver for the case where the desired role 
>> is the opposite of the role corresponding to .id.
> 
> Looking at this one more time, drivers/usb/cdns3/cdns3-ti.c is already 
> UCLASS_NOP , that is the parent driver that you are trying to probe in bind 
> here, right ?

Yes.

> 
> Could you instead flag that cdns3-ti.c with dev_or_flags(dev, 
> DM_FLAG_PROBE_AFTER_BIND); so it would probe early ? Would that resolve 
> this problem too ?

DM_FLAG_PROBE_AFTER_BIND will not fix the issue because:
1. cdns3-ti.c registers cdns3_bind() as its .bind() callback.
2. cdns3_bind() will be called before the .probe() callback of cdns3-ti.c
    even with DM_FLAG_PROBE_AFTER_BIND
3. With cdns3_bind() being called before .probe() of cdns3-ti.c, the USB
    Controller is still Powered OFF.
I have tested it nevertheless and have confirmed that the issue isn't fixed.

> 
> And just a follow up idea, if you were to stop DM scan at DT compatible = 
> "ti,j721e-usb"; in dts/upstream/src/arm64/ti/k3-j784s4-j742s2-main- 
> common.dtsi (i.e. the cdns3 driver wouldn't bind at all), maybe you could 
> make the cdns3-ti.c bind the right driver (peripheral or host) the same way

For cdns3-ti.c to bind the right driver, it still needs to read the OTG 
register when dr_mode is set to otg in the device-tree. This would mean 
that the USB Controller has to be powered ON. Since controller is powered 
ON by cdns_ti_probe (.probe callback of cdns3-ti.c), a different .bind 
callback in cdns3-ti.c will still not fix the issue.

> e.g. block device partitions or USB devices are bound during 'mmc rescan' 
> or 'usb reset' . All those devices are instantiated after the initial DM 
> scan and after the controller to which they are attached has been probed. 
> This might be the way to avoid probe in bind too ?

The device-tree node layout and the corresponding drivers / callbacks are 
as follows:

	usbss0: usb at 4104000 {
		compatible = "ti,j721e-usb"; => cdns3-ti.c driver
		[...]
		usb0: usb at 6000000 {
			compatible = "cdns,usb3";
			[...]
		};
	};

First, the cdns3-ti.c driver's .bind callback is invoked. Since cdns3-ti.c 
registered cdns3_bind() as the .bind callback, cdns3_bind() is invoked 
before the .probe callback of cdns3-ti.c driver is invoked. As a result,
the USB Controller is still powered OFF for non-USB Boot Modes when 
cdns3_bind() executes.

Regards,
Siddharth.