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

[Siddharth Vadapalli]

On Sun, 2026-02-15 at 01:04 +0100, Marek Vasut wrote:
> On 2/14/26 5:36 AM, Siddharth Vadapalli wrote:
> > Hello Marek,
> > 
> > On Fri, 2026-02-13 at 16:04 +0100, Marek Vasut wrote:
> > > On 2/13/26 6:15 AM, Siddharth Vadapalli wrote:
> > > 
> > > [...]
> > > 
> > > > +++ b/drivers/usb/cdns3/core.c
> > > > @@ -392,6 +392,63 @@ 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(ofnode node
> > > 
> > > Pass struct udevice *parent instead of node here ...
> > > 
> > > > , enum usb_dr_mode *mode)
> > > > +{
> > > > +	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) {
> > > > +		printf("%s: dr_mode is otg but otg register name is missing\n", __func__);
> > > > +		return -ENOENT;
> > > > +	}
> > > > +
> > > > +	otg_addr = ofnode_get_addr_index(node, otg_reg_index);
> > > > +	if (otg_addr == FDT_ADDR_T_NONE) {
> > > > +		printf("%s: address of otg registers is unspecified\n", __func__);
> > > > +		return -ENOENT;
> > > > +	}
> > > > +
> > > > +	otg_regs = map_physmem(otg_addr, 0, MAP_NOCACHE);
> > > > +	if (!otg_regs) {
> > > > +		printf("%s: failed to map otg registers\n", __func__);
> > > > +		return -EINVAL;
> > > > +	}
> > > 
> > > ... and replace all that ^ with devfdt_remap_addr_name(parent, "otg") .
> > > That should work.
> > 
> > It doesn't work because the 'parent' node doesn't have the 'otg' registers.
> > It is the child node which has it. The device-tree nodes for parent and
> > child are:
> > 
> > 	usbss0: usb at 4104000 {
> > 							=> PARENT
> > 		bootph-all;
> > 		compatible = "ti,j721e-usb";
> > 		reg = <0x00 0x4104000 0x00 0x100>;
> > 		dma-coherent;
> > 
> > 		[TRIMMED]
> > 
> > 		usb0: usb at 6000000
> > {							=> CHILD
> > 			bootph-all;
> > 			compatible = "cdns,usb3";
> > 			reg = <0x00 0x6000000 0x00 0x10000>,
> > 			      <0x00 0x6010000 0x00 0x10000>,
> > 			      <0x00 0x6020000 0x00 0x10000>;
> > 			reg-names = "otg", "xhci",
> > "dev";			=> OTG Register is defined here
> > 
> > 			[TRIMMED]
> > 	};
> > 
> > The 'udevice' for the child node is created in the cdns3_bind() function
> > __after__ the function in this patch is invoked. Therefore, we don't yet
> > have the child device which is created by:
> > 
> > 		device_bind_driver_to_node(parent, driver, name, node,
> > &dev);
> > 
> > I did consider the possibility of using the standard APIs
> > (devfdt_remap_addr_name), but since the child device doesn't exist yet,
> > they cannot be used. Hence, I had to implement custom logic.
> Maybe it would be possible to create a temporary udevice on stack and 
> pass it to devfdt_remap_addr_name() ? I think this should be complete 
> enough, although I might have missed something:
> 
> "
> struct udevice cdev = { .parent = parent; };
> dev_set_ofnode(cdev, node);

This works, except that I had to use the subnode rather than the parent's
node:
	dev_set_ofnode(&child, ofnode_first_subnode(dev_ofnode(parent)));

> "
> 
> devfdt_remap_addr_name() internally calls dev_of_offset(dev) on the 
> cdev, tests is device_get_uclass_id(dev->parent) == UCLASS_SIMPLE_BUS 
> (it is not) and that is it.

I have posted the v3 patch incorporating your feedback at:
https://lore.kernel.org/r/20260215123514.2288422-1-s-vadapalli@ti.com/
Thank you.

Regards,
Siddharth.