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

[Marek Vasut <marek.vasut@mailbox.org>]

On 8/29/26 6:19 PM, Krishna Kurapati wrote:

Hi,

>> +static int dwc3_renesas_rcar_gen5_init(struct dwc3_generic *dwc3g)
>> +{
>> +    struct device *dev = dwc3g->dev;
>> +    struct platform_device *pdev = to_platform_device(dev);
>> +    enum usb_device_speed speed = usb_get_maximum_speed(dev);
>> +    bool usb2only = false;
>> +    void __iomem *glue;
>> +
>> +    /* Wireless USB is not supported */
>> +    if (speed == USB_SPEED_WIRELESS)
>> +        return dev_err_probe(dev, -EINVAL, "Wireless USB not 
>> supported\n");
>> +
>> +    /* No USB 3 PHY in DT means this is surely USB 2 controller */
>> +    if (device_property_match_string(dev, "phy-names", "usb3-phy") < 0)
>> +        usb2only = true;
>> +
> 
> Why not just set the maximum-speed property to "high-speed" instead of 
> checking for "usb3-phy". That would be more clean.

Please see 1/2, maximum-speed DT property and the presence of the 
usb3-phy are both optional, hence this code. Also, this was discussed in 
previous version 2 of this patchset already.

>> +    if (speed >= USB_SPEED_LOW && speed <= USB_SPEED_HIGH)
>> +        usb2only = true;
> 
> And this check can be removed as well if we use "maximum-speed" property 
> in DT.

Please see above.

>> +
>> +    glue = devm_platform_ioremap_resource_byname(pdev, "glue");
>> +    if (IS_ERR(glue))
>> +        return PTR_ERR(glue);
>> +
>> +    /*
>> +     * The datasheet describes initialization procedure without full
>> +     * information about the registers. Therefore, the source code is
>> +     * based on the bare metal code shared by the board team.
>> +     */
>> +    writew(0x211, glue + 0x26);
>> +
>> +    /* USB2 does need additional register programming. */
>> +    if (!usb2only)
>> +        return 0;
>> +
>> +    writew(0x11, glue + 0x81c);
>> +    writew(0x0, glue + 0x81a);
>> +    writew(0x1, glue + 0x802);
>> +
>> +    usleep_range(10000, 20000);
>> +
>> +    writew(0x0, glue + 0x802);
>> +    writew(0x1, glue + 0x2a);
>> +    writew(0x1, glue + 0x81a);
>> +
>> +    usleep_range(10000, 20000);
>> +
> 
> Use #defines for register offsets and register values like in 
> dwc3_eic7700_init.

Please see the comment above this section, it clarifies why the magic 
numbers, and it was discussed in previous version 1 of this patchset too.