Message-ID: <CAGXv+5HRaYVMjwiqeVVm2-abnyga_-hzWM_tXw9hrqo74x47hA@mail.gmail.com>
Date: Wed, 26 Nov 2025 15:22:16 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>
Cc: Manivannan Sadhasivam <mani@...nel.org>, 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>, 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>
Subject: Re: [PATCH 0/5] PCI/pwrctrl: Major rework to integrate pwrctrl
 devices with controller drivers

On Tue, Nov 25, 2025 at 3:13 PM Manivannan Sadhasivam
<manivannan.sadhasivam@....qualcomm.com> 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. 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@...8000 { // 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@....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

I see this is still handled in the driver. Reading the cover letter,
it seems the plan is to move this into pwrctrl, perhaps as a separate
function?

>       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

Tested-by: Chen-Yu Tsai <wenst@...omium.org>

on an MT8188-based Chromebook which has on-board PCIe-based WiFi.
The pcie-mediatek-gen3.c driver was reworked to integrate the new API.


>  drivers/pci/controller/dwc/pcie-qcom.c   | 124 +++++++++++++---
>  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 +-
>  7 files changed, 407 insertions(+), 136 deletions(-)
> ---
> base-commit: 3a8660878839faadb4f1a6dd72c3179c1df56787
> change-id: 20251124-pci-pwrctrl-rework-c91a6e16c2f6
>
> Best regards,
> --
> Manivannan Sadhasivam <manivannan.sadhasivam@....qualcomm.com>
>

Powered by blists - more mailing lists