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

[Siddharth Vadapalli]

On 01/03/26 06:50, Marek Vasut wrote:
> On 3/1/26 2:02 AM, Siddharth Vadapalli wrote:
> 
>>>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>>>> index 10bc4cabed4..a775567b3bc 100644
>>>> --- a/drivers/usb/cdns3/core.c
>>>> +++ b/drivers/usb/cdns3/core.c
>>>> @@ -461,9 +461,18 @@ int cdns3_bind(struct udevice *parent)
>>>>       /* Use VBUS Valid to determine role */
>>>>       if (dr_mode == USB_DR_MODE_OTG) {
>>>> +        /*
>>>> +         * Since we perform register accesses in cdns3_get_otg_mode(),
>>>> +         * we need to ensure that the parent has been probed and that
>>>> +         * the USB Controller is powered ON.
>>>> +         */
>>>> +        ret = device_probe(parent);
>>>> +        if (ret)
>>>> +            goto fail;
>>>> +
>>>>           ret = cdns3_get_otg_mode(parent, &dr_mode);
>>>>           if (ret < 0)
>>>> -            return ret;
>>>> +            goto fail;
>>>>       }
>>> Can you instead move this logic into driver .probe() ? Then you wouldn't 
>>> need the device_probe() hackage.
>>>
>>
>> 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.

[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 when 'dr_mode' is OTG because the .probe() callback 
itself is determined by cdns3_bind(). Whether it is "cdns-usb3-host" 
driver's .probe() callback or "cdns-usb3-peripheral" driver's .probe() 
callback depends on cdns3_bind() being able to decide which driver to bind 
to the DT node. And for cdns3_bind() to decide the driver when 'dr_mode' is 
OTG, we need to read the VBUS Valid field. So cdns3_bind() needs to ensure 
that the USB Controller is powered ON (device_probe() of parent).