Hello Marek, On Thu, 2026-02-05 at 06:41 +0530, Siddharth Vadapalli wrote: > On 04/02/26 7:39 PM, Marek Vasut wrote: > > On 2/4/26 1:12 PM, Siddharth Vadapalli wrote: > > > The cdns3_bind() function is responsible for identifying the appropriate > > > driver to bind to the USB Controller's device-tree node. If the > > > device-tree > > > node has the 'dr_mode' property set to 'otg', the existing approach fails > > > to bind a driver, leading to loss of functionality. > > > > > > To address this, use the VBUS Valid field of the OTG Status register to > > > determine the role as follows: > > > - If VBUS Valid field is set, it indicates that a USB Host is supplying > > > power and the Controller should assume the Peripheral role. > > > - If VBUS Valid field is clear, it indicates the absence of a USB Host > > > and > > > the Controller should assume the Host role. > > > > > > Additionally, when 'dr_mode' happens to be 'otg' and the STRAP settings > > > are not specified, use VBUS Valid to determine the role in > > > cdns3_drd_init() > > > and assign it to cdns->dr_mode. > > > > > > Signed-off-by: Siddharth Vadapalli <[email protected]> > > > --- > > > > > > Hello, > > > > > > This patch is based on commit > > > a8d982e1f17 x86: cpu: Fix crash on FTRACE enabled builds > > > of the master branch of U-Boot. > > > > > > Patch has been tested in the context of USB DFU boot on the J784S4 SoC > > > with a Cadence USB 3.0 Controller. Since the Linux device-tree for the > > > J784S4-EVM specifies the 'dr_mode' as OTG, due to reuse of Linux > > > device-tree by U-Boot, the 'dr_mode' of 'otg' causes USB DFU Boot to > > > fail. > > > While one approach to fix it is to update 'dr_mode' to 'peripheral' in > > > the 'k3-j784s4-u-boot.dtsi' file as a HACK, it doesn't scale to other > > > SoCs and all of their 'u-boot.dtsi' files will have to be udpated as > > > well. > > > While I am providing Texas Instruments SoCs as an example, I presume that > > > SoCs from other vendors are also affected by the 'dr_mode' being 'otg'. > > > This patch addresses the issue by using the VBUS Valid bit of the OTG > > > status register to override 'dr_mode' from 'otg' to a deterministic role > > > to enable functionality. > > > > > > The USB DFU Boot Logs for the R5 SPL stage on J784S4 SoC with the current > > > U-Boot master as of commit > > > a8d982e1f17 x86: cpu: Fix crash on FTRACE enabled builds > > > are: > > > https://gist.github.com/Siddharth-Vadapalli-at-TI/ > > > b910f326b6c9fad760190bd299ae3ff3 > > > and the logs indicate that USB DFU Boot fails. The failure is > > > specifically > > > due to 'dr_mode' being 'otg' and the cdns3_bind() function failing to > > > bind > > > the device-tree node to the peripheral driver required for DFU Boot. > > > > > > The USB DFU Boot Logs for the R5 SPL stage with just this patch applied > > > while leaving 'dr_mode' at its existing value of 'otg' are: > > > https://gist.github.com/Siddharth-Vadapalli-at- > > > TI/033b9772b6ab105b96806adf890848fe > > > > > > Regards, > > > Siddharth. > > > > > > drivers/usb/cdns3/core.c | 53 ++++++++++++++++++++++++++++++++++++++++ > > > drivers/usb/cdns3/drd.c | 11 +++++++++ > > > 2 files changed, 64 insertions(+) > > > > > > diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c > > > index 4434dc15bec..b7be5c48fc9 100644 > > > --- a/drivers/usb/cdns3/core.c > > > +++ b/drivers/usb/cdns3/core.c > > > @@ -392,6 +392,55 @@ 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 OTG > > > role. > > > + */ > > > +static enum usb_dr_mode cdns3_get_otg_mode(ofnode node) > > > +{ > > > + struct cdns3 cdns, *cdnsp; > > > + void __iomem *otg_regs; > > > + fdt_addr_t otg_addr; > > > + int otg_reg_index; > > > + int vbus; > > > + > > > + otg_reg_index = ofnode_stringlist_search(node, "reg-names", "otg"); > > > + if (otg_reg_index < 0) > > > + return USB_DR_MODE_OTG; > > This is wrong, the function should return error and possibly mode via > > function parameter , do not conflate return value and mode please . > > Regarding returning an error, the patch maintains status quo for the > case where we are unable to read the USB OTG Status register. Please > refer to the following diff in the patch: > > > @@ -413,6 +462,10 @@ 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) > + dr_mode = cdns3_get_otg_mode(node); > + > switch (dr_mode) { > #if defined(CONFIG_SPL_USB_HOST) || \ > (!defined(CONFIG_XPL_BUILD) && defined(CONFIG_USB_HOST)) > > Only if the 'dr_mode' is already 'OTG', the newly added function > cdns3_get_otg_mode() is invoked. In case cdns3_get_otg_mode() cannot > determine the mode (otg status register cannot be read), we return otg > mode to maintain existing behavior, following which, the code will use > the CONFIGS to determine the role. > > Returning an error will alter existing behavior rather than extending it > for enhancing functionality. > > Regarding passing the mode via a function parameter, I could do so, but > I followed the implementation of other functions such as > usb_get_dr_mode() > which 'returns' the mode rather than using a function parameter to fill > in the mode. > > Please let me know what you think.
Gentle reminder that I am waiting on your feedback. This patch has been marked with the 'Changes Requested' state on patchworks: https://patchwork.ozlabs.org/project/uboot/patch/[email protected]/ although the discussion is still open. Regards, Siddharth.
