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

Ze Huang
2025-04-10T01:44:38Z

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@whut.edu.cn>
>> ---
>> 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@spacemit.com>");
> 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