[PATCH 0/3] PCI: qcom: Add D3cold support

[PATCH 0/3] PCI: qcom: Add D3cold support

[Krishna Chaitanya Chundru]

This series adds a common helper to determine when a PCIe host bridge
can safely enter D3cold, converts the DesignWare host driver to use that
helper, and enables D3cold support for Qualcomm PCIe controllers.

The first patch introduces pci_host_common_can_enter_d3cold() in
pci-host-common. The helper walks all devices on the bridge's bus and
only allows the host bridge to enter D3cold if all PCIe endpoints are
already in PCI_D3hot. For devices that may wake the system, it
additionally requires that the device support PME wakeup from D3cold
(via WAKE#). Devices without wakeup enabled are not restricted by this
check and may be left in D3cold.

The second patch updates the DesignWare host driver to use this common
helper in dw_pcie_suspend_noirq(). Previously, the driver skipped
putting the link into L2 / device 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 helper's requirement that all
endpoints be in D3hot before the host bridge may enter D3cold. Using the
shared helper removes this coarse heuristic and centralizes the D3cold
eligibility policy.

The third patch enables D3cold support for Qualcomm PCIe controllers. It
adds pme_turn_off() support and switches to the DesignWare common
suspend/resume methods for device D3cold entry and exit. If the
controller is not kept in D3cold, the existing paths are used so that
ICC votes, OPP votes, and other resources remain managed as before. In
addition, qcom_pcie_deinit_2_7_0() is updated to explicitly disable
PCIe clocks and resets in the controller, and the now-unused "suspended"
flag is removed from struct qcom_pcie.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Krishna Chaitanya Chundru (3):
      PCI: host-common: Add shared D3cold eligibility helper for host bridges
      PCI: dwc: Use common D3cold eligibility helper in suspend path
      PCI: qcom: Add D3cold support

 drivers/pci/controller/dwc/pcie-designware-host.c |   7 +-
 drivers/pci/controller/dwc/pcie-designware.h      |   1 +
 drivers/pci/controller/dwc/pcie-qcom.c            | 114 +++++++++++++---------
 drivers/pci/controller/pci-host-common.c          |  29 ++++++
 drivers/pci/controller/pci-host-common.h          |   2 +
 5 files changed, 101 insertions(+), 52 deletions(-)
---
base-commit: 590a64365d9bcc13ee644a3e73ffdc3df26cf23c
change-id: 20251229-d3cold-bf99921960bb

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

[PATCH 1/3] PCI: host-common: Add shared D3cold eligibility helper for host bridges

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

The helper walks all devices on the bridge's bus and only allows the
host bridge to enter D3cold if all PCIe endpoints are already in
PCI_D3hot. For devices that may wake the system, it additionally
requires that the device supports PME wakeup from D3cold(with WAKE#).
Devices without wakeup enabled are not restricted by this check and can
be allowed to keep device in D3cold.

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

diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
index c473e7c03bacad2de07c798768f99652443431e0..225472c5ac82c6c5b44257d68a0fc503ec046ff1 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -106,5 +106,34 @@ 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->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;
+}
+
+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

[PATCH 2/3] PCI: dwc: Use common D3cold eligibility helper in suspend path

[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 | 7 +------
 drivers/pci/controller/dwc/pcie-designware.h      | 1 +
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 372207c33a857b4c98572bb1e9b61fa0080bc871..2c8056761addf7febc1b0e06ddf8ba4dd4ad1684 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -1157,15 +1157,10 @@ 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);
 	u32 val;
 	int ret;
 
-	/*
-	 * 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) {
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 31685951a080456b8834aab2bf79a36c78f46639..18d8f7d5d23088b2fa177e84a21d900c98850fcd 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -26,6 +26,7 @@
 #include <linux/pci-epc.h>
 #include <linux/pci-epf.h>
 
+#include "../pci-host-common.h"
 #include "../../pci.h"
 
 /* DWC PCIe IP-core versions (native support since v4.70a) */

-- 
2.34.1

[PATCH 3/3] PCI: qcom: Add D3cold support

[Krishna Chaitanya Chundru]

Add pme_turn_off() support and use DWC common suspend resume methods
for device D3cold entry & exit. If the device is not kept in D3cold
use existing methods like keeping icc votes, opp votes etc.. intact.

In qcom_pcie_deinit_2_7_0(), explicitly disable PCIe clocks and resets
in the controller.

Remove suspended flag from qcom_pcie structure as it is no longer needed.

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

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 7b92e7a1c0d9364a9cefe1450818f9cbfc7fd3ac..bb4e5a29c452a6c0d4b31b2a9ff3aa62b984fb64 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -150,6 +150,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;
 };
 
@@ -1075,6 +1075,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;
+
+	/* Disable PCIe clocks and resets */
+	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);
 
@@ -1355,10 +1361,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 */
@@ -2016,53 +2030,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;
 }
@@ -2076,20 +2088,30 @@ 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);
+		ret = icc_enable(pcie->icc_mem);
+		if (ret) {
+			dev_err(dev, "Failed to enable PCIe-MEM interconnect path: %d\n", ret);
+			return ret;
+		}
+		ret = dw_pcie_resume_noirq(pcie->pci);
 		if (ret)
 			return ret;
-
-		pcie->suspended = false;
+	} 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);

-- 
2.34.1

Re: [PATCH 3/3] PCI: qcom: Add D3cold support

[Konrad Dybcio]

On 1/28/26 12:40 PM, Krishna Chaitanya Chundru wrote:
> Add pme_turn_off() support and use DWC common suspend resume methods
> for device D3cold entry & exit. If the device is not kept in D3cold
> use existing methods like keeping icc votes, opp votes etc.. intact.
> 
> In qcom_pcie_deinit_2_7_0(), explicitly disable PCIe clocks and resets
> in the controller.
> 
> Remove suspended flag from qcom_pcie structure as it is no longer needed.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---

[...]

> +		/*
> +		 * 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.
> +		 */

I think someone internally once tracked down what that access was

Can we fix that instead?

Konrad

Re: [PATCH 1/3] PCI: host-common: Add shared D3cold eligibility helper for host bridges

[Bjorn Andersson]

On Wed, Jan 28, 2026 at 05:10:41PM +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.

Please read https://docs.kernel.org/process/submitting-patches.html#describe-your-changes

It clearly says that you're supposed to start your commit message with a
description of the problem you're solving. In fact, even after reading
the entire commit message a few times I only know what the patch does,
but it's not clear to me why this patch exists.

> 
> The helper walks all devices on the bridge's bus and only allows the
> host bridge to enter D3cold if all PCIe endpoints are already in
> PCI_D3hot.

The code below does walk the bus, but it doesn't allow/disallow anything
as far as I can tell, it queries their type, state, and if they are wake
capable?

> For devices that may wake the system, it additionally
> requires that the device supports PME wakeup from D3cold(with WAKE#).
> Devices without wakeup enabled are not restricted by this check and can
> be allowed to keep device in D3cold.
> 

Again, this code doesn't perform any action, it doesn't
allow/disallow/restrict the devices from doing anything, it merely
queries a bunch of things across all devices, and the commit message
fails to capture why this is useful.

> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  drivers/pci/controller/pci-host-common.c | 29 +++++++++++++++++++++++++++++
>  drivers/pci/controller/pci-host-common.h |  2 ++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> index c473e7c03bacad2de07c798768f99652443431e0..225472c5ac82c6c5b44257d68a0fc503ec046ff1 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c
> @@ -106,5 +106,34 @@ 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->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;
> +}
> +
> +bool pci_host_common_can_enter_d3cold(struct pci_host_bridge *bridge)

Please add kernel-doc for any EXPORT_SYMBOL() functions, so that it's
clear to the next guy what the API does.

Regards,
Bjorn

> +{
> +	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
> 
> 

Re: [PATCH 2/3] PCI: dwc: Use common D3cold eligibility helper in suspend path

[Bjorn Andersson]

On Wed, Jan 28, 2026 at 05:10:42PM +0530, Krishna Chaitanya Chundru wrote:
> 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 | 7 +------
>  drivers/pci/controller/dwc/pcie-designware.h      | 1 +
>  2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 372207c33a857b4c98572bb1e9b61fa0080bc871..2c8056761addf7febc1b0e06ddf8ba4dd4ad1684 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -1157,15 +1157,10 @@ 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);
>  	u32 val;
>  	int ret;
>  
> -	/*
> -	 * 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) {
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 31685951a080456b8834aab2bf79a36c78f46639..18d8f7d5d23088b2fa177e84a21d900c98850fcd 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -26,6 +26,7 @@
>  #include <linux/pci-epc.h>
>  #include <linux/pci-epf.h>
>  
> +#include "../pci-host-common.h"

Why doesn't this include go in the c file? I don't see that all c files
including pcie-designware.h now needs this.

Regards,
Bjorn

>  #include "../../pci.h"
>  
>  /* DWC PCIe IP-core versions (native support since v4.70a) */
> 
> -- 
> 2.34.1
> 
> 

Re: [PATCH 3/3] PCI: qcom: Add D3cold support

[Bjorn Andersson]

On Wed, Jan 28, 2026 at 05:10:43PM +0530, Krishna Chaitanya Chundru wrote:
> Add pme_turn_off() support and use DWC common suspend resume methods
> for device D3cold entry & exit. If the device is not kept in D3cold
> use existing methods like keeping icc votes, opp votes etc.. intact.
> 
> In qcom_pcie_deinit_2_7_0(), explicitly disable PCIe clocks and resets
> in the controller.
> 
> Remove suspended flag from qcom_pcie structure as it is no longer needed.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 114 ++++++++++++++++++++-------------
>  1 file changed, 68 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
[..]
> @@ -2016,53 +2030,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) {

I think this is okay for now, but I'd prefer changing the return value
of dw_pcie_suspend_noirq() to indicate if it did stop the link or not
(two different success values) - rather than deriving that information
by peeking into the dw_pcie struct and conclude that
dw_pcie_suspend_noirq() did reach the end.

> +		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;
>  }
> @@ -2076,20 +2088,30 @@ 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);
> +		ret = icc_enable(pcie->icc_mem);
> +		if (ret) {
> +			dev_err(dev, "Failed to enable PCIe-MEM interconnect path: %d\n", ret);

I think you should revert icc_enable(pcie->icc_cpu) here, to avoid
leaving the bus voted for with the PCIe controller resume aborted.

> +			return ret;
> +		}
> +		ret = dw_pcie_resume_noirq(pcie->pci);
>  		if (ret)

And Both icc_cpu and icc_mem here.

Regards,
Bjorn

>  			return ret;
> -
> -		pcie->suspended = false;
> +	} 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);
> 
> -- 
> 2.34.1
> 
> 

Re: [PATCH 0/3] PCI: qcom: Add D3cold support

[Bjorn Andersson]

On Wed, Jan 28, 2026 at 05:10:40PM +0530, Krishna Chaitanya Chundru wrote:
> This series adds a common helper to determine when a PCIe host bridge
> can safely enter D3cold, converts the DesignWare host driver to use that
> helper, and enables D3cold support for Qualcomm PCIe controllers.
> 

You only modify qcom_pcie_deinit_2_7_0() so which targets is this
expected to work on and which targets have you tested it on. How can I
test it and what outcome should I expect?

> The first patch introduces pci_host_common_can_enter_d3cold() in
> pci-host-common. The helper walks all devices on the bridge's bus and
> only allows the host bridge to enter D3cold if all PCIe endpoints are
> already in PCI_D3hot. For devices that may wake the system, it
> additionally requires that the device support PME wakeup from D3cold
> (via WAKE#). Devices without wakeup enabled are not restricted by this
> check and may be left in D3cold.
> 
> The second patch updates the DesignWare host driver to use this common
> helper in dw_pcie_suspend_noirq(). Previously, the driver skipped
> putting the link into L2 / device 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 helper's requirement that all
> endpoints be in D3hot before the host bridge may enter D3cold. Using the
> shared helper removes this coarse heuristic and centralizes the D3cold
> eligibility policy.
> 
> The third patch enables D3cold support for Qualcomm PCIe controllers. It
> adds pme_turn_off() support and switches to the DesignWare common
> suspend/resume methods for device D3cold entry and exit. If the
> controller is not kept in D3cold, the existing paths are used so that
> ICC votes, OPP votes, and other resources remain managed as before. In
> addition, qcom_pcie_deinit_2_7_0() is updated to explicitly disable
> PCIe clocks and resets in the controller, and the now-unused "suspended"
> flag is removed from struct qcom_pcie.
> 

This is effectively just duplicating the commit messages. Lacking from
both is a good explanation of the problem statement, but that might just
be me not getting it?

Could you please help me understand what the actual outcome of this
series is? I was under the impression that this work would lead us
towards unblocking CXPC, but the other patch you sent will prevent CXPC.

Regards,
Bjorn

> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
> Krishna Chaitanya Chundru (3):
>       PCI: host-common: Add shared D3cold eligibility helper for host bridges
>       PCI: dwc: Use common D3cold eligibility helper in suspend path
>       PCI: qcom: Add D3cold support
> 
>  drivers/pci/controller/dwc/pcie-designware-host.c |   7 +-
>  drivers/pci/controller/dwc/pcie-designware.h      |   1 +
>  drivers/pci/controller/dwc/pcie-qcom.c            | 114 +++++++++++++---------
>  drivers/pci/controller/pci-host-common.c          |  29 ++++++
>  drivers/pci/controller/pci-host-common.h          |   2 +
>  5 files changed, 101 insertions(+), 52 deletions(-)
> ---
> base-commit: 590a64365d9bcc13ee644a3e73ffdc3df26cf23c
> change-id: 20251229-d3cold-bf99921960bb
> 
> Best regards,
> -- 
> Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> 
> 

Re: [PATCH 0/3] PCI: qcom: Add D3cold support

[Krishna Chaitanya Chundru]

On 1/28/2026 8:25 PM, Bjorn Andersson wrote:
> On Wed, Jan 28, 2026 at 05:10:40PM +0530, Krishna Chaitanya Chundru wrote:
>> This series adds a common helper to determine when a PCIe host bridge
>> can safely enter D3cold, converts the DesignWare host driver to use that
>> helper, and enables D3cold support for Qualcomm PCIe controllers.
>>
> You only modify qcom_pcie_deinit_2_7_0() so which targets is this
> expected to work on and which targets have you tested it on. How can I
> test it and what outcome should I expect?
we modified qcom_pcie_deinit_2_7_() because we are trying to undo what 
we are doing as part of init for other platforms, in init() we are just 
turning on the resources. I tested this on lemans evk device. After this 
series we can expect PCIe link will go to D3cold(provided there is no 
NVMe attach), and cxpc can be achieved. For NVMe devices, mani is 
working on, in which requires some psci changes[1]
>> The first patch introduces pci_host_common_can_enter_d3cold() in
>> pci-host-common. The helper walks all devices on the bridge's bus and
>> only allows the host bridge to enter D3cold if all PCIe endpoints are
>> already in PCI_D3hot. For devices that may wake the system, it
>> additionally requires that the device support PME wakeup from D3cold
>> (via WAKE#). Devices without wakeup enabled are not restricted by this
>> check and may be left in D3cold.
>>
>> The second patch updates the DesignWare host driver to use this common
>> helper in dw_pcie_suspend_noirq(). Previously, the driver skipped
>> putting the link into L2 / device 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 helper's requirement that all
>> endpoints be in D3hot before the host bridge may enter D3cold. Using the
>> shared helper removes this coarse heuristic and centralizes the D3cold
>> eligibility policy.
>>
>> The third patch enables D3cold support for Qualcomm PCIe controllers. It
>> adds pme_turn_off() support and switches to the DesignWare common
>> suspend/resume methods for device D3cold entry and exit. If the
>> controller is not kept in D3cold, the existing paths are used so that
>> ICC votes, OPP votes, and other resources remain managed as before. In
>> addition, qcom_pcie_deinit_2_7_0() is updated to explicitly disable
>> PCIe clocks and resets in the controller, and the now-unused "suspended"
>> flag is removed from struct qcom_pcie.
>>
> This is effectively just duplicating the commit messages. Lacking from
> both is a good explanation of the problem statement, but that might just
> be me not getting it?
We are adding support for D3cold for qcom controllers, this is a PCIe 
feature,
I haven't added reference to qcom internal power state like CXPC since 
this alone
will not achieve this. But I should have added to this as ultimate 
purpose is to
have CXPC and main blocking is currently PCIe.
>
> Could you please help me understand what the actual outcome of this
> series is? I was under the impression that this work would lead us
> towards unblocking CXPC, but the other patch you sent will prevent CXPC.
This will keep PCIe in D3cold and achieve CXPC if there is no NVMe 
endpoints.

No other patch is not preventing CXPC, it is just trying to tell genpd 
framework that
don't turn off GENPD, if the controller is not suspended. if we don't 
have that patch
when device is not suspended i.e not kept in D3cold the gdsc is getting 
turned off
and PCIe link is going down. Until PCIe state is in D3cold, gdsc should 
not be off
even in cxpc case.

Mani,
To avoid the confusion, can I club this patch[2] to this series in next 
verision

[1] 
https://lore.kernel.org/all/20251231162126.7728-1-manivannan.sadhasivam@oss.qualcomm.com
[2] [PATCH] PCI: qcom: Prevent GDSC power down on suspend - Krishna 
Chaitanya Chundru 
<https://lore.kernel.org/all/20260128-genpd_fix-v1-1-cd45a249d12f@oss.qualcomm.com/>

- Krishna Chaitanya.
>
> Regards,
> Bjorn
>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
>> Krishna Chaitanya Chundru (3):
>>        PCI: host-common: Add shared D3cold eligibility helper for host bridges
>>        PCI: dwc: Use common D3cold eligibility helper in suspend path
>>        PCI: qcom: Add D3cold support
>>
>>   drivers/pci/controller/dwc/pcie-designware-host.c |   7 +-
>>   drivers/pci/controller/dwc/pcie-designware.h      |   1 +
>>   drivers/pci/controller/dwc/pcie-qcom.c            | 114 +++++++++++++---------
>>   drivers/pci/controller/pci-host-common.c          |  29 ++++++
>>   drivers/pci/controller/pci-host-common.h          |   2 +
>>   5 files changed, 101 insertions(+), 52 deletions(-)
>> ---
>> base-commit: 590a64365d9bcc13ee644a3e73ffdc3df26cf23c
>> change-id: 20251229-d3cold-bf99921960bb
>>
>> Best regards,
>> -- 
>> Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>>
>>

Re: [PATCH 3/3] PCI: qcom: Add D3cold support

[Krishna Chaitanya Chundru]

On 1/28/2026 5:58 PM, Konrad Dybcio wrote:
> On 1/28/26 12:40 PM, Krishna Chaitanya Chundru wrote:
>> Add pme_turn_off() support and use DWC common suspend resume methods
>> for device D3cold entry & exit. If the device is not kept in D3cold
>> use existing methods like keeping icc votes, opp votes etc.. intact.
>>
>> In qcom_pcie_deinit_2_7_0(), explicitly disable PCIe clocks and resets
>> in the controller.
>>
>> Remove suspended flag from qcom_pcie structure as it is no longer needed.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
> [...]
>
>> +		/*
>> +		 * 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.
>> +		 */
> I think someone internally once tracked down what that access was
As per last debug which I have done few years back we see access coming 
IRQ driver to mask the interrupts
as part of disabling non boot CPU's.
> Can we fix that instead?
The only proper fix is to keep device in D3cold which this patch is 
doing. if some client drivers like NVMe
doesn't want to go D3cold we need to honor it, but Mani is working on it 
to allow NVMe drivers to go to D3cold.

- Krishna Chaitanya.
>
> Konrad

Re: [PATCH 3/3] PCI: qcom: Add D3cold support

[Krishna Chaitanya Chundru]

On 1/28/2026 8:17 PM, Bjorn Andersson wrote:
> On Wed, Jan 28, 2026 at 05:10:43PM +0530, Krishna Chaitanya Chundru wrote:
>> Add pme_turn_off() support and use DWC common suspend resume methods
>> for device D3cold entry & exit. If the device is not kept in D3cold
>> use existing methods like keeping icc votes, opp votes etc.. intact.
>>
>> In qcom_pcie_deinit_2_7_0(), explicitly disable PCIe clocks and resets
>> in the controller.
>>
>> Remove suspended flag from qcom_pcie structure as it is no longer needed.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
>>   drivers/pci/controller/dwc/pcie-qcom.c | 114 ++++++++++++++++++++-------------
>>   1 file changed, 68 insertions(+), 46 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> [..]
>> @@ -2016,53 +2030,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) {
> I think this is okay for now, but I'd prefer changing the return value
> of dw_pcie_suspend_noirq() to indicate if it did stop the link or not
> (two different success values) - rather than deriving that information
> by peeking into the dw_pcie struct and conclude that
> dw_pcie_suspend_noirq() did reach the end.
>
>> +		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;
>>   }
>> @@ -2076,20 +2088,30 @@ 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);
>> +		ret = icc_enable(pcie->icc_mem);
>> +		if (ret) {
>> +			dev_err(dev, "Failed to enable PCIe-MEM interconnect path: %d\n", ret);
> I think you should revert icc_enable(pcie->icc_cpu) here, to avoid
> leaving the bus voted for with the PCIe controller resume aborted.
>
>> +			return ret;
>> +		}
>> +		ret = dw_pcie_resume_noirq(pcie->pci);
>>   		if (ret)
> And Both icc_cpu and icc_mem here.
Ack, I will do this in V2.

- Krishna Chaitanya.
> Regards,
> Bjorn
>
>>   			return ret;
>> -
>> -		pcie->suspended = false;
>> +	} 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);
>>
>> -- 
>> 2.34.1
>>
>>

Re: [PATCH 2/3] PCI: dwc: Use common D3cold eligibility helper in suspend path

[Krishna Chaitanya Chundru]

On 1/28/2026 7:58 PM, Bjorn Andersson wrote:
> On Wed, Jan 28, 2026 at 05:10:42PM +0530, Krishna Chaitanya Chundru wrote:
>> 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 | 7 +------
>>   drivers/pci/controller/dwc/pcie-designware.h      | 1 +
>>   2 files changed, 2 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>> index 372207c33a857b4c98572bb1e9b61fa0080bc871..2c8056761addf7febc1b0e06ddf8ba4dd4ad1684 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>> @@ -1157,15 +1157,10 @@ 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);
>>   	u32 val;
>>   	int ret;
>>   
>> -	/*
>> -	 * 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) {
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>> index 31685951a080456b8834aab2bf79a36c78f46639..18d8f7d5d23088b2fa177e84a21d900c98850fcd 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>> @@ -26,6 +26,7 @@
>>   #include <linux/pci-epc.h>
>>   #include <linux/pci-epf.h>
>>   
>> +#include "../pci-host-common.h"
> Why doesn't this include go in the c file? I don't see that all c files
Ack, will do this in v2.

- Krishna Chaitanya.
> including pcie-designware.h now needs this.
>
> Regards,
> Bjorn
>
>>   #include "../../pci.h"
>>   
>>   /* DWC PCIe IP-core versions (native support since v4.70a) */
>>
>> -- 
>> 2.34.1
>>
>>

Re: [PATCH 1/3] PCI: host-common: Add shared D3cold eligibility helper for host bridges

[Krishna Chaitanya Chundru]

On 1/28/2026 7:56 PM, Bjorn Andersson wrote:
> On Wed, Jan 28, 2026 at 05:10:41PM +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.
> Please read https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
>
> It clearly says that you're supposed to start your commit message with a
> description of the problem you're solving. In fact, even after reading
> the entire commit message a few times I only know what the patch does,
> but it's not clear to me why this patch exists.
>
>> The helper walks all devices on the bridge's bus and only allows the
>> host bridge to enter D3cold if all PCIe endpoints are already in
>> PCI_D3hot.
> The code below does walk the bus, but it doesn't allow/disallow anything
> as far as I can tell, it queries their type, state, and if they are wake
> capable?
>
>> For devices that may wake the system, it additionally
>> requires that the device supports PME wakeup from D3cold(with WAKE#).
>> Devices without wakeup enabled are not restricted by this check and can
>> be allowed to keep device in D3cold.
>>
> Again, this code doesn't perform any action, it doesn't
> allow/disallow/restrict the devices from doing anything, it merely
> queries a bunch of things across all devices, and the commit message
> fails to capture why this is useful.
This is a helper function used by controller drivers, to know if the 
controller
driver can keep in D3cold state or not. we use endpoint states and their 
wakeup
capability support to determine d3cold can be supported or not. The 
return value
of this function will tell controller drivers to know if we can allow 
D3cold or not.
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
>>   drivers/pci/controller/pci-host-common.c | 29 +++++++++++++++++++++++++++++
>>   drivers/pci/controller/pci-host-common.h |  2 ++
>>   2 files changed, 31 insertions(+)
>>
>> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
>> index c473e7c03bacad2de07c798768f99652443431e0..225472c5ac82c6c5b44257d68a0fc503ec046ff1 100644
>> --- a/drivers/pci/controller/pci-host-common.c
>> +++ b/drivers/pci/controller/pci-host-common.c
>> @@ -106,5 +106,34 @@ 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->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;
>> +}
>> +
>> +bool pci_host_common_can_enter_d3cold(struct pci_host_bridge *bridge)
> Please add kernel-doc for any EXPORT_SYMBOL() functions, so that it's
> clear to the next guy what the API does.
Initially, I had a change in my workspace which has kernel-doc, but 
after seeing this
file I see none of the exported API's had a kernel-doc. Following it I 
dropped the kernel
-doc at last minute. I will add this in v2.

- Krishna Chaitanya.
>
> Regards,
> Bjorn
>
>> +{
>> +	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
>>
>>

Re: [PATCH 1/3] PCI: host-common: Add shared D3cold eligibility helper for host bridges

[Bjorn Andersson]

On Thu, Jan 29, 2026 at 11:08:57AM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 1/28/2026 7:56 PM, Bjorn Andersson wrote:
> > On Wed, Jan 28, 2026 at 05:10:41PM +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.
> > Please read https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
> > 
> > It clearly says that you're supposed to start your commit message with a
> > description of the problem you're solving. In fact, even after reading
> > the entire commit message a few times I only know what the patch does,
> > but it's not clear to me why this patch exists.
> > 
> > > The helper walks all devices on the bridge's bus and only allows the
> > > host bridge to enter D3cold if all PCIe endpoints are already in
> > > PCI_D3hot.
> > The code below does walk the bus, but it doesn't allow/disallow anything
> > as far as I can tell, it queries their type, state, and if they are wake
> > capable?
> > 
> > > For devices that may wake the system, it additionally
> > > requires that the device supports PME wakeup from D3cold(with WAKE#).
> > > Devices without wakeup enabled are not restricted by this check and can
> > > be allowed to keep device in D3cold.
> > > 
> > Again, this code doesn't perform any action, it doesn't
> > allow/disallow/restrict the devices from doing anything, it merely
> > queries a bunch of things across all devices, and the commit message
> > fails to capture why this is useful.
> This is a helper function used by controller drivers, to know if the
> controller
> driver can keep in D3cold state or not. we use endpoint states and their
> wakeup
> capability support to determine d3cold can be supported or not. The return
> value
> of this function will tell controller drivers to know if we can allow D3cold
> or not.

Please make sure that the commit message captures this.

Regards,
Bjorn

> > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > ---
> > >   drivers/pci/controller/pci-host-common.c | 29 +++++++++++++++++++++++++++++
> > >   drivers/pci/controller/pci-host-common.h |  2 ++
> > >   2 files changed, 31 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> > > index c473e7c03bacad2de07c798768f99652443431e0..225472c5ac82c6c5b44257d68a0fc503ec046ff1 100644
> > > --- a/drivers/pci/controller/pci-host-common.c
> > > +++ b/drivers/pci/controller/pci-host-common.c
> > > @@ -106,5 +106,34 @@ 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->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;
> > > +}
> > > +
> > > +bool pci_host_common_can_enter_d3cold(struct pci_host_bridge *bridge)
> > Please add kernel-doc for any EXPORT_SYMBOL() functions, so that it's
> > clear to the next guy what the API does.
> Initially, I had a change in my workspace which has kernel-doc, but after
> seeing this
> file I see none of the exported API's had a kernel-doc. Following it I
> dropped the kernel
> -doc at last minute. I will add this in v2.
> 
> - Krishna Chaitanya.
> > 
> > Regards,
> > Bjorn
> > 
> > > +{
> > > +	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
> > > 
> > > 
> 

Re: [PATCH 0/3] PCI: qcom: Add D3cold support

[Bjorn Andersson]

On Thu, Jan 29, 2026 at 10:53:43AM +0530, Krishna Chaitanya Chundru wrote:
> 
> 
> On 1/28/2026 8:25 PM, Bjorn Andersson wrote:
> > On Wed, Jan 28, 2026 at 05:10:40PM +0530, Krishna Chaitanya Chundru wrote:
> > > This series adds a common helper to determine when a PCIe host bridge
> > > can safely enter D3cold, converts the DesignWare host driver to use that
> > > helper, and enables D3cold support for Qualcomm PCIe controllers.
> > > 
> > You only modify qcom_pcie_deinit_2_7_0() so which targets is this
> > expected to work on and which targets have you tested it on. How can I
> > test it and what outcome should I expect?
> we modified qcom_pcie_deinit_2_7_() because we are trying to undo what we
> are doing as part of init for other platforms, in init() we are just turning
> on the resources. I tested this on lemans evk device. After this series we
> can expect PCIe link will go to D3cold(provided there is no NVMe attach),
> and cxpc can be achieved. For NVMe devices, mani is working on, in which
> requires some psci changes[1]

I have no objections to start with one platform, please consider what
will happen to others (don't break existing users) and please document
in the commit message that this only affects X, Y, Z platforms.

> > > The first patch introduces pci_host_common_can_enter_d3cold() in
> > > pci-host-common. The helper walks all devices on the bridge's bus and
> > > only allows the host bridge to enter D3cold if all PCIe endpoints are
> > > already in PCI_D3hot. For devices that may wake the system, it
> > > additionally requires that the device support PME wakeup from D3cold
> > > (via WAKE#). Devices without wakeup enabled are not restricted by this
> > > check and may be left in D3cold.
> > > 
> > > The second patch updates the DesignWare host driver to use this common
> > > helper in dw_pcie_suspend_noirq(). Previously, the driver skipped
> > > putting the link into L2 / device 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 helper's requirement that all
> > > endpoints be in D3hot before the host bridge may enter D3cold. Using the
> > > shared helper removes this coarse heuristic and centralizes the D3cold
> > > eligibility policy.
> > > 
> > > The third patch enables D3cold support for Qualcomm PCIe controllers. It
> > > adds pme_turn_off() support and switches to the DesignWare common
> > > suspend/resume methods for device D3cold entry and exit. If the
> > > controller is not kept in D3cold, the existing paths are used so that
> > > ICC votes, OPP votes, and other resources remain managed as before. In
> > > addition, qcom_pcie_deinit_2_7_0() is updated to explicitly disable
> > > PCIe clocks and resets in the controller, and the now-unused "suspended"
> > > flag is removed from struct qcom_pcie.
> > > 
> > This is effectively just duplicating the commit messages. Lacking from
> > both is a good explanation of the problem statement, but that might just
> > be me not getting it?
> We are adding support for D3cold for qcom controllers, this is a PCIe
> feature,
> I haven't added reference to qcom internal power state like CXPC since this
> alone
> will not achieve this. But I should have added to this as ultimate purpose
> is to
> have CXPC and main blocking is currently PCIe.

Thank you.

> > 
> > Could you please help me understand what the actual outcome of this
> > series is? I was under the impression that this work would lead us
> > towards unblocking CXPC, but the other patch you sent will prevent CXPC.
> This will keep PCIe in D3cold and achieve CXPC if there is no NVMe
> endpoints.
> 
> No other patch is not preventing CXPC, it is just trying to tell genpd
> framework that
> don't turn off GENPD, if the controller is not suspended. if we don't have
> that patch
> when device is not suspended i.e not kept in D3cold the gdsc is getting
> turned off
> and PCIe link is going down. Until PCIe state is in D3cold, gdsc should not
> be off
> even in cxpc case.
> 

Please do include such information in your cover letters or commit
messages. By not assuming that recipients can read your mind, you will
be able to get more engagement surrounding your patches - and this is a
topic that we're very much interested in.

Regards,
Bjorn

> Mani,
> To avoid the confusion, can I club this patch[2] to this series in next
> verision
> 
> [1] https://lore.kernel.org/all/20251231162126.7728-1-manivannan.sadhasivam@oss.qualcomm.com
> [2] [PATCH] PCI: qcom: Prevent GDSC power down on suspend - Krishna
> Chaitanya Chundru <https://lore.kernel.org/all/20260128-genpd_fix-v1-1-cd45a249d12f@oss.qualcomm.com/>
> 
> - Krishna Chaitanya.
> > 
> > Regards,
> > Bjorn
> > 
> > > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > ---
> > > Krishna Chaitanya Chundru (3):
> > >        PCI: host-common: Add shared D3cold eligibility helper for host bridges
> > >        PCI: dwc: Use common D3cold eligibility helper in suspend path
> > >        PCI: qcom: Add D3cold support
> > > 
> > >   drivers/pci/controller/dwc/pcie-designware-host.c |   7 +-
> > >   drivers/pci/controller/dwc/pcie-designware.h      |   1 +
> > >   drivers/pci/controller/dwc/pcie-qcom.c            | 114 +++++++++++++---------
> > >   drivers/pci/controller/pci-host-common.c          |  29 ++++++
> > >   drivers/pci/controller/pci-host-common.h          |   2 +
> > >   5 files changed, 101 insertions(+), 52 deletions(-)
> > > ---
> > > base-commit: 590a64365d9bcc13ee644a3e73ffdc3df26cf23c
> > > change-id: 20251229-d3cold-bf99921960bb
> > > 
> > > Best regards,
> > > -- 
> > > Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> > > 
> > > 
> 

Re: [PATCH 3/3] PCI: qcom: Add D3cold support

[Konrad Dybcio]

On 1/29/26 6:27 AM, Krishna Chaitanya Chundru wrote:
> 
> 
> On 1/28/2026 5:58 PM, Konrad Dybcio wrote:
>> On 1/28/26 12:40 PM, Krishna Chaitanya Chundru wrote:
>>> Add pme_turn_off() support and use DWC common suspend resume methods
>>> for device D3cold entry & exit. If the device is not kept in D3cold
>>> use existing methods like keeping icc votes, opp votes etc.. intact.
>>>
>>> In qcom_pcie_deinit_2_7_0(), explicitly disable PCIe clocks and resets
>>> in the controller.
>>>
>>> Remove suspended flag from qcom_pcie structure as it is no longer needed.
>>>
>>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>>> ---
>> [...]
>>
>>> +        /*
>>> +         * 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.
>>> +         */
>> I think someone internally once tracked down what that access was
> As per last debug which I have done few years back we see access coming IRQ driver to mask the interrupts
> as part of disabling non boot CPU's.
>> Can we fix that instead?
> The only proper fix is to keep device in D3cold which this patch is doing. if some client drivers like NVMe
> doesn't want to go D3cold we need to honor it, but Mani is working on it to allow NVMe drivers to go to D3cold.

That doesn't sound right - if there's an unclocked access, we should
either ensure that the PCIe controller is online for that write, or skip
the write if it's not possible for $reasons

Konrad