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

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