Hi Vinod,

Thank you for your reply.

quoted

Introduce support for Aspeed PCIe PHY controller available in
AST2600/2700.

What is with the uppercase "PHY" in patch title instead of lowercase 'phy' as is
the convention

Got it. I will change the title to lowercase 'phy' in next version.

quoted

+++ b/drivers/phy/aspeed/Kconfig

@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0-only # # Phy drivers for Aspeed
+platforms # config PHY_ASPEED_PCIE
+	tristate "ASPEED PCIe PHY driver"
+	select GENERIC_PHY
+	depends on ARCH_ASPEED
+	default y

NO! why should this driver be default!

Agreed - this should not be default.
I'll remove `default y` in the next revision.

quoted

+	help
+	  This option enables support for the ASPEED PCIe PHY driver.
+	  The driver provides the necessary interface to control and
+	  configure the PCIe PHY hardware found on ASPEED SoCs.
+	  It is required for proper operation of PCIe devices on
+	  platforms using ASPEED chips.
\ No newline at end of file

??

I'll add the missing newline at end of file in the next revision.

quoted

diff --git a/drivers/phy/aspeed/Makefile b/drivers/phy/aspeed/Makefile
new file mode 100644 index 000000000000..7203152f44bf
--- /dev/null
+++ b/drivers/phy/aspeed/Makefile

@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_PHY_ASPEED_PCIE)		+= phy-aspeed-pcie.o

\ No newline at end of file

Are we expecting more drivers for aspeed, if not move it to drivers/phy/ once
we have couple of them we can add a directory

Yes, we are expecting more PHY drivers for ASPEED.

This is the first ASPEED PHY driver, and additional ASPEED-specific
PHY drivers will be submitted later, so having a dedicated
drivers/phy/aspeed/ directory is intentional.

quoted

diff --git a/drivers/phy/aspeed/phy-aspeed-pcie.c

b/drivers/phy/aspeed/phy-aspeed-pcie.c
new file mode 100644
index 000000000000..3de43a86ac17

--- /dev/null
+++ b/drivers/phy/aspeed/phy-aspeed-pcie.c

@@ -0,0 +1,209 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2025 Aspeed Technology Inc.
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>

why do you need this

quoted

+#include <linux/phy/pcie.h>
+#include <linux/phy/phy.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h> #include <linux/reset.h>
+#include <linux/slab.h>

Do you need all headers here?

Thanks for pointing this out.

I'll confirm which headers are actually required and clean up
the unused includes in the next revision.

Thanks,
Jacky