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

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

[Manivannan Sadhasivam via B4 Relay]

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. I
plan to replace probe deferral with blocking wait in the coming days.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
Changes in v4:
- Used platform_device_put()
- Changed the return value of power_off() callback to 'int'
- Splitted patch 6 into two and reworded the commit message
- Collected tags
- Link to v3: https://lore.kernel.org/r/20251229-pci-pwrctrl-rework-v3-0-c7d5918cd0db@oss.qualcomm.com

Changes in v3:
- Integrated TC9563 change
- Reworked the power_on API to properly power off the devices in error path
- Fixed the error path in pcie-qcom.c to not destroy pwrctrl devices during
  probe deferral
- Rebased on top of pci/controller/dwc-qcom branch and dropped the PERST# patch
- Added a patch for TC9563 to fix the refcount dropping for i2c adapter
- Added patches to drop the assert_perst callback and rename the PERST# helpers in
  pcie-qcom.c
- Link to v2: https://lore.kernel.org/r/20251216-pci-pwrctrl-rework-v2-0-745a563b9be6@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 (7):
      PCI/pwrctrl: tc9563: Use put_device() instead of i2c_put_adapter()
      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
      PCI: qcom: Drop the assert_perst() callbacks
      PCI: Drop the assert_perst() callback
      PCI: qcom: Rename PERST# assert/deassert helpers for uniformity

 drivers/pci/bus.c                                 |  19 --
 drivers/pci/controller/dwc/pcie-designware-host.c |   9 -
 drivers/pci/controller/dwc/pcie-designware.h      |   9 -
 drivers/pci/controller/dwc/pcie-qcom.c            |  54 +++--
 drivers/pci/of.c                                  |   1 +
 drivers/pci/probe.c                               |  59 -----
 drivers/pci/pwrctrl/core.c                        | 260 ++++++++++++++++++++--
 drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c          |  30 ++-
 drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c          |  48 ++--
 drivers/pci/pwrctrl/slot.c                        |  48 ++--
 drivers/pci/remove.c                              |  20 --
 include/linux/pci-pwrctrl.h                       |  16 +-
 include/linux/pci.h                               |   1 -
 13 files changed, 367 insertions(+), 207 deletions(-)
---
base-commit: 3e7f562e20ee87a25e104ef4fce557d39d62fa85
change-id: 20251124-pci-pwrctrl-rework-c91a6e16c2f6
prerequisite-message-id: 20251126081718.8239-1-mani@kernel.org
prerequisite-patch-id: db9ff6c713e2303c397e645935280fd0d277793a
prerequisite-patch-id: b5351b0a41f618435f973ea2c3275e51d46f01c5

Best regards,
-- 
Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

[PATCH v4 1/8] PCI/pwrctrl: tc9563: Use put_device() instead of i2c_put_adapter()

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

The API comment for of_find_i2c_adapter_by_node() recommends using
put_device() to drop the reference count of I2C adapter instead of using
i2c_put_adapter(). So replace i2c_put_adapter() with put_device().

Fixes: 4c9c7be47310 ("PCI: pwrctrl: Add power control driver for TC9563")
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
index ec423432ac65..0a63add84d09 100644
--- a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
+++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
@@ -533,7 +533,7 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
 	ctx->client = i2c_new_dummy_device(ctx->adapter, addr);
 	if (IS_ERR(ctx->client)) {
 		dev_err(dev, "Failed to create I2C client\n");
-		i2c_put_adapter(ctx->adapter);
+		put_device(&ctx->adapter->dev);
 		return PTR_ERR(ctx->client);
 	}
 
@@ -613,7 +613,7 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
 	tc9563_pwrctrl_power_off(ctx);
 remove_i2c:
 	i2c_unregister_device(ctx->client);
-	i2c_put_adapter(ctx->adapter);
+	put_device(&ctx->adapter->dev);
 	return ret;
 }
 
@@ -623,7 +623,7 @@ static void tc9563_pwrctrl_remove(struct platform_device *pdev)
 
 	tc9563_pwrctrl_power_off(ctx);
 	i2c_unregister_device(ctx->client);
-	i2c_put_adapter(ctx->adapter);
+	put_device(&ctx->adapter->dev);
 }
 
 static const struct of_device_id tc9563_pwrctrl_of_match[] = {

-- 
2.48.1

[PATCH v4 2/8] PCI/pwrctrl: Add 'struct pci_pwrctrl::power_{on/off}' callbacks

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

To allow the pwrctrl core to control the power on/off sequences of the
pwrctrl drivers, add the 'struct pci_pwrctrl::power_{on/off}' callbacks and
populate them in the respective pwrctrl drivers.

The pwrctrl drivers still power on the resources on their own now. So there
is no functional change.

Co-developed-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c | 27 ++++++++++++++---
 drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c | 22 ++++++++++----
 drivers/pci/pwrctrl/slot.c               | 50 +++++++++++++++++++++++---------
 include/linux/pci-pwrctrl.h              |  4 +++
 4 files changed, 79 insertions(+), 24 deletions(-)

diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
index 4e664e7b8dd2..7e7093cbff3c 100644
--- a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
+++ b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
@@ -52,11 +52,27 @@ static const struct pci_pwrctrl_pwrseq_pdata pci_pwrctrl_pwrseq_qcom_wcn_pdata =
 	.validate_device = pci_pwrctrl_pwrseq_qcm_wcn_validate_device,
 };
 
+static int pci_pwrctrl_pwrseq_power_on(struct pci_pwrctrl *ctx)
+{
+	struct pci_pwrctrl_pwrseq_data *data = container_of(ctx, struct pci_pwrctrl_pwrseq_data,
+							    ctx);
+
+	return pwrseq_power_on(data->pwrseq);
+}
+
+static int pci_pwrctrl_pwrseq_power_off(struct pci_pwrctrl *ctx)
+{
+	struct pci_pwrctrl_pwrseq_data *data = container_of(ctx, struct pci_pwrctrl_pwrseq_data,
+							    ctx);
+
+	return pwrseq_power_off(data->pwrseq);
+}
+
 static void devm_pci_pwrctrl_pwrseq_power_off(void *data)
 {
-	struct pwrseq_desc *pwrseq = data;
+	struct pci_pwrctrl_pwrseq_data *pwrseq_data = data;
 
-	pwrseq_power_off(pwrseq);
+	pci_pwrctrl_pwrseq_power_off(&pwrseq_data->ctx);
 }
 
 static int pci_pwrctrl_pwrseq_probe(struct platform_device *pdev)
@@ -85,16 +101,19 @@ static int pci_pwrctrl_pwrseq_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(data->pwrseq),
 				     "Failed to get the power sequencer\n");
 
-	ret = pwrseq_power_on(data->pwrseq);
+	ret = pci_pwrctrl_pwrseq_power_on(&data->ctx);
 	if (ret)
 		return dev_err_probe(dev, ret,
 				     "Failed to power-on the device\n");
 
 	ret = devm_add_action_or_reset(dev, devm_pci_pwrctrl_pwrseq_power_off,
-				       data->pwrseq);
+				       data);
 	if (ret)
 		return ret;
 
+	data->ctx.power_on = pci_pwrctrl_pwrseq_power_on;
+	data->ctx.power_off = pci_pwrctrl_pwrseq_power_off;
+
 	pci_pwrctrl_init(&data->ctx, dev);
 
 	ret = devm_pci_pwrctrl_device_set_ready(dev, &data->ctx);
diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
index 0a63add84d09..616d3caedb34 100644
--- a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
+++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
@@ -434,15 +434,22 @@ static int tc9563_pwrctrl_parse_device_dt(struct tc9563_pwrctrl_ctx *ctx, struct
 	return 0;
 }
 
-static void tc9563_pwrctrl_power_off(struct tc9563_pwrctrl_ctx *ctx)
+static int tc9563_pwrctrl_power_off(struct pci_pwrctrl *pwrctrl)
 {
+	struct tc9563_pwrctrl_ctx *ctx = container_of(pwrctrl,
+					struct tc9563_pwrctrl_ctx, pwrctrl);
+
 	gpiod_set_value(ctx->reset_gpio, 1);
 
 	regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies);
+
+	return 0;
 }
 
-static int tc9563_pwrctrl_bring_up(struct tc9563_pwrctrl_ctx *ctx)
+static int tc9563_pwrctrl_power_on(struct pci_pwrctrl *pwrctrl)
 {
+	struct tc9563_pwrctrl_ctx *ctx = container_of(pwrctrl,
+					struct tc9563_pwrctrl_ctx, pwrctrl);
 	struct tc9563_pwrctrl_cfg *cfg;
 	int ret, i;
 
@@ -502,7 +509,7 @@ static int tc9563_pwrctrl_bring_up(struct tc9563_pwrctrl_ctx *ctx)
 		return 0;
 
 power_off:
-	tc9563_pwrctrl_power_off(ctx);
+	tc9563_pwrctrl_power_off(&ctx->pwrctrl);
 	return ret;
 }
 
@@ -591,7 +598,7 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
 			goto remove_i2c;
 	}
 
-	ret = tc9563_pwrctrl_bring_up(ctx);
+	ret = tc9563_pwrctrl_power_on(&ctx->pwrctrl);
 	if (ret)
 		goto remove_i2c;
 
@@ -601,6 +608,9 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
 			goto power_off;
 	}
 
+	ctx->pwrctrl.power_on = tc9563_pwrctrl_power_on;
+	ctx->pwrctrl.power_off = tc9563_pwrctrl_power_off;
+
 	ret = devm_pci_pwrctrl_device_set_ready(dev, &ctx->pwrctrl);
 	if (ret)
 		goto power_off;
@@ -610,7 +620,7 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
 	return 0;
 
 power_off:
-	tc9563_pwrctrl_power_off(ctx);
+	tc9563_pwrctrl_power_off(&ctx->pwrctrl);
 remove_i2c:
 	i2c_unregister_device(ctx->client);
 	put_device(&ctx->adapter->dev);
@@ -621,7 +631,7 @@ static void tc9563_pwrctrl_remove(struct platform_device *pdev)
 {
 	struct tc9563_pwrctrl_ctx *ctx = platform_get_drvdata(pdev);
 
-	tc9563_pwrctrl_power_off(ctx);
+	tc9563_pwrctrl_power_off(&ctx->pwrctrl);
 	i2c_unregister_device(ctx->client);
 	put_device(&ctx->adapter->dev);
 }
diff --git a/drivers/pci/pwrctrl/slot.c b/drivers/pci/pwrctrl/slot.c
index 3320494b62d8..9f91f04b4ae0 100644
--- a/drivers/pci/pwrctrl/slot.c
+++ b/drivers/pci/pwrctrl/slot.c
@@ -17,13 +17,38 @@ struct pci_pwrctrl_slot_data {
 	struct pci_pwrctrl ctx;
 	struct regulator_bulk_data *supplies;
 	int num_supplies;
+	struct clk *clk;
 };
 
-static void devm_pci_pwrctrl_slot_power_off(void *data)
+static int pci_pwrctrl_slot_power_on(struct pci_pwrctrl *ctx)
 {
-	struct pci_pwrctrl_slot_data *slot = data;
+	struct pci_pwrctrl_slot_data *slot = container_of(ctx, struct pci_pwrctrl_slot_data, ctx);
+	int ret;
+
+	ret = regulator_bulk_enable(slot->num_supplies, slot->supplies);
+	if (ret < 0) {
+		dev_err(slot->ctx.dev, "Failed to enable slot regulators\n");
+		return ret;
+	}
+
+	return clk_prepare_enable(slot->clk);
+}
+
+static int pci_pwrctrl_slot_power_off(struct pci_pwrctrl *ctx)
+{
+	struct pci_pwrctrl_slot_data *slot = container_of(ctx, struct pci_pwrctrl_slot_data, ctx);
 
 	regulator_bulk_disable(slot->num_supplies, slot->supplies);
+	clk_disable_unprepare(slot->clk);
+
+	return 0;
+}
+
+static void devm_pci_pwrctrl_slot_release(void *data)
+{
+	struct pci_pwrctrl_slot_data *slot = data;
+
+	pci_pwrctrl_slot_power_off(&slot->ctx);
 	regulator_bulk_free(slot->num_supplies, slot->supplies);
 }
 
@@ -31,7 +56,6 @@ static int pci_pwrctrl_slot_probe(struct platform_device *pdev)
 {
 	struct pci_pwrctrl_slot_data *slot;
 	struct device *dev = &pdev->dev;
-	struct clk *clk;
 	int ret;
 
 	slot = devm_kzalloc(dev, sizeof(*slot), GFP_KERNEL);
@@ -46,23 +70,21 @@ static int pci_pwrctrl_slot_probe(struct platform_device *pdev)
 	}
 
 	slot->num_supplies = ret;
-	ret = regulator_bulk_enable(slot->num_supplies, slot->supplies);
-	if (ret < 0) {
-		dev_err_probe(dev, ret, "Failed to enable slot regulators\n");
-		regulator_bulk_free(slot->num_supplies, slot->supplies);
-		return ret;
-	}
 
-	ret = devm_add_action_or_reset(dev, devm_pci_pwrctrl_slot_power_off,
+	ret = devm_add_action_or_reset(dev, devm_pci_pwrctrl_slot_release,
 				       slot);
 	if (ret)
 		return ret;
 
-	clk = devm_clk_get_optional_enabled(dev, NULL);
-	if (IS_ERR(clk)) {
-		return dev_err_probe(dev, PTR_ERR(clk),
+	slot->clk = devm_clk_get_optional(dev, NULL);
+	if (IS_ERR(slot->clk))
+		return dev_err_probe(dev, PTR_ERR(slot->clk),
 				     "Failed to enable slot clock\n");
-	}
+
+	pci_pwrctrl_slot_power_on(&slot->ctx);
+
+	slot->ctx.power_on = pci_pwrctrl_slot_power_on;
+	slot->ctx.power_off = pci_pwrctrl_slot_power_off;
 
 	pci_pwrctrl_init(&slot->ctx, dev);
 
diff --git a/include/linux/pci-pwrctrl.h b/include/linux/pci-pwrctrl.h
index 4aefc7901cd1..435b822c841e 100644
--- a/include/linux/pci-pwrctrl.h
+++ b/include/linux/pci-pwrctrl.h
@@ -31,6 +31,8 @@ struct device_link;
 /**
  * struct pci_pwrctrl - PCI device power control context.
  * @dev: Address of the power controlling device.
+ * @power_on: Callback to power on the power controlling device.
+ * @power_off: Callback to power off the power controlling device.
  *
  * An object of this type must be allocated by the PCI power control device and
  * passed to the pwrctrl subsystem to trigger a bus rescan and setup a device
@@ -38,6 +40,8 @@ struct device_link;
  */
 struct pci_pwrctrl {
 	struct device *dev;
+	int (*power_on)(struct pci_pwrctrl *pwrctrl);
+	int (*power_off)(struct pci_pwrctrl *pwrctrl);
 
 	/* private: internal use only */
 	struct notifier_block nb;

-- 
2.48.1

[PATCH v4 3/8] PCI/pwrctrl: Add APIs for explicitly creating and destroying pwrctrl devices

[Manivannan Sadhasivam via B4 Relay]

From: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

Previously, the PCI core created pwrctrl devices during pci_scan_device()
on its own and then skipped enumeration of those devices, hoping the
pwrctrl driver would power them on and trigger a bus rescan.

This approach works for endpoint devices directly connected to Root Ports,
but it fails for PCIe switches acting as bus extenders. When the switch
requires pwrctrl support, and the pwrctrl driver is not available during
the pwrctrl device creation, it's enumeration will be skipped during the
initial PCI bus scan.

This premature scan leads the PCI core to allocate resources (bridge
windows, bus numbers) for the upstream bridge based on available downstream
buses at scan time. For non-hotplug capable bridges, PCI core typically
allocates resources based on the number of buses available during the
initial bus scan, which happens to be just one if the switch is not powered
on and enumerated at that time. When the switch gets enumerated later on,
it will fail due to the lack of upstream resources.

As a result, a PCIe switch powered on by the pwrctrl driver cannot be
reliably enumerated currently. Either the switch has to be enabled in the
bootloader or the switch pwrctrl driver has to be loaded during the pwrctrl
device creation time to workaround these issues.

This commit introduces new APIs to explicitly create and destroy pwrctrl
devices from controller drivers by recursively scanning the PCI child nodes
of the controller. These APIs allow creating pwrctrl devices based on the
original criteria and are intended to be called during controller probe and
removal.

These APIs, together with the upcoming APIs for power on/off will allow the
controller drivers to power on all the devices before starting the initial
bus scan, thereby solving the resource allocation issue.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
[mani: splitted the patch, cleaned up the code, and rewrote description]
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/of.c            |   1 +
 drivers/pci/pwrctrl/core.c  | 113 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci-pwrctrl.h |   8 +++-
 3 files changed, 121 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 3579265f1198..9bb5f258759b 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -867,6 +867,7 @@ bool of_pci_supply_present(struct device_node *np)
 
 	return false;
 }
+EXPORT_SYMBOL_GPL(of_pci_supply_present);
 
 #endif /* CONFIG_PCI */
 
diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
index 6bdbfed584d6..4bd67b688a68 100644
--- a/drivers/pci/pwrctrl/core.c
+++ b/drivers/pci/pwrctrl/core.c
@@ -3,14 +3,21 @@
  * Copyright (C) 2024 Linaro Ltd.
  */
 
+#define dev_fmt(fmt) "Pwrctrl: " fmt
+
 #include <linux/device.h>
 #include <linux/export.h>
 #include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/pci.h>
 #include <linux/pci-pwrctrl.h>
+#include <linux/platform_device.h>
 #include <linux/property.h>
 #include <linux/slab.h>
 
+#include "../pci.h"
+
 static int pci_pwrctrl_notify(struct notifier_block *nb, unsigned long action,
 			      void *data)
 {
@@ -145,6 +152,112 @@ int devm_pci_pwrctrl_device_set_ready(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_pci_pwrctrl_device_set_ready);
 
+static int pci_pwrctrl_create_device(struct device_node *np, struct device *parent)
+{
+	struct platform_device *pdev;
+	int ret;
+
+	for_each_available_child_of_node_scoped(np, child) {
+		ret = pci_pwrctrl_create_device(child, parent);
+		if (ret)
+			return ret;
+	}
+
+	/* Bail out if the platform device is already available for the node */
+	pdev = of_find_device_by_node(np);
+	if (pdev) {
+		platform_device_put(pdev);
+		return 0;
+	}
+
+	/*
+	 * Sanity check to make sure that the node has the compatible property
+	 * to allow driver binding.
+	 */
+	if (!of_property_present(np, "compatible"))
+		return 0;
+
+	/*
+	 * Check whether the pwrctrl device really needs to be created or not.
+	 * This is decided based on at least one of the power supplies being
+	 * defined in the devicetree node of the device.
+	 */
+	if (!of_pci_supply_present(np)) {
+		dev_dbg(parent, "Skipping OF node: %s\n", np->name);
+		return 0;
+	}
+
+	/* Now create the pwrctrl device */
+	pdev = of_platform_device_create(np, NULL, parent);
+	if (!pdev) {
+		dev_err(parent, "Failed to create pwrctrl device for node: %s\n", np->name);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/**
+ * pci_pwrctrl_create_devices - Create pwrctrl devices
+ *
+ * @parent: Parent PCI device for which the pwrctrl devices need to be created.
+ *
+ * This function recursively creates pwrctrl devices for the child nodes
+ * of the specified PCI parent device in a depth first manner. On error, all
+ * created devices will be destroyed.
+ *
+ * Returns: 0 on success, negative error number on error.
+ */
+int pci_pwrctrl_create_devices(struct device *parent)
+{
+	int ret;
+
+	for_each_available_child_of_node_scoped(parent->of_node, child) {
+		ret = pci_pwrctrl_create_device(child, parent);
+		if (ret) {
+			pci_pwrctrl_destroy_devices(parent);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pci_pwrctrl_create_devices);
+
+static void pci_pwrctrl_destroy_device(struct device_node *np)
+{
+	struct platform_device *pdev;
+
+	for_each_available_child_of_node_scoped(np, child)
+		pci_pwrctrl_destroy_device(child);
+
+	pdev = of_find_device_by_node(np);
+	if (!pdev)
+		return;
+
+	of_device_unregister(pdev);
+	platform_device_put(pdev);
+
+	of_node_clear_flag(np, OF_POPULATED);
+}
+
+/**
+ * pci_pwrctrl_destroy_devices - Destroy pwrctrl devices
+ *
+ * @parent: Parent PCI device for which the pwrctrl devices need to be destroyed.
+ *
+ * This function recursively destroys pwrctrl devices for the child nodes
+ * of the specified PCI parent device in a depth first manner.
+ */
+void pci_pwrctrl_destroy_devices(struct device *parent)
+{
+	struct device_node *np = parent->of_node;
+
+	for_each_available_child_of_node_scoped(np, child)
+		pci_pwrctrl_destroy_device(child);
+}
+EXPORT_SYMBOL_GPL(pci_pwrctrl_destroy_devices);
+
 MODULE_AUTHOR("Bartosz Golaszewski <bartosz.golaszewski@linaro.org>");
 MODULE_DESCRIPTION("PCI Device Power Control core driver");
 MODULE_LICENSE("GPL");
diff --git a/include/linux/pci-pwrctrl.h b/include/linux/pci-pwrctrl.h
index 435b822c841e..44f66872d090 100644
--- a/include/linux/pci-pwrctrl.h
+++ b/include/linux/pci-pwrctrl.h
@@ -54,5 +54,11 @@ int pci_pwrctrl_device_set_ready(struct pci_pwrctrl *pwrctrl);
 void pci_pwrctrl_device_unset_ready(struct pci_pwrctrl *pwrctrl);
 int devm_pci_pwrctrl_device_set_ready(struct device *dev,
 				     struct pci_pwrctrl *pwrctrl);
-
+#if IS_ENABLED(CONFIG_PCI_PWRCTRL)
+int pci_pwrctrl_create_devices(struct device *parent);
+void pci_pwrctrl_destroy_devices(struct device *parent);
+#else
+static inline int pci_pwrctrl_create_devices(struct device *parent) { return 0; }
+static void pci_pwrctrl_destroy_devices(struct device *parent) { }
+#endif
 #endif /* __PCI_PWRCTRL_H__ */

-- 
2.48.1

[PATCH v4 4/8] PCI/pwrctrl: Add APIs to power on/off the pwrctrl devices

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

To fix PCIe bridge resource allocation issues when powering PCIe
switches with the pwrctrl driver, introduce APIs to explicitly power
on and off all related devices simultaneously.

Previously, the individual pwrctrl drivers powered on/off the PCIe devices
autonomously, without any control from the controller drivers. But to
enforce ordering w.r.t powering on the devices, these APIs will power
on/off all the devices at the same time.

The pci_pwrctrl_power_on_devices() API recursively scans the PCI child
nodes, makes sure that pwrctrl drivers are bind to devices, and calls their
power_on() callbacks. If any of the pwrctrl driver is not bound, it will
return -EPROBE_DEFER.

Similarly, pci_pwrctrl_power_off_devices() API powers off devices
recursively via their power_off() callbacks.

These APIs are expected to be called during the controller probe and
suspend/resume time to power on/off the devices. But before calling these
APIs, the pwrctrl devices should've been created beforehand using the
pci_pwrctrl_{create/destroy}_devices() APIs.

Co-developed-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/pwrctrl/core.c  | 132 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci-pwrctrl.h |   4 ++
 2 files changed, 136 insertions(+)

diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
index 4bd67b688a68..cb7947e347d3 100644
--- a/drivers/pci/pwrctrl/core.c
+++ b/drivers/pci/pwrctrl/core.c
@@ -65,6 +65,7 @@ void pci_pwrctrl_init(struct pci_pwrctrl *pwrctrl, struct device *dev)
 {
 	pwrctrl->dev = dev;
 	INIT_WORK(&pwrctrl->work, rescan_work_func);
+	dev_set_drvdata(dev, pwrctrl);
 }
 EXPORT_SYMBOL_GPL(pci_pwrctrl_init);
 
@@ -152,6 +153,137 @@ int devm_pci_pwrctrl_device_set_ready(struct device *dev,
 }
 EXPORT_SYMBOL_GPL(devm_pci_pwrctrl_device_set_ready);
 
+static int __pci_pwrctrl_power_off_device(struct device *dev)
+{
+	struct pci_pwrctrl *pwrctrl = dev_get_drvdata(dev);
+
+	if (!pwrctrl)
+		return 0;
+
+	return pwrctrl->power_off(pwrctrl);
+}
+
+static void pci_pwrctrl_power_off_device(struct device_node *np)
+{
+	struct platform_device *pdev;
+	int ret;
+
+	for_each_available_child_of_node_scoped(np, child)
+		pci_pwrctrl_power_off_device(child);
+
+	pdev = of_find_device_by_node(np);
+	if (pdev) {
+		if (device_is_bound(&pdev->dev)) {
+			ret = __pci_pwrctrl_power_off_device(&pdev->dev);
+			if (ret)
+				dev_err(&pdev->dev, "Failed to power off device: %d", ret);
+		}
+
+		platform_device_put(pdev);
+	}
+}
+
+/**
+ * pci_pwrctrl_power_off_devices - Power off the pwrctrl devices
+ *
+ * @parent: Parent PCI device for which the pwrctrl devices need to be powered
+ * off.
+ *
+ * This function recursively traverses all pwrctrl devices for the child nodes
+ * of the specified PCI parent device, and powers them off in a depth first
+ * manner.
+ */
+void pci_pwrctrl_power_off_devices(struct device *parent)
+{
+	struct device_node *np = parent->of_node;
+
+	for_each_available_child_of_node_scoped(np, child)
+		pci_pwrctrl_power_off_device(child);
+}
+EXPORT_SYMBOL_GPL(pci_pwrctrl_power_off_devices);
+
+static int __pci_pwrctrl_power_on_device(struct device *dev)
+{
+	struct pci_pwrctrl *pwrctrl = dev_get_drvdata(dev);
+
+	if (!pwrctrl)
+		return 0;
+
+	return pwrctrl->power_on(pwrctrl);
+}
+
+/*
+ * Power on the devices in a depth first manner. Before powering on the device,
+ * make sure its driver is bound.
+ */
+static int pci_pwrctrl_power_on_device(struct device_node *np)
+{
+	struct platform_device *pdev;
+	int ret;
+
+	for_each_available_child_of_node_scoped(np, child) {
+		ret = pci_pwrctrl_power_on_device(child);
+		if (ret)
+			return ret;
+	}
+
+	pdev = of_find_device_by_node(np);
+	if (pdev) {
+		if (!device_is_bound(&pdev->dev)) {
+			/* FIXME: Use blocking wait instead of probe deferral */
+			dev_dbg(&pdev->dev, "driver is not bound\n");
+			ret = -EPROBE_DEFER;
+		} else {
+			ret = __pci_pwrctrl_power_on_device(&pdev->dev);
+		}
+		platform_device_put(pdev);
+
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * pci_pwrctrl_power_on_devices - Power on the pwrctrl devices
+ *
+ * @parent: Parent PCI device for which the pwrctrl devices need to be powered
+ * on.
+ *
+ * This function recursively traverses all pwrctrl devices for the child nodes
+ * of the specified PCI parent device, and powers them on in a depth first
+ * manner. On error, all powered on devices will be powered off.
+ *
+ * Returns: 0 on success, -EPROBE_DEFER if any pwrctrl driver is not bound, an
+ * appropriate error code otherwise.
+ */
+int pci_pwrctrl_power_on_devices(struct device *parent)
+{
+	struct device_node *np = parent->of_node;
+	struct device_node *child = NULL;
+	int ret;
+
+	for_each_available_child_of_node(np, child) {
+		ret = pci_pwrctrl_power_on_device(child);
+		if (ret)
+			goto err_power_off;
+	}
+
+	return 0;
+
+err_power_off:
+	for_each_available_child_of_node_scoped(np, tmp) {
+		if (tmp == child)
+			break;
+		pci_pwrctrl_power_off_device(tmp);
+	}
+	of_node_put(child);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pci_pwrctrl_power_on_devices);
+
 static int pci_pwrctrl_create_device(struct device_node *np, struct device *parent)
 {
 	struct platform_device *pdev;
diff --git a/include/linux/pci-pwrctrl.h b/include/linux/pci-pwrctrl.h
index 44f66872d090..1192a2527521 100644
--- a/include/linux/pci-pwrctrl.h
+++ b/include/linux/pci-pwrctrl.h
@@ -57,8 +57,12 @@ int devm_pci_pwrctrl_device_set_ready(struct device *dev,
 #if IS_ENABLED(CONFIG_PCI_PWRCTRL)
 int pci_pwrctrl_create_devices(struct device *parent);
 void pci_pwrctrl_destroy_devices(struct device *parent);
+int pci_pwrctrl_power_on_devices(struct device *parent);
+void pci_pwrctrl_power_off_devices(struct device *parent);
 #else
 static inline int pci_pwrctrl_create_devices(struct device *parent) { return 0; }
 static void pci_pwrctrl_destroy_devices(struct device *parent) { }
+static inline int pci_pwrctrl_power_on_devices(struct device *parent) { return 0; }
+static void pci_pwrctrl_power_off_devices(struct device *parent) { }
 #endif
 #endif /* __PCI_PWRCTRL_H__ */

-- 
2.48.1

[PATCH v4 5/8] PCI/pwrctrl: Switch to the new pwrctrl APIs

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

Adopt the recently introduced pwrctrl APIs to create, power on, destroy,
and power off pwrctrl devices. In qcom_pcie_host_init(), call
pci_pwrctrl_create_devices() to create devices, then
pci_pwrctrl_power_on_devices() to power them on, both after controller
resource initialization. Once successful, deassert PERST# for all devices.

In qcom_pcie_host_deinit(), call pci_pwrctrl_power_off_devices() after
asserting PERST#. Note that pci_pwrctrl_destroy_devices() is not called
here, as deinit is only invoked during system suspend where device
destruction is unnecessary. If the driver becomes removable in future,
pci_pwrctrl_destroy_devices() should be called in the remove() handler.

At last, remove the old pwrctrl framework code from the PCI core (including
devlinks) as the new APIs are now the sole consumer of pwrctrl
functionality. And also do not power on the pwrctrl drivers during probe()
as this is now handled by the APIs.

Co-developed-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/bus.c                        | 19 ----------
 drivers/pci/controller/dwc/pcie-qcom.c   | 23 +++++++++++--
 drivers/pci/probe.c                      | 59 --------------------------------
 drivers/pci/pwrctrl/core.c               | 15 --------
 drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c |  5 ---
 drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c | 24 ++-----------
 drivers/pci/pwrctrl/slot.c               |  2 --
 drivers/pci/remove.c                     | 20 -----------
 8 files changed, 24 insertions(+), 143 deletions(-)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 4383a36fd6ca..d60d5f108212 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -344,7 +344,6 @@ void __weak pcibios_bus_add_device(struct pci_dev *pdev) { }
 void pci_bus_add_device(struct pci_dev *dev)
 {
 	struct device_node *dn = dev->dev.of_node;
-	struct platform_device *pdev;
 
 	/*
 	 * Can not put in pci_device_add yet because resources
@@ -361,24 +360,6 @@ void pci_bus_add_device(struct pci_dev *dev)
 	/* Save config space for error recoverability */
 	pci_save_state(dev);
 
-	/*
-	 * If the PCI device is associated with a pwrctrl device with a
-	 * power supply, create a device link between the PCI device and
-	 * pwrctrl device.  This ensures that pwrctrl drivers are probed
-	 * before PCI client drivers.
-	 */
-	pdev = of_find_device_by_node(dn);
-	if (pdev) {
-		if (of_pci_supply_present(dn)) {
-			if (!device_link_add(&dev->dev, &pdev->dev,
-					     DL_FLAG_AUTOREMOVE_CONSUMER)) {
-				pci_err(dev, "failed to add device link to power control device %s\n",
-					pdev->name);
-			}
-		}
-		put_device(&pdev->dev);
-	}
-
 	if (!dn || of_device_is_available(dn))
 		pci_dev_allow_binding(dev);
 
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 1ca58d0264e3..198befb5be2c 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -24,6 +24,7 @@
 #include <linux/of_pci.h>
 #include <linux/pci.h>
 #include <linux/pci-ecam.h>
+#include <linux/pci-pwrctrl.h>
 #include <linux/pm_opp.h>
 #include <linux/pm_runtime.h>
 #include <linux/platform_device.h>
@@ -1317,10 +1318,18 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 	if (ret)
 		goto err_deinit;
 
+	ret = pci_pwrctrl_create_devices(pci->dev);
+	if (ret)
+		goto err_disable_phy;
+
+	ret = pci_pwrctrl_power_on_devices(pci->dev);
+	if (ret)
+		goto err_pwrctrl_destroy;
+
 	if (pcie->cfg->ops->post_init) {
 		ret = pcie->cfg->ops->post_init(pcie);
 		if (ret)
-			goto err_disable_phy;
+			goto err_pwrctrl_power_off;
 	}
 
 	qcom_pcie_clear_aspm_l0s(pcie->pci);
@@ -1337,6 +1346,11 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 
 err_assert_reset:
 	qcom_ep_reset_assert(pcie);
+err_pwrctrl_power_off:
+	pci_pwrctrl_power_off_devices(pci->dev);
+err_pwrctrl_destroy:
+	if (ret != -EPROBE_DEFER)
+		pci_pwrctrl_destroy_devices(pci->dev);
 err_disable_phy:
 	qcom_pcie_phy_power_off(pcie);
 err_deinit:
@@ -1351,6 +1365,11 @@ static void qcom_pcie_host_deinit(struct dw_pcie_rp *pp)
 	struct qcom_pcie *pcie = to_qcom_pcie(pci);
 
 	qcom_ep_reset_assert(pcie);
+	/*
+	 * No need to destroy pwrctrl devices as this function only gets called
+	 * during system suspend as of now.
+	 */
+	pci_pwrctrl_power_off_devices(pci->dev);
 	qcom_pcie_phy_power_off(pcie);
 	pcie->cfg->ops->deinit(pcie);
 }
@@ -2029,7 +2048,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 
 	ret = dw_pcie_host_init(pp);
 	if (ret) {
-		dev_err(dev, "cannot initialize host\n");
+		dev_err_probe(dev, ret, "cannot initialize host\n");
 		goto err_phy_exit;
 	}
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 41183aed8f5d..6e7252404b58 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2563,56 +2563,6 @@ bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
 }
 EXPORT_SYMBOL(pci_bus_read_dev_vendor_id);
 
-#if IS_ENABLED(CONFIG_PCI_PWRCTRL)
-static struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus, int devfn)
-{
-	struct pci_host_bridge *host = pci_find_host_bridge(bus);
-	struct platform_device *pdev;
-	struct device_node *np;
-
-	np = of_pci_find_child_device(dev_of_node(&bus->dev), devfn);
-	if (!np)
-		return NULL;
-
-	pdev = of_find_device_by_node(np);
-	if (pdev) {
-		put_device(&pdev->dev);
-		goto err_put_of_node;
-	}
-
-	/*
-	 * First check whether the pwrctrl device really needs to be created or
-	 * not. This is decided based on at least one of the power supplies
-	 * being defined in the devicetree node of the device.
-	 */
-	if (!of_pci_supply_present(np)) {
-		pr_debug("PCI/pwrctrl: Skipping OF node: %s\n", np->name);
-		goto err_put_of_node;
-	}
-
-	/* Now create the pwrctrl device */
-	pdev = of_platform_device_create(np, NULL, &host->dev);
-	if (!pdev) {
-		pr_err("PCI/pwrctrl: Failed to create pwrctrl device for node: %s\n", np->name);
-		goto err_put_of_node;
-	}
-
-	of_node_put(np);
-
-	return pdev;
-
-err_put_of_node:
-	of_node_put(np);
-
-	return NULL;
-}
-#else
-static struct platform_device *pci_pwrctrl_create_device(struct pci_bus *bus, int devfn)
-{
-	return NULL;
-}
-#endif
-
 /*
  * Read the config data for a PCI device, sanity-check it,
  * and fill in the dev structure.
@@ -2622,15 +2572,6 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)
 	struct pci_dev *dev;
 	u32 l;
 
-	/*
-	 * Create pwrctrl device (if required) for the PCI device to handle the
-	 * power state. If the pwrctrl device is created, then skip scanning
-	 * further as the pwrctrl core will rescan the bus after powering on
-	 * the device.
-	 */
-	if (pci_pwrctrl_create_device(bus, devfn))
-		return NULL;
-
 	if (!pci_bus_read_dev_vendor_id(bus, devfn, &l, 60*1000))
 		return NULL;
 
diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
index cb7947e347d3..5973a83fe0d0 100644
--- a/drivers/pci/pwrctrl/core.c
+++ b/drivers/pci/pwrctrl/core.c
@@ -45,16 +45,6 @@ static int pci_pwrctrl_notify(struct notifier_block *nb, unsigned long action,
 	return NOTIFY_DONE;
 }
 
-static void rescan_work_func(struct work_struct *work)
-{
-	struct pci_pwrctrl *pwrctrl = container_of(work,
-						   struct pci_pwrctrl, work);
-
-	pci_lock_rescan_remove();
-	pci_rescan_bus(to_pci_host_bridge(pwrctrl->dev->parent)->bus);
-	pci_unlock_rescan_remove();
-}
-
 /**
  * pci_pwrctrl_init() - Initialize the PCI power control context struct
  *
@@ -64,7 +54,6 @@ static void rescan_work_func(struct work_struct *work)
 void pci_pwrctrl_init(struct pci_pwrctrl *pwrctrl, struct device *dev)
 {
 	pwrctrl->dev = dev;
-	INIT_WORK(&pwrctrl->work, rescan_work_func);
 	dev_set_drvdata(dev, pwrctrl);
 }
 EXPORT_SYMBOL_GPL(pci_pwrctrl_init);
@@ -95,8 +84,6 @@ int pci_pwrctrl_device_set_ready(struct pci_pwrctrl *pwrctrl)
 	if (ret)
 		return ret;
 
-	schedule_work(&pwrctrl->work);
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(pci_pwrctrl_device_set_ready);
@@ -109,8 +96,6 @@ EXPORT_SYMBOL_GPL(pci_pwrctrl_device_set_ready);
  */
 void pci_pwrctrl_device_unset_ready(struct pci_pwrctrl *pwrctrl)
 {
-	cancel_work_sync(&pwrctrl->work);
-
 	/*
 	 * We don't have to delete the link here. Typically, this function
 	 * is only called when the power control device is being detached. If
diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
index 7e7093cbff3c..42b87d480fe8 100644
--- a/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
+++ b/drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c
@@ -101,11 +101,6 @@ static int pci_pwrctrl_pwrseq_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(data->pwrseq),
 				     "Failed to get the power sequencer\n");
 
-	ret = pci_pwrctrl_pwrseq_power_on(&data->ctx);
-	if (ret)
-		return dev_err_probe(dev, ret,
-				     "Failed to power-on the device\n");
-
 	ret = devm_add_action_or_reset(dev, devm_pci_pwrctrl_pwrseq_power_off,
 				       data);
 	if (ret)
diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
index 616d3caedb34..66132d437ddb 100644
--- a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
+++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
@@ -515,8 +515,6 @@ static int tc9563_pwrctrl_power_on(struct pci_pwrctrl *pwrctrl)
 
 static int tc9563_pwrctrl_probe(struct platform_device *pdev)
 {
-	struct pci_host_bridge *bridge = to_pci_host_bridge(pdev->dev.parent);
-	struct pci_bus *bus = bridge->bus;
 	struct device *dev = &pdev->dev;
 	enum tc9563_pwrctrl_ports port;
 	struct tc9563_pwrctrl_ctx *ctx;
@@ -592,22 +590,6 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
 		goto remove_i2c;
 	}
 
-	if (bridge->ops->assert_perst) {
-		ret = bridge->ops->assert_perst(bus, true);
-		if (ret)
-			goto remove_i2c;
-	}
-
-	ret = tc9563_pwrctrl_power_on(&ctx->pwrctrl);
-	if (ret)
-		goto remove_i2c;
-
-	if (bridge->ops->assert_perst) {
-		ret = bridge->ops->assert_perst(bus, false);
-		if (ret)
-			goto power_off;
-	}
-
 	ctx->pwrctrl.power_on = tc9563_pwrctrl_power_on;
 	ctx->pwrctrl.power_off = tc9563_pwrctrl_power_off;
 
@@ -615,8 +597,6 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
 	if (ret)
 		goto power_off;
 
-	platform_set_drvdata(pdev, ctx);
-
 	return 0;
 
 power_off:
@@ -629,7 +609,9 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
 
 static void tc9563_pwrctrl_remove(struct platform_device *pdev)
 {
-	struct tc9563_pwrctrl_ctx *ctx = platform_get_drvdata(pdev);
+	struct pci_pwrctrl *pwrctrl = dev_get_drvdata(&pdev->dev);
+	struct tc9563_pwrctrl_ctx *ctx = container_of(pwrctrl,
+					struct tc9563_pwrctrl_ctx, pwrctrl);
 
 	tc9563_pwrctrl_power_off(&ctx->pwrctrl);
 	i2c_unregister_device(ctx->client);
diff --git a/drivers/pci/pwrctrl/slot.c b/drivers/pci/pwrctrl/slot.c
index 9f91f04b4ae0..dcd6ab79ca82 100644
--- a/drivers/pci/pwrctrl/slot.c
+++ b/drivers/pci/pwrctrl/slot.c
@@ -81,8 +81,6 @@ static int pci_pwrctrl_slot_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(slot->clk),
 				     "Failed to enable slot clock\n");
 
-	pci_pwrctrl_slot_power_on(&slot->ctx);
-
 	slot->ctx.power_on = pci_pwrctrl_slot_power_on;
 	slot->ctx.power_off = pci_pwrctrl_slot_power_off;
 
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 417a9ea59117..e9d519993853 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -17,25 +17,6 @@ static void pci_free_resources(struct pci_dev *dev)
 	}
 }
 
-static void pci_pwrctrl_unregister(struct device *dev)
-{
-	struct device_node *np;
-	struct platform_device *pdev;
-
-	np = dev_of_node(dev);
-	if (!np)
-		return;
-
-	pdev = of_find_device_by_node(np);
-	if (!pdev)
-		return;
-
-	of_device_unregister(pdev);
-	put_device(&pdev->dev);
-
-	of_node_clear_flag(np, OF_POPULATED);
-}
-
 static void pci_stop_dev(struct pci_dev *dev)
 {
 	pci_pme_active(dev, false);
@@ -73,7 +54,6 @@ static void pci_destroy_dev(struct pci_dev *dev)
 	pci_ide_destroy(dev);
 	pcie_aspm_exit_link_state(dev);
 	pci_bridge_d3_update(dev);
-	pci_pwrctrl_unregister(&dev->dev);
 	pci_free_resources(dev);
 	put_device(&dev->dev);
 }

-- 
2.48.1

[PATCH v4 6/8] PCI: qcom: Drop the assert_perst() callbacks

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

Previously, the pcie-qcom driver probed first, deasserted PERST#, enabled
link training and scanned the bus. By the time the pwrctrl driver probe got
called, link training was already enabled by the controller driver.

Thus the pwrctrl drivers had to call the .assert_perst() callback, to
assert PERST#, power on the needed resources, and then call the
.assert_perst() callback to deassert PERST#.

Now since all pwrctrl drivers and this controller driver have been
converted to the new pwrctrl design where the pwrctrl drivers will first
power on the devices before this driver deasserts PERST# and scan the bus.
So there is no longer a need for .assert_perst() callback in this driver
and in DWC core driver. Hence, drop them.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c |  9 ---------
 drivers/pci/controller/dwc/pcie-designware.h      |  9 ---------
 drivers/pci/controller/dwc/pcie-qcom.c            | 13 -------------
 3 files changed, 31 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 372207c33a85..4862a3a059c7 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -857,19 +857,10 @@ static void __iomem *dw_pcie_ecam_conf_map_bus(struct pci_bus *bus, unsigned int
 	return pci->dbi_base + where;
 }
 
-static int dw_pcie_op_assert_perst(struct pci_bus *bus, bool assert)
-{
-	struct dw_pcie_rp *pp = bus->sysdata;
-	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
-
-	return dw_pcie_assert_perst(pci, assert);
-}
-
 static struct pci_ops dw_pcie_ops = {
 	.map_bus = dw_pcie_own_conf_map_bus,
 	.read = pci_generic_config_read,
 	.write = pci_generic_config_write,
-	.assert_perst = dw_pcie_op_assert_perst,
 };
 
 static struct pci_ops dw_pcie_ecam_ops = {
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 31685951a080..da32bb5f936c 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -493,7 +493,6 @@ struct dw_pcie_ops {
 	enum dw_pcie_ltssm (*get_ltssm)(struct dw_pcie *pcie);
 	int	(*start_link)(struct dw_pcie *pcie);
 	void	(*stop_link)(struct dw_pcie *pcie);
-	int	(*assert_perst)(struct dw_pcie *pcie, bool assert);
 };
 
 struct debugfs_info {
@@ -798,14 +797,6 @@ static inline void dw_pcie_stop_link(struct dw_pcie *pci)
 		pci->ops->stop_link(pci);
 }
 
-static inline int dw_pcie_assert_perst(struct dw_pcie *pci, bool assert)
-{
-	if (pci->ops && pci->ops->assert_perst)
-		return pci->ops->assert_perst(pci, assert);
-
-	return 0;
-}
-
 static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
 {
 	u32 val;
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 198befb5be2c..6de3ff555b9d 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -650,18 +650,6 @@ static int qcom_pcie_post_init_1_0_0(struct qcom_pcie *pcie)
 	return 0;
 }
 
-static int qcom_pcie_assert_perst(struct dw_pcie *pci, bool assert)
-{
-	struct qcom_pcie *pcie = to_qcom_pcie(pci);
-
-	if (assert)
-		qcom_ep_reset_assert(pcie);
-	else
-		qcom_ep_reset_deassert(pcie);
-
-	return 0;
-}
-
 static void qcom_pcie_2_3_2_ltssm_enable(struct qcom_pcie *pcie)
 {
 	u32 val;
@@ -1523,7 +1511,6 @@ static const struct qcom_pcie_cfg cfg_fw_managed = {
 static const struct dw_pcie_ops dw_pcie_ops = {
 	.link_up = qcom_pcie_link_up,
 	.start_link = qcom_pcie_start_link,
-	.assert_perst = qcom_pcie_assert_perst,
 };
 
 static int qcom_pcie_icc_init(struct qcom_pcie *pcie)

-- 
2.48.1

[PATCH v4 7/8] PCI: Drop the assert_perst() callback

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

Now since all .assert_callback() implementations have been removed from the
controller drivers, drop the .assert_callback callback from pci.h.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 include/linux/pci.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index 864775651c6f..3eb8fd975ad9 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -854,7 +854,6 @@ struct pci_ops {
 	void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where);
 	int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val);
 	int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val);
-	int (*assert_perst)(struct pci_bus *bus, bool assert);
 };
 
 /*

-- 
2.48.1

[PATCH v4 8/8] PCI: qcom: Rename PERST# assert/deassert helpers for uniformity

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

Rename the PERST# assert/deassert helpers from
qcom_ep_reset_{assert/deassert}() to qcom_pcie_perst_{assert/deassert}() to
maintain uniformity.

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 6de3ff555b9d..a9293502e565 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -295,7 +295,7 @@ struct qcom_pcie {
 
 #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
 
-static void qcom_perst_assert(struct qcom_pcie *pcie, bool assert)
+static void __qcom_pcie_perst_assert(struct qcom_pcie *pcie, bool assert)
 {
 	struct qcom_pcie_perst *perst;
 	struct qcom_pcie_port *port;
@@ -309,16 +309,16 @@ static void qcom_perst_assert(struct qcom_pcie *pcie, bool assert)
 	usleep_range(PERST_DELAY_US, PERST_DELAY_US + 500);
 }
 
-static void qcom_ep_reset_assert(struct qcom_pcie *pcie)
+static void qcom_pcie_perst_assert(struct qcom_pcie *pcie)
 {
-	qcom_perst_assert(pcie, true);
+	__qcom_pcie_perst_assert(pcie, true);
 }
 
-static void qcom_ep_reset_deassert(struct qcom_pcie *pcie)
+static void qcom_pcie_perst_deassert(struct qcom_pcie *pcie)
 {
 	/* Ensure that PERST has been asserted for at least 100 ms */
 	msleep(PCIE_T_PVPERL_MS);
-	qcom_perst_assert(pcie, false);
+	__qcom_pcie_perst_assert(pcie, false);
 }
 
 static int qcom_pcie_start_link(struct dw_pcie *pci)
@@ -1296,7 +1296,7 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 	struct qcom_pcie *pcie = to_qcom_pcie(pci);
 	int ret;
 
-	qcom_ep_reset_assert(pcie);
+	qcom_pcie_perst_assert(pcie);
 
 	ret = pcie->cfg->ops->init(pcie);
 	if (ret)
@@ -1322,7 +1322,7 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 
 	qcom_pcie_clear_aspm_l0s(pcie->pci);
 
-	qcom_ep_reset_deassert(pcie);
+	qcom_pcie_perst_deassert(pcie);
 
 	if (pcie->cfg->ops->config_sid) {
 		ret = pcie->cfg->ops->config_sid(pcie);
@@ -1333,7 +1333,7 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 	return 0;
 
 err_assert_reset:
-	qcom_ep_reset_assert(pcie);
+	qcom_pcie_perst_assert(pcie);
 err_pwrctrl_power_off:
 	pci_pwrctrl_power_off_devices(pci->dev);
 err_pwrctrl_destroy:
@@ -1352,7 +1352,7 @@ static void qcom_pcie_host_deinit(struct dw_pcie_rp *pp)
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct qcom_pcie *pcie = to_qcom_pcie(pci);
 
-	qcom_ep_reset_assert(pcie);
+	qcom_pcie_perst_assert(pcie);
 	/*
 	 * No need to destroy pwrctrl devices as this function only gets called
 	 * during system suspend as of now.

-- 
2.48.1

Re: [PATCH v4 2/8] PCI/pwrctrl: Add 'struct pci_pwrctrl::power_{on/off}' callbacks

[Bartosz Golaszewski]

On Mon, 5 Jan 2026 14:55:42 +0100, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.oss.qualcomm.com@kernel.org> said:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> To allow the pwrctrl core to control the power on/off sequences of the
> pwrctrl drivers, add the 'struct pci_pwrctrl::power_{on/off}' callbacks and
> populate them in the respective pwrctrl drivers.
>
> The pwrctrl drivers still power on the resources on their own now. So there
> is no functional change.
>
> Co-developed-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Re: [PATCH v4 7/8] PCI: Drop the assert_perst() callback

[Bartosz Golaszewski]

On Mon, 5 Jan 2026 14:55:47 +0100, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.oss.qualcomm.com@kernel.org> said:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> Now since all .assert_callback() implementations have been removed from the
> controller drivers, drop the .assert_callback callback from pci.h.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  include/linux/pci.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 864775651c6f..3eb8fd975ad9 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -854,7 +854,6 @@ struct pci_ops {
>  	void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where);
>  	int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val);
>  	int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val);
> -	int (*assert_perst)(struct pci_bus *bus, bool assert);
>  };
>
>  /*
>
> --
> 2.48.1
>
>
>

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Re: [PATCH v4 2/8] PCI/pwrctrl: Add 'struct pci_pwrctrl::power_{on/off}' callbacks

[Bjorn Helgaas]

On Mon, Jan 05, 2026 at 07:25:42PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> To allow the pwrctrl core to control the power on/off sequences of the
> pwrctrl drivers, add the 'struct pci_pwrctrl::power_{on/off}' callbacks and
> populate them in the respective pwrctrl drivers.
> 
> The pwrctrl drivers still power on the resources on their own now. So there
> is no functional change.
> 
> Co-developed-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c | 27 ++++++++++++++---
>  drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c | 22 ++++++++++----
>  drivers/pci/pwrctrl/slot.c               | 50 +++++++++++++++++++++++---------
>  include/linux/pci-pwrctrl.h              |  4 +++
>  4 files changed, 79 insertions(+), 24 deletions(-)

I had a hard time reading this because of the gratuitous differences
in names of pwrseq, tc9563, and slot structs, members, and pointers.
These are all corresponding private structs that could be named
similarly:

  struct pci_pwrctrl_pwrseq_data
  struct tc9563_pwrctrl_ctx
  struct pci_pwrctrl_slot_data

These are all corresponding members:

  struct pci_pwrctrl_pwrseq_data.ctx
  struct tc9563_pwrctrl_ctx.pwrctrl (last in struct instead of first)
  struct pci_pwrctrl_slot_data.ctx
  
And these are all corresponding pointers or parameters:

  struct pci_pwrctrl_pwrseq_data *data
  struct tc9563_pwrctrl_ctx *ctx
  struct pci_pwrctrl_slot_data *slot

There's no need for this confusion, so I reworked those names to make
them a *little* more consistent:

  structs:
    struct pci_pwrctrl_pwrseq
    struct pci_pwrctrl_tc9563
    struct pci_pwrctrl_slot

  member:
    struct pci_pwrctrl pwrctrl (for all)

  pointers/parameters:
    struct pci_pwrctrl_pwrseq *pwrseq
    struct pci_pwrctrl_tc9563 *tc9563
    struct pci_pwrctrl_slot *slot

The file names, function names, and driver names are still not very
consistent, but I didn't do anything with them:

  pci-pwrctrl-pwrseq.c  pci_pwrctrl_pwrseq_probe()  "pci-pwrctrl-pwrseq"
  pci-pwrctrl-tc9563.c  tc9563_pwrctrl_probe()      "pwrctrl-tc9563"
  slot.c                pci_pwrctrl_slot_probe()    ""pci-pwrctrl-slot"

> +++ b/drivers/pci/pwrctrl/slot.c
> @@ -17,13 +17,38 @@ struct pci_pwrctrl_slot_data {
>  	struct pci_pwrctrl ctx;
>  	struct regulator_bulk_data *supplies;
>  	int num_supplies;
> +	struct clk *clk;
>  };
>  
> -static void devm_pci_pwrctrl_slot_power_off(void *data)
> +static int pci_pwrctrl_slot_power_on(struct pci_pwrctrl *ctx)
>  {
> -	struct pci_pwrctrl_slot_data *slot = data;
> +	struct pci_pwrctrl_slot_data *slot = container_of(ctx, struct pci_pwrctrl_slot_data, ctx);
> +	int ret;
> +
> +	ret = regulator_bulk_enable(slot->num_supplies, slot->supplies);
> +	if (ret < 0) {
> +		dev_err(slot->ctx.dev, "Failed to enable slot regulators\n");
> +		return ret;
> +	}
> +
> +	return clk_prepare_enable(slot->clk);

It would be nice if we could add a preparatory patch to factor out
pci_pwrctrl_slot_power_on() before this one.  Then the slot.c patch
would look more like the pwrseq and tc9563 ones.

Bjorn

Re: [PATCH v4 4/8] PCI/pwrctrl: Add APIs to power on/off the pwrctrl devices

[Bjorn Helgaas]

On Mon, Jan 05, 2026 at 07:25:44PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> To fix PCIe bridge resource allocation issues when powering PCIe
> switches with the pwrctrl driver, introduce APIs to explicitly power
> on and off all related devices simultaneously.
> ...

> +static void pci_pwrctrl_power_off_device(struct device_node *np)
> +{
> +	struct platform_device *pdev;
> +	int ret;
> +
> +	for_each_available_child_of_node_scoped(np, child)
> +		pci_pwrctrl_power_off_device(child);
> +
> +	pdev = of_find_device_by_node(np);
> +	if (pdev) {
> +		if (device_is_bound(&pdev->dev)) {
> +			ret = __pci_pwrctrl_power_off_device(&pdev->dev);
> +			if (ret)
> +				dev_err(&pdev->dev, "Failed to power off device: %d", ret);
> +		}
> +
> +		platform_device_put(pdev);
> +	}

Suggest this to reduce indentation:

  pdev = of_find_device_by_node(np);
  if (!pdev)
    return;

  if (device_is_bound(&pdev->dev)) {
    ...
  }

  platform_device_put(pdev);

> +static int pci_pwrctrl_power_on_device(struct device_node *np)
> +{
> +	struct platform_device *pdev;
> +	int ret;
> +
> +	for_each_available_child_of_node_scoped(np, child) {
> +		ret = pci_pwrctrl_power_on_device(child);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	pdev = of_find_device_by_node(np);
> +	if (pdev) {
> +		if (!device_is_bound(&pdev->dev)) {
> +			/* FIXME: Use blocking wait instead of probe deferral */
> +			dev_dbg(&pdev->dev, "driver is not bound\n");
> +			ret = -EPROBE_DEFER;
> +		} else {
> +			ret = __pci_pwrctrl_power_on_device(&pdev->dev);
> +		}
> +		platform_device_put(pdev);
> +
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;

And this:

  pdev = of_find_device_by_node(np);
  if (!pdev)
    return 0;

  if (device_is_bound(&pdev->dev)) {
    ...
  } else {
    ...
  }

  platform_device_put(pdev);

  return ret;

Re: [PATCH v4 0/8] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

[Bjorn Helgaas]

On Mon, Jan 05, 2026 at 07:25:40PM +0530, Manivannan Sadhasivam via B4 Relay 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.

I pushed a pci/pwrctrl-v5 that incorporates some of the comments I
sent.  If it's useful, you can use it as a basis for a v6; if not, no
worries.

Bjorn

Re: [PATCH v4 0/8] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

[Manivannan Sadhasivam]

On Sun, Jan 11, 2026 at 09:31:32PM -0600, Bjorn Helgaas wrote:
> On Mon, Jan 05, 2026 at 07:25:40PM +0530, Manivannan Sadhasivam via B4 Relay 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.
> 
> I pushed a pci/pwrctrl-v5 that incorporates some of the comments I
> sent.  If it's useful, you can use it as a basis for a v6; if not, no
> worries.
> 

Thanks for making the changes, they look good to me. Do you expect me to send v6
or you intend to merge this pwrctrl-v5 branch to pci/next?

- Mani

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

Re: [PATCH v4 0/8] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

[Bjorn Helgaas]

On Mon, Jan 12, 2026 at 01:23:02PM +0530, Manivannan Sadhasivam wrote:
> On Sun, Jan 11, 2026 at 09:31:32PM -0600, Bjorn Helgaas wrote:
> > On Mon, Jan 05, 2026 at 07:25:40PM +0530, Manivannan Sadhasivam via B4 Relay 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.
> > 
> > I pushed a pci/pwrctrl-v5 that incorporates some of the comments I
> > sent.  If it's useful, you can use it as a basis for a v6; if not,
> > no worries.
> > 
> 
> Thanks for making the changes, they look good to me. Do you expect
> me to send v6 or you intend to merge this pwrctrl-v5 branch to
> pci/next?

I'm not planning to merge pwrctrl-v5 yet.

Still hoping pci_pwrctrl_slot_power_on() could be factored out earlier
to simplify "PCI/pwrctrl: Add 'struct pci_pwrctrl::power_{on/off}'
callbacks".  It wasn't quite obvious to me how to do that.

Re: [PATCH v4 0/8] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

[Sean Anderson]

On 1/5/26 08:55, Manivannan Sadhasivam via B4 Relay wrote:
> Hi,

I asked substantially similar questions on v2, but since I never got a
response I want to reiterate them on v4 to make sure they don't get
lost.

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

OK, so to clarify the problem is an architecture like

    RP
    |-- Bridge 1 (automatic)
    |   |-- Device 1
    |   `-- Bridge 2 (needs pwrseq)
    |       `-- Device 2
    `-- Bridge 3 (automatic)
        `-- Device 3

where Bridge 2 has a devicetree node with a pwrseq binding? So we do the
initial scan and allocate resources for bridge/devices 1 and 3 with the
resources for bridge 3 immediately above those for bridge 1. Then when
bridge 2 shows up we can't resize bridge 1's windows since bridge 3's
windows are in the way?

But is it even valid to have a pwrseq node on bridge 2 without one on
bridge 1? If bridge 1 is automatically controlled, then I would expect
bridge 2 to be as well. E.g. I would expect bridge 2's reset sequence to
be controlled by the secondary bus reset bit in bridge 1's bridge
control register.

And a very similar architecture like

    RP
    |-- Bridge 4 (pwrseq)
    |   |-- Device 4
    `-- Bridge 5 (automatic)
        `-- Device 5

has no problems since the resources for bridge 4 can be allocated above
those for bridge 5 whenever it shows up.

These problems seem very similar to what hotplug bridges have to handle
(except that we (usually) only need to do one hotplug per boot). So
maybe we should set is_hotplug_bridge on bridges with a pwrseq node.
That way they'll get resources distributed for when the downstream port
shows up. As an optimization, we could then release those resources once
the downstream port is scanned.

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

(just for the record)

I think the existing design is quite elegant, since the operations
associated with the bridge correspond directly to device lifecycle
operations. It also avoids problems related to the root port trying to
look up its own child (possibly missing a driver) during probe.

> This integration allows better coordination
> between controller drivers and the pwrctrl framework, enabling enhanced features
> such as D3Cold support.


I think this should be handled by the power sequencing driver,
especially as there are timing requirements for the other resources
referenced to PERST? If we are going to touch each driver, it would
be much better to consolidate things by removing the ad-hoc PERST
support.

Different drivers control PERST in various ways, but I think this can
be abstracted behind a GPIO controller (if necessary for e.g. MMIO-based
control). If there's no reset-gpios property in the pwrseq node then we
could automatically look up the GPIO on the root port.

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

Can you elaborate on this? Previously you said

| Some platforms do LTSSM during phy_init(), so they will fail if the
| device is not powered ON at that time.

What do you mean by "do LTSSM during phy_init()"? Do you have a specific
driver in mind?

I would expect that the LTSSM would remain in the Detect state until the
pwrseq driver is being probed.

> 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. I
> plan to replace probe deferral with blocking wait in the coming days.

You can only do a blocking wait after deferring at least once, since the
root port may be probed synchronously during boot. I really think this
is rather messy and something we should avoid architecturally while we
have the chance.

--Sean

> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> Changes in v4:
> - Used platform_device_put()
> - Changed the return value of power_off() callback to 'int'
> - Splitted patch 6 into two and reworded the commit message
> - Collected tags
> - Link to v3: https://lore.kernel.org/r/20251229-pci-pwrctrl-rework-v3-0-c7d5918cd0db@oss.qualcomm.com
> 
> Changes in v3:
> - Integrated TC9563 change
> - Reworked the power_on API to properly power off the devices in error path
> - Fixed the error path in pcie-qcom.c to not destroy pwrctrl devices during
>   probe deferral
> - Rebased on top of pci/controller/dwc-qcom branch and dropped the PERST# patch
> - Added a patch for TC9563 to fix the refcount dropping for i2c adapter
> - Added patches to drop the assert_perst callback and rename the PERST# helpers in
>   pcie-qcom.c
> - Link to v2: https://lore.kernel.org/r/20251216-pci-pwrctrl-rework-v2-0-745a563b9be6@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 (7):
>       PCI/pwrctrl: tc9563: Use put_device() instead of i2c_put_adapter()
>       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
>       PCI: qcom: Drop the assert_perst() callbacks
>       PCI: Drop the assert_perst() callback
>       PCI: qcom: Rename PERST# assert/deassert helpers for uniformity
> 
>  drivers/pci/bus.c                                 |  19 --
>  drivers/pci/controller/dwc/pcie-designware-host.c |   9 -
>  drivers/pci/controller/dwc/pcie-designware.h      |   9 -
>  drivers/pci/controller/dwc/pcie-qcom.c            |  54 +++--
>  drivers/pci/of.c                                  |   1 +
>  drivers/pci/probe.c                               |  59 -----
>  drivers/pci/pwrctrl/core.c                        | 260 ++++++++++++++++++++--
>  drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c          |  30 ++-
>  drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c          |  48 ++--
>  drivers/pci/pwrctrl/slot.c                        |  48 ++--
>  drivers/pci/remove.c                              |  20 --
>  include/linux/pci-pwrctrl.h                       |  16 +-
>  include/linux/pci.h                               |   1 -
>  13 files changed, 367 insertions(+), 207 deletions(-)
> ---
> base-commit: 3e7f562e20ee87a25e104ef4fce557d39d62fa85
> change-id: 20251124-pci-pwrctrl-rework-c91a6e16c2f6
> prerequisite-message-id: 20251126081718.8239-1-mani@kernel.org
> prerequisite-patch-id: db9ff6c713e2303c397e645935280fd0d277793a
> prerequisite-patch-id: b5351b0a41f618435f973ea2c3275e51d46f01c5
> 
> Best regards,

Re: [PATCH v4 0/8] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

[Manivannan Sadhasivam]

On Tue, Jan 13, 2026 at 12:15:01PM -0500, Sean Anderson wrote:
> On 1/5/26 08:55, Manivannan Sadhasivam via B4 Relay wrote:
> > Hi,
> 
> I asked substantially similar questions on v2, but since I never got a
> response I want to reiterate them on v4 to make sure they don't get
> lost.
> 

I did respond to your queries in v2, but lost your last reply in that thread:
https://lore.kernel.org/linux-pci/8269249f-48a9-4136-a326-23f5076be487@linux.dev/

Apologies!

> > 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.
> 
> OK, so to clarify the problem is an architecture like
> 
>     RP
>     |-- Bridge 1 (automatic)
>     |   |-- Device 1
>     |   `-- Bridge 2 (needs pwrseq)
>     |       `-- Device 2
>     `-- Bridge 3 (automatic)
>         `-- Device 3
> 

This topology is not possible with PCIe. A single Root Port can only connect to
a single bridge. But applies to PCI.

> where Bridge 2 has a devicetree node with a pwrseq binding? So we do the
> initial scan and allocate resources for bridge/devices 1 and 3 with the
> resources for bridge 3 immediately above those for bridge 1. Then when
> bridge 2 shows up we can't resize bridge 1's windows since bridge 3's
> windows are in the way?
> 

It is not a problem with resizing, it is the problem with how much you can
resize. And also if that bridge 2 is a switch and if it exposes multiple
downstream busses, then the upstream bridge 1 will run out of resources.
If bridge 2 is a hotplug bridge, then no issues. But I was only referring to
non-hotplug capable switches.

> But is it even valid to have a pwrseq node on bridge 2 without one on
> bridge 1? If bridge 1 is automatically controlled, then I would expect
> bridge 2 to be as well. E.g. I would expect bridge 2's reset sequence to
> be controlled by the secondary bus reset bit in bridge 1's bridge
> control register.
> 

Technically it is possible for Bridge 2 to have a pwrctrl requirement. What is
limiting from spec PoV?

> And a very similar architecture like
> 
>     RP
>     |-- Bridge 4 (pwrseq)
>     |   |-- Device 4
>     `-- Bridge 5 (automatic)
>         `-- Device 5
> 
> has no problems since the resources for bridge 4 can be allocated above
> those for bridge 5 whenever it shows up.
> 

Again, if bridge 4 is not hotplug capable and if it is a switch, the problem is
still applicable.

> These problems seem very similar to what hotplug bridges have to handle
> (except that we (usually) only need to do one hotplug per boot). So
> maybe we should set is_hotplug_bridge on bridges with a pwrseq node.
> That way they'll get resources distributed for when the downstream port
> shows up. As an optimization, we could then release those resources once
> the downstream port is scanned.
> 

That would be incorrect. You cannot set 'is_hotplug_bridge' to 'true' for a
non-hotplug capable bridge. You can call it as a hack, but there is no place
for that in upstream.

> > 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.
> 
> (just for the record)
> 
> I think the existing design is quite elegant, since the operations
> associated with the bridge correspond directly to device lifecycle
> operations. It also avoids problems related to the root port trying to
> look up its own child (possibly missing a driver) during probe.
> 

I agree with you that it is elegant and I even was very reluctant to move out of
it [1]. But lately, I understood that we cannot scale the pwrctrl framework if we
do not give flexibility to the controller drivers [2].

[1] https://lore.kernel.org/linux-pci/eix65qdwtk5ocd7lj6sw2lslidivauzyn6h5cc4mc2nnci52im@qfmbmwy2zjbe/
[2] https://lore.kernel.org/linux-pci/aG3IWdZIhnk01t2A@google.com/

> > This integration allows better coordination
> > between controller drivers and the pwrctrl framework, enabling enhanced features
> > such as D3Cold support.
> 
> 
> I think this should be handled by the power sequencing driver,
> especially as there are timing requirements for the other resources
> referenced to PERST? If we are going to touch each driver, it would
> be much better to consolidate things by removing the ad-hoc PERST
> support.
> 
> Different drivers control PERST in various ways, but I think this can
> be abstracted behind a GPIO controller (if necessary for e.g. MMIO-based
> control). If there's no reset-gpios property in the pwrseq node then we
> could automatically look up the GPIO on the root port.
> 

Not at all. We cannot model PERST# as a GPIO in all the cases. Some drivers
implement PERST# as a set of MMIO operations in the Root Complex MMIO space and
that space belongs to the controller driver.

FYI, I did try something similar before:
https://lore.kernel.org/linux-pci/20250707-pci-pwrctrl-perst-v1-0-c3c7e513e312@kernel.org/

> > 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.
> 
> Can you elaborate on this? Previously you said
> 
> | Some platforms do LTSSM during phy_init(), so they will fail if the
> | device is not powered ON at that time.
> 
> What do you mean by "do LTSSM during phy_init()"? Do you have a specific
> driver in mind?
> 

I believe the Mediatek PCIe controller driver used in Chromebooks exhibit this
behavior. Chen talked about it in his LPC session:
https://lpc.events/event/19/contributions/2023/

> I would expect that the LTSSM would remain in the Detect state until the
> pwrseq driver is being probed.
> 

True, but if the API (phy_init()) expects the LTSSM to move to L0, then it will
fail, right? It might be what's happening with above mentioned platform.

> > 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. I
> > plan to replace probe deferral with blocking wait in the coming days.
> 
> You can only do a blocking wait after deferring at least once, since the
> root port may be probed synchronously during boot. I really think this
> is rather messy and something we should avoid architecturally while we
> have the chance.
> 

By blocking wait I meant that the controller probe itself will do a blocking
wait until the pwrctrl drivers gets bound. Since this happens way before the PCI
bus scan, there won't be any Root Port probed synchronously.

- Mani

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

Re: [PATCH v4 0/8] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

[Chen-Yu Tsai]

On Wed, Jan 14, 2026 at 4:48 PM Manivannan Sadhasivam <mani@kernel.org> wrote:

[...]

> > > 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.
> >
> > Can you elaborate on this? Previously you said
> >
> > | Some platforms do LTSSM during phy_init(), so they will fail if the
> > | device is not powered ON at that time.
> >
> > What do you mean by "do LTSSM during phy_init()"? Do you have a specific
> > driver in mind?
> >
>
> I believe the Mediatek PCIe controller driver used in Chromebooks exhibit this
> behavior. Chen talked about it in his LPC session:
> https://lpc.events/event/19/contributions/2023/

I don't remember all the details off the top of my head, but at least the
MediaTek and old (non-DesignWare) Rockchip drivers both did this:

    Wait for link up during the probe function; if it times out then
    nothing is there, and just fail the probe.

And this probably makes sense if the controller does not support hotplug,
and you want to keep unused devices / interfaces disabled to save power.

> > I would expect that the LTSSM would remain in the Detect state until the
> > pwrseq driver is being probed.
> >
>
> True, but if the API (phy_init()) expects the LTSSM to move to L0, then it will
> fail, right? It might be what's happening with above mentioned platform.

I can't remember if any drivers expected this. IIRC they waited for link up
in the probe function before registering the PCI host.

[...]


ChenYu

Re: [PATCH v4 2/8] PCI/pwrctrl: Add 'struct pci_pwrctrl::power_{on/off}' callbacks

[Bjorn Helgaas]

On Sun, Jan 11, 2026 at 09:27:11PM -0600, Bjorn Helgaas wrote:
> On Mon, Jan 05, 2026 at 07:25:42PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > 
> > To allow the pwrctrl core to control the power on/off sequences of the
> > pwrctrl drivers, add the 'struct pci_pwrctrl::power_{on/off}' callbacks and
> > populate them in the respective pwrctrl drivers.
> > 
> > The pwrctrl drivers still power on the resources on their own now. So there
> > is no functional change.
> > 
> > Co-developed-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >  drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c | 27 ++++++++++++++---
> >  drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c | 22 ++++++++++----
> >  drivers/pci/pwrctrl/slot.c               | 50 +++++++++++++++++++++++---------
> >  include/linux/pci-pwrctrl.h              |  4 +++
> >  4 files changed, 79 insertions(+), 24 deletions(-)

> > +++ b/drivers/pci/pwrctrl/slot.c
> > @@ -17,13 +17,38 @@ struct pci_pwrctrl_slot_data {
> >  	struct pci_pwrctrl ctx;
> >  	struct regulator_bulk_data *supplies;
> >  	int num_supplies;
> > +	struct clk *clk;
> >  };
> >  
> > -static void devm_pci_pwrctrl_slot_power_off(void *data)
> > +static int pci_pwrctrl_slot_power_on(struct pci_pwrctrl *ctx)
> >  {
> > -	struct pci_pwrctrl_slot_data *slot = data;
> > +	struct pci_pwrctrl_slot_data *slot = container_of(ctx, struct pci_pwrctrl_slot_data, ctx);
> > +	int ret;
> > +
> > +	ret = regulator_bulk_enable(slot->num_supplies, slot->supplies);
> > +	if (ret < 0) {
> > +		dev_err(slot->ctx.dev, "Failed to enable slot regulators\n");
> > +		return ret;
> > +	}
> > +
> > +	return clk_prepare_enable(slot->clk);
> 
> It would be nice if we could add a preparatory patch to factor out
> pci_pwrctrl_slot_power_on() before this one.  Then the slot.c patch
> would look more like the pwrseq and tc9563 ones.

tc9563 implements power control functions:

  tc9563_pwrctrl_bring_up(struct tc9563_pwrctrl_ctx *ctx)
  tc9563_pwrctrl_power_off(struct pci_pwrctrl_tc9563 *tc9563)

and this patch updates these to make the signature generic so they can
be used as callbacks:

  tc9563_pwrctrl_power_on(struct pci_pwrctrl *pwrctrl)
  tc9563_pwrctrl_power_off(struct pci_pwrctrl *pwrctrl)

This part of the patch is super straightforward -- make the signature
generic and extract the per-driver struct using the generic pointer.

I was thinking that if a preparatory patch factored out the slot power
on/off, e.g.:

  pci_pwrctrl_slot_power_on(struct pci_pwrctrl_slot_data *slot)

Then the slot.c part of this patch wouldn't move any code around, so
the structure would be identical to the tc9563 part.

pwrseq doesn't currently have the power-on/off functions factored out
either because they're so trivial, but I might even consider factoring
those out first, e.g.:

  pci_pwrctrl_pwrseq_power_on(struct pci_pwrctrl_pwrseq *pwrseq)

If we did that, this patch would be strictly conversion from
driver-specific pointer to "struct pci_pwrctrl *pwrctrl" followed by
"<driver-specific pointer = container_of(...)", so all three driver
changes would be identical and trivial to describe and review:

  - pci_pwrctrl_pwrseq_power_on(struct pci_pwrctrl_pwrseq *pwrseq)
  + pci_pwrctrl_pwrseq_power_on(struct pci_pwrctrl *pwrctrl)
  +   struct pci_pwrctrl_pwrseq *pwrseq = container_of(...);

  - tc9563_pwrctrl_bring_up(struct pci_pwrctrl_tc9563 *tc9563)
  + tc9563_pwrctrl_power_on(struct pci_pwrctrl *pwrctrl)
  +   struct pci_pwrctrl_tc9563 *tc9563 = container_of(...);

  - pci_pwrctrl_slot_power_on(struct pci_pwrctrl_slot *slot)
  + pci_pwrctrl_slot_power_on(struct pci_pwrctrl *pwrctrl)
  +   struct pci_pwrctrl_slot *slot = container_of(...);

Re: [PATCH v4 2/8] PCI/pwrctrl: Add 'struct pci_pwrctrl::power_{on/off}' callbacks

[Manivannan Sadhasivam]

On Sun, Jan 11, 2026 at 09:27:11PM -0600, Bjorn Helgaas wrote:
> On Mon, Jan 05, 2026 at 07:25:42PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > 
> > To allow the pwrctrl core to control the power on/off sequences of the
> > pwrctrl drivers, add the 'struct pci_pwrctrl::power_{on/off}' callbacks and
> > populate them in the respective pwrctrl drivers.
> > 
> > The pwrctrl drivers still power on the resources on their own now. So there
> > is no functional change.
> > 
> > Co-developed-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > Tested-by: Chen-Yu Tsai <wenst@chromium.org>
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >  drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c | 27 ++++++++++++++---
> >  drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c | 22 ++++++++++----
> >  drivers/pci/pwrctrl/slot.c               | 50 +++++++++++++++++++++++---------
> >  include/linux/pci-pwrctrl.h              |  4 +++
> >  4 files changed, 79 insertions(+), 24 deletions(-)
> 
> I had a hard time reading this because of the gratuitous differences
> in names of pwrseq, tc9563, and slot structs, members, and pointers.
> These are all corresponding private structs that could be named
> similarly:
> 
>   struct pci_pwrctrl_pwrseq_data
>   struct tc9563_pwrctrl_ctx
>   struct pci_pwrctrl_slot_data
> 
> These are all corresponding members:
> 
>   struct pci_pwrctrl_pwrseq_data.ctx
>   struct tc9563_pwrctrl_ctx.pwrctrl (last in struct instead of first)
>   struct pci_pwrctrl_slot_data.ctx
>   
> And these are all corresponding pointers or parameters:
> 
>   struct pci_pwrctrl_pwrseq_data *data
>   struct tc9563_pwrctrl_ctx *ctx
>   struct pci_pwrctrl_slot_data *slot
> 
> There's no need for this confusion, so I reworked those names to make
> them a *little* more consistent:
> 
>   structs:
>     struct pci_pwrctrl_pwrseq
>     struct pci_pwrctrl_tc9563
>     struct pci_pwrctrl_slot
> 
>   member:
>     struct pci_pwrctrl pwrctrl (for all)
> 
>   pointers/parameters:
>     struct pci_pwrctrl_pwrseq *pwrseq
>     struct pci_pwrctrl_tc9563 *tc9563
>     struct pci_pwrctrl_slot *slot
> 
> The file names, function names, and driver names are still not very
> consistent, but I didn't do anything with them:
> 
>   pci-pwrctrl-pwrseq.c  pci_pwrctrl_pwrseq_probe()  "pci-pwrctrl-pwrseq"
>   pci-pwrctrl-tc9563.c  tc9563_pwrctrl_probe()      "pwrctrl-tc9563"
>   slot.c                pci_pwrctrl_slot_probe()    ""pci-pwrctrl-slot"
> 

Yeah, because all 3 were written by 3 different developers and Bartosz didn't
pay attention to the detail :) I can unify them in the upcoming patches. Thanks
for spotting the differences.

> > +++ b/drivers/pci/pwrctrl/slot.c
> > @@ -17,13 +17,38 @@ struct pci_pwrctrl_slot_data {
> >  	struct pci_pwrctrl ctx;
> >  	struct regulator_bulk_data *supplies;
> >  	int num_supplies;
> > +	struct clk *clk;
> >  };
> >  
> > -static void devm_pci_pwrctrl_slot_power_off(void *data)
> > +static int pci_pwrctrl_slot_power_on(struct pci_pwrctrl *ctx)
> >  {
> > -	struct pci_pwrctrl_slot_data *slot = data;
> > +	struct pci_pwrctrl_slot_data *slot = container_of(ctx, struct pci_pwrctrl_slot_data, ctx);
> > +	int ret;
> > +
> > +	ret = regulator_bulk_enable(slot->num_supplies, slot->supplies);
> > +	if (ret < 0) {
> > +		dev_err(slot->ctx.dev, "Failed to enable slot regulators\n");
> > +		return ret;
> > +	}
> > +
> > +	return clk_prepare_enable(slot->clk);
> 
> It would be nice if we could add a preparatory patch to factor out
> pci_pwrctrl_slot_power_on() before this one.  Then the slot.c patch
> would look more like the pwrseq and tc9563 ones.
> 

Agree, other two drivers atleast had a helper to do power on/off, so that made
them look nicer in diff.

- Mani

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

Re: [PATCH v4 0/8] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

[Sean Anderson]

On 1/14/26 03:48, Manivannan Sadhasivam wrote:
> On Tue, Jan 13, 2026 at 12:15:01PM -0500, Sean Anderson wrote:
>> On 1/5/26 08:55, Manivannan Sadhasivam via B4 Relay wrote:
>> > Hi,
>> 
>> I asked substantially similar questions on v2, but since I never got a
>> response I want to reiterate them on v4 to make sure they don't get
>> lost.
>> 
> 
> I did respond to your queries in v2, but lost your last reply in that thread:
> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2flore.kernel.org%2flinux%2dpci%2f8269249f%2d48a9%2d4136%2da326%2d23f5076be487%40linux.dev%2f&umid=db5ea813-d162-4dc2-9847-b6f01a3e22ce&rct=1768380513&auth=d807158c60b7d2502abde8a2fc01f40662980862-8453843623be88c725b7c9a8baf78220575003f2
> 
> Apologies!
> 
>> > 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.
>> 
>> OK, so to clarify the problem is an architecture like
>> 
>>     RP
>>     |-- Bridge 1 (automatic)
>>     |   |-- Device 1
>>     |   `-- Bridge 2 (needs pwrseq)
>>     |       `-- Device 2
>>     `-- Bridge 3 (automatic)
>>         `-- Device 3
>> 
> 
> This topology is not possible with PCIe. A single Root Port can only connect to
> a single bridge. But applies to PCI.

OK, well imagine it like

     RP
     `-- Host Bridge (automatic)
         |-- Bridge 1 (automatic)
         |   |-- Device 1
         |   `-- Bridge 2 (needs pwrseq)
         |       `-- Device 2
         `-- Bridge 3 (automatic)
             `-- Device 3

You raised the problem, so what I am asking is: is this such a
problematic topology? And if not, please describe one.

>> where Bridge 2 has a devicetree node with a pwrseq binding? So we do the
>> initial scan and allocate resources for bridge/devices 1 and 3 with the
>> resources for bridge 3 immediately above those for bridge 1. Then when
>> bridge 2 shows up we can't resize bridge 1's windows since bridge 3's
>> windows are in the way?
>> 
> 
> It is not a problem with resizing, it is the problem with how much you can
> resize. And also if that bridge 2 is a switch and if it exposes multiple
> downstream busses, then the upstream bridge 1 will run out of resources.

OK, but what I am saying is that I don't believe Bridge 2 can need
pwrseq if Bridge 1 doesn't. So I don't think the topology as-illustrated
can exist.

It's possible that there could be a problem with multiple levels of
bridges all needing pwrseq, but does such a system exist?

> If bridge 2 is a hotplug bridge, then no issues. But I was only referring to
> non-hotplug capable switches.
> 
>> But is it even valid to have a pwrseq node on bridge 2 without one on
>> bridge 1? If bridge 1 is automatically controlled, then I would expect
>> bridge 2 to be as well. E.g. I would expect bridge 2's reset sequence to
>> be controlled by the secondary bus reset bit in bridge 1's bridge
>> control register.
>> 
> 
> Technically it is possible for Bridge 2 to have a pwrctrl requirement. What is
> limiting from spec PoV?

If this is the case then we need to be able to handle the resource
constraint problem. But if it doesn't exist then there is no problem
with the existing architecture. Only this sort of design has resource
problems, while most designs like

     RP
     `-- Bridge 1 (pwrseq)
         |-- Bridge 2 (automatic)
         |   |-- Device 1
         |   |-- Device 2
         `-- Bridge 3 (automatic)
             `-- Device 3

have no resource problems even with the current subsystem.

>> And a very similar architecture like
>> 
>>     RP
>>     |-- Bridge 4 (pwrseq)
>>     |   |-- Device 4
>>     `-- Bridge 5 (automatic)
>>         `-- Device 5
>> 
>> has no problems since the resources for bridge 4 can be allocated above
>> those for bridge 5 whenever it shows up.
>> 
> 
> Again, if bridge 4 is not hotplug capable and if it is a switch, the problem is
> still applicable.

This doesn't apply even if bridge 4 is not hotplug capable. It will show
up after bridge 5 gets probed and just grab the next available
resources.

>> These problems seem very similar to what hotplug bridges have to handle
>> (except that we (usually) only need to do one hotplug per boot). So
>> maybe we should set is_hotplug_bridge on bridges with a pwrseq node.
>> That way they'll get resources distributed for when the downstream port
>> shows up. As an optimization, we could then release those resources once
>> the downstream port is scanned.
>> 
> 
> That would be incorrect. You cannot set 'is_hotplug_bridge' to 'true' for a
> non-hotplug capable bridge. You can call it as a hack, but there is no place
> for that in upstream.

Introduce a new boolean called 'is_pwrseq_bridge' and check for it when
allocating 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.
>> 
>> (just for the record)
>> 
>> I think the existing design is quite elegant, since the operations
>> associated with the bridge correspond directly to device lifecycle
>> operations. It also avoids problems related to the root port trying to
>> look up its own child (possibly missing a driver) during probe.
>> 
> 
> I agree with you that it is elegant and I even was very reluctant to move out of
> it [1]. But lately, I understood that we cannot scale the pwrctrl framework if we
> do not give flexibility to the controller drivers [2].
> 
> [1] https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2flore.kernel.org%2flinux%2dpci%2feix65qdwtk5ocd7lj6sw2lslidivauzyn6h5cc4mc2nnci52im%40qfmbmwy2zjbe%2f&umid=db5ea813-d162-4dc2-9847-b6f01a3e22ce&rct=1768380513&auth=d807158c60b7d2502abde8a2fc01f40662980862-377ad79c69a5ff9c69de76d9fcf5f030d066027a
> [2] https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2flore.kernel.org%2flinux%2dpci%2faG3IWdZIhnk01t2A%40google.com%2f&umid=db5ea813-d162-4dc2-9847-b6f01a3e22ce&rct=1768380513&auth=d807158c60b7d2502abde8a2fc01f40662980862-9a33d827cf703f2827fca86fd99acf563ca26bd9
> 
>> > This integration allows better coordination
>> > between controller drivers and the pwrctrl framework, enabling enhanced features
>> > such as D3Cold support.
>> 
>> 
>> I think this should be handled by the power sequencing driver,
>> especially as there are timing requirements for the other resources
>> referenced to PERST? If we are going to touch each driver, it would
>> be much better to consolidate things by removing the ad-hoc PERST
>> support.
>> 
>> Different drivers control PERST in various ways, but I think this can
>> be abstracted behind a GPIO controller (if necessary for e.g. MMIO-based
>> control). If there's no reset-gpios property in the pwrseq node then we
>> could automatically look up the GPIO on the root port.
>> 
> 
> Not at all. We cannot model PERST# as a GPIO in all the cases. Some drivers
> implement PERST# as a set of MMIO operations in the Root Complex MMIO space and
> that space belongs to the controller driver.

That's what I mean. Implement a GPIO driver with one GPIO and perform
the MMIO operations as requested.

Or we can invert things and add a reset op to pci_ops. If present then
call it, and if absent use the PERST GPIO on the bridge.

> FYI, I did try something similar before:
> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2flore.kernel.org%2flinux%2dpci%2f20250707%2dpci%2dpwrctrl%2dperst%2dv1%2d0%2dc3c7e513e312%40kernel.org%2f&umid=db5ea813-d162-4dc2-9847-b6f01a3e22ce&rct=1768380513&auth=d807158c60b7d2502abde8a2fc01f40662980862-e06652b06144d91b37cae1f9289747fe7cbe0762
>> > 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.
>> 
>> Can you elaborate on this? Previously you said
>> 
>> | Some platforms do LTSSM during phy_init(), so they will fail if the
>> | device is not powered ON at that time.
>> 
>> What do you mean by "do LTSSM during phy_init()"? Do you have a specific
>> driver in mind?
>> 
> 
> I believe the Mediatek PCIe controller driver used in Chromebooks exhibit this
> behavior. Chen talked about it in his LPC session:
> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2flpc.events%2fevent%2f19%2fcontributions%2f2023%2f&umid=db5ea813-d162-4dc2-9847-b6f01a3e22ce&rct=1768380513&auth=d807158c60b7d2502abde8a2fc01f40662980862-59ecd8a94baa970f1f962febb6fe20f15058ef42

I went through 

mediatek/phy-mtk-pcie.c
mediatek/phy-mtk-tphy.c
mediatek/phy-mtk-xsphy.c
ralink/phy-mt7621-pci.c

and didn't see anything where they wait for the link to come up or check
the link state and fail.

The mtk PCIe drivers may check for this, but I'm saying that we
*shouldn't* do that in probe.

>> I would expect that the LTSSM would remain in the Detect state until the
>> pwrseq driver is being probed.
>> 
> 
> True, but if the API (phy_init()) expects the LTSSM to move to L0, then it will
> fail, right? It might be what's happening with above mentioned platform.

How can the API expect this?

I'm not saying that such a situation cannot exist, but I don't think
it's a common case.

>> > 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. I
>> > plan to replace probe deferral with blocking wait in the coming days.
>> 
>> You can only do a blocking wait after deferring at least once, since the
>> root port may be probed synchronously during boot. I really think this
>> is rather messy and something we should avoid architecturally while we
>> have the chance.
>> 
> 
> By blocking wait I meant that the controller probe itself will do a blocking
> wait until the pwrctrl drivers gets bound. Since this happens way before the PCI
> bus scan, there won't be any Root Port probed synchronously.

You can't do that because the pwrctrl driver may *never* be loaded. And
this may deadlock the boot sequence because the initial probe is
performed synchronously from the initcall. i.e.

do_initcalls
  my_driver_init
    driver_register
      bus_add_driver
        driver_attach
          driver_probe_device

If the PCI controller is probed before the device that has the module
you will deadlock! So you can only sleep indefinitely if you are being
probed asynchronously.

-----

Maybe the best way to address this is to add assert_reset/link_up/
link_down callbacks to pci_ops. Then pwrctrl_slot probe could look like

    bridge = to_pci_host_bridge(dev->parent);
    of_regulator_bulk_get_all();
    regulator_bulk_enable();
    devm_clk_get_optional_enabled();
    devm_gpiod_get_optional(/* "reset" */);
    if (bridge && bridge->ops->assert_reset)
        ret = bridge->ops->assert_reset(bridge, slot)
    else
        ret = assert_reset_gpio(slot);

    if (ret != ALREADY_ASSERTED)
	    fdelay(100000);

    /* Deassert PERST and bring the link up */
    if (bridge && bridge->ops->link_up)
        bridge->ops->link_up(bridge, slot);
    else
        slot_deassert_reset(slot);

    devm_add_action_or_reset(link_down);
    pci_pwrctrl_init();
    devm_pci_pwrctrl_device_set_ready();

--Sean

Re: [PATCH v4 0/8] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

[Manivannan Sadhasivam]

On Thu, Jan 15, 2026 at 02:26:32PM -0500, Sean Anderson wrote:

[...]

> >> OK, so to clarify the problem is an architecture like
> >> 
> >>     RP
> >>     |-- Bridge 1 (automatic)
> >>     |   |-- Device 1
> >>     |   `-- Bridge 2 (needs pwrseq)
> >>     |       `-- Device 2
> >>     `-- Bridge 3 (automatic)
> >>         `-- Device 3
> >> 
> > 
> > This topology is not possible with PCIe. A single Root Port can only connect to
> > a single bridge. But applies to PCI.
> 
> OK, well imagine it like
> 
>      RP
>      `-- Host Bridge (automatic)
>          |-- Bridge 1 (automatic)
>          |   |-- Device 1
>          |   `-- Bridge 2 (needs pwrseq)
>          |       `-- Device 2
>          `-- Bridge 3 (automatic)
>              `-- Device 3
> 
> You raised the problem, so what I am asking is: is this such a
> problematic topology? And if not, please describe one.
> 

Again, this topology is also incorrect, but my point is that in whatever
topology, if you have a PCIe switch and that requires a pwrctrl driver to power
it on, then there will be a resource allocation problem.

> >> where Bridge 2 has a devicetree node with a pwrseq binding? So we do the
> >> initial scan and allocate resources for bridge/devices 1 and 3 with the
> >> resources for bridge 3 immediately above those for bridge 1. Then when
> >> bridge 2 shows up we can't resize bridge 1's windows since bridge 3's
> >> windows are in the way?
> >> 
> > 
> > It is not a problem with resizing, it is the problem with how much you can
> > resize. And also if that bridge 2 is a switch and if it exposes multiple
> > downstream busses, then the upstream bridge 1 will run out of resources.
> 
> OK, but what I am saying is that I don't believe Bridge 2 can need
> pwrseq if Bridge 1 doesn't. So I don't think the topology as-illustrated
> can exist.
> 
> It's possible that there could be a problem with multiple levels of
> bridges all needing pwrseq, but does such a system exist?
> 

Yes, it does exists atm. Below is the TC9563 PCIe switch topology in Qcom
RB3Gen2 board:

	Host bridge
	`--> Root Port (auto)
	     `--> TC9563 (pwrctrl)

https://lore.kernel.org/linux-arm-msm/20260105-tc9563-v1-1-642fd1fe7893@oss.qualcomm.com/

And then there is also a design which is underway that connects one more TC9653
to the downstream of existing one for peripheral expansion. So the topology will
become:

	Host bridge
	`--> Root Port (auto)
	     `--> TC9563 (pwrctrl)
		  `--> TC9563 (pwrctrl)

This is just one example and the OEMs may come up with many such designs and we
cannot deny them.

> > If bridge 2 is a hotplug bridge, then no issues. But I was only referring to
> > non-hotplug capable switches.
> > 
> >> But is it even valid to have a pwrseq node on bridge 2 without one on
> >> bridge 1? If bridge 1 is automatically controlled, then I would expect
> >> bridge 2 to be as well. E.g. I would expect bridge 2's reset sequence to
> >> be controlled by the secondary bus reset bit in bridge 1's bridge
> >> control register.
> >> 
> > 
> > Technically it is possible for Bridge 2 to have a pwrctrl requirement. What is
> > limiting from spec PoV?
> 
> If this is the case then we need to be able to handle the resource
> constraint problem. But if it doesn't exist then there is no problem
> with the existing architecture. Only this sort of design has resource
> problems, while most designs like
> 
>      RP
>      `-- Bridge 1 (pwrseq)
>          |-- Bridge 2 (automatic)
>          |   |-- Device 1
>          |   |-- Device 2
>          `-- Bridge 3 (automatic)
>              `-- Device 3
> 
> have no resource problems even with the current subsystem.
> 

Not at all. I think you don't get the issue. The Root Port is just a PCI bridge.
If the downstream device is not found during the initial scan, and if RP is a
non-hotplug capable device, then the PCI core will allocate resources for only
one downstream bus. And if the PCIe switch shows up on the downstream bus later,
then it will fail to enumerate due to resource constraint for the switch's
downstream busses.

This is pretty much what happens with the single switch TC9563 design in RB3Gen2
that I referenced above.

> >> And a very similar architecture like
> >> 
> >>     RP
> >>     |-- Bridge 4 (pwrseq)
> >>     |   |-- Device 4
> >>     `-- Bridge 5 (automatic)
> >>         `-- Device 5
> >> 
> >> has no problems since the resources for bridge 4 can be allocated above
> >> those for bridge 5 whenever it shows up.
> >> 
> > 
> > Again, if bridge 4 is not hotplug capable and if it is a switch, the problem is
> > still applicable.
> 
> This doesn't apply even if bridge 4 is not hotplug capable. It will show
> up after bridge 5 gets probed and just grab the next available
> resources.
> 

See above. The next available resources are very limited if the upstream bridge
is not hotplug capable. And we can't blame PCI core for this because, we are
pretty much emulating hotplug on a non-hotplug capable bridge, which is not
ideal.

> >> These problems seem very similar to what hotplug bridges have to handle
> >> (except that we (usually) only need to do one hotplug per boot). So
> >> maybe we should set is_hotplug_bridge on bridges with a pwrseq node.
> >> That way they'll get resources distributed for when the downstream port
> >> shows up. As an optimization, we could then release those resources once
> >> the downstream port is scanned.
> >> 
> > 
> > That would be incorrect. You cannot set 'is_hotplug_bridge' to 'true' for a
> > non-hotplug capable bridge. You can call it as a hack, but there is no place
> > for that in upstream.
> 
> Introduce a new boolean called 'is_pwrseq_bridge' and check for it when
> allocating resources.
> 

Sorry, I'm not upto introducing such hacks.

> >> > 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.
> >> 
> >> (just for the record)
> >> 
> >> I think the existing design is quite elegant, since the operations
> >> associated with the bridge correspond directly to device lifecycle
> >> operations. It also avoids problems related to the root port trying to
> >> look up its own child (possibly missing a driver) during probe.
> >> 
> > 
> > I agree with you that it is elegant and I even was very reluctant to move out of
> > it [1]. But lately, I understood that we cannot scale the pwrctrl framework if we
> > do not give flexibility to the controller drivers [2].
> > 
> > [1] https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2flore.kernel.org%2flinux%2dpci%2feix65qdwtk5ocd7lj6sw2lslidivauzyn6h5cc4mc2nnci52im%40qfmbmwy2zjbe%2f&umid=db5ea813-d162-4dc2-9847-b6f01a3e22ce&rct=1768380513&auth=d807158c60b7d2502abde8a2fc01f40662980862-377ad79c69a5ff9c69de76d9fcf5f030d066027a
> > [2] https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2flore.kernel.org%2flinux%2dpci%2faG3IWdZIhnk01t2A%40google.com%2f&umid=db5ea813-d162-4dc2-9847-b6f01a3e22ce&rct=1768380513&auth=d807158c60b7d2502abde8a2fc01f40662980862-9a33d827cf703f2827fca86fd99acf563ca26bd9
> > 
> >> > This integration allows better coordination
> >> > between controller drivers and the pwrctrl framework, enabling enhanced features
> >> > such as D3Cold support.
> >> 
> >> 
> >> I think this should be handled by the power sequencing driver,
> >> especially as there are timing requirements for the other resources
> >> referenced to PERST? If we are going to touch each driver, it would
> >> be much better to consolidate things by removing the ad-hoc PERST
> >> support.
> >> 
> >> Different drivers control PERST in various ways, but I think this can
> >> be abstracted behind a GPIO controller (if necessary for e.g. MMIO-based
> >> control). If there's no reset-gpios property in the pwrseq node then we
> >> could automatically look up the GPIO on the root port.
> >> 
> > 
> > Not at all. We cannot model PERST# as a GPIO in all the cases. Some drivers
> > implement PERST# as a set of MMIO operations in the Root Complex MMIO space and
> > that space belongs to the controller driver.
> 
> That's what I mean. Implement a GPIO driver with one GPIO and perform
> the MMIO operations as requested.
> 
> Or we can invert things and add a reset op to pci_ops. If present then
> call it, and if absent use the PERST GPIO on the bridge.
> 

Having a callback for controlling the PERST# will work for the addressing the
PERST# issue, but it won't solve the PCIe switch issue we were talking above.
And this API design will fix both the problems.

But even in this callback design, you need to have modifications in the existing
controller drivers to integrate pwrctrl. So how that is different from calling
just two (or one unified API for create/power_on)?

> > FYI, I did try something similar before:
> > https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2flore.kernel.org%2flinux%2dpci%2f20250707%2dpci%2dpwrctrl%2dperst%2dv1%2d0%2dc3c7e513e312%40kernel.org%2f&umid=db5ea813-d162-4dc2-9847-b6f01a3e22ce&rct=1768380513&auth=d807158c60b7d2502abde8a2fc01f40662980862-e06652b06144d91b37cae1f9289747fe7cbe0762
> >> > 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.
> >> 
> >> Can you elaborate on this? Previously you said
> >> 
> >> | Some platforms do LTSSM during phy_init(), so they will fail if the
> >> | device is not powered ON at that time.
> >> 
> >> What do you mean by "do LTSSM during phy_init()"? Do you have a specific
> >> driver in mind?
> >> 
> > 
> > I believe the Mediatek PCIe controller driver used in Chromebooks exhibit this
> > behavior. Chen talked about it in his LPC session:
> > https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2flpc.events%2fevent%2f19%2fcontributions%2f2023%2f&umid=db5ea813-d162-4dc2-9847-b6f01a3e22ce&rct=1768380513&auth=d807158c60b7d2502abde8a2fc01f40662980862-59ecd8a94baa970f1f962febb6fe20f15058ef42
> 
> I went through 
> 
> mediatek/phy-mtk-pcie.c
> mediatek/phy-mtk-tphy.c
> mediatek/phy-mtk-xsphy.c
> ralink/phy-mt7621-pci.c
> 
> and didn't see anything where they wait for the link to come up or check
> the link state and fail.
> 

See tegra_pcie_config_rp().

> The mtk PCIe drivers may check for this, but I'm saying that we
> *shouldn't* do that in probe.
> 

Such drivers already exist. Sure they can just leave the LTSSM in detect state
instead of failing probe. But if their Root Port is not hotplug capable, why
should they expect a device to get attached to the bus after probe?

That being said, we do have DWC drivers ignoring the link up failure expecting
the link to come up later. But I may just fix that in the coming days once the
pwrctrl APIs are added. There is no reason to wait for hotplug if the RP is not
hotplug capable. It is just asking for troubles.

> >> I would expect that the LTSSM would remain in the Detect state until the
> >> pwrseq driver is being probed.
> >> 
> > 
> > True, but if the API (phy_init()) expects the LTSSM to move to L0, then it will
> > fail, right? It might be what's happening with above mentioned platform.
> 
> How can the API expect this?
> 
> I'm not saying that such a situation cannot exist, but I don't think
> it's a common case.
>

Starting LTSSM in phy_init() is weird I agree. I for sure know that someone
raised this up earlier, but don't exactly remember which driver is doing it.
 
> >> > 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. I
> >> > plan to replace probe deferral with blocking wait in the coming days.
> >> 
> >> You can only do a blocking wait after deferring at least once, since the
> >> root port may be probed synchronously during boot. I really think this
> >> is rather messy and something we should avoid architecturally while we
> >> have the chance.
> >> 
> > 
> > By blocking wait I meant that the controller probe itself will do a blocking
> > wait until the pwrctrl drivers gets bound. Since this happens way before the PCI
> > bus scan, there won't be any Root Port probed synchronously.
> 
> You can't do that because the pwrctrl driver may *never* be loaded. And
> this may deadlock the boot sequence because the initial probe is
> performed synchronously from the initcall. i.e.
> 
> do_initcalls
>   my_driver_init
>     driver_register
>       bus_add_driver
>         driver_attach
>           driver_probe_device
> 
> If the PCI controller is probed before the device that has the module
> you will deadlock! So you can only sleep indefinitely if you are being
> probed asynchronously.
> 

Yes, I was thinking about controller drivers setting PROBE_PREFER_ASYNCHRONOUS
flag. We can restrict the blocking wait to such drivers.

> -----
> 
> Maybe the best way to address this is to add assert_reset/link_up/
> link_down callbacks to pci_ops. Then pwrctrl_slot probe could look like
> 
>     bridge = to_pci_host_bridge(dev->parent);
>     of_regulator_bulk_get_all();
>     regulator_bulk_enable();
>     devm_clk_get_optional_enabled();
>     devm_gpiod_get_optional(/* "reset" */);
>     if (bridge && bridge->ops->assert_reset)
>         ret = bridge->ops->assert_reset(bridge, slot)
>     else
>         ret = assert_reset_gpio(slot);
> 
>     if (ret != ALREADY_ASSERTED)
> 	    fdelay(100000);
> 
>     /* Deassert PERST and bring the link up */
>     if (bridge && bridge->ops->link_up)
>         bridge->ops->link_up(bridge, slot);
>     else
>         slot_deassert_reset(slot);
> 
>     devm_add_action_or_reset(link_down);
>     pci_pwrctrl_init();
>     devm_pci_pwrctrl_device_set_ready();
> 

Sorry, I'm not inclined to take this route for the reasons mentioned above.

- Mani

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

Re: [PATCH v4 0/8] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

[Shawn Lin]

在 2026/01/05 星期一 21:55, Manivannan Sadhasivam via B4 Relay 写道:
> 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.
> 

This series looks great.

In practice, some PCIe devices may need to be powered down dynamically 
at runtime. For example, users might want to disable a PCIe Wi-Fi module 
when there's no internet usage — typically, commands like ifconfig wlan0 
downonly bring the interface down but leave the Wi-Fi hardware powered. 
Is there a mechanism that would allow the Endpoint driver to leverage 
pwrctrl dynamically to support such power management scenarios?


> 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. I
> plan to replace probe deferral with blocking wait in the coming days.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
> Changes in v4:
> - Used platform_device_put()
> - Changed the return value of power_off() callback to 'int'
> - Splitted patch 6 into two and reworded the commit message
> - Collected tags
> - Link to v3: https://lore.kernel.org/r/20251229-pci-pwrctrl-rework-v3-0-c7d5918cd0db@oss.qualcomm.com
> 
> Changes in v3:
> - Integrated TC9563 change
> - Reworked the power_on API to properly power off the devices in error path
> - Fixed the error path in pcie-qcom.c to not destroy pwrctrl devices during
>    probe deferral
> - Rebased on top of pci/controller/dwc-qcom branch and dropped the PERST# patch
> - Added a patch for TC9563 to fix the refcount dropping for i2c adapter
> - Added patches to drop the assert_perst callback and rename the PERST# helpers in
>    pcie-qcom.c
> - Link to v2: https://lore.kernel.org/r/20251216-pci-pwrctrl-rework-v2-0-745a563b9be6@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 (7):
>        PCI/pwrctrl: tc9563: Use put_device() instead of i2c_put_adapter()
>        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
>        PCI: qcom: Drop the assert_perst() callbacks
>        PCI: Drop the assert_perst() callback
>        PCI: qcom: Rename PERST# assert/deassert helpers for uniformity
> 
>   drivers/pci/bus.c                                 |  19 --
>   drivers/pci/controller/dwc/pcie-designware-host.c |   9 -
>   drivers/pci/controller/dwc/pcie-designware.h      |   9 -
>   drivers/pci/controller/dwc/pcie-qcom.c            |  54 +++--
>   drivers/pci/of.c                                  |   1 +
>   drivers/pci/probe.c                               |  59 -----
>   drivers/pci/pwrctrl/core.c                        | 260 ++++++++++++++++++++--
>   drivers/pci/pwrctrl/pci-pwrctrl-pwrseq.c          |  30 ++-
>   drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c          |  48 ++--
>   drivers/pci/pwrctrl/slot.c                        |  48 ++--
>   drivers/pci/remove.c                              |  20 --
>   include/linux/pci-pwrctrl.h                       |  16 +-
>   include/linux/pci.h                               |   1 -
>   13 files changed, 367 insertions(+), 207 deletions(-)
> ---
> base-commit: 3e7f562e20ee87a25e104ef4fce557d39d62fa85
> change-id: 20251124-pci-pwrctrl-rework-c91a6e16c2f6
> prerequisite-message-id: 20251126081718.8239-1-mani@kernel.org
> prerequisite-patch-id: db9ff6c713e2303c397e645935280fd0d277793a
> prerequisite-patch-id: b5351b0a41f618435f973ea2c3275e51d46f01c5
> 
> Best regards,

Re: [PATCH v4 0/8] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

[Manivannan Sadhasivam]

On Fri, Jan 16, 2026 at 04:02:38PM +0800, Shawn Lin wrote:
> 
> 在 2026/01/05 星期一 21:55, Manivannan Sadhasivam via B4 Relay 写道:
> > 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.
> > 
> 
> This series looks great.
> 
> In practice, some PCIe devices may need to be powered down dynamically at
> runtime. For example, users might want to disable a PCIe Wi-Fi module when
> there's no internet usage — typically, commands like ifconfig wlan0 downonly
> bring the interface down but leave the Wi-Fi hardware powered. Is there a
> mechanism that would allow the Endpoint driver to leverage pwrctrl
> dynamically to support such power management scenarios?
> 

Glad that you've brought it up. You are talking about the usecase similar to
Airplane mode in mobiles, and we at Qcom are looking into this usecase in
upstream.

They way to handle this would be by using runtime PM ops. Once your WiFi or a
NIC driver runtime suspends, it will trigger the controller driver runtime
suspend callback. By that time, the controller driver can see if the device is
active or not (checking D states), whether wakeup is requested or not and then
initiate the D3Cold sequence using the APIs introduced in this series.

But that comes with a cost though, which is resume latency. It is generally not
advised to do D3Cold during runtime PM due to the latency and also device
lifetime issues (wearout etc...). So technically it is possible, but there are
challenges.

Krishna is going to post a series that allows the pcie-qcom driver to do D3Cold
during system suspend with these APIs. And we do have plans to extend it to
Airplane mode and similar usecases in the future.

- Mani

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

Re: [PATCH v4 0/8] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

[Shawn Lin]

在 2026/01/16 星期五 16:30, Manivannan Sadhasivam 写道:
> On Fri, Jan 16, 2026 at 04:02:38PM +0800, Shawn Lin wrote:
>>
>> 在 2026/01/05 星期一 21:55, Manivannan Sadhasivam via B4 Relay 写道:
>>> 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.
>>>
>>
>> This series looks great.
>>
>> In practice, some PCIe devices may need to be powered down dynamically at
>> runtime. For example, users might want to disable a PCIe Wi-Fi module when
>> there's no internet usage — typically, commands like ifconfig wlan0 downonly
>> bring the interface down but leave the Wi-Fi hardware powered. Is there a
>> mechanism that would allow the Endpoint driver to leverage pwrctrl
>> dynamically to support such power management scenarios?
>>
> 
> Glad that you've brought it up. You are talking about the usecase similar to
> Airplane mode in mobiles, and we at Qcom are looking into this usecase in
> upstream.
> 
> They way to handle this would be by using runtime PM ops. Once your WiFi or a
> NIC driver runtime suspends, it will trigger the controller driver runtime
> suspend callback. By that time, the controller driver can see if the device is
> active or not (checking D states), whether wakeup is requested or not and then
> initiate the D3Cold sequence using the APIs introduced in this series.
> 
> But that comes with a cost though, which is resume latency. It is generally not
> advised to do D3Cold during runtime PM due to the latency and also device
> lifetime issues (wearout etc...). So technically it is possible, but there are
> challenges.
> 

Indeed, that's a fundamental power-performance trade-off for 
battery-powered devices.

> Krishna is going to post a series that allows the pcie-qcom driver to do D3Cold
> during system suspend with these APIs. And we do have plans to extend it to
> Airplane mode and similar usecases in the future.
> 

Thanks for sharing this details.

> - Mani
> 

Re: [PATCH v4 0/8] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

[Niklas Cassel]

On Fri, Jan 16, 2026 at 11:54:26AM +0530, Manivannan Sadhasivam wrote:
> On Thu, Jan 15, 2026 at 02:26:32PM -0500, Sean Anderson wrote:
> > > 
> > > Not at all. We cannot model PERST# as a GPIO in all the cases. Some drivers
> > > implement PERST# as a set of MMIO operations in the Root Complex MMIO space and
> > > that space belongs to the controller driver.
> > 
> > That's what I mean. Implement a GPIO driver with one GPIO and perform
> > the MMIO operations as requested.
> > 
> > Or we can invert things and add a reset op to pci_ops. If present then
> > call it, and if absent use the PERST GPIO on the bridge.
> > 
> 
> Having a callback for controlling the PERST# will work for the addressing the
> PERST# issue, but it won't solve the PCIe switch issue we were talking above.
> And this API design will fix both the problems.
> 
> But even in this callback design, you need to have modifications in the existing
> controller drivers to integrate pwrctrl. So how that is different from calling
> just two (or one unified API for create/power_on)?

FWIW, I do think that it is a good idea to create a reset op to pci_ops
that will implement PERST# assertion/deassertion.

Right now, it is a mess, with various drivers doing this at various
different places.

Having a specific callback, the driver implement it however they want
GPIO, MMIO, whatever, and it could even be called by (in case of DWC,
the host_init, by pwrctrl, or potentially by the PCI core itself before
enumerating the bus.

If we don't do something about it now, the problem will just get worse
with time. Yes, it will take time before all drivers have migrated and
been tested to have a dedicated PERST# reset op, but for the long term
maintainability, I think it is something that we should do.

I also know that some drivers have some loops with retry logic, where
they might go down in link speed, but right now I don't see why those
drivers shouldn't be able to keep that retry logic just because we
add a dedicated PERST# callback.


All that said, that would be a separate endeavor and can be implemented
later.


Kind regards,
Niklas

Re: [PATCH v4 0/8] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

[Manivannan Sadhasivam]

On Fri, Jan 16, 2026 at 02:40:36PM +0100, Niklas Cassel wrote:
> On Fri, Jan 16, 2026 at 11:54:26AM +0530, Manivannan Sadhasivam wrote:
> > On Thu, Jan 15, 2026 at 02:26:32PM -0500, Sean Anderson wrote:
> > > > 
> > > > Not at all. We cannot model PERST# as a GPIO in all the cases. Some drivers
> > > > implement PERST# as a set of MMIO operations in the Root Complex MMIO space and
> > > > that space belongs to the controller driver.
> > > 
> > > That's what I mean. Implement a GPIO driver with one GPIO and perform
> > > the MMIO operations as requested.
> > > 
> > > Or we can invert things and add a reset op to pci_ops. If present then
> > > call it, and if absent use the PERST GPIO on the bridge.
> > > 
> > 
> > Having a callback for controlling the PERST# will work for the addressing the
> > PERST# issue, but it won't solve the PCIe switch issue we were talking above.
> > And this API design will fix both the problems.
> > 
> > But even in this callback design, you need to have modifications in the existing
> > controller drivers to integrate pwrctrl. So how that is different from calling
> > just two (or one unified API for create/power_on)?
> 
> FWIW, I do think that it is a good idea to create a reset op to pci_ops
> that will implement PERST# assertion/deassertion.
> 
> Right now, it is a mess, with various drivers doing this at various
> different places.
> 
> Having a specific callback, the driver implement it however they want
> GPIO, MMIO, whatever, and it could even be called by (in case of DWC,
> the host_init, by pwrctrl, or potentially by the PCI core itself before
> enumerating the bus.
> 
> If we don't do something about it now, the problem will just get worse
> with time. Yes, it will take time before all drivers have migrated and
> been tested to have a dedicated PERST# reset op, but for the long term
> maintainability, I think it is something that we should do.
> 
> I also know that some drivers have some loops with retry logic, where
> they might go down in link speed, but right now I don't see why those
> drivers shouldn't be able to keep that retry logic just because we
> add a dedicated PERST# callback.
> 
> 
> All that said, that would be a separate endeavor and can be implemented
> later.
> 

Agree. Having unification always helps!

- Mani

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

Re: [PATCH v4 0/8] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

[Shawn Lin]

在 2026/01/16 星期五 21:40, Niklas Cassel 写道:
> On Fri, Jan 16, 2026 at 11:54:26AM +0530, Manivannan Sadhasivam wrote:
>> On Thu, Jan 15, 2026 at 02:26:32PM -0500, Sean Anderson wrote:
>>>>
>>>> Not at all. We cannot model PERST# as a GPIO in all the cases. Some drivers
>>>> implement PERST# as a set of MMIO operations in the Root Complex MMIO space and
>>>> that space belongs to the controller driver.
>>>
>>> That's what I mean. Implement a GPIO driver with one GPIO and perform
>>> the MMIO operations as requested.
>>>
>>> Or we can invert things and add a reset op to pci_ops. If present then
>>> call it, and if absent use the PERST GPIO on the bridge.
>>>
>>
>> Having a callback for controlling the PERST# will work for the addressing the
>> PERST# issue, but it won't solve the PCIe switch issue we were talking above.
>> And this API design will fix both the problems.
>>
>> But even in this callback design, you need to have modifications in the existing
>> controller drivers to integrate pwrctrl. So how that is different from calling
>> just two (or one unified API for create/power_on)?
> 
> FWIW, I do think that it is a good idea to create a reset op to pci_ops
> that will implement PERST# assertion/deassertion.
> 

That's exactly what I had in mind when looking at different PCIe host
drivers that why individual drivers should implement their own powering
up sequence, regarding to the face that bringing devices up should
always follow the timing defined PCI Express Card Electromechanical
Specification R6.0.1, section 2.2.1 "Initial Power Up(G3 to S0)"


> Right now, it is a mess, with various drivers doing this at various
> different places.
> 
> Having a specific callback, the driver implement it however they want
> GPIO, MMIO, whatever, and it could even be called by (in case of DWC,
> the host_init, by pwrctrl, or potentially by the PCI core itself before
> enumerating the bus.
> 
> If we don't do something about it now, the problem will just get worse
> with time. Yes, it will take time before all drivers have migrated and
> been tested to have a dedicated PERST# reset op, but for the long term
> maintainability, I think it is something that we should do.

Ack. That will also consolidate more timing relate improvements
together. For instance, almost all drivers holds PERST# for 100ms
Tpvperl before release it, but 100ms is the minimal value defined by
spec. I have to say it's broken for several EP cards I have debugged in 
practice these years. With holding all powering up timing together into
pwrctrl design could really helpful in the future.


> 
> I also know that some drivers have some loops with retry logic, where
> they might go down in link speed, but right now I don't see why those
> drivers shouldn't be able to keep that retry logic just because we
> add a dedicated PERST# callback.
> 
> 
> All that said, that would be a separate endeavor and can be implemented
> later.
> 
> 
> Kind regards,
> Niklas
> 
> 

Re: [PATCH v4 0/8] PCI/pwrctrl: Major rework to integrate pwrctrl devices with controller drivers

[Manivannan Sadhasivam]

On Fri, Jan 16, 2026 at 02:40:36PM +0100, Niklas Cassel wrote:
> On Fri, Jan 16, 2026 at 11:54:26AM +0530, Manivannan Sadhasivam wrote:
> > On Thu, Jan 15, 2026 at 02:26:32PM -0500, Sean Anderson wrote:
> > > > 
> > > > Not at all. We cannot model PERST# as a GPIO in all the cases. Some drivers
> > > > implement PERST# as a set of MMIO operations in the Root Complex MMIO space and
> > > > that space belongs to the controller driver.
> > > 
> > > That's what I mean. Implement a GPIO driver with one GPIO and perform
> > > the MMIO operations as requested.
> > > 
> > > Or we can invert things and add a reset op to pci_ops. If present then
> > > call it, and if absent use the PERST GPIO on the bridge.
> > > 
> > 
> > Having a callback for controlling the PERST# will work for the addressing the
> > PERST# issue, but it won't solve the PCIe switch issue we were talking above.
> > And this API design will fix both the problems.
> > 
> > But even in this callback design, you need to have modifications in the existing
> > controller drivers to integrate pwrctrl. So how that is different from calling
> > just two (or one unified API for create/power_on)?
> 
> FWIW, I do think that it is a good idea to create a reset op to pci_ops
> that will implement PERST# assertion/deassertion.
> 
> Right now, it is a mess, with various drivers doing this at various
> different places.
> 
> Having a specific callback, the driver implement it however they want
> GPIO, MMIO, whatever, and it could even be called by (in case of DWC,
> the host_init, by pwrctrl, or potentially by the PCI core itself before
> enumerating the bus.
> 
> If we don't do something about it now, the problem will just get worse
> with time. Yes, it will take time before all drivers have migrated and
> been tested to have a dedicated PERST# reset op, but for the long term
> maintainability, I think it is something that we should do.
> 
> I also know that some drivers have some loops with retry logic, where
> they might go down in link speed, but right now I don't see why those
> drivers shouldn't be able to keep that retry logic just because we
> add a dedicated PERST# callback.
> 
> 
> All that said, that would be a separate endeavor and can be implemented
> later.
> 

[looks like my previous reply was lost, so resending it]

Agree. Having unification always helps!

- Mani

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