[PATCH v2 0/5] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

Manivannan Sadhasivam posted 5 patches 1 month, 3 weeks ago
There is a newer version of this series
drivers/pci/controller/dwc/pcie-qcom.c   | 124 +++++++++++++---
drivers/pci/of.c                         |   1 +
drivers/pci/probe.c                      |  59 --------
drivers/pci/pwrctrl/core.c               | 248 +++++++++++++++++++++++++++++--
drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c |  30 +++-
drivers/pci/pwrctrl/slot.c               |  46 ++++--
drivers/pci/remove.c                     |  20 ---
include/linux/pci-pwrctrl.h              |  16 +-
8 files changed, 408 insertions(+), 136 deletions(-)
[PATCH v2 0/5] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers
Posted by Manivannan Sadhasivam 1 month, 3 weeks ago
Hi,

This series provides a major rework for the PCI power control (pwrctrl)
framework to enable the pwrctrl devices to be controlled by the PCI controller
drivers.

Problem Statement
=================

Currently, the pwrctrl framework faces two major issues:

1. Missing PERST# integration
2. Inability to properly handle bus extenders such as PCIe switch devices

First issue arises from the disconnect between the PCI controller drivers and
pwrctrl framework. At present, the pwrctrl framework just operates on its own
with the help of the PCI core. The pwrctrl devices are created by the PCI core
during initial bus scan and the pwrctrl drivers once bind, just power on the
PCI devices during their probe(). This design conflicts with the PCI Express
Card Electromechanical Specification requirements for PERST# timing. The reason
is, PERST# signals are mostly handled by the controller drivers and often
deasserted even before the pwrctrl drivers probe. According to the spec, PERST#
should be deasserted only after power and reference clock to the device are
stable, within predefined timing parameters.

The second issue stems from the PCI bus scan completing before pwrctrl drivers
probe. This poses a significant problem for PCI bus extenders like switches
because the PCI core allocates upstream bridge resources during the initial
scan. If the upstream bridge is not hotplug capable, resources are allocated
only for the number of downstream buses detected at scan time, which might be
just one if the switch was not powered and enumerated at that time. Later, when
the pwrctrl driver powers on and enumerates the switch, enumeration fails due to
insufficient upstream bridge resources.

Proposal
========

This series addresses both issues by introducing new individual APIs for pwrctrl
device creation, destruction, power on, and power off operations. Controller
drivers are expected to invoke these APIs during their probe(), remove(),
suspend(), and resume() operations. This integration allows better coordination
between controller drivers and the pwrctrl framework, enabling enhanced features
such as D3Cold support.

The original design aimed to avoid modifying controller drivers for pwrctrl
integration. However, this approach lacked scalability because different
controllers have varying requirements for when devices should be powered on. For
example, controller drivers require devices to be powered on early for
successful PHY initialization.

By using these explicit APIs, controller drivers gain fine grained control over
their associated pwrctrl devices.

This series modified the pcie-qcom driver (only consumer of pwrctrl framework)
to adopt to these APIs and also removed the old pwrctrl code from PCI core. This
could be used as a reference to add pwrctrl support for other controller drivers
also.

For example, to control the 3.3v supply to the PCI slot where the NVMe device is
connected, below modifications are required:

Devicetree
----------

	// In SoC dtsi:

	pci@1bf8000 { // controller node
		...
		pcie1_port0: pcie@0 { // PCI Root Port node
			compatible = "pciclass,0604"; // required for pwrctrl
							 driver bind
			...
		};
	};

	// In board dts:

	&pcie1_port0 {
		reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>; // optional
		vpcie3v3-supply = <&vreg_nvme>; // NVMe power supply
	};

Controller driver
-----------------

	// Select PCI_PWRCTRL_SLOT in controller Kconfig

	probe() {
		...
		// Initialize controller resources
		pci_pwrctrl_create_devices(&pdev->dev);
		pci_pwrctrl_power_on_devices(&pdev->dev);
		// Deassert PERST# (optional)
		...
		pci_host_probe(); // Allocate host bridge and start bus scan
	}

	suspend {
		// PME_Turn_Off broadcast
		// Assert PERST# (optional)
		pci_pwrctrl_power_off_devices(&pdev->dev);
		...
	}

	resume {
		...
		pci_pwrctrl_power_on_devices(&pdev->dev);
		// Deassert PERST# (optional)
	}

I will add a documentation for the pwrctrl framework in the coming days to make
it easier to use.

Testing
=======

This series is tested on the Lenovo Thinkpad T14s laptop based on Qcom X1E
chipset and RB3Gen2 development board with TC9563 switch based on Qcom QCS6490
chipset.

**NOTE**: With this series, the controller driver may undergo multiple probe
deferral if the pwrctrl driver was not available during the controller driver
probe. This is pretty much required to avoid the resource allocation issue.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
Changes in v2:
- Exported of_pci_supply_present() API
- Demoted the -EPROBE_DEFER log to dev_dbg()
- Collected tags and rebased on top of v6.19-rc1
- Link to v1: https://lore.kernel.org/r/20251124-pci-pwrctrl-rework-v1-0-78a72627683d@oss.qualcomm.com

---
Krishna Chaitanya Chundru (1):
      PCI/pwrctrl: Add APIs for explicitly creating and destroying pwrctrl devices

Manivannan Sadhasivam (4):
      PCI: qcom: Parse PERST# from all PCIe bridge nodes
      PCI/pwrctrl: Add 'struct pci_pwrctrl::power_{on/off}' callbacks
      PCI/pwrctrl: Add APIs to power on/off the pwrctrl devices
      PCI/pwrctrl: Switch to the new pwrctrl APIs

 drivers/pci/controller/dwc/pcie-qcom.c   | 124 +++++++++++++---
 drivers/pci/of.c                         |   1 +
 drivers/pci/probe.c                      |  59 --------
 drivers/pci/pwrctrl/core.c               | 248 +++++++++++++++++++++++++++++--
 drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c |  30 +++-
 drivers/pci/pwrctrl/slot.c               |  46 ++++--
 drivers/pci/remove.c                     |  20 ---
 include/linux/pci-pwrctrl.h              |  16 +-
 8 files changed, 408 insertions(+), 136 deletions(-)
---
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
change-id: 20251124-pci-pwrctrl-rework-c91a6e16c2f6

Best regards,
-- 
Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
Re: [PATCH v2 0/5] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers
Posted by Sean Anderson 1 month, 2 weeks ago
Hi,

I have a few comments on the overall architecture. I did some work to
add PERST as well (sent as replies to this message).

On 12/16/25 07:51, Manivannan Sadhasivam wrote:
> Hi,
> 
> This series provides a major rework for the PCI power control (pwrctrl)
> framework to enable the pwrctrl devices to be controlled by the PCI controller
> drivers.
> 
> Problem Statement
> =================
> 
> Currently, the pwrctrl framework faces two major issues:
> 
> 1. Missing PERST# integration
> 2. Inability to properly handle bus extenders such as PCIe switch devices
> 
> First issue arises from the disconnect between the PCI controller drivers and
> pwrctrl framework. At present, the pwrctrl framework just operates on its own
> with the help of the PCI core. The pwrctrl devices are created by the PCI core
> during initial bus scan and the pwrctrl drivers once bind, just power on the
> PCI devices during their probe(). This design conflicts with the PCI Express
> Card Electromechanical Specification requirements for PERST# timing. The reason
> is, PERST# signals are mostly handled by the controller drivers and often
> deasserted even before the pwrctrl drivers probe. According to the spec, PERST#
> should be deasserted only after power and reference clock to the device are
> stable, within predefined timing parameters.
> 
> The second issue stems from the PCI bus scan completing before pwrctrl drivers
> probe. This poses a significant problem for PCI bus extenders like switches
> because the PCI core allocates upstream bridge resources during the initial
> scan. If the upstream bridge is not hotplug capable, resources are allocated
> only for the number of downstream buses detected at scan time, which might be
> just one if the switch was not powered and enumerated at that time. Later, when
> the pwrctrl driver powers on and enumerates the switch, enumeration fails due to
> insufficient upstream bridge resources.
>
>
> Proposal
> ========
> 
> This series addresses both issues by introducing new individual APIs for pwrctrl
> device creation, destruction, power on, and power off operations.

I wrote my own PCI power sequencing subsystem last year but didn't get
around to upstreaming it before the current subsystem was merged. This
approach (individual APIs for each power sequence) was how I did it. But
I think the approach to do everything in probe is rather elegant, since
it integrates with the existing device model and we don't need to modify
existing drivers.

To contrast, in U-Boot the second issue doesn't apply because driver
probing happens synchronously and config space accesses are done after
devices get probed. So you have something like

host bridge probe()
pci_scan_child_bus()
   discover root port
   root port probe()
      initialize slot resources (power supplies, clocks, etc.)
   allocate root port BARs
   discover root port children

I guess your approach is the only way to do it in Linux given the
asynchronous nature of driver binding. What is the overhead for hotplug
switches like? Maybe it makes sense to just treat all switches as
hotplug capable when PCI power sequencing is enabled?

> Controller
> drivers are expected to invoke these APIs during their probe(), remove(),
> suspend(), and resume() operations. This integration allows better coordination
> between controller drivers and the pwrctrl framework, enabling enhanced features
> such as D3Cold support.

How does PERST work? The only reference I can find to GPIOs in this
series is in the first patch. Is every driver supposed to add support
for PERST and then call these new functions? Shouldn't this be handled
by the power sequencing driver, especially as there are timing
requirements for the other resources referenced to PERST? IMO if we are
going to touch each driver, it would be much better to consolidate
things by removing the ad-hoc PERST support.

> The original design aimed to avoid modifying controller drivers for pwrctrl
> integration. However, this approach lacked scalability because different
> controllers have varying requirements for when devices should be powered on. For
> example, controller drivers require devices to be powered on early for
> successful PHY initialization.

Is this the case for qcom? The device I tested (nwl) was perfectly
happy to have the PCI device show up some time after the host bridge
got probed.

--Sean

> By using these explicit APIs, controller drivers gain fine grained control over
> their associated pwrctrl devices.
> 
> This series modified the pcie-qcom driver (only consumer of pwrctrl framework)
> to adopt to these APIs and also removed the old pwrctrl code from PCI core. This
> could be used as a reference to add pwrctrl support for other controller drivers
> also.
> 
> For example, to control the 3.3v supply to the PCI slot where the NVMe device is
> connected, below modifications are required:
> 
> Devicetree
> ----------
> 
> 	// In SoC dtsi:
> 
> 	pci@1bf8000 { // controller node
> 		...
> 		pcie1_port0: pcie@0 { // PCI Root Port node
> 			compatible = "pciclass,0604"; // required for pwrctrl
> 							 driver bind
> 			...
> 		};
> 	};
> 
> 	// In board dts:
> 
> 	&pcie1_port0 {
> 		reset-gpios = <&tlmm 152 GPIO_ACTIVE_LOW>; // optional
> 		vpcie3v3-supply = <&vreg_nvme>; // NVMe power supply
> 	};
> 
> Controller driver
> -----------------
> 
> 	// Select PCI_PWRCTRL_SLOT in controller Kconfig
> 
> 	probe() {
> 		...
> 		// Initialize controller resources
> 		pci_pwrctrl_create_devices(&pdev->dev);
> 		pci_pwrctrl_power_on_devices(&pdev->dev);
> 		// Deassert PERST# (optional)
> 		...
> 		pci_host_probe(); // Allocate host bridge and start bus scan
> 	}
> 
> 	suspend {
> 		// PME_Turn_Off broadcast
> 		// Assert PERST# (optional)
> 		pci_pwrctrl_power_off_devices(&pdev->dev);
> 		...
> 	}
> 
> 	resume {
> 		...
> 		pci_pwrctrl_power_on_devices(&pdev->dev);
> 		// Deassert PERST# (optional)
> 	}
> 
> I will add a documentation for the pwrctrl framework in the coming days to make
> it easier to use.
> 
> Testing
> =======
> 
> This series is tested on the Lenovo Thinkpad T14s laptop based on Qcom X1E
> chipset and RB3Gen2 development board with TC9563 switch based on Qcom QCS6490
> chipset.
> 
> **NOTE**: With this series, the controller driver may undergo multiple probe
> deferral if the pwrctrl driver was not available during the controller driver
> probe. This is pretty much required to avoid the resource allocation issue.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> Changes in v2:
> - Exported of_pci_supply_present() API
> - Demoted the -EPROBE_DEFER log to dev_dbg()
> - Collected tags and rebased on top of v6.19-rc1
> - Link to v1: https://lore.kernel.org/r/20251124-pci-pwrctrl-rework-v1-0-78a72627683d@oss.qualcomm.com
> 
> ---
> Krishna Chaitanya Chundru (1):
>       PCI/pwrctrl: Add APIs for explicitly creating and destroying pwrctrl devices
> 
> Manivannan Sadhasivam (4):
>       PCI: qcom: Parse PERST# from all PCIe bridge nodes
>       PCI/pwrctrl: Add 'struct pci_pwrctrl::power_{on/off}' callbacks
>       PCI/pwrctrl: Add APIs to power on/off the pwrctrl devices
>       PCI/pwrctrl: Switch to the new pwrctrl APIs
> 
>  drivers/pci/controller/dwc/pcie-qcom.c   | 124 +++++++++++++---
>  drivers/pci/of.c                         |   1 +
>  drivers/pci/probe.c                      |  59 --------
>  drivers/pci/pwrctrl/core.c               | 248 +++++++++++++++++++++++++++++--
>  drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c |  30 +++-
>  drivers/pci/pwrctrl/slot.c               |  46 ++++--
>  drivers/pci/remove.c                     |  20 ---
>  include/linux/pci-pwrctrl.h              |  16 +-
>  8 files changed, 408 insertions(+), 136 deletions(-)
> ---
> base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
> change-id: 20251124-pci-pwrctrl-rework-c91a6e16c2f6
> 
> Best regards,
Re: [PATCH v2 0/5] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers
Posted by Manivannan Sadhasivam 1 month, 2 weeks ago
On Fri, Dec 19, 2025 at 12:19:36PM -0500, Sean Anderson wrote:
> Hi,
> 
> I have a few comments on the overall architecture. I did some work to
> add PERST as well (sent as replies to this message).
> 
> On 12/16/25 07:51, Manivannan Sadhasivam wrote:
> > Hi,
> > 
> > This series provides a major rework for the PCI power control (pwrctrl)
> > framework to enable the pwrctrl devices to be controlled by the PCI controller
> > drivers.
> > 
> > Problem Statement
> > =================
> > 
> > Currently, the pwrctrl framework faces two major issues:
> > 
> > 1. Missing PERST# integration
> > 2. Inability to properly handle bus extenders such as PCIe switch devices
> > 
> > First issue arises from the disconnect between the PCI controller drivers and
> > pwrctrl framework. At present, the pwrctrl framework just operates on its own
> > with the help of the PCI core. The pwrctrl devices are created by the PCI core
> > during initial bus scan and the pwrctrl drivers once bind, just power on the
> > PCI devices during their probe(). This design conflicts with the PCI Express
> > Card Electromechanical Specification requirements for PERST# timing. The reason
> > is, PERST# signals are mostly handled by the controller drivers and often
> > deasserted even before the pwrctrl drivers probe. According to the spec, PERST#
> > should be deasserted only after power and reference clock to the device are
> > stable, within predefined timing parameters.
> > 
> > The second issue stems from the PCI bus scan completing before pwrctrl drivers
> > probe. This poses a significant problem for PCI bus extenders like switches
> > because the PCI core allocates upstream bridge resources during the initial
> > scan. If the upstream bridge is not hotplug capable, resources are allocated
> > only for the number of downstream buses detected at scan time, which might be
> > just one if the switch was not powered and enumerated at that time. Later, when
> > the pwrctrl driver powers on and enumerates the switch, enumeration fails due to
> > insufficient upstream bridge resources.
> >
> >
> > Proposal
> > ========
> > 
> > This series addresses both issues by introducing new individual APIs for pwrctrl
> > device creation, destruction, power on, and power off operations.
> 
> I wrote my own PCI power sequencing subsystem last year but didn't get
> around to upstreaming it before the current subsystem was merged. This
> approach (individual APIs for each power sequence) was how I did it. But
> I think the approach to do everything in probe is rather elegant, since
> it integrates with the existing device model and we don't need to modify
> existing drivers.
> 
> To contrast, in U-Boot the second issue doesn't apply because driver
> probing happens synchronously and config space accesses are done after
> devices get probed. So you have something like
> 
> host bridge probe()
> pci_scan_child_bus()
>    discover root port
>    root port probe()
>       initialize slot resources (power supplies, clocks, etc.)
>    allocate root port BARs
>    discover root port children
> 
> I guess your approach is the only way to do it in Linux given the
> asynchronous nature of driver binding. What is the overhead for hotplug
> switches like? Maybe it makes sense to just treat all switches as
> hotplug capable when PCI power sequencing is enabled?
> 

Pwrctrl doesn't care if the underlying bridge is hotplug capable or not. It just
handles the power control for the device if it happens to have resource
dependency in DT. For example, if the PCIe switch requires pwrctrl and the
corresponding DT node has the resources described, pwrctrl framework will just
turn ON the switch. Then during PCI bus scan, PCI core will enumerate the switch
and check whether the downstream ports are hotplug capable or not and handles
the bus resource accordingly.

> > Controller
> > drivers are expected to invoke these APIs during their probe(), remove(),
> > suspend(), and resume() operations. This integration allows better coordination
> > between controller drivers and the pwrctrl framework, enabling enhanced features
> > such as D3Cold support.
> 
> How does PERST work? The only reference I can find to GPIOs in this
> series is in the first patch. Is every driver supposed to add support
> for PERST and then call these new functions?

Yes. We can come up with some generic controller specific APIs later to reduce
duplication, especially if GPIO is used for PERST#. But that's currently not in
scope for this series.

> Shouldn't this be handled
> by the power sequencing driver, especially as there are timing
> requirements for the other resources referenced to PERST? IMO if we are
> going to touch each driver, it would be much better to consolidate
> things by removing the ad-hoc PERST support.
> 

It is not that straightforward. Initially, my goal was to abstract pwrctrl away
from controller drivers, but then that didn't scale. Because, each controller
drivers have different requirement, some may use GPIO for PERST# and some use
MMIO. Also, some drivers do more work during the PERST# deassert, like checking
for Link up as in drivers/pci/controller/pci-tegra.c.

For sure, it would be doable to rework those drivers for pwrctrl, but that is
not practically possible and requires platform maintainer support. So to make
the pwrctrl adoption easier, I went with the explicit APIs that the drivers can
call from relevant places.

> > The original design aimed to avoid modifying controller drivers for pwrctrl
> > integration. However, this approach lacked scalability because different
> > controllers have varying requirements for when devices should be powered on. For
> > example, controller drivers require devices to be powered on early for
> > successful PHY initialization.
> 
> Is this the case for qcom? The device I tested (nwl) was perfectly
> happy to have the PCI device show up some time after the host bridge
> got probed.
> 

Not for Qcom, but some platforms do LTSSM during phy_init(), so they will fail
if the device is not powered ON at that time.

The challenge with the pwrctrl framework is that, it has to work across all
platforms and with the existing drivers without major rework. The initial design
worked for Qcom (somewhat), but I couldn't get it to scale across other
platforms.

- Mani

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v2 0/5] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers
Posted by Sean Anderson 1 month ago
On 12/23/25 09:05, Manivannan Sadhasivam wrote:
> On Fri, Dec 19, 2025 at 12:19:36PM -0500, Sean Anderson wrote:
>> Hi,
>> 
>> I have a few comments on the overall architecture. I did some work to
>> add PERST as well (sent as replies to this message).
>> 
>> On 12/16/25 07:51, Manivannan Sadhasivam wrote:
>> > Hi,
>> > 
>> > This series provides a major rework for the PCI power control (pwrctrl)
>> > framework to enable the pwrctrl devices to be controlled by the PCI controller
>> > drivers.
>> > 
>> > Problem Statement
>> > =================
>> > 
>> > Currently, the pwrctrl framework faces two major issues:
>> > 
>> > 1. Missing PERST# integration
>> > 2. Inability to properly handle bus extenders such as PCIe switch devices
>> > 
>> > First issue arises from the disconnect between the PCI controller drivers and
>> > pwrctrl framework. At present, the pwrctrl framework just operates on its own
>> > with the help of the PCI core. The pwrctrl devices are created by the PCI core
>> > during initial bus scan and the pwrctrl drivers once bind, just power on the
>> > PCI devices during their probe(). This design conflicts with the PCI Express
>> > Card Electromechanical Specification requirements for PERST# timing. The reason
>> > is, PERST# signals are mostly handled by the controller drivers and often
>> > deasserted even before the pwrctrl drivers probe. According to the spec, PERST#
>> > should be deasserted only after power and reference clock to the device are
>> > stable, within predefined timing parameters.
>> > 
>> > The second issue stems from the PCI bus scan completing before pwrctrl drivers
>> > probe. This poses a significant problem for PCI bus extenders like switches
>> > because the PCI core allocates upstream bridge resources during the initial
>> > scan. If the upstream bridge is not hotplug capable, resources are allocated
>> > only for the number of downstream buses detected at scan time, which might be
>> > just one if the switch was not powered and enumerated at that time. Later, when
>> > the pwrctrl driver powers on and enumerates the switch, enumeration fails due to
>> > insufficient upstream bridge resources.
>> >
>> >
>> > Proposal
>> > ========
>> > 
>> > This series addresses both issues by introducing new individual APIs for pwrctrl
>> > device creation, destruction, power on, and power off operations.
>> 
>> I wrote my own PCI power sequencing subsystem last year but didn't get
>> around to upstreaming it before the current subsystem was merged. This
>> approach (individual APIs for each power sequence) was how I did it. But
>> I think the approach to do everything in probe is rather elegant, since
>> it integrates with the existing device model and we don't need to modify
>> existing drivers.
>> 
>> To contrast, in U-Boot the second issue doesn't apply because driver
>> probing happens synchronously and config space accesses are done after
>> devices get probed. So you have something like
>> 
>> host bridge probe()
>> pci_scan_child_bus()
>>    discover root port
>>    root port probe()
>>       initialize slot resources (power supplies, clocks, etc.)
>>    allocate root port BARs
>>    discover root port children
>> 
>> I guess your approach is the only way to do it in Linux given the
>> asynchronous nature of driver binding. What is the overhead for hotplug
>> switches like? Maybe it makes sense to just treat all switches as
>> hotplug capable when PCI power sequencing is enabled?
>> 
> 
> Pwrctrl doesn't care if the underlying bridge is hotplug capable or not. It just
> handles the power control for the device if it happens to have resource
> dependency in DT. For example, if the PCIe switch requires pwrctrl and the
> corresponding DT node has the resources described, pwrctrl framework will just
> turn ON the switch. Then during PCI bus scan, PCI core will enumerate the switch
> and check whether the downstream ports are hotplug capable or not and handles
> the bus resource accordingly.
> 

I'm saying that we allocate enough address space for each bridge as if
it was hotplug capable, to ensure we don't run out of space later on.

>> > Controller
>> > drivers are expected to invoke these APIs during their probe(), remove(),
>> > suspend(), and resume() operations. This integration allows better coordination
>> > between controller drivers and the pwrctrl framework, enabling enhanced features
>> > such as D3Cold support.
>> 
>> How does PERST work? The only reference I can find to GPIOs in this
>> series is in the first patch. Is every driver supposed to add support
>> for PERST and then call these new functions?
> 
> Yes. We can come up with some generic controller specific APIs later to reduce
> duplication, especially if GPIO is used for PERST#. But that's currently not in
> scope for this series.
> 
>> Shouldn't this be handled
>> by the power sequencing driver, especially as there are timing
>> requirements for the other resources referenced to PERST? IMO if we are
>> going to touch each driver, it would be much better to consolidate
>> things by removing the ad-hoc PERST support.
>> 
> 
> It is not that straightforward. Initially, my goal was to abstract pwrctrl away
> from controller drivers, but then that didn't scale. Because, each controller
> drivers have different requirement, some may use GPIO for PERST# and some use
> MMIO.

Can't you resolve that by exposing a GPIO controller for the signal?

Of course for compatibility we will want to support old devicetrees, so
maybe we should just add a method on the host bridge to control PERST.
And then pwrctrl could try to use it as a fallback.

> Also, some drivers do more work during the PERST# deassert, like checking
> for Link up as in drivers/pci/controller/pci-tegra.c.

Yeah, but is this actually necessary? We should just be able to toggle
the reset and then try to read a config register after 100 ms. If the
link is up the access will succeed. If it's down at that point then the
device doesn't follow the PCIe spec and we can't really be sure the link
will ever come up. The relevant code even has a comment

 * FIXME: If there are no PCIe cards attached, then calling this function
 * can result in the increase of the bootup time as there are big timeout
 * loops.

Which would be avoided by delaying asynchronously in the pwrseq driver.

> For sure, it would be doable to rework those drivers for pwrctrl, but that is
> not practically possible and requires platform maintainer support. So to make
> the pwrctrl adoption easier, I went with the explicit APIs that the drivers can
> call from relevant places.
> 
>> > The original design aimed to avoid modifying controller drivers for pwrctrl
>> > integration. However, this approach lacked scalability because different
>> > controllers have varying requirements for when devices should be powered on. For
>> > example, controller drivers require devices to be powered on early for
>> > successful PHY initialization.
>> 
>> Is this the case for qcom? The device I tested (nwl) was perfectly
>> happy to have the PCI device show up some time after the host bridge
>> got probed.
>> 
> 
> Not for Qcom, but some platforms do LTSSM during phy_init(), so they will fail
> if the device is not powered ON at that time.

Do you have a specific driver in mind?

> The challenge with the pwrctrl framework is that, it has to work across all
> platforms and with the existing drivers without major rework. The initial design
> worked for Qcom (somewhat), but I couldn't get it to scale across other
> platforms.

--Sean
[PATCH 1/3] PCI/pwrctrl: Bind a pwrctrl device if clocks are present
Posted by Sean Anderson 1 month, 2 weeks ago
Since commit 66db1d3cbdb0 ("PCI/pwrctrl: Add optional slot clock for PCI
slots"), power supplies are not the only resource PCI slots may create.
Also create a pwrctrl device if there are any clocks.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 drivers/pci/of.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 3579265f1198..07546a16ac86 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -847,7 +847,7 @@ void of_pci_make_host_bridge_node(struct pci_host_bridge *bridge)
  * @np: Device tree node
  *
  * Check if the power supply for the PCI device is present in the device tree
- * node or not.
+ * node or not. Clocks may also create a device.
  *
  * Return: true if at least one power supply exists; false otherwise.
  */
@@ -860,6 +860,9 @@ bool of_pci_supply_present(struct device_node *np)
 		return false;
 
 	for_each_property_of_node(np, prop) {
+		if (!strcmp(prop->name, "clocks"))
+			return true;
+
 		supply = strrchr(prop->name, '-');
 		if (supply && !strcmp(supply, "-supply"))
 			return true;
-- 
2.35.1.1320.gc452695387.dirty
Re: [PATCH 1/3] PCI/pwrctrl: Bind a pwrctrl device if clocks are present
Posted by Bartosz Golaszewski 1 month, 1 week ago
On Fri, Dec 19, 2025 at 6:23 PM Sean Anderson <sean.anderson@linux.dev> wrote:
>
> Since commit 66db1d3cbdb0 ("PCI/pwrctrl: Add optional slot clock for PCI
> slots"), power supplies are not the only resource PCI slots may create.
> Also create a pwrctrl device if there are any clocks.
>
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
>

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
[PATCH 2/3] PCI/pwrctrl: Add appropriate delays for slot power/clocks
Posted by Sean Anderson 1 month, 2 weeks ago
Each of the PCIe electromechanical specifications requires a delay
between when power and clocks are stable and when PERST is released.
Delay for the specified time before continuing with initialization. If
there are no power supplies/clock, skip the associated delay as we
assume that they have been initialized by the bootloader (and that
booting up to this point has taken longer than the delay).

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 drivers/pci/pwrctrl/slot.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/pci/pwrctrl/slot.c b/drivers/pci/pwrctrl/slot.c
index 3320494b62d8..1c56fcd49f2b 100644
--- a/drivers/pci/pwrctrl/slot.c
+++ b/drivers/pci/pwrctrl/slot.c
@@ -5,6 +5,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/delay.h>
 #include <linux/device.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
@@ -31,6 +32,7 @@ static int pci_pwrctrl_slot_probe(struct platform_device *pdev)
 {
 	struct pci_pwrctrl_slot_data *slot;
 	struct device *dev = &pdev->dev;
+	unsigned long delay = 0;
 	struct clk *clk;
 	int ret;
 
@@ -64,6 +66,17 @@ static int pci_pwrctrl_slot_probe(struct platform_device *pdev)
 				     "Failed to enable slot clock\n");
 	}
 
+	if (slot->num_supplies)
+		/*
+		 * Delay for T_PVPERL. This could be reduced to 1 ms/50 ms
+		 * (T_PVPGL) for Mini/M.2 slots.
+		 */
+		delay = 100000;
+	else if (clk)
+		/* Delay for T_PERST-CLK (100 us for all slot types) */
+		delay = 100;
+
+	fsleep(delay)
 	pci_pwrctrl_init(&slot->ctx, dev);
 
 	ret = devm_pci_pwrctrl_device_set_ready(dev, &slot->ctx);
-- 
2.35.1.1320.gc452695387.dirty
[PATCH 3/3] PCI/pwrctrl: Support PERST GPIO in slot driver
Posted by Sean Anderson 1 month, 2 weeks ago
On many embedded platforms PERST is controlled by a GPIO. This was
traditionally handled by the host bridge. However, PERST may be released
before slot resources are initialized if there is a pwrctrl device. To
ensure we follow the power sequence, add support for controlling PERST
to the slot driver.

The host bridge could have already grabbed the GPIO. If this happens the
power sequence might be violated but there's really nothing we can do so
we just ignore the GPIO.

PERST must be asserted for at least T_PERST, such as when
entering/exiting S3/S4. As an optimization, we skip this delay when
PERST was already asserted, which may be the case when booting (such as
if the system has a pull-down on the line).

If the link is already up (e.g. the bootloader configured the power
supplies, clocks, and PERST) we will reset the link anyway. I don't
really know how to avoid this. I think we're OK because the root port
will be probed before we probe the endpoint so we shouldn't reset the
link while we're reading the configuration space.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 drivers/pci/pwrctrl/slot.c | 52 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pwrctrl/slot.c b/drivers/pci/pwrctrl/slot.c
index 1c56fcd49f2b..59dc92c4bc04 100644
--- a/drivers/pci/pwrctrl/slot.c
+++ b/drivers/pci/pwrctrl/slot.c
@@ -7,6 +7,7 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/gpio/consumer.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/pci-pwrctrl.h>
@@ -18,6 +19,7 @@ struct pci_pwrctrl_slot_data {
 	struct pci_pwrctrl ctx;
 	struct regulator_bulk_data *supplies;
 	int num_supplies;
+	struct gpio_desc *perst;
 };
 
 static void devm_pci_pwrctrl_slot_power_off(void *data)
@@ -28,6 +30,13 @@ static void devm_pci_pwrctrl_slot_power_off(void *data)
 	regulator_bulk_free(slot->num_supplies, slot->supplies);
 }
 
+static void devm_pci_pwrctrl_slot_assert_perst(void *data)
+{
+	struct pci_pwrctrl_slot_data *slot = data;
+
+	gpiod_set_value_cansleep(slot->perst, 1);
+}
+
 static int pci_pwrctrl_slot_probe(struct platform_device *pdev)
 {
 	struct pci_pwrctrl_slot_data *slot;
@@ -66,6 +75,14 @@ static int pci_pwrctrl_slot_probe(struct platform_device *pdev)
 				     "Failed to enable slot clock\n");
 	}
 
+	slot->perst = devm_gpiod_get_optional(dev, "reset", GPIOD_ASIS);
+	if (IS_ERR(slot->perst)) {
+		/* The PCIe host bridge may have already grabbed the reset */
+		if (PTR_ERR(slot->perst) != -EBUSY)
+			return dev_err_probe(dev, ret, "failed to get PERST\n");
+		slot->perst = NULL;
+	}
+
 	if (slot->num_supplies)
 		/*
 		 * Delay for T_PVPERL. This could be reduced to 1 ms/50 ms
@@ -76,7 +93,40 @@ static int pci_pwrctrl_slot_probe(struct platform_device *pdev)
 		/* Delay for T_PERST-CLK (100 us for all slot types) */
 		delay = 100;
 
-	fsleep(delay)
+	if (slot->perst) {
+		/*
+		 * If PERST is inactive, the following call to
+		 * gpiod_direction_output will be the first time we assert it
+		 * and we will need to delay for T_PERST.
+		 */
+		if (gpiod_get_value_cansleep(slot->perst) != 1)
+			delay = 100000;
+
+		ret = gpiod_direction_output(slot->perst, 1);
+		if (ret) {
+			dev_err(dev, "failed to assert PERST\n");
+			return ret;
+		}
+	}
+
+	fsleep(delay);
+	if (slot->perst) {
+		gpiod_set_value(slot->perst, 0);
+		ret = devm_add_action_or_reset(dev,
+					       devm_pci_pwrctrl_slot_assert_perst,
+					       slot);
+		if (ret)
+			return ret;
+
+		/*
+		 * PCIe section 6.6.1:
+		 * > ... software must wait a minimum of 100 ms before sending a
+		 * > Configuration Request to the device immediately below that
+		 * > Port.
+		 */
+		msleep(100);
+	}
+
 	pci_pwrctrl_init(&slot->ctx, dev);
 
 	ret = devm_pci_pwrctrl_device_set_ready(dev, &slot->ctx);
-- 
2.35.1.1320.gc452695387.dirty
Re: (subset) [PATCH v2 0/5] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers
Posted by Manivannan Sadhasivam 1 month, 3 weeks ago
On Tue, 16 Dec 2025 18:21:42 +0530, Manivannan Sadhasivam wrote:
> This series provides a major rework for the PCI power control (pwrctrl)
> framework to enable the pwrctrl devices to be controlled by the PCI controller
> drivers.
> 
> Problem Statement
> =================
> 
> [...]

Applied, thanks!

[1/5] PCI: qcom: Parse PERST# from all PCIe bridge nodes
      commit: 36777244652a7fe773bd7a1fe46bd3803712d3be

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>