Re: [PATCH v3 2/3] phy: aspeed: Add AST2700 USB3.2 PHY driver

[Manivannan Sadhasivam]

On Wed, Jul 01, 2026 at 02:58:18PM +0800, Ryan Chen wrote:
> Add AST2700 USB3.2 PHY driver support.
> 
> Signed-off-by: Ryan Chen <ryan_chen@xxxxxxxxxxxxxx>

Just couple of nitpicks below. Once they are addressed:

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>

> ---
>  drivers/phy/Kconfig                  |   1 +
>  drivers/phy/Makefile                 |   1 +
>  drivers/phy/aspeed/Kconfig           |  15 +++
>  drivers/phy/aspeed/Makefile          |   2 +
>  drivers/phy/aspeed/phy-aspeed-usb3.c | 236 +++++++++++++++++++++++++++++++++++
>  5 files changed, 255 insertions(+)
> 
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 19f3b7d12b7d..85fa381978f8 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -171,6 +171,7 @@ config PHY_XGENE
>  source "drivers/phy/allwinner/Kconfig"
>  source "drivers/phy/amlogic/Kconfig"
>  source "drivers/phy/apple/Kconfig"
> +source "drivers/phy/aspeed/Kconfig"
>  source "drivers/phy/axiado/Kconfig"
>  source "drivers/phy/broadcom/Kconfig"
>  source "drivers/phy/cadence/Kconfig"
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index d7aa516bcc49..c6dd02003bbe 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -23,6 +23,7 @@ obj-$(CONFIG_PHY_XGENE)			+= phy-xgene.o
>  obj-$(CONFIG_GENERIC_PHY)		+= allwinner/	\
>  					   amlogic/	\
>  					   apple/	\
> +					   aspeed/	\
>  					   axiado/	\
>  					   broadcom/	\
>  					   cadence/	\
> diff --git a/drivers/phy/aspeed/Kconfig b/drivers/phy/aspeed/Kconfig
> new file mode 100644
> index 000000000000..7b5f48db2be8
> --- /dev/null
> +++ b/drivers/phy/aspeed/Kconfig
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +#
> +# PHY drivers for ASPEED
> +#
> +
> +config PHY_ASPEED_USB3
> +	tristate "ASPEED USB3 PHY driver"
> +	select GENERIC_PHY
> +	depends on (ARCH_ASPEED || COMPILE_TEST)
> +	help
> +	  Enable this to support the USB 3.2 PHY on the Aspeed AST2700 SoC.
> +	  It supports SuperSpeedPlus Gen2x1 (10 Gbps), SuperSpeed (5 Gbps),
> +	  High Speed (480 Mbps), Full Speed (12 Mbps) and Low Speed
> +	  (1.5 Mbps), and is paired with the DWC3 USB controller.
> diff --git a/drivers/phy/aspeed/Makefile b/drivers/phy/aspeed/Makefile
> new file mode 100644
> index 000000000000..d96d9d73a009
> --- /dev/null
> +++ b/drivers/phy/aspeed/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_PHY_ASPEED_USB3)		+= phy-aspeed-usb3.o
> diff --git a/drivers/phy/aspeed/phy-aspeed-usb3.c b/drivers/phy/aspeed/phy-aspeed-usb3.c
> new file mode 100644
> index 000000000000..eff148faa14c
> --- /dev/null
> +++ b/drivers/phy/aspeed/phy-aspeed-usb3.c
> @@ -0,0 +1,236 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2026 Aspeed Technology Inc.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/phy/phy.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +
> +#define PHY3S00		0x00
> +#define PHY3S00_INIT_DONE		BIT(15)
> +#define PHY3S00_SRAM_BYPASS		BIT(7)
> +#define PHY3S00_SRAM_EXT_LOAD	BIT(6)

If you want to mix register offsets and bitfield definitions, then you need to
use indent to differentiate:

	#define PHY3S00			0x00
	#define  PHY3S00_INIT_DONE	BIT(15)
	#define  PHY3S00_SRAM_BYPASS	BIT(7)
	...
	#define PHY3S04			0x04
	...

> +#define PHY3S04		0x04
> +#define PHY3C00		0x08
> +#define PHY3C04		0x0C

Use lowercaps for all hex values

- Mani

-- 
மணிவண்ணன் சதாசிவம்