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

Thinh Nguyen <[email protected]>
Newsgroups org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel,org.kernel.vger.linux-renesas-soc,org.kernel.vger.linux-usb
Message-ID <anJ6HkPgDb7I4xeL@vbox>
On Tue, Jul 28, 2026, Marek Vasut wrote:
> From: Thanh Quan <[email protected]>
> 
> 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 <[email protected]>
> Signed-off-by: Marek Vasut <[email protected]>
> ---
> Cc: Conor Dooley <[email protected]>
> Cc: Geert Uytterhoeven <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Thinh Nguyen <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
>  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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.