Re: [PATCH 6/7] usb: dwc3: add spacemit dwc3 glue layer driver

[Ze Huang]

On 4/10/25 6:34 AM, Thinh Nguyen wrote:
> On Mon, Apr 07, 2025, Ze Huang wrote:
> > Add SpacemiT glue logic to support dwc3 HC on K1 SoC. The driver manages
> > clock, reset and interrupt resource.
> >
> > Signed-off-by: Ze Huang <huangze@xxxxxxxxxxx>
> > ---
> >   drivers/usb/dwc3/Kconfig         |   7 +++
> >   drivers/usb/dwc3/Makefile        |   1 +
> >   drivers/usb/dwc3/dwc3-spacemit.c | 127 +++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 135 insertions(+)
> >
> > diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
> > index 310d182e10b50b253d7e5a51674806e6ec442a2a..3c30680fa4f83565fc03c6800e867c6ced0fe101 100644
> > --- a/drivers/usb/dwc3/Kconfig
> > +++ b/drivers/usb/dwc3/Kconfig
> > @@ -189,4 +189,11 @@ config USB_DWC3_RTK
> >   	  or dual-role mode.
> >   	  Say 'Y' or 'M' if you have such device.
> >   
> > +config USB_DWC3_SPACEMIT
> > +	tristate "Spacemit Platforms"
> Does this depend on other configs like OF and COMMON_CLK?

Yes, depends on them, will fix Kconfig entries in next version

> > +	default USB_DWC3
> > +	help
> > +	  Support SPACEMIT platforms with DesignWare Core USB3 IP.
> > +	  Say 'Y' or 'M' here if you have one such device
> > +
> >   endif
> > diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
> > index 124eda2522d9c1f4caab222ec9770d0deaf655fc..61a87765c0c591e0a53c33b5a6544db056166f96 100644
> > --- a/drivers/usb/dwc3/Makefile
> > +++ b/drivers/usb/dwc3/Makefile
> > @@ -56,3 +56,4 @@ obj-$(CONFIG_USB_DWC3_IMX8MP)		+= dwc3-imx8mp.o
> >   obj-$(CONFIG_USB_DWC3_XILINX)		+= dwc3-xilinx.o
> >   obj-$(CONFIG_USB_DWC3_OCTEON)		+= dwc3-octeon.o
> >   obj-$(CONFIG_USB_DWC3_RTK)		+= dwc3-rtk.o
> > +obj-$(CONFIG_USB_DWC3_SPACEMIT)		+= dwc3-spacemit.o
...
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int dwc3_spacemit_suspend(struct device *dev)
> > +{
> > +	struct dwc3_spacemit *spacemit = dev_get_drvdata(dev);
> > +
> > +	clk_disable_unprepare(spacemit->clk);
> > +
> > +	return 0;
> > +}
> > +
> > +static int dwc3_spacemit_resume(struct device *dev)
> > +{
> > +	struct dwc3_spacemit *spacemit = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	ret = clk_prepare_enable(spacemit->clk);
> > +
> > +	return ret;
> > +}
> > +
> > +static const struct dev_pm_ops dwc3_spacemit_dev_pm_ops = {
> > +	SET_SYSTEM_SLEEP_PM_OPS(dwc3_spacemit_suspend, dwc3_spacemit_resume)
> > +};
> > +#endif /* CONFIG_PM_SLEEP */
> > +
> Use DEFINE_SIMPLE_DEV_PM_OPS to remove the CONFIG_PM_SLEEP guards.

thanks

> > +static struct platform_driver dwc3_spacemit_driver = {
> > +	.probe		= dwc3_spacemit_probe,
> > +	.remove		= dwc3_spacemit_remove,
> > +	.driver		= {
> > +		.name	= "spacemit-dwc3",
> > +		.of_match_table = spacemit_dwc3_match,
> > +#ifdef CONFIG_PM_SLEEP
> > +		.pm	= &dwc3_spacemit_dev_pm_ops,
> > +#endif /* CONFIG_PM_SLEEP */
> > +	},
> > +};
> > +module_platform_driver(dwc3_spacemit_driver);
> > +
> > +MODULE_AUTHOR("Wilson <long.wan@xxxxxxxxxxxx>");
> The author is different than the commiter? Also, is there a last name?

You're right, I missed Signed-off-by tag for Wilson in commit message

> > +MODULE_LICENSE("GPL");
> > +MODULE_DESCRIPTION("DesignWare USB3 Spacemit Glue Layer");
> >
> > --
> > 2.49.0
> The logic in this glue driver looks quite simple. Can this platform work
> as dwc3-of-simple?

Yes, indeed simple.
As Krzysztof mentioned, creating glue nodes is not ideal for DWC USB. I
would drop the glue driver and use dwc3/core.c as fallback, similar to
what Rockchip did[1].

[1] 
https://www.kernel.org/doc/Documentation/devicetree/bindings/usb/rockchip%2Cdwc3.yaml

> Thanks,
> Thinh