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

[Siddharth Vadapalli]

Hello Marek,

On 13/03/26 09:29, 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)
> 
> 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.
Please note that this revert patch is required to fix non-USB Boot Mode. If 
you have a suggestion for an alternative, please let me know.

Regards,
Siddharth.