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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] R-Car Gen5 platform-specific glue logic registers lose their state across system suspend/resume cycles, permanently breaking the USB controller upon wake.
--

commit 3972c6a9582e00349a3238f262e250712caa62f9
Author: Thanh Quan <thanh.quan.xn@renesas.com>

usb: dwc3: dwc3-generic-plat: Add Renesas R-Car Gen5 DWC3 xHCI USB controller glue

The Renesas R-Car Gen5 SoC contains multiple instances of DWC3 USB controller
with glue logic wrapper around them. Extend the generic DWC3 platform driver
with Renesas R-Car Gen5 glue logic specifics.

> diff --git a/drivers/usb/dwc3/dwc3-generic-plat.c b/drivers/usb/dwc3/dwc3-generic-plat.c
> index ca69ac0eb07ce..32497ab6f065e 100644
> --- a/drivers/usb/dwc3/dwc3-generic-plat.c
> +++ b/drivers/usb/dwc3/dwc3-generic-plat.c

[ ... ]

> @@ -71,6 +71,51 @@ static int dwc3_eic7700_init(struct dwc3_generic *dwc3g)
>  	return 0;
>  }
>  
> +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;
> +
> +	/* 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;
> +
> +	if (speed >= USB_SPEED_LOW && speed <= USB_SPEED_HIGH)
> +		usb2only = true;
> +
> +	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);

[Severity: High]
Does this setup break across system suspend/resume cycles?

Since the 'glue' pointer is instantiated as a local variable and discarded
after this initialization function returns, how are these hardware registers
restored when the controller's power domain is turned off during a
suspend-to-RAM (S3) or deep sleep cycle?

Looking at dwc3_generic_resume(), it restores the DWC3 core but doesn't seem
to have a configuration callback to re-initialize the platform-specific glue
wrapper. Even if there were a resume callback, it wouldn't be able to access
the remapped 'glue' __iomem pointer.

Will this cause the USB controller to become permanently non-functional after
system resume?

> +
> +	/* 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);
> +
> +	return 0;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903213031.314473-1-marek.vasut+renesas@mailbox.org?part=2