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

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

On Tue, Jul 28, 2026, Marek Vasut wrote:
> From: Thanh Quan <thanh.quan.xn@renesas.com>
> 
> The Renesas R-Car Gen5 SoC contains multiple instances of DWC3 USB
> controller with glue logic wrapper around them. Add driver for the
> glue logic around the DWC3 USB controller core.
> 
> Signed-off-by: Thanh Quan <thanh.quan.xn@renesas.com>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> ---
> Cc: Conor Dooley <conor+dt@kernel.org>
> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-renesas-soc@vger.kernel.org
> Cc: linux-usb@vger.kernel.org
> ---
>  drivers/usb/dwc3/Kconfig          |  10 ++
>  drivers/usb/dwc3/Makefile         |   1 +
>  drivers/usb/dwc3/dwc3-rcar-gen5.c | 150 ++++++++++++++++++++++++++++++
>  3 files changed, 161 insertions(+)
>  create mode 100644 drivers/usb/dwc3/dwc3-rcar-gen5.c
> 
> diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> index 18169727a413e..7d8ee192b1241 100644
> --- a/drivers/usb/dwc3/Kconfig
> +++ b/drivers/usb/dwc3/Kconfig
> @@ -234,4 +234,14 @@ config USB_DWC3_GOOGLE
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called dwc3-google.ko.
>  
> +config USB_DWC3_RCAR_GEN5
> +	tristate "Renesas R-Car Gen5 DWC3 Platform Driver"
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	default USB_DWC3
> +	help
> +	  Support the DesignWare Core USB3 IP found on Renesas R-Car Gen5
> +	  SoC. This driver provides the glue layer for the USB controller
> +	  on Renesas R-Car Gen5 platform.
> +	  Say 'Y' or 'M' if you have such device.
> +
>  endif
> diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> index f37971197203e..99bbd147f0b5f 100644
> --- a/drivers/usb/dwc3/Makefile
> +++ b/drivers/usb/dwc3/Makefile
> @@ -61,3 +61,4 @@ obj-$(CONFIG_USB_DWC3_OCTEON)		+= dwc3-octeon.o
>  obj-$(CONFIG_USB_DWC3_RTK)		+= dwc3-rtk.o
>  obj-$(CONFIG_USB_DWC3_GENERIC_PLAT)	+= dwc3-generic-plat.o
>  obj-$(CONFIG_USB_DWC3_GOOGLE)		+= dwc3-google.o
> +obj-$(CONFIG_USB_DWC3_RCAR_GEN5)	+= dwc3-rcar-gen5.o
> diff --git a/drivers/usb/dwc3/dwc3-rcar-gen5.c b/drivers/usb/dwc3/dwc3-rcar-gen5.c
> new file mode 100644
> index 0000000000000..344d0851559f5
> --- /dev/null
> +++ b/drivers/usb/dwc3/dwc3-rcar-gen5.c
> @@ -0,0 +1,150 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Renesas USB device driver with DWC3 integration
> + *
> + * Copyright (C) 2025-2026 Renesas Electronics Corporation
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/reset.h>
> +
> +struct dwc3_rcar_gen5_priv {
> +	void __iomem		*base;
> +	struct clk		*clk;
> +	bool			use_usb3_flow;
> +};
> +
> +static int dwc3_rcar_gen5_init(struct dwc3_rcar_gen5_priv *priv)
> +{
> +	/*
> +	 * 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, priv->base + 0x26);
> +
> +	/* USB3 does not need additional register programming. */
> +	if (priv->use_usb3_flow)
> +		return 0;
> +
> +	writew(0x11, priv->base + 0x81c);
> +	writew(0x0, priv->base + 0x81a);
> +	writew(0x1, priv->base + 0x802);
> +
> +	usleep_range(10000, 20000);
> +
> +	writew(0x0, priv->base + 0x802);
> +	writew(0x1, priv->base + 0x2a);
> +	writew(0x1, priv->base + 0x81a);

Use macros for these magic numbers.

> +
> +	usleep_range(10000, 20000);
> +
> +	return 0;
> +}
> +
> +static int dwc3_rcar_gen5_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *subnode;
> +	struct reset_control *reset;
> +	const char *maximum_speed;
> +	struct dwc3_rcar_gen5_priv *priv;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	priv->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(priv->base))
> +		return dev_err_probe(dev, PTR_ERR(priv->base), "Failed to map registers\n");
> +
> +	reset = devm_reset_control_get(dev, NULL);

Where is the "reset" being used? How is this reset asserted/deasserted?

> +	if (IS_ERR(reset))
> +		return dev_err_probe(dev, PTR_ERR(reset), "Failed to get reset control\n");
> +
> +	priv->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(priv->clk))
> +		return dev_err_probe(dev, PTR_ERR(priv->clk), "Failed to get clock control\n");
> +
> +	subnode = of_get_compatible_child(dev->of_node, "synopsys,dwc3");
> +	if (!subnode)
> +		return dev_err_probe(dev, -ENODEV, "Failed to find DWC3 subnode node\n");
> +
> +	ret = of_property_read_string(subnode, "maximum-speed", &maximum_speed);
> +	of_node_put(subnode);
> +	if (ret)
> +		return dev_err_probe(dev, -ENODEV, "Failed to determine maximum speed\n");

Why is maximum-speed a hard requirement? It should be optional.

> +
> +	priv->use_usb3_flow = !strcmp(maximum_speed, "super-speed-plus") ||
> +			      !strcmp(maximum_speed, "super-speed");
> +
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to enable runtime PM\n");

Do you really want this to be a hard error?

> +
> +	ret = pm_runtime_resume_and_get(dev);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to resume runtime PM\n");

Same question here.

> +
> +	return devm_of_platform_populate(dev);
> +}
> +
> +static int __maybe_unused dwc3_rcar_gen5_suspend(struct device *dev)

Avoid using __maybe_unused. If it's not used, remove it.

> +{
> +	struct dwc3_rcar_gen5_priv *priv = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(priv->clk);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused dwc3_rcar_gen5_resume(struct device *dev)
> +{
> +	struct dwc3_rcar_gen5_priv *priv = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = clk_prepare_enable(priv->clk);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable clock on resume: %d\n", ret);
> +		return ret;
> +	}
> +
> +	usleep_range(10000, 20000);
> +
> +	return dwc3_rcar_gen5_init(priv);
> +}
> +
> +static DEFINE_RUNTIME_DEV_PM_OPS(dwc3_rcar_gen5_pm_ops,
> +				 dwc3_rcar_gen5_suspend,
> +				 dwc3_rcar_gen5_resume, NULL);
> +
> +static const struct of_device_id dwc3_rcar_gen5_of_match[] = {
> +	{ .compatible = "renesas,rcar-gen5-usb" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, dwc3_rcar_gen5_of_match);
> +
> +static struct platform_driver dwc3_rcar_gen5_driver = {
> +	.probe		= dwc3_rcar_gen5_probe,
> +	.driver		= {
> +		.name	= "renesas-rcar-gen5-usb",
> +		.of_match_table = dwc3_rcar_gen5_of_match,
> +		.pm	= &dwc3_rcar_gen5_pm_ops,
> +	},
> +};
> +
> +module_platform_driver(dwc3_rcar_gen5_driver);
> +
> +MODULE_AUTHOR("Thanh Quan");
> +MODULE_DESCRIPTION("Renesas R-Car X5H USB Glue Layer Driver");
> +MODULE_LICENSE("GPL");
> -- 
> 2.53.0

Please use the new flatten glue model. The logic for this glue looks
simple. Can you review and evaluate if we can enhance and whether we
should use the dwc3-generic-plat glue driver for this?

BR,
Thinh