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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d500b879-a4fe-4f29-a9d0-c29826d21e31@whut.edu.cn>
Date: Thu, 10 Apr 2025 09:34:38 +0800
From: Ze Huang <huangze@...t.edu.cn>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham I
 <kishon@...nel.org>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Yixun Lan <dlan@...too.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Philipp Zabel <p.zabel@...gutronix.de>,
 Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
 <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 Alexandre Ghiti <alex@...ti.fr>,
 "linux-phy@...ts.infradead.org" <linux-phy@...ts.infradead.org>,
 "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
 "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
 "spacemit@...ts.linux.dev" <spacemit@...ts.linux.dev>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
Subject: Re: [PATCH 6/7] usb: dwc3: add spacemit dwc3 glue layer driver

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@...t.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@...cemit.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