Re: [PATCH 2/5] usb: dwc3: rockchip: introduce glue driver

[Sebastian Reichel]

Hi,

On Wed, Sep 23, 2026 at 01:31:03AM +0000, Thinh Nguyen wrote:
> On Tue, Sep 15, 2026, Sebastian Reichel wrote:
> > Introduce Rockchip specific glue code for the Synopsys DWC3 USB driver.
> > For now this handles things identical to the default glue.
> > 
> > Tested-by: Igor Paunovic <royalnet026@xxxxxxxxx> # Orange Pi 5 Plus
> > Signed-off-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>
> > ---
> >  drivers/usb/dwc3/Kconfig         |  11 ++++
> >  drivers/usb/dwc3/Makefile        |   1 +
> >  drivers/usb/dwc3/core.c          |  15 ++++++
> >  drivers/usb/dwc3/dwc3-rockchip.c | 106 +++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 133 insertions(+)
> > 
> 
> <...>
> 
> >  
> > +/*
> > + * List of compatibles, which have "synopsys,dwc3" as a fallback
> > + * compatible, but have a vendor specific glue driver that should
> > + * be used instead of this one.
> > + */
> 
> Shouldn't these be guarded by CONFIG_USB_DWC3_ROCKCHIP so that
> we can still fall back to the generic DWC3 driver?

I think it's better to not probe at all. The reason for introducing
the new glue driver is avoiding PHY startup problems. They already
happen every now and then (so the bug is not introduced by my USBDP
patches, but rather its an existing race condition). Fixing the hangs
also requires driving the reset notifier from the USBDP PHY, which is
not part of this series. But I doubt we get a lot from making
patch series interdependencies even more complicated by temporarily
continueing to allow using the generic DWC3 driver on Rockchip.

Note that the PHY startup hang probability increases a lot by my
USBDP series. So once that landed the generic code is no longer
usable without doing lots of reboots and getting very lucky.

> > +static const char *const dwc3_compatible_blocklist[] = {
> 
> #if IS_ENABLED(CONFIG_USB_DWC3_ROCKCHIP)
> 
> > +	"rockchip,rk3588-dwc3",
> > +	"rockchip,rk3576-dwc3",
> 
> #endif
> 
> > +};
> > +
> 
> 
> 
> >  static int dwc3_probe(struct platform_device *pdev)
> >  {
> >  	struct dwc3_probe_data probe_data = {};
> >  	struct resource *res;
> >  	struct dwc3 *dwc;
> > +	int i;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(dwc3_compatible_blocklist); i++)
> > +		if (device_is_compatible(&pdev->dev, dwc3_compatible_blocklist[i]))
> > +			return -ENODEV;
> >  
> >  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	if (!res) {
> 
> 
> 
> <...>
> 
> 
> 
> > diff --git a/drivers/usb/dwc3/dwc3-rockchip.c b/drivers/usb/dwc3/dwc3-rockchip.c
> > new file mode 100644
> > index 000000000000..62f2a03b08a2
> > --- /dev/null
> > +++ b/drivers/usb/dwc3/dwc3-rockchip.c
> > @@ -0,0 +1,106 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright (c) 2026, Collabora Ltd. */
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include "glue.h"
> > +
> > +struct dwc3_rockchip {
> > +	struct dwc3		dwc;
> > +};
> > +
> > +static int dwc3_rockchip_probe(struct platform_device *pdev)
> > +{
> > +	struct dwc3_probe_data probe_data = {};
> > +	struct resource *res;
> > +	struct dwc3_rockchip *dwc_rk;
> > +
> > +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +	if (!res) {
> > +		dev_err(&pdev->dev, "missing memory resource\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	dwc_rk = devm_kzalloc(&pdev->dev, sizeof(*dwc_rk), GFP_KERNEL);
> > +	if (!dwc_rk)
> > +		return -ENOMEM;
> > +
> > +	dwc_rk->dwc.dev = &pdev->dev;
> > +	dwc_rk->dwc.glue_ops = NULL;
> > +
> > +	probe_data.dwc = &dwc_rk->dwc;
> > +	probe_data.res = res;
> > +	probe_data.properties = DWC3_DEFAULT_PROPERTIES;
> > +
> > +	return dwc3_core_probe(&probe_data);
> > +}
> > +
> > +static void dwc3_rockchip_remove(struct platform_device *pdev)
> > +{
> > +	dwc3_core_remove(platform_get_drvdata(pdev));
> > +}
> > +
> > +static int __maybe_unused dwc3_rockchip_runtime_suspend(struct device *dev)
> 
> Please use the same PM callback pattern as dwc3-generic-plat.c. That
> should avoid the need for __maybe_unused on each callback.

Will do. FWIW this wastes a bunch of memory for !CONFIG_PM by simply
always including the code, even when it's not needed. I suppose
that's fine as disabled CONFIG_PM is uncommon config anyways.

> > +{
> > +	return dwc3_runtime_suspend(dev_get_drvdata(dev));
> > +}
> > +
> > +static int __maybe_unused dwc3_rockchip_runtime_resume(struct device *dev)
> > +{
> > +	return dwc3_runtime_resume(dev_get_drvdata(dev));
> > +}
> > +
> > +static int __maybe_unused dwc3_rockchip_runtime_idle(struct device *dev)
> > +{
> > +	return dwc3_runtime_idle(dev_get_drvdata(dev));
> > +}
> > +
> > +static int __maybe_unused dwc3_rockchip_suspend(struct device *dev)
> > +{
> > +	return dwc3_pm_suspend(dev_get_drvdata(dev));
> > +}
> > +
> > +static int __maybe_unused dwc3_rockchip_resume(struct device *dev)
> > +{
> > +	return dwc3_pm_resume(dev_get_drvdata(dev));
> > +}
> > +
> > +static void __maybe_unused dwc3_rockchip_complete(struct device *dev)
> > +{
> > +	dwc3_pm_complete(dev_get_drvdata(dev));
> > +}
> > +
> > +static int __maybe_unused dwc3_rockchip_prepare(struct device *dev)
> > +{
> > +	return dwc3_pm_prepare(dev_get_drvdata(dev));
> > +}
> > +
> > +static const struct dev_pm_ops dwc3_rockchip_dev_pm_ops = {
> > +	SET_SYSTEM_SLEEP_PM_OPS(dwc3_rockchip_suspend, dwc3_rockchip_resume)
> > +	SET_RUNTIME_PM_OPS(dwc3_rockchip_runtime_suspend, dwc3_rockchip_runtime_resume,
> > +			   dwc3_rockchip_runtime_idle)
> > +	.complete = pm_sleep_ptr(dwc3_rockchip_complete),
> > +	.prepare = pm_sleep_ptr(dwc3_rockchip_prepare),
> > +};
> > +
> > +static const struct of_device_id dwc3_rockchip_of_match[] = {
> > +	{ .compatible = "rockchip,rk3588-dwc3" },
> > +	{ .compatible = "rockchip,rk3576-dwc3" },
> > +	{ }
> > +};
> > +MODULE_DEVICE_TABLE(of, dwc3_rockchip_of_match);
> > +
> > +static struct platform_driver dwc3_rockchip_driver = {
> > +	.probe		= dwc3_rockchip_probe,
> > +	.remove		= dwc3_rockchip_remove,
> > +	.driver		= {
> > +		.name	= "dwc3-rockchip",
> > +		.pm	= pm_ptr(&dwc3_rockchip_dev_pm_ops),
> > +		.of_match_table	= dwc3_rockchip_of_match,
> > +	},
> > +};
> > +
> > +module_platform_driver(dwc3_rockchip_driver);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_DESCRIPTION("DesignWare DWC3 Rockchip Glue Driver");
> > 
> 
> 
> Thanks,
> Thinh

Attachment: signature.asc
Description: PGP signature