public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v2 0/5] PCI: qcom: Add D3cold support
@ 2026-02-17 11:19 Krishna Chaitanya Chundru
  2026-02-17 11:19 ` [PATCH v2 1/5] PCI: host-common: Add helper to determine host bridge D3cold eligibility Krishna Chaitanya Chundru
                   ` (5 more replies)
  0 siblings, 6 replies; 22+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-02-17 11:19 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Will Deacon
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-arm-kernel,
	jonathanh, bjorn.andersson, Krishna Chaitanya Chundru

This series adds support for putting Qualcomm PCIe host bridges into D3cold
when downstream conditions allow it, and introduces a small common helper
to determine D3cold eligibility based on endpoint state.

On Qualcomm platforms, PCIe host controllers are currently kept powered
even when there are no active endpoints (i.e. all endpoints are already in
PCI_D3hot). This prevents the SoC from entering deeper low‑power states
such as CXPC.

While PCIe D3cold support exists in the PCI core, host controller drivers
lack a common mechanism to determine whether it is safe to power off the
host bridge without breaking active devices or wakeup functionality.
As a result, controllers either avoid entering D3cold or depend on rough,
driver‑specific workarounds.

This series addresses that gap.

1. Introduces pci_host_common_can_enter_d3cold(), a helper that determines
   whether a host bridge may enter D3cold based on downstream PCIe endpoint
   state. The helper permits D3cold only when all *active* endpoints are
   already in PCI_D3hot, and any wakeup‑enabled endpoint supports PME
   from D3cold.

2. Updates the Designware PCIe host driver to use this helper in the
   suspend_noirq() path, replacing the existing heuristic that blocked
   D3cold whenever L1 ASPM was enabled.

3. Enables D3cold support for Qualcomm PCIe controllers by wiring them into
   the DesignWare common suspend/resume flow and explicitly powering down
   controller resources when all endpoints are in D3hot.

The immediate outcome of this series is that Qualcomm PCIe host bridges can
enter D3cold when all endpoints are in D3hot.

This is a necessary but not sufficient step toward unblocking CXPC. With
this series applied, CXPC can be achieved on systems with no attached NVMe
devices. Support for NVMe‑attached systems requires additional changes
in NVMe driver, which are being worked on separately.

Tested on:
  - Qualcomm Lemans EVK, Monaco & sc7280 platforms.

Validation steps:
  - Boot without NVMe attach:
      * PCIe host enters D3cold during suspend
      * SoC is able to reach CXPC provided other drivers also remove
	their votes as part of suspend.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Changes in v2:
- Updated the cover letter (Bjorn Andersson)
- Add get_ltssm helper function to read LTSSM state from parf.
- Allow D3cold if there is no driver enabled for a endpoint.
- Added a seperate patch to make phy down in deinit part to avoid power
  leakage.
- Revert icc bw voting if resume fails(Bjorn Andersson).
- Link to v1: https://lore.kernel.org/r/20260128-d3cold-v1-0-dd8f3f0ce824@oss.qualcomm.com

---
Krishna Chaitanya Chundru (5):
      PCI: host-common: Add helper to determine host bridge D3cold eligibility
      PCI: dwc: Use common D3cold eligibility helper in suspend path
      PCI: qcom: Add .get_ltssm() helper
      PCI: qcom: Power down PHY via PARF_PHY_CTRL before disabling rails/clocks
      PCI: qcom: Add D3cold support

 drivers/pci/controller/dwc/pcie-designware-host.c |   9 +-
 drivers/pci/controller/dwc/pcie-qcom.c            | 162 +++++++++++++++-------
 drivers/pci/controller/pci-host-common.c          |  45 ++++++
 drivers/pci/controller/pci-host-common.h          |   2 +
 4 files changed, 162 insertions(+), 56 deletions(-)
---
base-commit: 9702969978695d9a699a1f34771580cdbb153b33
change-id: 20251229-d3cold-bf99921960bb

Best regards,
-- 
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 1/5] PCI: host-common: Add helper to determine host bridge D3cold eligibility
  2026-02-17 11:19 [PATCH v2 0/5] PCI: qcom: Add D3cold support Krishna Chaitanya Chundru
@ 2026-02-17 11:19 ` Krishna Chaitanya Chundru
  2026-02-23 18:10   ` Bjorn Helgaas
  2026-03-05  7:42   ` Manivannan Sadhasivam
  2026-02-17 11:19 ` [PATCH v2 2/5] PCI: dwc: Use common D3cold eligibility helper in suspend path Krishna Chaitanya Chundru
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 22+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-02-17 11:19 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Will Deacon
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-arm-kernel,
	jonathanh, bjorn.andersson, Krishna Chaitanya Chundru

Add a common helper, pci_host_common_can_enter_d3cold(), to determine
whether a PCI host bridge can safely transition to D3cold.

This helper is intended to be used by PCI host controller drivers to
decide whether they may safely put the host bridge into D3cold based on
the power state and wakeup capabilities of downstream endpoints.

The helper walks all devices on the bridge's primary bus and only allows
the host bridge to enter D3cold if all PCIe endpoints are already in
PCI_D3hot. This ensures that we do not power off the host bridge while
any active endpoint still requires the link to remain powered.

For devices that may wake the system, the helper additionally requires
that the device supports PME wake from D3cold (via WAKE#). Devices that
do not have wakeup enabled are not restricted by this check and do not
block the host bridge from entering D3cold.

Devices without a bound driver and with PCI not enabled via sysfs are
treated as inactive and therefore do not prevent the host bridge from
entering D3cold. This allows controllers to power down more aggressively
when there are no actively managed endpoints.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/pci-host-common.c | 45 ++++++++++++++++++++++++++++++++
 drivers/pci/controller/pci-host-common.h |  2 ++
 2 files changed, 47 insertions(+)

diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
index d6258c1cffe5ec480fd2a7e50b3af39ef6ac4c8c..b0a4a3c995e80e0245657f0273a349334071013c 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -106,5 +106,50 @@ void pci_host_common_remove(struct platform_device *pdev)
 }
 EXPORT_SYMBOL_GPL(pci_host_common_remove);
 
+static int pci_host_common_check_d3cold(struct pci_dev *pdev, void *userdata)
+{
+	bool *d3cold_allow = userdata;
+
+	if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ENDPOINT)
+		return 0;
+
+	if (!pdev->dev.driver && !pci_is_enabled(pdev))
+		return 0;
+
+	if (pdev->current_state != PCI_D3hot)
+		goto exit;
+
+	if (device_may_wakeup(&pdev->dev) && !pci_pme_capable(pdev, PCI_D3cold))
+		goto exit;
+
+	return 0;
+exit:
+	*d3cold_allow = false;
+	return -EBUSY;
+}
+
+/**
+ * pci_host_common_can_enter_d3cold - Determine whether a host bridge may enter D3cold
+ * @bridge: PCI host bridge to check
+ *
+ * Walk downstream PCIe endpoint devices and determine whether the host bridge
+ * is permitted to transition to D3cold.
+ *
+ * The host bridge may enter D3cold only if all active PCIe endpoints are in
+ * %PCI_D3hot and any wakeup-enabled endpoint is capable of generating PME from
+ * D3cold. Inactive endpoints are ignored.
+ *
+ * Return: %true if the host bridge may enter D3cold, otherwise %false.
+ */
+bool pci_host_common_can_enter_d3cold(struct pci_host_bridge *bridge)
+{
+	bool d3cold_allow = true;
+
+	pci_walk_bus(bridge->bus, pci_host_common_check_d3cold, &d3cold_allow);
+
+	return d3cold_allow;
+}
+EXPORT_SYMBOL_GPL(pci_host_common_can_enter_d3cold);
+
 MODULE_DESCRIPTION("Common library for PCI host controller drivers");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/controller/pci-host-common.h
index b5075d4bd7eb31fbf1dc946ef1a6afd5afb5b3c6..18a731bca058828340bca84776d0e91da1edbbf7 100644
--- a/drivers/pci/controller/pci-host-common.h
+++ b/drivers/pci/controller/pci-host-common.h
@@ -20,4 +20,6 @@ void pci_host_common_remove(struct platform_device *pdev);
 
 struct pci_config_window *pci_host_common_ecam_create(struct device *dev,
 	struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
+
+bool pci_host_common_can_enter_d3cold(struct pci_host_bridge *bridge);
 #endif

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 2/5] PCI: dwc: Use common D3cold eligibility helper in suspend path
  2026-02-17 11:19 [PATCH v2 0/5] PCI: qcom: Add D3cold support Krishna Chaitanya Chundru
  2026-02-17 11:19 ` [PATCH v2 1/5] PCI: host-common: Add helper to determine host bridge D3cold eligibility Krishna Chaitanya Chundru
@ 2026-02-17 11:19 ` Krishna Chaitanya Chundru
  2026-02-17 11:19 ` [PATCH v2 3/5] PCI: qcom: Add .get_ltssm() helper Krishna Chaitanya Chundru
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 22+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-02-17 11:19 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Will Deacon
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-arm-kernel,
	jonathanh, bjorn.andersson, Krishna Chaitanya Chundru

Previously, the driver skipped putting the link into L2/device state in
D3cold whenever L1 ASPM was enabled, since some devices (e.g. NVMe) expect
low resume latency and may not tolerate deeper power states. However, such
devices typically remain in D0 and are already covered by the new helper's
requirement that all endpoints be in D3hot before the host bridge may
enter D3cold.

So, replace the local L1/L1SS-based check in dw_pcie_suspend_noirq() with
the shared pci_host_common_can_enter_d3cold() helper to decide whether the
DesignWare host bridge can safely transition to D3cold.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 6ae6189e9b8a9021c99ece17504834650debd86b..713aa64553bfc988717cab2936935bb43aabd72c 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -16,9 +16,11 @@
 #include <linux/msi.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
+#include <linux/pci.h>
 #include <linux/pci_regs.h>
 #include <linux/platform_device.h>
 
+#include "../pci-host-common.h"
 #include "../../pci.h"
 #include "pcie-designware.h"
 
@@ -1218,18 +1220,13 @@ static int dw_pcie_pme_turn_off(struct dw_pcie *pci)
 
 int dw_pcie_suspend_noirq(struct dw_pcie *pci)
 {
-	u8 offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
 	int ret = 0;
 	u32 val;
 
 	if (!dw_pcie_link_up(pci))
 		goto stop_link;
 
-	/*
-	 * If L1SS is supported, then do not put the link into L2 as some
-	 * devices such as NVMe expect low resume latency.
-	 */
-	if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) & PCI_EXP_LNKCTL_ASPM_L1)
+	if (!pci_host_common_can_enter_d3cold(pci->pp.bridge))
 		return 0;
 
 	if (pci->pp.ops->pme_turn_off) {

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 3/5] PCI: qcom: Add .get_ltssm() helper
  2026-02-17 11:19 [PATCH v2 0/5] PCI: qcom: Add D3cold support Krishna Chaitanya Chundru
  2026-02-17 11:19 ` [PATCH v2 1/5] PCI: host-common: Add helper to determine host bridge D3cold eligibility Krishna Chaitanya Chundru
  2026-02-17 11:19 ` [PATCH v2 2/5] PCI: dwc: Use common D3cold eligibility helper in suspend path Krishna Chaitanya Chundru
@ 2026-02-17 11:19 ` Krishna Chaitanya Chundru
  2026-03-05  7:46   ` Manivannan Sadhasivam
  2026-02-17 11:19 ` [PATCH v2 4/5] PCI: qcom: Power down PHY via PARF_PHY_CTRL before disabling rails/clocks Krishna Chaitanya Chundru
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 22+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-02-17 11:19 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Will Deacon
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-arm-kernel,
	jonathanh, bjorn.andersson, Krishna Chaitanya Chundru

For older targets like sc7280, we see reading DBI after sending PME
turn off message is causing NOC error.

To avoid unsafe DBI accesses, introduce qcom_pcie_get_ltssm(), which
retrieves the LTSSM state from the PARF_LTSSM register instead.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 67a16af69ddc75fca1b123e70715e692a91a9135..2c4dc7134e006d3530a809f1bcc1a6488d4632ad 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -131,6 +131,7 @@
 
 /* PARF_LTSSM register fields */
 #define LTSSM_EN				BIT(8)
+#define PARF_LTSSM_STATE_MASK			GENMASK(5, 0)
 
 /* PARF_NO_SNOOP_OVERRIDE register fields */
 #define WR_NO_SNOOP_OVERRIDE_EN			BIT(1)
@@ -1255,6 +1256,15 @@ static bool qcom_pcie_link_up(struct dw_pcie *pci)
 	return val & PCI_EXP_LNKSTA_DLLLA;
 }
 
+static enum dw_pcie_ltssm qcom_pcie_get_ltssm(struct dw_pcie *pci)
+{
+	struct qcom_pcie *pcie = to_qcom_pcie(pci);
+	u32 val;
+
+	val = readl(pcie->parf + PARF_LTSSM);
+	return (enum dw_pcie_ltssm)FIELD_GET(PARF_LTSSM_STATE_MASK, val);
+}
+
 static void qcom_pcie_phy_power_off(struct qcom_pcie *pcie)
 {
 	struct qcom_pcie_port *port;
@@ -1507,6 +1517,7 @@ 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,
+	.get_ltssm = qcom_pcie_get_ltssm,
 };
 
 static int qcom_pcie_icc_init(struct qcom_pcie *pcie)

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 4/5] PCI: qcom: Power down PHY via PARF_PHY_CTRL before disabling rails/clocks
  2026-02-17 11:19 [PATCH v2 0/5] PCI: qcom: Add D3cold support Krishna Chaitanya Chundru
                   ` (2 preceding siblings ...)
  2026-02-17 11:19 ` [PATCH v2 3/5] PCI: qcom: Add .get_ltssm() helper Krishna Chaitanya Chundru
@ 2026-02-17 11:19 ` Krishna Chaitanya Chundru
  2026-03-05  7:49   ` Manivannan Sadhasivam
  2026-02-17 11:19 ` [PATCH v2 5/5] PCI: qcom: Add D3cold support Krishna Chaitanya Chundru
  2026-02-17 15:40 ` [PATCH v2 0/5] " Neil Armstrong
  5 siblings, 1 reply; 22+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-02-17 11:19 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Will Deacon
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-arm-kernel,
	jonathanh, bjorn.andersson, Krishna Chaitanya Chundru

Some Qcom PCIe controller variants bring the PHY out of test power-down
(PHY_TEST_PWR_DOWN) during init. When the link is later transitioned
towards D3cold and the driver disables PCIe clocks and/or regulators
without explicitly re-asserting PHY_TEST_PWR_DOWN, the PHY can remain
partially powered, leading to avoidable power leakage.

Update the init-path comments to reflect that PARF_PHY_CTRL is used to
power the PHY on. Also, for controller revisions that enable PHY power
in init (2.3.2, 2.3.3, 2.7.0 and 2.9.0), explicitly power the PHY down
via PARF_PHY_CTRL in the deinit path before disabling clocks/regulators.

This ensures the PHY is put into a defined low-power state prior to
removing its supplies, preventing leakage when entering D3cold.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 2c4dc7134e006d3530a809f1bcc1a6488d4632ad..b02c19bbdf2ea5db252c2a0281a569bb3a0cc497 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -513,7 +513,7 @@ static int qcom_pcie_post_init_2_1_0(struct qcom_pcie *pcie)
 	u32 val;
 	int ret;
 
-	/* enable PCIe clocks and resets */
+	/* PHY power ON */
 	val = readl(pcie->parf + PARF_PHY_CTRL);
 	val &= ~PHY_TEST_PWR_DOWN;
 	writel(val, pcie->parf + PARF_PHY_CTRL);
@@ -680,6 +680,12 @@ static int qcom_pcie_get_resources_2_3_2(struct qcom_pcie *pcie)
 static void qcom_pcie_deinit_2_3_2(struct qcom_pcie *pcie)
 {
 	struct qcom_pcie_resources_2_3_2 *res = &pcie->res.v2_3_2;
+	u32 val;
+
+	/* PHY Power down */
+	val = readl(pcie->parf + PARF_PHY_CTRL);
+	val |= PHY_TEST_PWR_DOWN;
+	writel(val, pcie->parf + PARF_PHY_CTRL);
 
 	clk_bulk_disable_unprepare(res->num_clks, res->clks);
 	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
@@ -712,7 +718,7 @@ static int qcom_pcie_post_init_2_3_2(struct qcom_pcie *pcie)
 {
 	u32 val;
 
-	/* enable PCIe clocks and resets */
+	/* PHY Power ON */
 	val = readl(pcie->parf + PARF_PHY_CTRL);
 	val &= ~PHY_TEST_PWR_DOWN;
 	writel(val, pcie->parf + PARF_PHY_CTRL);
@@ -844,6 +850,12 @@ static int qcom_pcie_get_resources_2_3_3(struct qcom_pcie *pcie)
 static void qcom_pcie_deinit_2_3_3(struct qcom_pcie *pcie)
 {
 	struct qcom_pcie_resources_2_3_3 *res = &pcie->res.v2_3_3;
+	u32 val;
+
+	/* PHY Power down */
+	val = readl(pcie->parf + PARF_PHY_CTRL);
+	val |= PHY_TEST_PWR_DOWN;
+	writel(val, pcie->parf + PARF_PHY_CTRL);
 
 	clk_bulk_disable_unprepare(res->num_clks, res->clks);
 }
@@ -994,7 +1006,7 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
 	/* configure PCIe to RC mode */
 	writel(DEVICE_TYPE_RC, pcie->parf + PARF_DEVICE_TYPE);
 
-	/* enable PCIe clocks and resets */
+	/* PHY power ON */
 	val = readl(pcie->parf + PARF_PHY_CTRL);
 	val &= ~PHY_TEST_PWR_DOWN;
 	writel(val, pcie->parf + PARF_PHY_CTRL);
@@ -1065,6 +1077,12 @@ static void qcom_pcie_host_post_init_2_7_0(struct qcom_pcie *pcie)
 static void qcom_pcie_deinit_2_7_0(struct qcom_pcie *pcie)
 {
 	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
+	u32 val;
+
+	/* PHY Power down */
+	val = readl(pcie->parf + PARF_PHY_CTRL);
+	val |= PHY_TEST_PWR_DOWN;
+	writel(val, pcie->parf + PARF_PHY_CTRL);
 
 	clk_bulk_disable_unprepare(res->num_clks, res->clks);
 
@@ -1169,6 +1187,12 @@ static int qcom_pcie_get_resources_2_9_0(struct qcom_pcie *pcie)
 static void qcom_pcie_deinit_2_9_0(struct qcom_pcie *pcie)
 {
 	struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
+	u32 val;
+
+	/* PHY Power down */
+	val = readl(pcie->parf + PARF_PHY_CTRL);
+	val |= PHY_TEST_PWR_DOWN;
+	writel(val, pcie->parf + PARF_PHY_CTRL);
 
 	clk_bulk_disable_unprepare(res->num_clks, res->clks);
 }

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 5/5] PCI: qcom: Add D3cold support
  2026-02-17 11:19 [PATCH v2 0/5] PCI: qcom: Add D3cold support Krishna Chaitanya Chundru
                   ` (3 preceding siblings ...)
  2026-02-17 11:19 ` [PATCH v2 4/5] PCI: qcom: Power down PHY via PARF_PHY_CTRL before disabling rails/clocks Krishna Chaitanya Chundru
@ 2026-02-17 11:19 ` Krishna Chaitanya Chundru
  2026-03-05  7:58   ` Manivannan Sadhasivam
  2026-02-17 15:40 ` [PATCH v2 0/5] " Neil Armstrong
  5 siblings, 1 reply; 22+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-02-17 11:19 UTC (permalink / raw)
  To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Will Deacon
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-arm-kernel,
	jonathanh, bjorn.andersson, Krishna Chaitanya Chundru

Add support for transitioning Qcom PCIe controllers into D3cold by
integrating with the DWC core suspend/resume helpers.

Implement PME_TurnOff message generation via ELBI_SYS_CTRL and hook it
into the DWC host operations so the controller follows the standard
PME_TurnOff-based power-down sequence before entering D3cold.

When the link is suspended into D3cold, fully tear down interconnect
bandwidth, OPP votes. If D3cold is not entered, retain existing behavior
by keeping the required interconnect and OPP votes.

Drop the qcom_pcie::suspended flag and rely on the existing
dw_pcie::suspended state, which now drives both the power-management
flow and the interconnect/OPP handling.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 121 ++++++++++++++++++++-------------
 1 file changed, 74 insertions(+), 47 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index b02c19bbdf2ea5db252c2a0281a569bb3a0cc497..37442bbe588c36b0b0414cc4d0016da2d8424a87 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -145,6 +145,7 @@
 
 /* ELBI_SYS_CTRL register fields */
 #define ELBI_SYS_CTRL_LT_ENABLE			BIT(0)
+#define ELBI_SYS_CTRL_PME_TURNOFF_MSG		BIT(4)
 
 /* AXI_MSTR_RESP_COMP_CTRL0 register fields */
 #define CFG_REMOTE_RD_REQ_BRIDGE_SIZE_2K	0x4
@@ -283,7 +284,6 @@ struct qcom_pcie {
 	const struct qcom_pcie_cfg *cfg;
 	struct dentry *debugfs;
 	struct list_head ports;
-	bool suspended;
 	bool use_pm_opp;
 };
 
@@ -1401,10 +1401,18 @@ static void qcom_pcie_host_post_init(struct dw_pcie_rp *pp)
 		pcie->cfg->ops->host_post_init(pcie);
 }
 
+static void qcom_pcie_host_pme_turn_off(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+
+	writel(ELBI_SYS_CTRL_PME_TURNOFF_MSG, pci->elbi_base + ELBI_SYS_CTRL);
+}
+
 static const struct dw_pcie_host_ops qcom_pcie_dw_ops = {
 	.init		= qcom_pcie_host_init,
 	.deinit		= qcom_pcie_host_deinit,
 	.post_init	= qcom_pcie_host_post_init,
+	.pme_turn_off	= qcom_pcie_host_pme_turn_off,
 };
 
 /* Qcom IP rev.: 2.1.0	Synopsys IP rev.: 4.01a */
@@ -2069,53 +2077,51 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
 	if (!pcie)
 		return 0;
 
-	/*
-	 * Set minimum bandwidth required to keep data path functional during
-	 * suspend.
-	 */
-	if (pcie->icc_mem) {
-		ret = icc_set_bw(pcie->icc_mem, 0, kBps_to_icc(1));
-		if (ret) {
-			dev_err(dev,
-				"Failed to set bandwidth for PCIe-MEM interconnect path: %d\n",
-				ret);
-			return ret;
-		}
-	}
+	ret = dw_pcie_suspend_noirq(pcie->pci);
+	if (ret)
+		return ret;
 
-	/*
-	 * Turn OFF the resources only for controllers without active PCIe
-	 * devices. For controllers with active devices, the resources are kept
-	 * ON and the link is expected to be in L0/L1 (sub)states.
-	 *
-	 * Turning OFF the resources for controllers with active PCIe devices
-	 * will trigger access violation during the end of the suspend cycle,
-	 * as kernel tries to access the PCIe devices config space for masking
-	 * MSIs.
-	 *
-	 * Also, it is not desirable to put the link into L2/L3 state as that
-	 * implies VDD supply will be removed and the devices may go into
-	 * powerdown state. This will affect the lifetime of the storage devices
-	 * like NVMe.
-	 */
-	if (!dw_pcie_link_up(pcie->pci)) {
-		qcom_pcie_host_deinit(&pcie->pci->pp);
-		pcie->suspended = true;
-	}
+	if (pcie->pci->suspended) {
+		ret = icc_disable(pcie->icc_mem);
+		if (ret)
+			dev_err(dev, "Failed to disable PCIe-MEM interconnect path: %d\n", ret);
 
-	/*
-	 * Only disable CPU-PCIe interconnect path if the suspend is non-S2RAM.
-	 * Because on some platforms, DBI access can happen very late during the
-	 * S2RAM and a non-active CPU-PCIe interconnect path may lead to NoC
-	 * error.
-	 */
-	if (pm_suspend_target_state != PM_SUSPEND_MEM) {
 		ret = icc_disable(pcie->icc_cpu);
 		if (ret)
 			dev_err(dev, "Failed to disable CPU-PCIe interconnect path: %d\n", ret);
 
 		if (pcie->use_pm_opp)
 			dev_pm_opp_set_opp(pcie->pci->dev, NULL);
+	} else {
+		/*
+		 * Set minimum bandwidth required to keep data path functional during
+		 * suspend.
+		 */
+		if (pcie->icc_mem) {
+			ret = icc_set_bw(pcie->icc_mem, 0, kBps_to_icc(1));
+			if (ret) {
+				dev_err(dev,
+					"Failed to set bandwidth for PCIe-MEM interconnect path: %d\n",
+					ret);
+				return ret;
+			}
+		}
+
+		/*
+		 * Only disable CPU-PCIe interconnect path if the suspend is non-S2RAM.
+		 * Because on some platforms, DBI access can happen very late during the
+		 * S2RAM and a non-active CPU-PCIe interconnect path may lead to NoC
+		 * error.
+		 */
+		if (pm_suspend_target_state != PM_SUSPEND_MEM) {
+			ret = icc_disable(pcie->icc_cpu);
+			if (ret)
+				dev_err(dev, "Failed to disable CPU-PCIe interconnect path: %d\n",
+					ret);
+
+			if (pcie->use_pm_opp)
+				dev_pm_opp_set_opp(pcie->pci->dev, NULL);
+		}
 	}
 	return ret;
 }
@@ -2129,25 +2135,46 @@ static int qcom_pcie_resume_noirq(struct device *dev)
 	if (!pcie)
 		return 0;
 
-	if (pm_suspend_target_state != PM_SUSPEND_MEM) {
+	if (pcie->pci->suspended) {
 		ret = icc_enable(pcie->icc_cpu);
 		if (ret) {
 			dev_err(dev, "Failed to enable CPU-PCIe interconnect path: %d\n", ret);
 			return ret;
 		}
-	}
 
-	if (pcie->suspended) {
-		ret = qcom_pcie_host_init(&pcie->pci->pp);
-		if (ret)
-			return ret;
+		ret = icc_enable(pcie->icc_mem);
+		if (ret) {
+			dev_err(dev, "Failed to enable PCIe-MEM interconnect path: %d\n", ret);
+			goto disable_icc_cpu;
+		}
 
-		pcie->suspended = false;
+		/*
+		 * Ignore -ETIMEDOUT here since it is expected when no endpoint is
+		 * connected to the PCIe link.
+		 */
+		ret = dw_pcie_resume_noirq(pcie->pci);
+		if (ret && (ret != -ETIMEDOUT))
+			goto disable_icc_mem;
+	} else {
+		if (pm_suspend_target_state != PM_SUSPEND_MEM) {
+			ret = icc_enable(pcie->icc_cpu);
+			if (ret) {
+				dev_err(dev, "Failed to enable CPU-PCIe interconnect path: %d\n",
+					ret);
+				return ret;
+			}
+		}
 	}
 
 	qcom_pcie_icc_opp_update(pcie);
 
 	return 0;
+disable_icc_mem:
+	icc_disable(pcie->icc_mem);
+disable_icc_cpu:
+	icc_disable(pcie->icc_cpu);
+
+	return ret;
 }
 
 static const struct of_device_id qcom_pcie_match[] = {

-- 
2.34.1



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 0/5] PCI: qcom: Add D3cold support
  2026-02-17 11:19 [PATCH v2 0/5] PCI: qcom: Add D3cold support Krishna Chaitanya Chundru
                   ` (4 preceding siblings ...)
  2026-02-17 11:19 ` [PATCH v2 5/5] PCI: qcom: Add D3cold support Krishna Chaitanya Chundru
@ 2026-02-17 15:40 ` Neil Armstrong
  5 siblings, 0 replies; 22+ messages in thread
From: Neil Armstrong @ 2026-02-17 15:40 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru, Jingoo Han, Manivannan Sadhasivam,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
	Bjorn Helgaas, Will Deacon
  Cc: linux-pci, linux-kernel, linux-arm-msm, linux-arm-kernel,
	jonathanh, bjorn.andersson

On 2/17/26 12:19, Krishna Chaitanya Chundru wrote:
> This series adds support for putting Qualcomm PCIe host bridges into D3cold
> when downstream conditions allow it, and introduces a small common helper
> to determine D3cold eligibility based on endpoint state.
> 
> On Qualcomm platforms, PCIe host controllers are currently kept powered
> even when there are no active endpoints (i.e. all endpoints are already in
> PCI_D3hot). This prevents the SoC from entering deeper low‑power states
> such as CXPC.
> 
> While PCIe D3cold support exists in the PCI core, host controller drivers
> lack a common mechanism to determine whether it is safe to power off the
> host bridge without breaking active devices or wakeup functionality.
> As a result, controllers either avoid entering D3cold or depend on rough,
> driver‑specific workarounds.
> 
> This series addresses that gap.
> 
> 1. Introduces pci_host_common_can_enter_d3cold(), a helper that determines
>     whether a host bridge may enter D3cold based on downstream PCIe endpoint
>     state. The helper permits D3cold only when all *active* endpoints are
>     already in PCI_D3hot, and any wakeup‑enabled endpoint supports PME
>     from D3cold.
> 
> 2. Updates the Designware PCIe host driver to use this helper in the
>     suspend_noirq() path, replacing the existing heuristic that blocked
>     D3cold whenever L1 ASPM was enabled.
> 
> 3. Enables D3cold support for Qualcomm PCIe controllers by wiring them into
>     the DesignWare common suspend/resume flow and explicitly powering down
>     controller resources when all endpoints are in D3hot.
> 
> The immediate outcome of this series is that Qualcomm PCIe host bridges can
> enter D3cold when all endpoints are in D3hot.
> 
> This is a necessary but not sufficient step toward unblocking CXPC. With
> this series applied, CXPC can be achieved on systems with no attached NVMe
> devices. Support for NVMe‑attached systems requires additional changes
> in NVMe driver, which are being worked on separately.
> 
> Tested on:
>    - Qualcomm Lemans EVK, Monaco & sc7280 platforms.
> 
> Validation steps:
>    - Boot without NVMe attach:
>        * PCIe host enters D3cold during suspend
>        * SoC is able to reach CXPC provided other drivers also remove
> 	their votes as part of suspend.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> Changes in v2:
> - Updated the cover letter (Bjorn Andersson)
> - Add get_ltssm helper function to read LTSSM state from parf.
> - Allow D3cold if there is no driver enabled for a endpoint.
> - Added a seperate patch to make phy down in deinit part to avoid power
>    leakage.
> - Revert icc bw voting if resume fails(Bjorn Andersson).
> - Link to v1: https://lore.kernel.org/r/20260128-d3cold-v1-0-dd8f3f0ce824@oss.qualcomm.com
> 
> ---
> Krishna Chaitanya Chundru (5):
>        PCI: host-common: Add helper to determine host bridge D3cold eligibility
>        PCI: dwc: Use common D3cold eligibility helper in suspend path
>        PCI: qcom: Add .get_ltssm() helper
>        PCI: qcom: Power down PHY via PARF_PHY_CTRL before disabling rails/clocks
>        PCI: qcom: Add D3cold support
> 
>   drivers/pci/controller/dwc/pcie-designware-host.c |   9 +-
>   drivers/pci/controller/dwc/pcie-qcom.c            | 162 +++++++++++++++-------
>   drivers/pci/controller/pci-host-common.c          |  45 ++++++
>   drivers/pci/controller/pci-host-common.h          |   2 +
>   4 files changed, 162 insertions(+), 56 deletions(-)
> ---
> base-commit: 9702969978695d9a699a1f34771580cdbb153b33
> change-id: 20251229-d3cold-bf99921960bb
> 
> Best regards,

With [1] to allow ath12k to go in d3cold:

Tested-by: Neil Armstrong <neil.armstrong@linaro.org> # on SM8650-HDK

[1] https://lore.kernel.org/all/20260217113142.9140-1-manivannan.sadhasivam@oss.qualcomm.com/


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/5] PCI: host-common: Add helper to determine host bridge D3cold eligibility
  2026-02-17 11:19 ` [PATCH v2 1/5] PCI: host-common: Add helper to determine host bridge D3cold eligibility Krishna Chaitanya Chundru
@ 2026-02-23 18:10   ` Bjorn Helgaas
  2026-02-23 18:46     ` Rafael J. Wysocki
  2026-03-05  7:42   ` Manivannan Sadhasivam
  1 sibling, 1 reply; 22+ messages in thread
From: Bjorn Helgaas @ 2026-02-23 18:10 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Will Deacon, linux-pci, linux-kernel, linux-arm-msm,
	linux-arm-kernel, jonathanh, bjorn.andersson, Rafael J. Wysocki,
	linux-pm

[+cc Rafael, linux-pm]

On Tue, Feb 17, 2026 at 04:49:06PM +0530, Krishna Chaitanya Chundru wrote:
> Add a common helper, pci_host_common_can_enter_d3cold(), to determine
> whether a PCI host bridge can safely transition to D3cold.
> 
> This helper is intended to be used by PCI host controller drivers to
> decide whether they may safely put the host bridge into D3cold based on
> the power state and wakeup capabilities of downstream endpoints.
> 
> The helper walks all devices on the bridge's primary bus and only allows
> the host bridge to enter D3cold if all PCIe endpoints are already in
> PCI_D3hot. This ensures that we do not power off the host bridge while
> any active endpoint still requires the link to remain powered.
> 
> For devices that may wake the system, the helper additionally requires
> that the device supports PME wake from D3cold (via WAKE#). Devices that
> do not have wakeup enabled are not restricted by this check and do not
> block the host bridge from entering D3cold.
> 
> Devices without a bound driver and with PCI not enabled via sysfs are
> treated as inactive and therefore do not prevent the host bridge from
> entering D3cold. This allows controllers to power down more aggressively
> when there are no actively managed endpoints.

This series is currently structured so it's only applicable to native
host bridge drivers, i.e., things using DT.  Is there anything that
prevents using D3cold for host bridges in ACPI systems?

> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  drivers/pci/controller/pci-host-common.c | 45 ++++++++++++++++++++++++++++++++
>  drivers/pci/controller/pci-host-common.h |  2 ++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> index d6258c1cffe5ec480fd2a7e50b3af39ef6ac4c8c..b0a4a3c995e80e0245657f0273a349334071013c 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c
> @@ -106,5 +106,50 @@ void pci_host_common_remove(struct platform_device *pdev)
>  }
>  EXPORT_SYMBOL_GPL(pci_host_common_remove);
>  
> +static int pci_host_common_check_d3cold(struct pci_dev *pdev, void *userdata)
> +{
> +	bool *d3cold_allow = userdata;
> +
> +	if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ENDPOINT)
> +		return 0;
> +
> +	if (!pdev->dev.driver && !pci_is_enabled(pdev))
> +		return 0;
> +
> +	if (pdev->current_state != PCI_D3hot)
> +		goto exit;
> +
> +	if (device_may_wakeup(&pdev->dev) && !pci_pme_capable(pdev, PCI_D3cold))
> +		goto exit;
> +
> +	return 0;
> +exit:
> +	*d3cold_allow = false;
> +	return -EBUSY;
> +}
> +
> +/**
> + * pci_host_common_can_enter_d3cold - Determine whether a host bridge may enter D3cold
> + * @bridge: PCI host bridge to check
> + *
> + * Walk downstream PCIe endpoint devices and determine whether the host bridge
> + * is permitted to transition to D3cold.
> + *
> + * The host bridge may enter D3cold only if all active PCIe endpoints are in
> + * %PCI_D3hot and any wakeup-enabled endpoint is capable of generating PME from
> + * D3cold. Inactive endpoints are ignored.
> + *
> + * Return: %true if the host bridge may enter D3cold, otherwise %false.
> + */
> +bool pci_host_common_can_enter_d3cold(struct pci_host_bridge *bridge)
> +{
> +	bool d3cold_allow = true;
> +
> +	pci_walk_bus(bridge->bus, pci_host_common_check_d3cold, &d3cold_allow);
> +
> +	return d3cold_allow;
> +}
> +EXPORT_SYMBOL_GPL(pci_host_common_can_enter_d3cold);
> +
>  MODULE_DESCRIPTION("Common library for PCI host controller drivers");
>  MODULE_LICENSE("GPL v2");
> diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/controller/pci-host-common.h
> index b5075d4bd7eb31fbf1dc946ef1a6afd5afb5b3c6..18a731bca058828340bca84776d0e91da1edbbf7 100644
> --- a/drivers/pci/controller/pci-host-common.h
> +++ b/drivers/pci/controller/pci-host-common.h
> @@ -20,4 +20,6 @@ void pci_host_common_remove(struct platform_device *pdev);
>  
>  struct pci_config_window *pci_host_common_ecam_create(struct device *dev,
>  	struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
> +
> +bool pci_host_common_can_enter_d3cold(struct pci_host_bridge *bridge);
>  #endif
> 
> -- 
> 2.34.1
> 


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/5] PCI: host-common: Add helper to determine host bridge D3cold eligibility
  2026-02-23 18:10   ` Bjorn Helgaas
@ 2026-02-23 18:46     ` Rafael J. Wysocki
  2026-02-23 19:55       ` Bjorn Helgaas
  0 siblings, 1 reply; 22+ messages in thread
From: Rafael J. Wysocki @ 2026-02-23 18:46 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Krishna Chaitanya Chundru, Jingoo Han, Manivannan Sadhasivam,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
	Bjorn Helgaas, Will Deacon, linux-pci, linux-kernel,
	linux-arm-msm, linux-arm-kernel, jonathanh, bjorn.andersson,
	Rafael J. Wysocki, linux-pm

On Mon, Feb 23, 2026 at 7:10 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> [+cc Rafael, linux-pm]
>
> On Tue, Feb 17, 2026 at 04:49:06PM +0530, Krishna Chaitanya Chundru wrote:
> > Add a common helper, pci_host_common_can_enter_d3cold(), to determine
> > whether a PCI host bridge can safely transition to D3cold.
> >
> > This helper is intended to be used by PCI host controller drivers to
> > decide whether they may safely put the host bridge into D3cold based on
> > the power state and wakeup capabilities of downstream endpoints.
> >
> > The helper walks all devices on the bridge's primary bus and only allows
> > the host bridge to enter D3cold if all PCIe endpoints are already in
> > PCI_D3hot. This ensures that we do not power off the host bridge while
> > any active endpoint still requires the link to remain powered.
> >
> > For devices that may wake the system, the helper additionally requires
> > that the device supports PME wake from D3cold (via WAKE#). Devices that
> > do not have wakeup enabled are not restricted by this check and do not
> > block the host bridge from entering D3cold.
> >
> > Devices without a bound driver and with PCI not enabled via sysfs are
> > treated as inactive and therefore do not prevent the host bridge from
> > entering D3cold. This allows controllers to power down more aggressively
> > when there are no actively managed endpoints.
>
> This series is currently structured so it's only applicable to native
> host bridge drivers, i.e., things using DT.  Is there anything that
> prevents using D3cold for host bridges in ACPI systems?

Do you mean in principle or in practice?

x86 platforms don't support PCI host bridge PM AFAICS and the ACPI
driver doesn't support PM either.  I'd rather not add it ad hoc
because of this patch series.

> > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > ---
> >  drivers/pci/controller/pci-host-common.c | 45 ++++++++++++++++++++++++++++++++
> >  drivers/pci/controller/pci-host-common.h |  2 ++
> >  2 files changed, 47 insertions(+)
> >
> > diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> > index d6258c1cffe5ec480fd2a7e50b3af39ef6ac4c8c..b0a4a3c995e80e0245657f0273a349334071013c 100644
> > --- a/drivers/pci/controller/pci-host-common.c
> > +++ b/drivers/pci/controller/pci-host-common.c
> > @@ -106,5 +106,50 @@ void pci_host_common_remove(struct platform_device *pdev)
> >  }
> >  EXPORT_SYMBOL_GPL(pci_host_common_remove);
> >
> > +static int pci_host_common_check_d3cold(struct pci_dev *pdev, void *userdata)
> > +{
> > +     bool *d3cold_allow = userdata;
> > +
> > +     if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ENDPOINT)
> > +             return 0;
> > +
> > +     if (!pdev->dev.driver && !pci_is_enabled(pdev))
> > +             return 0;
> > +
> > +     if (pdev->current_state != PCI_D3hot)
> > +             goto exit;
> > +
> > +     if (device_may_wakeup(&pdev->dev) && !pci_pme_capable(pdev, PCI_D3cold))
> > +             goto exit;
> > +
> > +     return 0;
> > +exit:
> > +     *d3cold_allow = false;
> > +     return -EBUSY;
> > +}
> > +
> > +/**
> > + * pci_host_common_can_enter_d3cold - Determine whether a host bridge may enter D3cold
> > + * @bridge: PCI host bridge to check
> > + *
> > + * Walk downstream PCIe endpoint devices and determine whether the host bridge
> > + * is permitted to transition to D3cold.
> > + *
> > + * The host bridge may enter D3cold only if all active PCIe endpoints are in
> > + * %PCI_D3hot and any wakeup-enabled endpoint is capable of generating PME from
> > + * D3cold. Inactive endpoints are ignored.
> > + *
> > + * Return: %true if the host bridge may enter D3cold, otherwise %false.
> > + */
> > +bool pci_host_common_can_enter_d3cold(struct pci_host_bridge *bridge)
> > +{
> > +     bool d3cold_allow = true;
> > +
> > +     pci_walk_bus(bridge->bus, pci_host_common_check_d3cold, &d3cold_allow);
> > +
> > +     return d3cold_allow;
> > +}
> > +EXPORT_SYMBOL_GPL(pci_host_common_can_enter_d3cold);
> > +
> >  MODULE_DESCRIPTION("Common library for PCI host controller drivers");
> >  MODULE_LICENSE("GPL v2");
> > diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/controller/pci-host-common.h
> > index b5075d4bd7eb31fbf1dc946ef1a6afd5afb5b3c6..18a731bca058828340bca84776d0e91da1edbbf7 100644
> > --- a/drivers/pci/controller/pci-host-common.h
> > +++ b/drivers/pci/controller/pci-host-common.h
> > @@ -20,4 +20,6 @@ void pci_host_common_remove(struct platform_device *pdev);
> >
> >  struct pci_config_window *pci_host_common_ecam_create(struct device *dev,
> >       struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
> > +
> > +bool pci_host_common_can_enter_d3cold(struct pci_host_bridge *bridge);
> >  #endif
> >
> > --
> > 2.34.1
> >


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/5] PCI: host-common: Add helper to determine host bridge D3cold eligibility
  2026-02-23 18:46     ` Rafael J. Wysocki
@ 2026-02-23 19:55       ` Bjorn Helgaas
  2026-02-23 20:03         ` Rafael J. Wysocki
  0 siblings, 1 reply; 22+ messages in thread
From: Bjorn Helgaas @ 2026-02-23 19:55 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Krishna Chaitanya Chundru, Jingoo Han, Manivannan Sadhasivam,
	Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
	Bjorn Helgaas, Will Deacon, linux-pci, linux-kernel,
	linux-arm-msm, linux-arm-kernel, jonathanh, bjorn.andersson,
	linux-pm

On Mon, Feb 23, 2026 at 07:46:08PM +0100, Rafael J. Wysocki wrote:
> On Mon, Feb 23, 2026 at 7:10 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Tue, Feb 17, 2026 at 04:49:06PM +0530, Krishna Chaitanya Chundru wrote:
> > > Add a common helper, pci_host_common_can_enter_d3cold(), to determine
> > > whether a PCI host bridge can safely transition to D3cold.
> > >
> > > This helper is intended to be used by PCI host controller drivers to
> > > decide whether they may safely put the host bridge into D3cold based on
> > > the power state and wakeup capabilities of downstream endpoints.
> > >
> > > The helper walks all devices on the bridge's primary bus and only allows
> > > the host bridge to enter D3cold if all PCIe endpoints are already in
> > > PCI_D3hot. This ensures that we do not power off the host bridge while
> > > any active endpoint still requires the link to remain powered.
> > >
> > > For devices that may wake the system, the helper additionally requires
> > > that the device supports PME wake from D3cold (via WAKE#). Devices that
> > > do not have wakeup enabled are not restricted by this check and do not
> > > block the host bridge from entering D3cold.
> > >
> > > Devices without a bound driver and with PCI not enabled via sysfs are
> > > treated as inactive and therefore do not prevent the host bridge from
> > > entering D3cold. This allows controllers to power down more aggressively
> > > when there are no actively managed endpoints.
> >
> > This series is currently structured so it's only applicable to native
> > host bridge drivers, i.e., things using DT.  Is there anything that
> > prevents using D3cold for host bridges in ACPI systems?
> 
> Do you mean in principle or in practice?

Just in principle.  I was hoping for some way to tie this decision
making back to a spec.  Everything downstream being in D3 already, and
any wakeup devices supporting PME from D3cold sounds plausible to me,
but I don't know about any prescriptive spec language, and I wouldn't
want to get locked into a scheme that couldn't be supported on ACPI.

> x86 platforms don't support PCI host bridge PM AFAICS and the ACPI
> driver doesn't support PM either.  I'd rather not add it ad hoc
> because of this patch series.

Agreed.

> > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > ---
> > >  drivers/pci/controller/pci-host-common.c | 45 ++++++++++++++++++++++++++++++++
> > >  drivers/pci/controller/pci-host-common.h |  2 ++
> > >  2 files changed, 47 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> > > index d6258c1cffe5ec480fd2a7e50b3af39ef6ac4c8c..b0a4a3c995e80e0245657f0273a349334071013c 100644
> > > --- a/drivers/pci/controller/pci-host-common.c
> > > +++ b/drivers/pci/controller/pci-host-common.c
> > > @@ -106,5 +106,50 @@ void pci_host_common_remove(struct platform_device *pdev)
> > >  }
> > >  EXPORT_SYMBOL_GPL(pci_host_common_remove);
> > >
> > > +static int pci_host_common_check_d3cold(struct pci_dev *pdev, void *userdata)
> > > +{
> > > +     bool *d3cold_allow = userdata;
> > > +
> > > +     if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ENDPOINT)
> > > +             return 0;
> > > +
> > > +     if (!pdev->dev.driver && !pci_is_enabled(pdev))
> > > +             return 0;
> > > +
> > > +     if (pdev->current_state != PCI_D3hot)
> > > +             goto exit;
> > > +
> > > +     if (device_may_wakeup(&pdev->dev) && !pci_pme_capable(pdev, PCI_D3cold))
> > > +             goto exit;
> > > +
> > > +     return 0;
> > > +exit:
> > > +     *d3cold_allow = false;
> > > +     return -EBUSY;
> > > +}
> > > +
> > > +/**
> > > + * pci_host_common_can_enter_d3cold - Determine whether a host bridge may enter D3cold
> > > + * @bridge: PCI host bridge to check
> > > + *
> > > + * Walk downstream PCIe endpoint devices and determine whether the host bridge
> > > + * is permitted to transition to D3cold.
> > > + *
> > > + * The host bridge may enter D3cold only if all active PCIe endpoints are in
> > > + * %PCI_D3hot and any wakeup-enabled endpoint is capable of generating PME from
> > > + * D3cold. Inactive endpoints are ignored.
> > > + *
> > > + * Return: %true if the host bridge may enter D3cold, otherwise %false.
> > > + */
> > > +bool pci_host_common_can_enter_d3cold(struct pci_host_bridge *bridge)
> > > +{
> > > +     bool d3cold_allow = true;
> > > +
> > > +     pci_walk_bus(bridge->bus, pci_host_common_check_d3cold, &d3cold_allow);
> > > +
> > > +     return d3cold_allow;
> > > +}
> > > +EXPORT_SYMBOL_GPL(pci_host_common_can_enter_d3cold);
> > > +
> > >  MODULE_DESCRIPTION("Common library for PCI host controller drivers");
> > >  MODULE_LICENSE("GPL v2");
> > > diff --git a/drivers/pci/controller/pci-host-common.h b/drivers/pci/controller/pci-host-common.h
> > > index b5075d4bd7eb31fbf1dc946ef1a6afd5afb5b3c6..18a731bca058828340bca84776d0e91da1edbbf7 100644
> > > --- a/drivers/pci/controller/pci-host-common.h
> > > +++ b/drivers/pci/controller/pci-host-common.h
> > > @@ -20,4 +20,6 @@ void pci_host_common_remove(struct platform_device *pdev);
> > >
> > >  struct pci_config_window *pci_host_common_ecam_create(struct device *dev,
> > >       struct pci_host_bridge *bridge, const struct pci_ecam_ops *ops);
> > > +
> > > +bool pci_host_common_can_enter_d3cold(struct pci_host_bridge *bridge);
> > >  #endif
> > >
> > > --
> > > 2.34.1
> > >


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/5] PCI: host-common: Add helper to determine host bridge D3cold eligibility
  2026-02-23 19:55       ` Bjorn Helgaas
@ 2026-02-23 20:03         ` Rafael J. Wysocki
  0 siblings, 0 replies; 22+ messages in thread
From: Rafael J. Wysocki @ 2026-02-23 20:03 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Rafael J. Wysocki, Krishna Chaitanya Chundru, Jingoo Han,
	Manivannan Sadhasivam, Lorenzo Pieralisi,
	Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
	Will Deacon, linux-pci, linux-kernel, linux-arm-msm,
	linux-arm-kernel, jonathanh, bjorn.andersson, linux-pm

On Mon, Feb 23, 2026 at 8:55 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Feb 23, 2026 at 07:46:08PM +0100, Rafael J. Wysocki wrote:
> > On Mon, Feb 23, 2026 at 7:10 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > > On Tue, Feb 17, 2026 at 04:49:06PM +0530, Krishna Chaitanya Chundru wrote:
> > > > Add a common helper, pci_host_common_can_enter_d3cold(), to determine
> > > > whether a PCI host bridge can safely transition to D3cold.
> > > >
> > > > This helper is intended to be used by PCI host controller drivers to
> > > > decide whether they may safely put the host bridge into D3cold based on
> > > > the power state and wakeup capabilities of downstream endpoints.
> > > >
> > > > The helper walks all devices on the bridge's primary bus and only allows
> > > > the host bridge to enter D3cold if all PCIe endpoints are already in
> > > > PCI_D3hot. This ensures that we do not power off the host bridge while
> > > > any active endpoint still requires the link to remain powered.
> > > >
> > > > For devices that may wake the system, the helper additionally requires
> > > > that the device supports PME wake from D3cold (via WAKE#). Devices that
> > > > do not have wakeup enabled are not restricted by this check and do not
> > > > block the host bridge from entering D3cold.
> > > >
> > > > Devices without a bound driver and with PCI not enabled via sysfs are
> > > > treated as inactive and therefore do not prevent the host bridge from
> > > > entering D3cold. This allows controllers to power down more aggressively
> > > > when there are no actively managed endpoints.
> > >
> > > This series is currently structured so it's only applicable to native
> > > host bridge drivers, i.e., things using DT.  Is there anything that
> > > prevents using D3cold for host bridges in ACPI systems?
> >
> > Do you mean in principle or in practice?
>
> Just in principle.  I was hoping for some way to tie this decision
> making back to a spec.  Everything downstream being in D3 already, and
> any wakeup devices supporting PME from D3cold sounds plausible to me,
> but I don't know about any prescriptive spec language, and I wouldn't
> want to get locked into a scheme that couldn't be supported on ACPI.

In principle, I guess someone might want to add ACPI PM support to a
PCI host bridge device object.

In theory, there may be a problem with message-based PME wakeup unless
the host bridge device has some D3cold aux power, but it at least
seems to be doable.

I think that this question is for non-x86 folks who use ACPI in their
platforms because they may just have this use case.


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/5] PCI: host-common: Add helper to determine host bridge D3cold eligibility
  2026-02-17 11:19 ` [PATCH v2 1/5] PCI: host-common: Add helper to determine host bridge D3cold eligibility Krishna Chaitanya Chundru
  2026-02-23 18:10   ` Bjorn Helgaas
@ 2026-03-05  7:42   ` Manivannan Sadhasivam
  1 sibling, 0 replies; 22+ messages in thread
From: Manivannan Sadhasivam @ 2026-03-05  7:42 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Bjorn Helgaas, Will Deacon, linux-pci, linux-kernel,
	linux-arm-msm, linux-arm-kernel, jonathanh, bjorn.andersson

On Tue, Feb 17, 2026 at 04:49:06PM +0530, Krishna Chaitanya Chundru wrote:
> Add a common helper, pci_host_common_can_enter_d3cold(), to determine
> whether a PCI host bridge can safely transition to D3cold.
> 
> This helper is intended to be used by PCI host controller drivers to
> decide whether they may safely put the host bridge into D3cold based on
> the power state and wakeup capabilities of downstream endpoints.
> 
> The helper walks all devices on the bridge's primary bus and only allows
> the host bridge to enter D3cold if all PCIe endpoints are already in
> PCI_D3hot. This ensures that we do not power off the host bridge while
> any active endpoint still requires the link to remain powered.
> 
> For devices that may wake the system, the helper additionally requires
> that the device supports PME wake from D3cold (via WAKE#). Devices that
> do not have wakeup enabled are not restricted by this check and do not
> block the host bridge from entering D3cold.
> 
> Devices without a bound driver and with PCI not enabled via sysfs are
> treated as inactive and therefore do not prevent the host bridge from
> entering D3cold. This allows controllers to power down more aggressively
> when there are no actively managed endpoints.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  drivers/pci/controller/pci-host-common.c | 45 ++++++++++++++++++++++++++++++++
>  drivers/pci/controller/pci-host-common.h |  2 ++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> index d6258c1cffe5ec480fd2a7e50b3af39ef6ac4c8c..b0a4a3c995e80e0245657f0273a349334071013c 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c
> @@ -106,5 +106,50 @@ void pci_host_common_remove(struct platform_device *pdev)
>  }
>  EXPORT_SYMBOL_GPL(pci_host_common_remove);
>  
> +static int pci_host_common_check_d3cold(struct pci_dev *pdev, void *userdata)
> +{
> +	bool *d3cold_allow = userdata;

d3cold_possible

> +
> +	if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ENDPOINT)
> +		return 0;
> +
> +	if (!pdev->dev.driver && !pci_is_enabled(pdev))
> +		return 0;
> +
> +	if (pdev->current_state != PCI_D3hot)
> +		goto exit;
> +
> +	if (device_may_wakeup(&pdev->dev) && !pci_pme_capable(pdev, PCI_D3cold))
> +		goto exit;
> +
> +	return 0;

Newline

> +exit:
> +	*d3cold_allow = false;

Newline

> +	return -EBUSY;

I think -EOPNOTSUPP should be returned here.

> +}
> +
> +/**
> + * pci_host_common_can_enter_d3cold - Determine whether a host bridge may enter D3cold

Since PCI core already has pci_bridge_d3_possible() API, we should try to be in
sync by calling this API as pci_host_common_d3cold_possible().

> + * @bridge: PCI host bridge to check
> + *
> + * Walk downstream PCIe endpoint devices and determine whether the host bridge
> + * is permitted to transition to D3cold.
> + *
> + * The host bridge may enter D3cold only if all active PCIe endpoints are in

s/may/can

> + * %PCI_D3hot and any wakeup-enabled endpoint is capable of generating PME from

Remove %

> + * D3cold. Inactive endpoints are ignored.
> + *
> + * Return: %true if the host bridge may enter D3cold, otherwise %false.
> + */
> +bool pci_host_common_can_enter_d3cold(struct pci_host_bridge *bridge)
> +{
> +	bool d3cold_allow = true;
> +
> +	pci_walk_bus(bridge->bus, pci_host_common_check_d3cold, &d3cold_allow);

s/pci_host_common_check_d3cold/__pci_host_common_d3cold_possible

- Mani

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 3/5] PCI: qcom: Add .get_ltssm() helper
  2026-02-17 11:19 ` [PATCH v2 3/5] PCI: qcom: Add .get_ltssm() helper Krishna Chaitanya Chundru
@ 2026-03-05  7:46   ` Manivannan Sadhasivam
  0 siblings, 0 replies; 22+ messages in thread
From: Manivannan Sadhasivam @ 2026-03-05  7:46 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Bjorn Helgaas, Will Deacon, linux-pci, linux-kernel,
	linux-arm-msm, linux-arm-kernel, jonathanh, bjorn.andersson

On Tue, Feb 17, 2026 at 04:49:08PM +0530, Krishna Chaitanya Chundru wrote:
> For older targets like sc7280, we see reading DBI after sending PME
> turn off message is causing NOC error.
> 
> To avoid unsafe DBI accesses, introduce qcom_pcie_get_ltssm(), which
> retrieves the LTSSM state from the PARF_LTSSM register instead.
> 

You completely missed describing the helper here and where it is used.

> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 67a16af69ddc75fca1b123e70715e692a91a9135..2c4dc7134e006d3530a809f1bcc1a6488d4632ad 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -131,6 +131,7 @@
>  
>  /* PARF_LTSSM register fields */
>  #define LTSSM_EN				BIT(8)
> +#define PARF_LTSSM_STATE_MASK			GENMASK(5, 0)
>  
>  /* PARF_NO_SNOOP_OVERRIDE register fields */
>  #define WR_NO_SNOOP_OVERRIDE_EN			BIT(1)
> @@ -1255,6 +1256,15 @@ static bool qcom_pcie_link_up(struct dw_pcie *pci)
>  	return val & PCI_EXP_LNKSTA_DLLLA;
>  }
>  
> +static enum dw_pcie_ltssm qcom_pcie_get_ltssm(struct dw_pcie *pci)
> +{
> +	struct qcom_pcie *pcie = to_qcom_pcie(pci);
> +	u32 val;
> +
> +	val = readl(pcie->parf + PARF_LTSSM);

Newline

- Mani

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 4/5] PCI: qcom: Power down PHY via PARF_PHY_CTRL before disabling rails/clocks
  2026-02-17 11:19 ` [PATCH v2 4/5] PCI: qcom: Power down PHY via PARF_PHY_CTRL before disabling rails/clocks Krishna Chaitanya Chundru
@ 2026-03-05  7:49   ` Manivannan Sadhasivam
  2026-03-05  8:56     ` Krishna Chaitanya Chundru
  0 siblings, 1 reply; 22+ messages in thread
From: Manivannan Sadhasivam @ 2026-03-05  7:49 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Bjorn Helgaas, Will Deacon, linux-pci, linux-kernel,
	linux-arm-msm, linux-arm-kernel, jonathanh, bjorn.andersson

On Tue, Feb 17, 2026 at 04:49:09PM +0530, Krishna Chaitanya Chundru wrote:
> Some Qcom PCIe controller variants bring the PHY out of test power-down
> (PHY_TEST_PWR_DOWN) during init. When the link is later transitioned
> towards D3cold and the driver disables PCIe clocks and/or regulators
> without explicitly re-asserting PHY_TEST_PWR_DOWN, the PHY can remain
> partially powered, leading to avoidable power leakage.
> 
> Update the init-path comments to reflect that PARF_PHY_CTRL is used to
> power the PHY on. Also, for controller revisions that enable PHY power
> in init (2.3.2, 2.3.3, 2.7.0 and 2.9.0), explicitly power the PHY down
> via PARF_PHY_CTRL in the deinit path before disabling clocks/regulators.
> 
> This ensures the PHY is put into a defined low-power state prior to
> removing its supplies, preventing leakage when entering D3cold.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index 2c4dc7134e006d3530a809f1bcc1a6488d4632ad..b02c19bbdf2ea5db252c2a0281a569bb3a0cc497 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -513,7 +513,7 @@ static int qcom_pcie_post_init_2_1_0(struct qcom_pcie *pcie)
>  	u32 val;
>  	int ret;
>  
> -	/* enable PCIe clocks and resets */
> +	/* PHY power ON */

This comment is confusing since we already have phy_power_on() API. What does
really happen in the 'test power down' case?

- Mani

>  	val = readl(pcie->parf + PARF_PHY_CTRL);
>  	val &= ~PHY_TEST_PWR_DOWN;
>  	writel(val, pcie->parf + PARF_PHY_CTRL);
> @@ -680,6 +680,12 @@ static int qcom_pcie_get_resources_2_3_2(struct qcom_pcie *pcie)
>  static void qcom_pcie_deinit_2_3_2(struct qcom_pcie *pcie)
>  {
>  	struct qcom_pcie_resources_2_3_2 *res = &pcie->res.v2_3_2;
> +	u32 val;
> +
> +	/* PHY Power down */
> +	val = readl(pcie->parf + PARF_PHY_CTRL);
> +	val |= PHY_TEST_PWR_DOWN;
> +	writel(val, pcie->parf + PARF_PHY_CTRL);
>  
>  	clk_bulk_disable_unprepare(res->num_clks, res->clks);
>  	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
> @@ -712,7 +718,7 @@ static int qcom_pcie_post_init_2_3_2(struct qcom_pcie *pcie)
>  {
>  	u32 val;
>  
> -	/* enable PCIe clocks and resets */
> +	/* PHY Power ON */
>  	val = readl(pcie->parf + PARF_PHY_CTRL);
>  	val &= ~PHY_TEST_PWR_DOWN;
>  	writel(val, pcie->parf + PARF_PHY_CTRL);
> @@ -844,6 +850,12 @@ static int qcom_pcie_get_resources_2_3_3(struct qcom_pcie *pcie)
>  static void qcom_pcie_deinit_2_3_3(struct qcom_pcie *pcie)
>  {
>  	struct qcom_pcie_resources_2_3_3 *res = &pcie->res.v2_3_3;
> +	u32 val;
> +
> +	/* PHY Power down */
> +	val = readl(pcie->parf + PARF_PHY_CTRL);
> +	val |= PHY_TEST_PWR_DOWN;
> +	writel(val, pcie->parf + PARF_PHY_CTRL);
>  
>  	clk_bulk_disable_unprepare(res->num_clks, res->clks);
>  }
> @@ -994,7 +1006,7 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>  	/* configure PCIe to RC mode */
>  	writel(DEVICE_TYPE_RC, pcie->parf + PARF_DEVICE_TYPE);
>  
> -	/* enable PCIe clocks and resets */
> +	/* PHY power ON */
>  	val = readl(pcie->parf + PARF_PHY_CTRL);
>  	val &= ~PHY_TEST_PWR_DOWN;
>  	writel(val, pcie->parf + PARF_PHY_CTRL);
> @@ -1065,6 +1077,12 @@ static void qcom_pcie_host_post_init_2_7_0(struct qcom_pcie *pcie)
>  static void qcom_pcie_deinit_2_7_0(struct qcom_pcie *pcie)
>  {
>  	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
> +	u32 val;
> +
> +	/* PHY Power down */
> +	val = readl(pcie->parf + PARF_PHY_CTRL);
> +	val |= PHY_TEST_PWR_DOWN;
> +	writel(val, pcie->parf + PARF_PHY_CTRL);
>  
>  	clk_bulk_disable_unprepare(res->num_clks, res->clks);
>  
> @@ -1169,6 +1187,12 @@ static int qcom_pcie_get_resources_2_9_0(struct qcom_pcie *pcie)
>  static void qcom_pcie_deinit_2_9_0(struct qcom_pcie *pcie)
>  {
>  	struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
> +	u32 val;
> +
> +	/* PHY Power down */
> +	val = readl(pcie->parf + PARF_PHY_CTRL);
> +	val |= PHY_TEST_PWR_DOWN;
> +	writel(val, pcie->parf + PARF_PHY_CTRL);
>  
>  	clk_bulk_disable_unprepare(res->num_clks, res->clks);
>  }
> 
> -- 
> 2.34.1
> 

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 5/5] PCI: qcom: Add D3cold support
  2026-02-17 11:19 ` [PATCH v2 5/5] PCI: qcom: Add D3cold support Krishna Chaitanya Chundru
@ 2026-03-05  7:58   ` Manivannan Sadhasivam
  2026-03-05  9:00     ` Krishna Chaitanya Chundru
  0 siblings, 1 reply; 22+ messages in thread
From: Manivannan Sadhasivam @ 2026-03-05  7:58 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Bjorn Helgaas, Will Deacon, linux-pci, linux-kernel,
	linux-arm-msm, linux-arm-kernel, jonathanh, bjorn.andersson

On Tue, Feb 17, 2026 at 04:49:10PM +0530, Krishna Chaitanya Chundru wrote:
> Add support for transitioning Qcom PCIe controllers into D3cold by

You cannot transition a 'PCIe controller' to D3Cold state, but only the
endpoints and bridges.

> integrating with the DWC core suspend/resume helpers.
> 
> Implement PME_TurnOff message generation via ELBI_SYS_CTRL and hook it
> into the DWC host operations so the controller follows the standard
> PME_TurnOff-based power-down sequence before entering D3cold.
> 
> When the link is suspended into D3cold, fully tear down interconnect

You cannot suspend a link into D3Cold. Link and D-State are different.

> bandwidth, OPP votes. If D3cold is not entered, retain existing behavior
> by keeping the required interconnect and OPP votes.
> 
> Drop the qcom_pcie::suspended flag and rely on the existing
> dw_pcie::suspended state, which now drives both the power-management
> flow and the interconnect/OPP handling.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 121 ++++++++++++++++++++-------------
>  1 file changed, 74 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index b02c19bbdf2ea5db252c2a0281a569bb3a0cc497..37442bbe588c36b0b0414cc4d0016da2d8424a87 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -145,6 +145,7 @@

[...]

> -	if (pcie->suspended) {
> -		ret = qcom_pcie_host_init(&pcie->pci->pp);
> -		if (ret)
> -			return ret;
> +		ret = icc_enable(pcie->icc_mem);
> +		if (ret) {
> +			dev_err(dev, "Failed to enable PCIe-MEM interconnect path: %d\n", ret);
> +			goto disable_icc_cpu;
> +		}
>  
> -		pcie->suspended = false;
> +		/*
> +		 * Ignore -ETIMEDOUT here since it is expected when no endpoint is
> +		 * connected to the PCIe link.
> +		 */
> +		ret = dw_pcie_resume_noirq(pcie->pci);
> +		if (ret && (ret != -ETIMEDOUT))

No, dw_pcie_resume_noirq() was reworked to return -ETIMEDOUT to indicate a hard
failure. If the device is not found, it will return -ENODEV. So you should
fail the resume if -ETIMEDOUT is returned.

- Mani

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 4/5] PCI: qcom: Power down PHY via PARF_PHY_CTRL before disabling rails/clocks
  2026-03-05  7:49   ` Manivannan Sadhasivam
@ 2026-03-05  8:56     ` Krishna Chaitanya Chundru
  2026-03-05  9:18       ` Manivannan Sadhasivam
  0 siblings, 1 reply; 22+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-03-05  8:56 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Bjorn Helgaas, Will Deacon, linux-pci, linux-kernel,
	linux-arm-msm, linux-arm-kernel, jonathanh, bjorn.andersson



On 3/5/2026 1:19 PM, Manivannan Sadhasivam wrote:
> On Tue, Feb 17, 2026 at 04:49:09PM +0530, Krishna Chaitanya Chundru wrote:
>> Some Qcom PCIe controller variants bring the PHY out of test power-down
>> (PHY_TEST_PWR_DOWN) during init. When the link is later transitioned
>> towards D3cold and the driver disables PCIe clocks and/or regulators
>> without explicitly re-asserting PHY_TEST_PWR_DOWN, the PHY can remain
>> partially powered, leading to avoidable power leakage.
>>
>> Update the init-path comments to reflect that PARF_PHY_CTRL is used to
>> power the PHY on. Also, for controller revisions that enable PHY power
>> in init (2.3.2, 2.3.3, 2.7.0 and 2.9.0), explicitly power the PHY down
>> via PARF_PHY_CTRL in the deinit path before disabling clocks/regulators.
>>
>> This ensures the PHY is put into a defined low-power state prior to
>> removing its supplies, preventing leakage when entering D3cold.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 30 +++++++++++++++++++++++++++---
>>   1 file changed, 27 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index 2c4dc7134e006d3530a809f1bcc1a6488d4632ad..b02c19bbdf2ea5db252c2a0281a569bb3a0cc497 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -513,7 +513,7 @@ static int qcom_pcie_post_init_2_1_0(struct qcom_pcie *pcie)
>>   	u32 val;
>>   	int ret;
>>   
>> -	/* enable PCIe clocks and resets */
>> +	/* PHY power ON */
> This comment is confusing since we already have phy_power_on() API. What does
> really happen in the 'test power down' case?
QCOM PCIe controller wrapper has way to force the entire PHY into lowest 
power
state by turning everything off, without this bit being cleared the phy 
will not be
powered on even after phy_power_on(), we can think this as a kind of switch
from the controller side to power on phy.

- Krishna Chaitanya.
> - Mani
>
>>   	val = readl(pcie->parf + PARF_PHY_CTRL);
>>   	val &= ~PHY_TEST_PWR_DOWN;
>>   	writel(val, pcie->parf + PARF_PHY_CTRL);
>> @@ -680,6 +680,12 @@ static int qcom_pcie_get_resources_2_3_2(struct qcom_pcie *pcie)
>>   static void qcom_pcie_deinit_2_3_2(struct qcom_pcie *pcie)
>>   {
>>   	struct qcom_pcie_resources_2_3_2 *res = &pcie->res.v2_3_2;
>> +	u32 val;
>> +
>> +	/* PHY Power down */
>> +	val = readl(pcie->parf + PARF_PHY_CTRL);
>> +	val |= PHY_TEST_PWR_DOWN;
>> +	writel(val, pcie->parf + PARF_PHY_CTRL);
>>   
>>   	clk_bulk_disable_unprepare(res->num_clks, res->clks);
>>   	regulator_bulk_disable(ARRAY_SIZE(res->supplies), res->supplies);
>> @@ -712,7 +718,7 @@ static int qcom_pcie_post_init_2_3_2(struct qcom_pcie *pcie)
>>   {
>>   	u32 val;
>>   
>> -	/* enable PCIe clocks and resets */
>> +	/* PHY Power ON */
>>   	val = readl(pcie->parf + PARF_PHY_CTRL);
>>   	val &= ~PHY_TEST_PWR_DOWN;
>>   	writel(val, pcie->parf + PARF_PHY_CTRL);
>> @@ -844,6 +850,12 @@ static int qcom_pcie_get_resources_2_3_3(struct qcom_pcie *pcie)
>>   static void qcom_pcie_deinit_2_3_3(struct qcom_pcie *pcie)
>>   {
>>   	struct qcom_pcie_resources_2_3_3 *res = &pcie->res.v2_3_3;
>> +	u32 val;
>> +
>> +	/* PHY Power down */
>> +	val = readl(pcie->parf + PARF_PHY_CTRL);
>> +	val |= PHY_TEST_PWR_DOWN;
>> +	writel(val, pcie->parf + PARF_PHY_CTRL);
>>   
>>   	clk_bulk_disable_unprepare(res->num_clks, res->clks);
>>   }
>> @@ -994,7 +1006,7 @@ static int qcom_pcie_init_2_7_0(struct qcom_pcie *pcie)
>>   	/* configure PCIe to RC mode */
>>   	writel(DEVICE_TYPE_RC, pcie->parf + PARF_DEVICE_TYPE);
>>   
>> -	/* enable PCIe clocks and resets */
>> +	/* PHY power ON */
>>   	val = readl(pcie->parf + PARF_PHY_CTRL);
>>   	val &= ~PHY_TEST_PWR_DOWN;
>>   	writel(val, pcie->parf + PARF_PHY_CTRL);
>> @@ -1065,6 +1077,12 @@ static void qcom_pcie_host_post_init_2_7_0(struct qcom_pcie *pcie)
>>   static void qcom_pcie_deinit_2_7_0(struct qcom_pcie *pcie)
>>   {
>>   	struct qcom_pcie_resources_2_7_0 *res = &pcie->res.v2_7_0;
>> +	u32 val;
>> +
>> +	/* PHY Power down */
>> +	val = readl(pcie->parf + PARF_PHY_CTRL);
>> +	val |= PHY_TEST_PWR_DOWN;
>> +	writel(val, pcie->parf + PARF_PHY_CTRL);
>>   
>>   	clk_bulk_disable_unprepare(res->num_clks, res->clks);
>>   
>> @@ -1169,6 +1187,12 @@ static int qcom_pcie_get_resources_2_9_0(struct qcom_pcie *pcie)
>>   static void qcom_pcie_deinit_2_9_0(struct qcom_pcie *pcie)
>>   {
>>   	struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
>> +	u32 val;
>> +
>> +	/* PHY Power down */
>> +	val = readl(pcie->parf + PARF_PHY_CTRL);
>> +	val |= PHY_TEST_PWR_DOWN;
>> +	writel(val, pcie->parf + PARF_PHY_CTRL);
>>   
>>   	clk_bulk_disable_unprepare(res->num_clks, res->clks);
>>   }
>>
>> -- 
>> 2.34.1
>>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 5/5] PCI: qcom: Add D3cold support
  2026-03-05  7:58   ` Manivannan Sadhasivam
@ 2026-03-05  9:00     ` Krishna Chaitanya Chundru
  2026-03-05  9:14       ` Manivannan Sadhasivam
  0 siblings, 1 reply; 22+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-03-05  9:00 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Bjorn Helgaas, Will Deacon, linux-pci, linux-kernel,
	linux-arm-msm, linux-arm-kernel, jonathanh, bjorn.andersson



On 3/5/2026 1:28 PM, Manivannan Sadhasivam wrote:
> On Tue, Feb 17, 2026 at 04:49:10PM +0530, Krishna Chaitanya Chundru wrote:
>> Add support for transitioning Qcom PCIe controllers into D3cold by
> You cannot transition a 'PCIe controller' to D3Cold state, but only the
> endpoints and bridges.
>
>> integrating with the DWC core suspend/resume helpers.
>>
>> Implement PME_TurnOff message generation via ELBI_SYS_CTRL and hook it
>> into the DWC host operations so the controller follows the standard
>> PME_TurnOff-based power-down sequence before entering D3cold.
>>
>> When the link is suspended into D3cold, fully tear down interconnect
> You cannot suspend a link into D3Cold. Link and D-State are different.
>
>> bandwidth, OPP votes. If D3cold is not entered, retain existing behavior
>> by keeping the required interconnect and OPP votes.
>>
>> Drop the qcom_pcie::suspended flag and rely on the existing
>> dw_pcie::suspended state, which now drives both the power-management
>> flow and the interconnect/OPP handling.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 121 ++++++++++++++++++++-------------
>>   1 file changed, 74 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>> index b02c19bbdf2ea5db252c2a0281a569bb3a0cc497..37442bbe588c36b0b0414cc4d0016da2d8424a87 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>> @@ -145,6 +145,7 @@
> [...]
>
>> -	if (pcie->suspended) {
>> -		ret = qcom_pcie_host_init(&pcie->pci->pp);
>> -		if (ret)
>> -			return ret;
>> +		ret = icc_enable(pcie->icc_mem);
>> +		if (ret) {
>> +			dev_err(dev, "Failed to enable PCIe-MEM interconnect path: %d\n", ret);
>> +			goto disable_icc_cpu;
>> +		}
>>   
>> -		pcie->suspended = false;
>> +		/*
>> +		 * Ignore -ETIMEDOUT here since it is expected when no endpoint is
>> +		 * connected to the PCIe link.
>> +		 */
>> +		ret = dw_pcie_resume_noirq(pcie->pci);
>> +		if (ret && (ret != -ETIMEDOUT))
> No, dw_pcie_resume_noirq() was reworked to return -ETIMEDOUT to indicate a hard
> failure. If the device is not found, it will return -ENODEV. So you should
> fail the resume if -ETIMEDOUT is returned.
Ack, didn't noticed the reworked changes, I will change -ETIMEDOUT to 
-ENODEV.

- Krishna Chaitanya.
> - Mani
>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 5/5] PCI: qcom: Add D3cold support
  2026-03-05  9:00     ` Krishna Chaitanya Chundru
@ 2026-03-05  9:14       ` Manivannan Sadhasivam
  2026-03-05  9:25         ` Krishna Chaitanya Chundru
  0 siblings, 1 reply; 22+ messages in thread
From: Manivannan Sadhasivam @ 2026-03-05  9:14 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Bjorn Helgaas, Will Deacon, linux-pci, linux-kernel,
	linux-arm-msm, linux-arm-kernel, jonathanh, bjorn.andersson

On Thu, Mar 05, 2026 at 02:30:17PM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 3/5/2026 1:28 PM, Manivannan Sadhasivam wrote:
> > On Tue, Feb 17, 2026 at 04:49:10PM +0530, Krishna Chaitanya Chundru wrote:
> > > Add support for transitioning Qcom PCIe controllers into D3cold by
> > You cannot transition a 'PCIe controller' to D3Cold state, but only the
> > endpoints and bridges.
> > 
> > > integrating with the DWC core suspend/resume helpers.
> > > 
> > > Implement PME_TurnOff message generation via ELBI_SYS_CTRL and hook it
> > > into the DWC host operations so the controller follows the standard
> > > PME_TurnOff-based power-down sequence before entering D3cold.
> > > 
> > > When the link is suspended into D3cold, fully tear down interconnect
> > You cannot suspend a link into D3Cold. Link and D-State are different.
> > 
> > > bandwidth, OPP votes. If D3cold is not entered, retain existing behavior
> > > by keeping the required interconnect and OPP votes.
> > > 
> > > Drop the qcom_pcie::suspended flag and rely on the existing
> > > dw_pcie::suspended state, which now drives both the power-management
> > > flow and the interconnect/OPP handling.
> > > 
> > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > ---
> > >   drivers/pci/controller/dwc/pcie-qcom.c | 121 ++++++++++++++++++++-------------
> > >   1 file changed, 74 insertions(+), 47 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > index b02c19bbdf2ea5db252c2a0281a569bb3a0cc497..37442bbe588c36b0b0414cc4d0016da2d8424a87 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > @@ -145,6 +145,7 @@
> > [...]
> > 
> > > -	if (pcie->suspended) {
> > > -		ret = qcom_pcie_host_init(&pcie->pci->pp);
> > > -		if (ret)
> > > -			return ret;
> > > +		ret = icc_enable(pcie->icc_mem);
> > > +		if (ret) {
> > > +			dev_err(dev, "Failed to enable PCIe-MEM interconnect path: %d\n", ret);
> > > +			goto disable_icc_cpu;
> > > +		}
> > > -		pcie->suspended = false;
> > > +		/*
> > > +		 * Ignore -ETIMEDOUT here since it is expected when no endpoint is
> > > +		 * connected to the PCIe link.
> > > +		 */
> > > +		ret = dw_pcie_resume_noirq(pcie->pci);
> > > +		if (ret && (ret != -ETIMEDOUT))
> > No, dw_pcie_resume_noirq() was reworked to return -ETIMEDOUT to indicate a hard
> > failure. If the device is not found, it will return -ENODEV. So you should
> > fail the resume if -ETIMEDOUT is returned.
> Ack, didn't noticed the reworked changes, I will change -ETIMEDOUT to
> -ENODEV.
> 

No, that's what not I meant. I meant, you should do:

	if (ret == -ETIMEDOUT)
		goto fail;

- Mani

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 4/5] PCI: qcom: Power down PHY via PARF_PHY_CTRL before disabling rails/clocks
  2026-03-05  8:56     ` Krishna Chaitanya Chundru
@ 2026-03-05  9:18       ` Manivannan Sadhasivam
  2026-03-06 10:31         ` Krishna Chaitanya Chundru
  0 siblings, 1 reply; 22+ messages in thread
From: Manivannan Sadhasivam @ 2026-03-05  9:18 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Bjorn Helgaas, Will Deacon, linux-pci, linux-kernel,
	linux-arm-msm, linux-arm-kernel, jonathanh, bjorn.andersson

On Thu, Mar 05, 2026 at 02:26:22PM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 3/5/2026 1:19 PM, Manivannan Sadhasivam wrote:
> > On Tue, Feb 17, 2026 at 04:49:09PM +0530, Krishna Chaitanya Chundru wrote:
> > > Some Qcom PCIe controller variants bring the PHY out of test power-down
> > > (PHY_TEST_PWR_DOWN) during init. When the link is later transitioned
> > > towards D3cold and the driver disables PCIe clocks and/or regulators
> > > without explicitly re-asserting PHY_TEST_PWR_DOWN, the PHY can remain
> > > partially powered, leading to avoidable power leakage.
> > > 
> > > Update the init-path comments to reflect that PARF_PHY_CTRL is used to
> > > power the PHY on. Also, for controller revisions that enable PHY power
> > > in init (2.3.2, 2.3.3, 2.7.0 and 2.9.0), explicitly power the PHY down
> > > via PARF_PHY_CTRL in the deinit path before disabling clocks/regulators.
> > > 
> > > This ensures the PHY is put into a defined low-power state prior to
> > > removing its supplies, preventing leakage when entering D3cold.
> > > 
> > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > ---
> > >   drivers/pci/controller/dwc/pcie-qcom.c | 30 +++++++++++++++++++++++++++---
> > >   1 file changed, 27 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > index 2c4dc7134e006d3530a809f1bcc1a6488d4632ad..b02c19bbdf2ea5db252c2a0281a569bb3a0cc497 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > @@ -513,7 +513,7 @@ static int qcom_pcie_post_init_2_1_0(struct qcom_pcie *pcie)
> > >   	u32 val;
> > >   	int ret;
> > > -	/* enable PCIe clocks and resets */
> > > +	/* PHY power ON */
> > This comment is confusing since we already have phy_power_on() API. What does
> > really happen in the 'test power down' case?
> QCOM PCIe controller wrapper has way to force the entire PHY into lowest
> power
> state by turning everything off, without this bit being cleared the phy will
> not be
> powered on even after phy_power_on(), we can think this as a kind of switch
> from the controller side to power on phy.
> 

We never cared to set/clear this bit so far. So I'm assuming that if we simply
set it during init, it will not do any harm and allow the PHY to fully power
down itself when phy_power_off() is called?

- Mani

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 5/5] PCI: qcom: Add D3cold support
  2026-03-05  9:14       ` Manivannan Sadhasivam
@ 2026-03-05  9:25         ` Krishna Chaitanya Chundru
  2026-03-05  9:34           ` Manivannan Sadhasivam
  0 siblings, 1 reply; 22+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-03-05  9:25 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Bjorn Helgaas, Will Deacon, linux-pci, linux-kernel,
	linux-arm-msm, linux-arm-kernel, jonathanh, bjorn.andersson



On 3/5/2026 2:44 PM, Manivannan Sadhasivam wrote:
> On Thu, Mar 05, 2026 at 02:30:17PM +0530, Krishna Chaitanya Chundru wrote:
>>
>> On 3/5/2026 1:28 PM, Manivannan Sadhasivam wrote:
>>> On Tue, Feb 17, 2026 at 04:49:10PM +0530, Krishna Chaitanya Chundru wrote:
>>>> Add support for transitioning Qcom PCIe controllers into D3cold by
>>> You cannot transition a 'PCIe controller' to D3Cold state, but only the
>>> endpoints and bridges.
>>>
>>>> integrating with the DWC core suspend/resume helpers.
>>>>
>>>> Implement PME_TurnOff message generation via ELBI_SYS_CTRL and hook it
>>>> into the DWC host operations so the controller follows the standard
>>>> PME_TurnOff-based power-down sequence before entering D3cold.
>>>>
>>>> When the link is suspended into D3cold, fully tear down interconnect
>>> You cannot suspend a link into D3Cold. Link and D-State are different.
>>>
>>>> bandwidth, OPP votes. If D3cold is not entered, retain existing behavior
>>>> by keeping the required interconnect and OPP votes.
>>>>
>>>> Drop the qcom_pcie::suspended flag and rely on the existing
>>>> dw_pcie::suspended state, which now drives both the power-management
>>>> flow and the interconnect/OPP handling.
>>>>
>>>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>>>> ---
>>>>    drivers/pci/controller/dwc/pcie-qcom.c | 121 ++++++++++++++++++++-------------
>>>>    1 file changed, 74 insertions(+), 47 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>>>> index b02c19bbdf2ea5db252c2a0281a569bb3a0cc497..37442bbe588c36b0b0414cc4d0016da2d8424a87 100644
>>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>>> @@ -145,6 +145,7 @@
>>> [...]
>>>
>>>> -	if (pcie->suspended) {
>>>> -		ret = qcom_pcie_host_init(&pcie->pci->pp);
>>>> -		if (ret)
>>>> -			return ret;
>>>> +		ret = icc_enable(pcie->icc_mem);
>>>> +		if (ret) {
>>>> +			dev_err(dev, "Failed to enable PCIe-MEM interconnect path: %d\n", ret);
>>>> +			goto disable_icc_cpu;
>>>> +		}
>>>> -		pcie->suspended = false;
>>>> +		/*
>>>> +		 * Ignore -ETIMEDOUT here since it is expected when no endpoint is
>>>> +		 * connected to the PCIe link.
>>>> +		 */
>>>> +		ret = dw_pcie_resume_noirq(pcie->pci);
>>>> +		if (ret && (ret != -ETIMEDOUT))
>>> No, dw_pcie_resume_noirq() was reworked to return -ETIMEDOUT to indicate a hard
>>> failure. If the device is not found, it will return -ENODEV. So you should
>>> fail the resume if -ETIMEDOUT is returned.
>> Ack, didn't noticed the reworked changes, I will change -ETIMEDOUT to
>> -ENODEV.
>>
> No, that's what not I meant. I meant, you should do:
>
> 	if (ret == -ETIMEDOUT)
> 		goto fail;
there can be other failures also right, where we should fail,
like pci->pp.ops->init(&pci->pp); can return different error other than 
-ETIMEDOUT in that case we should fail here. - Krishna Chaitanya.
> - Mani
>



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 5/5] PCI: qcom: Add D3cold support
  2026-03-05  9:25         ` Krishna Chaitanya Chundru
@ 2026-03-05  9:34           ` Manivannan Sadhasivam
  0 siblings, 0 replies; 22+ messages in thread
From: Manivannan Sadhasivam @ 2026-03-05  9:34 UTC (permalink / raw)
  To: Krishna Chaitanya Chundru
  Cc: Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Bjorn Helgaas, Will Deacon, linux-pci, linux-kernel,
	linux-arm-msm, linux-arm-kernel, jonathanh, bjorn.andersson

On Thu, Mar 05, 2026 at 02:55:25PM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 3/5/2026 2:44 PM, Manivannan Sadhasivam wrote:
> > On Thu, Mar 05, 2026 at 02:30:17PM +0530, Krishna Chaitanya Chundru wrote:
> > > 
> > > On 3/5/2026 1:28 PM, Manivannan Sadhasivam wrote:
> > > > On Tue, Feb 17, 2026 at 04:49:10PM +0530, Krishna Chaitanya Chundru wrote:
> > > > > Add support for transitioning Qcom PCIe controllers into D3cold by
> > > > You cannot transition a 'PCIe controller' to D3Cold state, but only the
> > > > endpoints and bridges.
> > > > 
> > > > > integrating with the DWC core suspend/resume helpers.
> > > > > 
> > > > > Implement PME_TurnOff message generation via ELBI_SYS_CTRL and hook it
> > > > > into the DWC host operations so the controller follows the standard
> > > > > PME_TurnOff-based power-down sequence before entering D3cold.
> > > > > 
> > > > > When the link is suspended into D3cold, fully tear down interconnect
> > > > You cannot suspend a link into D3Cold. Link and D-State are different.
> > > > 
> > > > > bandwidth, OPP votes. If D3cold is not entered, retain existing behavior
> > > > > by keeping the required interconnect and OPP votes.
> > > > > 
> > > > > Drop the qcom_pcie::suspended flag and rely on the existing
> > > > > dw_pcie::suspended state, which now drives both the power-management
> > > > > flow and the interconnect/OPP handling.
> > > > > 
> > > > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > > > ---
> > > > >    drivers/pci/controller/dwc/pcie-qcom.c | 121 ++++++++++++++++++++-------------
> > > > >    1 file changed, 74 insertions(+), 47 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > index b02c19bbdf2ea5db252c2a0281a569bb3a0cc497..37442bbe588c36b0b0414cc4d0016da2d8424a87 100644
> > > > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > > > @@ -145,6 +145,7 @@
> > > > [...]
> > > > 
> > > > > -	if (pcie->suspended) {
> > > > > -		ret = qcom_pcie_host_init(&pcie->pci->pp);
> > > > > -		if (ret)
> > > > > -			return ret;
> > > > > +		ret = icc_enable(pcie->icc_mem);
> > > > > +		if (ret) {
> > > > > +			dev_err(dev, "Failed to enable PCIe-MEM interconnect path: %d\n", ret);
> > > > > +			goto disable_icc_cpu;
> > > > > +		}
> > > > > -		pcie->suspended = false;
> > > > > +		/*
> > > > > +		 * Ignore -ETIMEDOUT here since it is expected when no endpoint is
> > > > > +		 * connected to the PCIe link.
> > > > > +		 */
> > > > > +		ret = dw_pcie_resume_noirq(pcie->pci);
> > > > > +		if (ret && (ret != -ETIMEDOUT))
> > > > No, dw_pcie_resume_noirq() was reworked to return -ETIMEDOUT to indicate a hard
> > > > failure. If the device is not found, it will return -ENODEV. So you should
> > > > fail the resume if -ETIMEDOUT is returned.
> > > Ack, didn't noticed the reworked changes, I will change -ETIMEDOUT to
> > > -ENODEV.
> > > 
> > No, that's what not I meant. I meant, you should do:
> > 
> > 	if (ret == -ETIMEDOUT)
> > 		goto fail;
> there can be other failures also right, where we should fail,
> like pci->pp.ops->init(&pci->pp); can return different error other than
> -ETIMEDOUT in that case we should fail here. - Krishna Chaitanya.

Hmm, I overlooked the init() callback. In that case, you should skip both
-ENODEV and -EIO and fail resume() for other errors.

- Mani

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


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 4/5] PCI: qcom: Power down PHY via PARF_PHY_CTRL before disabling rails/clocks
  2026-03-05  9:18       ` Manivannan Sadhasivam
@ 2026-03-06 10:31         ` Krishna Chaitanya Chundru
  0 siblings, 0 replies; 22+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-03-06 10:31 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
	Rob Herring, Bjorn Helgaas, Will Deacon, linux-pci, linux-kernel,
	linux-arm-msm, linux-arm-kernel, jonathanh, bjorn.andersson



On 3/5/2026 2:48 PM, Manivannan Sadhasivam wrote:
> On Thu, Mar 05, 2026 at 02:26:22PM +0530, Krishna Chaitanya Chundru wrote:
>>
>> On 3/5/2026 1:19 PM, Manivannan Sadhasivam wrote:
>>> On Tue, Feb 17, 2026 at 04:49:09PM +0530, Krishna Chaitanya Chundru wrote:
>>>> Some Qcom PCIe controller variants bring the PHY out of test power-down
>>>> (PHY_TEST_PWR_DOWN) during init. When the link is later transitioned
>>>> towards D3cold and the driver disables PCIe clocks and/or regulators
>>>> without explicitly re-asserting PHY_TEST_PWR_DOWN, the PHY can remain
>>>> partially powered, leading to avoidable power leakage.
>>>>
>>>> Update the init-path comments to reflect that PARF_PHY_CTRL is used to
>>>> power the PHY on. Also, for controller revisions that enable PHY power
>>>> in init (2.3.2, 2.3.3, 2.7.0 and 2.9.0), explicitly power the PHY down
>>>> via PARF_PHY_CTRL in the deinit path before disabling clocks/regulators.
>>>>
>>>> This ensures the PHY is put into a defined low-power state prior to
>>>> removing its supplies, preventing leakage when entering D3cold.
>>>>
>>>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>>>> ---
>>>>    drivers/pci/controller/dwc/pcie-qcom.c | 30 +++++++++++++++++++++++++++---
>>>>    1 file changed, 27 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
>>>> index 2c4dc7134e006d3530a809f1bcc1a6488d4632ad..b02c19bbdf2ea5db252c2a0281a569bb3a0cc497 100644
>>>> --- a/drivers/pci/controller/dwc/pcie-qcom.c
>>>> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
>>>> @@ -513,7 +513,7 @@ static int qcom_pcie_post_init_2_1_0(struct qcom_pcie *pcie)
>>>>    	u32 val;
>>>>    	int ret;
>>>> -	/* enable PCIe clocks and resets */
>>>> +	/* PHY power ON */
>>> This comment is confusing since we already have phy_power_on() API. What does
>>> really happen in the 'test power down' case?
>> QCOM PCIe controller wrapper has way to force the entire PHY into lowest
>> power
>> state by turning everything off, without this bit being cleared the phy will
>> not be
>> powered on even after phy_power_on(), we can think this as a kind of switch
>> from the controller side to power on phy.
>>
> We never cared to set/clear this bit so far. So I'm assuming that if we simply
> set it during init, it will not do any harm and allow the PHY to fully power
> down itself when phy_power_off() is called?
we are already doing set/clear of this bit, its not newly introduced 
one, I am
updating the  comment  to correctly reflect hw behaviour. PHY power on 
looks little
confusing I will update the commet to "Force PHY out of lowest mode".

- Krishna Chaitanya.
> - Mani
>



^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2026-03-06 10:32 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-17 11:19 [PATCH v2 0/5] PCI: qcom: Add D3cold support Krishna Chaitanya Chundru
2026-02-17 11:19 ` [PATCH v2 1/5] PCI: host-common: Add helper to determine host bridge D3cold eligibility Krishna Chaitanya Chundru
2026-02-23 18:10   ` Bjorn Helgaas
2026-02-23 18:46     ` Rafael J. Wysocki
2026-02-23 19:55       ` Bjorn Helgaas
2026-02-23 20:03         ` Rafael J. Wysocki
2026-03-05  7:42   ` Manivannan Sadhasivam
2026-02-17 11:19 ` [PATCH v2 2/5] PCI: dwc: Use common D3cold eligibility helper in suspend path Krishna Chaitanya Chundru
2026-02-17 11:19 ` [PATCH v2 3/5] PCI: qcom: Add .get_ltssm() helper Krishna Chaitanya Chundru
2026-03-05  7:46   ` Manivannan Sadhasivam
2026-02-17 11:19 ` [PATCH v2 4/5] PCI: qcom: Power down PHY via PARF_PHY_CTRL before disabling rails/clocks Krishna Chaitanya Chundru
2026-03-05  7:49   ` Manivannan Sadhasivam
2026-03-05  8:56     ` Krishna Chaitanya Chundru
2026-03-05  9:18       ` Manivannan Sadhasivam
2026-03-06 10:31         ` Krishna Chaitanya Chundru
2026-02-17 11:19 ` [PATCH v2 5/5] PCI: qcom: Add D3cold support Krishna Chaitanya Chundru
2026-03-05  7:58   ` Manivannan Sadhasivam
2026-03-05  9:00     ` Krishna Chaitanya Chundru
2026-03-05  9:14       ` Manivannan Sadhasivam
2026-03-05  9:25         ` Krishna Chaitanya Chundru
2026-03-05  9:34           ` Manivannan Sadhasivam
2026-02-17 15:40 ` [PATCH v2 0/5] " Neil Armstrong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox