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

[Marek Vasut]

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.