Re: [PATCH v2 2/2] usb: dwc3: dwc3-generic-plat: Add Renesas R-Car Gen5 DWC3 xHCI USB controller glue

[Marek Vasut]

On 8/11/26 1:18 AM, Thinh Nguyen wrote:

> > > > +static int dwc3_renesas_rcar_gen5_init(struct dwc3_generic *dwc3g)
> > > > +{
> > > > +	struct device *dev = dwc3g->dev;
> > > > +	struct platform_device *pdev = to_platform_device(dev);
> > > > +	const char *maximum_speed;
> > > > +	bool use_usb3_flow;
> > > > +	void __iomem *glue;
> > > > +	int ret;
> > > > +
> > > > +	glue = devm_platform_ioremap_resource_byname(pdev, "glue");
> > > > +	if (IS_ERR(glue))
> > > > +		return PTR_ERR(glue);
> > > > +
> > > > +	ret = of_property_read_string(dev->of_node, "maximum-speed", &maximum_speed);
> > > > +	if (ret)
> > > > +		return dev_err_probe(dev, -ENODEV, "Failed to determine maximum speed\n");
> > >
> > > We shouldn't overload the generic "maximum-speed" definition.
> > >
> > > Can we check for the presence of the "usb3-phy" instead?
> > This could be made to work, but I don't think this would work nicely, please
> > see below.
> >
> > The Gen5 contains four DWC3 controllers, two are USB2-only and two are
> > combined USB2+USB3.2 ; the USB2-only controllers each use one PHY, the
> > USB2+USB3.2 controllers each use two PHYs -- one USB2 and one USB3 PHY.
> > -> The PHYs are always present and always connected inside the SoC, so I
> >     believe the PHYs should always be described in the SoC DT, and the
> >     DWC3 controller node should always have phandle(s) to its matching
> >     PHYs.
> >
> > The USB2-only controllers are a non-issue, there we could get away with
> > checking for the presence of "usb3-phy" and if this is missing in DT, apply
> > the magic code ("use_usb3_flow" is always false).
> >
> > It is the USB2+USB3.2 controllers that are more complicated. These
> > controllers can be downgraded to USB2-only mode. Currently, this is done via
> > this "maximum-speed" DT property, which, if set to "high-speed" or lower
> > triggers the application of the magic code. If the "maximum-speed" DT
> > property is "super-speed" or "super-speed-plus", the magic code is not
> > applied (this is the "use_usb3_flow").
> >
> > If we were to opt for checking for presence of "usb3-phy" , then the board
> > DTs (which include the SoC DT) for board which would require the USB2+USB3.2
> > controller to be downgraded to USB2-only mode would have to adjust the PHY
> > phandles of such controller DT node in board DT, which itself does not seem
> > right (DT describes the SoC hardware, and the PHY is still in the SoC and
> > connected to the controller, please see "->" above).
> >
> > That is why using "maximum-speed" is I think a bit better.
> >
> > But maybe there is yet another option that I missed ?
> >
> > Thank you for your help !
>
> I see.
>
> My concern is that "maximum-speed" is a generic property that describes
> the maximum speed the controller is allowed to operate at. Per the
> binding, if it is not present the controller should default to its
> maximum HW capability.
>
> Would this work for you? (Given that the usb3 phy phandle must be
> present)
>
> 	if (!has_usb3_phy || maximum_speed <= USB_SPEED_HIGH)
> 		use_usb3_flow = false;
>
> This way, "maximum-speed" retains its generic meaning as a speed limit
> rather than being used by itself to select Renesas specific flow.

This works nicely.

I will include a slightly reworked variant of this in V3.

Thank you !

--
Best regards,
Marek Vasut