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

[Marek Vasut]

On 3/13/26 4:59 AM, Siddharth Vadapalli wrote:
> On 12/03/26 20:31, Marek Vasut wrote:
>> On 3/12/26 2:50 PM, Tom Rini wrote:
>>> On Thu, Mar 12, 2026 at 07:02:47PM +0530, Prasanth Babu Mantena wrote:
>>>
>>>> While USB DFU boot works with this patch, but the non USB boot modes 
>>>> like
>>>> SD Boot and flash boot fails for J784S4 EVM device.
>>>>
>>>> So, Reverting this patch.
>>>>
>>>> This reverts commit bfb530e06ca6c19f66c079601e568c761a001993.
>>>>
>>>> Signed-off-by: Prasanth Babu Mantena <p-mantena at ti.com>
>>>> ---
>>>>   drivers/usb/cdns3/core.c | 53 
>>>> ----------------------------------------
>>>>   drivers/usb/cdns3/drd.c  | 11 ---------
>>>>   2 files changed, 64 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>>>> index 10bc4cabed4..4434dc15bec 100644
>>>> --- a/drivers/usb/cdns3/core.c
>>>> +++ b/drivers/usb/cdns3/core.c
>>>> @@ -392,52 +392,6 @@ 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 an 
>>>> error.
>>>> - */
>>>> -static int cdns3_get_otg_mode(struct udevice *parent, enum 
>>>> usb_dr_mode *mode)
>>>> -{
>>>> -    /* Create a temporary child device for using 
>>>> devfdt_remap_addr_name() */
>>>> -    struct udevice child = {
>>>> -        .parent = parent,
>>>> -    };
>>>> -    struct cdns3 cdns, *cdnsp;
>>>> -    void __iomem *otg_regs;
>>>> -
>>>> -    dev_set_ofnode(&child, ofnode_first_subnode(dev_ofnode(parent)));
>>>> -    otg_regs = devfdt_remap_addr_name(&child, "otg");
>>>> -    if (!otg_regs) {
>>>> -        dev_err(parent, "failed to get otg registers for child 
>>>> node\n");
>>>> -        return -ENXIO;
>>>> -    }
>>>> -
>>>> -    /*
>>>> -     * As mentioned in drivers/usb/cdns3/drd.c, there are two versions
>>>> -     * of the Controller. The following logic detects the version 
>>>> of the
>>>> -     * Controller and interprets the register layout accordingly.
>>>> -     */
>>>> -    cdnsp = &cdns;
>>>> -    cdnsp->otg_v0_regs = otg_regs;
>>>> -    if (!readl(&cdnsp->otg_v0_regs->cmd)) {
>>>> -        cdnsp->otg_regs = otg_regs;
>>>> -    } else {
>>>> -        cdnsp->otg_v1_regs = otg_regs;
>>>> -        cdnsp->otg_regs = (void *)&cdnsp->otg_v1_regs->cmd;
>>>> -    }
>>>> -
>>>> -    /* Use VBUS Valid to determine role */
>>>> -    if (readl(&cdnsp->otg_regs->sts) & OTGSTS_VBUS_VALID)
>>>> -        *mode = USB_DR_MODE_PERIPHERAL;
>>>> -    else
>>>> -        *mode = USB_DR_MODE_HOST;
>>>> -
>>>> -    return 0;
>>>> -}
>>>> -
>>>>   int cdns3_bind(struct udevice *parent)
>>>>   {
>>>>       enum usb_dr_mode dr_mode;
>>>> @@ -459,13 +413,6 @@ 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) {
>>>> -        ret = cdns3_get_otg_mode(parent, &dr_mode);
>>>> -        if (ret < 0)
>>>> -            return ret;
>>>> -    }
>>>> -
>>>>       switch (dr_mode) {
>>>>   #if defined(CONFIG_SPL_USB_HOST) || \
>>>>       (!defined(CONFIG_XPL_BUILD) && defined(CONFIG_USB_HOST))
>>>> diff --git a/drivers/usb/cdns3/drd.c b/drivers/usb/cdns3/drd.c
>>>> index 0ca40a5cc8d..cbb13342343 100644
>>>> --- a/drivers/usb/cdns3/drd.c
>>>> +++ b/drivers/usb/cdns3/drd.c
>>>> @@ -301,17 +301,6 @@ int cdns3_drd_init(struct cdns3 *cdns)
>>>>           cdns->dr_mode = USB_DR_MODE_PERIPHERAL;
>>>>       }
>>>> -    /*
>>>> -     * In the absence of STRAP configuration, use VBUS Valid to
>>>> -     * determine the appropriate role to be assigned to dr_mode.
>>>> -     */
>>>> -    if (cdns->dr_mode == USB_DR_MODE_OTG) {
>>>> -        if (cdns3_get_vbus(cdns))
>>>> -            cdns->dr_mode = USB_DR_MODE_PERIPHERAL;
>>>> -        else
>>>> -            cdns->dr_mode = USB_DR_MODE_HOST;
>>>> -    }
>>>> -
>>>>       state = readl(&cdns->otg_regs->sts);
>>>>       if (OTGSTS_OTG_NRDY(state) != 0) {
>>>>           dev_err(cdns->dev, "Cadence USB3 OTG device not ready\n");
>>>
>>> Adding in the USB custodian as this revert is intended for master and
>>> v2026.04 I assume as the commit being reverted is also in master.
>>> Thanks.
>> This may be related to
>>
>> [PATCH] usb: cdns3: fix cdns3_bind() to avoid returning error and 
>> probe parent
>>
>> ?
>>
>> Can you coordinate with Siddharth Vadapalli ?
> Please refer to my response at:
> https://lore.kernel.org/r/17a341aa-4711-4389-8ea1-a8169616baeb@ti.com/
> on the patch quoted above
> ([PATCH] usb: cdns3: fix cdns3_bind() to avoid returning error and probe 
> parent)

I just replied to it (sorry, I am completely drowning in emails).

> A common .probe as a stub will not work and probing the parent in 
> cdns3_bind() doesn't seem to be acceptable. So reverting the commit 
> appears to be the only option at the moment to fix non-USB Boot Modes.
If this is a stop-gap measure for this release, to avoid breakage on TI 
platforms, sure, go with the revert.

But it would be nice if the bind in probe could be fixed properly at 
some point. I think it might be doable, see my reply to [PATCH] usb: 
cdns3: fix cdns3_bind() to avoid returning error and probe parent .