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

[Siddharth Vadapalli]

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)

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.

Regards,
Siddharth.