Message-ID: <n2vboqjh45bwhs3czpey3alxwi7msohir7m3lk45mecouddwew@byi2emazszqq>
Date: Tue, 23 Dec 2025 19:35:32 +0530
From: Manivannan Sadhasivam <mani@...nel.org>
To: Sean Anderson <sean.anderson@...ux.dev>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>, 
	Lorenzo Pieralisi <lpieralisi@...nel.org>, Krzysztof Wilczyński <kwilczynski@...nel.org>, 
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>, 
	Bartosz Golaszewski <brgl@...ev.pl>, Bartosz Golaszewski <brgl@...nel.org>, linux-pci@...r.kernel.org, 
	linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org, Chen-Yu Tsai <wens@...nel.org>, 
	Brian Norris <briannorris@...omium.org>, Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>, 
	Niklas Cassel <cassel@...nel.org>, Alex Elder <elder@...cstar.com>, 
	Chen-Yu Tsai <wenst@...omium.org>, Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: Re: [PATCH v2 0/5] PCI/pwrctrl: Major rework to integrate pwrctrl
 devices with controller drivers

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

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

Powered by blists - more mailing lists