linux-kernel - [PATCH v7 4/7] PHY: aspeed: Add ASPEED PCIe PHY 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 for Android: free password hash cracker in your pocket

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

Message-ID:
 <SEYPR06MB51340F64E40B19975A2FD9D79DB2A@SEYPR06MB5134.apcprd06.prod.outlook.com>
Date: Wed, 24 Dec 2025 05:32:00 +0000
From: Jacky Chou <jacky_chou@...eedtech.com>
To: Vinod Koul <vkoul@...nel.org>
CC: Kishon Vijay Abraham I <kishon@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
	Joel Stanley <joel@....id.au>, Andrew Jeffery <andrew@...econstruct.com.au>,
	Bjorn Helgaas <bhelgaas@...gle.com>, Lorenzo Pieralisi
	<lpieralisi@...nel.org>, Krzysztof Wilczyński
	<kwilczynski@...nel.org>, Manivannan Sadhasivam <mani@...nel.org>, Linus
 Walleij <linus.walleij@...aro.org>, Philipp Zabel <p.zabel@...gutronix.de>,
	Neil Armstrong <neil.armstrong@...aro.org>, "linux-aspeed@...ts.ozlabs.org"
	<linux-aspeed@...ts.ozlabs.org>, "linux-pci@...r.kernel.org"
	<linux-pci@...r.kernel.org>, "linux-phy@...ts.infradead.org"
	<linux-phy@...ts.infradead.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, Andrew Jeffery <andrew@...id.au>,
	"openbmc@...ts.ozlabs.org" <openbmc@...ts.ozlabs.org>,
	"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>
Subject: [PATCH v7 4/7] PHY: aspeed: Add ASPEED PCIe PHY driver

Hi Vinod,

Thank you for your reply.

> > 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.

> > +++ 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.

> > +	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.

> > 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.

> > 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
> 
> > +#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