[PATCH v2 0/4] PCI: Introduce pci_suspend_retains_context() API

[PATCH v2 0/4] PCI: Introduce pci_suspend_retains_context() API

[Manivannan Sadhasivam via B4 Relay]

Hi all,

This series introduces a new PCI API, pci_suspend_retains_context() to
let the client drivers know whether they can expect context retention across
suspend/resume or not and uses it in the NVMe PCI host driver.

This new API is targeted to abstract the PCI power management details away from
the client drivers. This is needed because client drivers like NVMe make use of
APIs such as pm_suspend_via_firmware() and decide to keep the device in low
power mode if this API returns 'false'. But some platforms may have other
limitations like in the case of Qcom, where if the RC driver removes the PCIe RC
resource vote to allow the SoC to enter low power mode, it cannot reliably exit
the L1ss state when the endpoint asserts CLKREQ#. So in this case also, the
client drivers cannot keep the device in low power state during suspend and
expect context retention.

And these limitations may just keep adding in the future. Without a unified
API, the client drivers have to implement their own logic which may cause code
duplication and may also lead to drivers missing some of the platform
limitations.

Once this series gets merged, we can extend this API usage to other client
drivers as well.

Testing
=======

This series is tested on Qualcomm Hamoa based Lenovo Thinkpad T14s latop with
NVMe drive.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
Changes in v2:
- Renamed the API to pci_suspend_retains_context()
- Reworded the commit messages to include L10_REFCLK_ON + T_COMMONMODE as the
  L1ss exit latency
- Rebased on top of v7.1-rc1

---
Manivannan Sadhasivam (4):
      PCI: Introduce an API to check if RC/platform can retain device context during suspend
      PCI: Indicate context lost if L1ss exit is broken during resume from system suspend
      PCI: qcom: Indicate broken L1ss exit during resume from system suspend
      nvme-pci: Use pci_suspend_retains_context() API during suspend

 drivers/nvme/host/pci.c                |  3 ++-
 drivers/pci/controller/dwc/pcie-qcom.c | 12 ++++++++++++
 drivers/pci/pci.c                      | 34 ++++++++++++++++++++++++++++++++++
 include/linux/pci.h                    |  9 +++++++++
 4 files changed, 57 insertions(+), 1 deletion(-)
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260414-l1ss-fix-6c9cf2451944

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

[PATCH v2 1/4] PCI: Introduce an API to check if RC/platform can retain device context during suspend

[Manivannan Sadhasivam via B4 Relay]

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

Currently, the PCI endpoint drivers like NVMe checks whether the device
context will be retained or not during system suspend, with the help of
pm_suspend_via_firmware() API.

But it is possible that the device context might be lost due to some
platform limitation as well. Having those checks in the endpoint drivers
will not scale and will cause a lot of code duplication.

So introduce an API that acts as a sole point of truth that the endpoint
drivers can rely on to check whether they can expect the device context
to be retained or not.

If the API returns 'false', then the client drivers need to prepare for
context loss by performing actions such as resetting the device, saving
the context, shutting it down etc... If it returns 'true', then the drivers
do not need to perform any special action and can leave the device in
active state.

Right now, this API only incorporates the pm_suspend_via_firmware() check.
But will be extended in the future commits.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/pci.c   | 23 +++++++++++++++++++++++
 include/linux/pci.h |  7 +++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 8f7cfcc00090..38cc5172d259 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -33,6 +33,7 @@
 #include <asm/dma.h>
 #include <linux/aer.h>
 #include <linux/bitfield.h>
+#include <linux/suspend.h>
 #include "pci.h"
 
 DEFINE_MUTEX(pci_slot_mutex);
@@ -2899,6 +2900,28 @@ void pci_config_pm_runtime_put(struct pci_dev *pdev)
 		pm_runtime_put_sync(parent);
 }
 
+/**
+ * pci_suspend_retains_context - Check if the platform can retain the device
+ *				 context during system suspend
+ * @pdev: PCI device to check
+ *
+ * Returns true if the platform can guarantee to retain the device context,
+ * false otherwise.
+ */
+bool pci_suspend_retains_context(struct pci_dev *pdev)
+{
+	/*
+	 * If the platform firmware (like ACPI) is involved at the end of system
+	 * suspend, device context may not be retained.
+	 */
+	if (pm_suspend_via_firmware())
+		return false;
+
+	/* Assume that the context is retained by default */
+	return true;
+}
+EXPORT_SYMBOL_GPL(pci_suspend_retains_context);
+
 static const struct dmi_system_id bridge_d3_blacklist[] = {
 #ifdef CONFIG_X86
 	{
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 2c4454583c11..f60f9e4e7b39 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2086,6 +2086,8 @@ pci_release_mem_regions(struct pci_dev *pdev)
 			    pci_select_bars(pdev, IORESOURCE_MEM));
 }
 
+bool pci_suspend_retains_context(struct pci_dev *pdev);
+
 #else /* CONFIG_PCI is not enabled */
 
 static inline void pci_set_flags(int flags) { }
@@ -2244,6 +2246,11 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
 static inline void pci_free_irq_vectors(struct pci_dev *dev)
 {
 }
+
+static inline bool pci_suspend_retains_context(struct pci_dev *pdev)
+{
+	return true;
+}
 #endif /* CONFIG_PCI */
 
 /* Include architecture-dependent settings and functions */

-- 
2.48.1

[PATCH v2 2/4] PCI: Indicate context lost if L1ss exit is broken during resume from system suspend

[Manivannan Sadhasivam via B4 Relay]

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

The PCIe spec v7.0, sec 5.5.3.3.1, states that for exiting L1.2 due to an
endpoint asserting CLKREQ# signal, the refclk must be turned on no earlier
than TL10_REFCLK_ON, and within the latency advertised in the LTR message.
This same behavior applies to L1.1 as well.

On some platforms like Qcom, these requirements are satisfied during OS
runtime, but not while resuming from the system suspend. This happens
because the PCIe RC driver may remove all resource votes and turns off the
analog circuitry of PHY during suspend to maximize power savings while
keeping the link in L1ss.

Consequently, when the endpoint asserts CLKREQ# to wake up, the OS must
first resume and the RC driver must restore the PHY and enable the REFCLK.
When this recovery process exceeds the L1ss exit latency time (roughly
L10_REFCLK_ON + T_COMMONMODE), the endpoint may treat it as a fatal
condition and triger Link Down (LDn). If the endpoint device is used to
host the RootFS, it will result in an OS crash. For other endpoints, it
may result in a complete device reset/recovery.

So to indicate this platform limitation to the client drivers, introduce a
new flag 'pci_host_bridge::broken_l1ss_resume' and check it in the
pci_suspend_retains_context() API. If the flag is set by the RC driver, the
API will return 'false' indicating the client drivers that the device
context may not be retained and the drivers must be prepared for context
loss.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/pci.c   | 11 +++++++++++
 include/linux/pci.h |  2 ++
 2 files changed, 13 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 38cc5172d259..a7d2cb69b42e 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2910,6 +2910,8 @@ void pci_config_pm_runtime_put(struct pci_dev *pdev)
  */
 bool pci_suspend_retains_context(struct pci_dev *pdev)
 {
+	struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
+
 	/*
 	 * If the platform firmware (like ACPI) is involved at the end of system
 	 * suspend, device context may not be retained.
@@ -2917,6 +2919,15 @@ bool pci_suspend_retains_context(struct pci_dev *pdev)
 	if (pm_suspend_via_firmware())
 		return false;
 
+	/*
+	 * Some host bridges power off the PHY to enter deep low-power modes
+	 * during system suspend. Exiting L1 PM Substates from this condition
+	 * violates strict timing requirements and results in Link Down (LDn).
+	 * On such platforms, the endpoint must be prepared for context loss.
+	 */
+	if (bridge && bridge->broken_l1ss_resume)
+		return false;
+
 	/* Assume that the context is retained by default */
 	return true;
 }
diff --git a/include/linux/pci.h b/include/linux/pci.h
index f60f9e4e7b39..1e5b59fa258a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -660,6 +660,8 @@ struct pci_host_bridge {
 	unsigned int	preserve_config:1;	/* Preserve FW resource setup */
 	unsigned int	size_windows:1;		/* Enable root bus sizing */
 	unsigned int	msi_domain:1;		/* Bridge wants MSI domain */
+	unsigned int	broken_l1ss_resume:1;	/* Resuming from L1ss during
+						   system suspend is broken */
 
 	/* Resource alignment requirements */
 	resource_size_t (*align_resource)(struct pci_dev *dev,

-- 
2.48.1

[PATCH v2 3/4] PCI: qcom: Indicate broken L1ss exit during resume from system suspend

[Manivannan Sadhasivam via B4 Relay]

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

Qcom PCIe RCs can successfully exit from L1ss during OS runtime. However,
during system suspend, the Qcom PCIe RC driver may remove all resource
votes and turns off the PHY to maximize power savings.

Consequently, when the host is in system suspend with the link in L1ss and
the endpoint asserts CLKREQ#, the OS must first wake up and the RC driver
must restore the PHY and enable the refclk. This recovery process causes
the strict L1ss exit latency time to be exceeded (roughly L10_REFCLK_ON +
T_COMMONMODE). If the RC driver were to retain all votes during suspend,
L1ss exit would succeed without issue, but at the expense of higher power
consumption.

So when the host fails to move the link from L1ss to L0 within the
L10_REFCLK_ON + T_COMMONMODE time, the endpoint may treat it as a fatal
condition and trigger Link Down (LDn) during resume. This LDn can crash the
OS if the endpoint hosts the RootFS, and for other types of devices, it may
result in a full device reset/recovery.

So to ensure that the client drivers can properly handle this scenario, let
them know about this platform limitation by setting the
'pci_host_bridge::broken_l1ss_resume' flag.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index af6bf5cce65b..75bb6cb5e35e 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1368,6 +1368,18 @@ static void qcom_pcie_host_post_init(struct dw_pcie_rp *pp)
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
 	struct qcom_pcie *pcie = to_qcom_pcie(pci);
 
+	/*
+	 * During system suspend, the Qcom RC driver may turn off the analog
+	 * circuitry of PHY and remove controller votes to save power. If the
+	 * link is in L1ss and the endpoint asserts CLKREQ# to exit L1ss, the
+	 * time required to wake the system and restore the PHY/refclk will
+	 * exceed the strict L1ss exit timing (L10_REFCLK_ON + T_COMMONMODE),
+	 * resulting in Link Down (LDn) condition. Set this flag to indicate
+	 * this limitation to client drivers so that they can avoid relying on
+	 * L1ss during system suspend.
+	 */
+	pp->bridge->broken_l1ss_resume = true;
+
 	if (pcie->cfg->ops->host_post_init)
 		pcie->cfg->ops->host_post_init(pcie);
 }

-- 
2.48.1

[PATCH v2 4/4] nvme-pci: Use pci_suspend_retains_context() API during suspend

[Manivannan Sadhasivam via B4 Relay]

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

The pci_suspend_retains_context() API lets PCI client drivers know if the
platform can retain the device context during suspend. This is decided
based on several factors like:

1. Firmware involvement at the end of suspend
2. Any platform limitation in waking from low power state (L1ss)

And this API might also get extended in the future to cover other platform
specific issues impacting the device low power mode during system suspend.

So use this API instead of checks like pm_suspend_via_firmware(). When this
API returns false, then assume that the platform cannot retain the context
and shutdown the controller. If it returns true, then assume that the
context will be retained and keep the device in low power mode.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/nvme/host/pci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index db5fc9bf6627..a6664983ce5d 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -3915,6 +3915,7 @@ static int nvme_suspend(struct device *dev)
 	 * use host managed nvme power settings for lowest idle power if
 	 * possible. This should have quicker resume latency than a full device
 	 * shutdown.  But if the firmware is involved after the suspend or the
+	 * platform has any limitation in waking from low power states or the
 	 * device does not support any non-default power states, shut down the
 	 * device fully.
 	 *
@@ -3923,7 +3924,7 @@ static int nvme_suspend(struct device *dev)
 	 * down, so as to allow the platform to achieve its minimum low-power
 	 * state (which may not be possible if the link is up).
 	 */
-	if (pm_suspend_via_firmware() || !ctrl->npss ||
+	if (!pci_suspend_retains_context(pdev) || !ctrl->npss ||
 	    !pcie_aspm_enabled(pdev) ||
 	    (ndev->ctrl.quirks & NVME_QUIRK_SIMPLE_SUSPEND))
 		return nvme_disable_prepare_reset(ndev, true);

-- 
2.48.1

Re: [PATCH v2 4/4] nvme-pci: Use pci_suspend_retains_context() API during suspend

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v2 3/4] PCI: qcom: Indicate broken L1ss exit during resume from system suspend

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Initialization order bug: `broken_l1ss_resume` is set after child PCI devices have already been enumerated and probed.
- [High] Commit message directly contradicts the actual driver implementation regarding PHY power state during suspend.
--

commit 3ebc1e1ccc09940f1b1046dd596cb5ba9e52cf1e
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

PCI: qcom: Indicate broken L1ss exit during resume from system suspend

This commit sets the broken_l1ss_resume flag on the PCI host bridge for
Qualcomm platforms to signal to client drivers that the RC driver will
turn off the PHY during suspend. This aims to prevent fatal link down
conditions by avoiding L1ss during system suspend.

>     Qcom PCIe RCs can successfully exit from L1ss during OS runtime. However,
>     during system suspend, the Qcom PCIe RC driver may remove all resource
>     votes and turns off the PHY to maximize power savings.

Does the commit message accurately describe the driver's suspend behavior?

Looking at qcom_pcie_suspend_noirq(), the PHY is only turned off if no
devices are active:

drivers/pci/controller/dwc/pcie-qcom.c:qcom_pcie_suspend_noirq() {
	/*
	 * 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.
	...
	 */
	if (!dw_pcie_link_up(pcie->pci)) {
		qcom_pcie_host_deinit(&pcie->pci->pp);
		pcie->suspended = true;
	}
...
}

Because the resources are kept active when an endpoint device is attached, it
seems the stated condition for link down would not occur. Could the commit
message be clarified to match the implementation, or is there a missing
patch that changes this suspend behavior?

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index af6bf5cce65be..75bb6cb5e35e5 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1368,6 +1368,18 @@ static void qcom_pcie_host_post_init(struct dw_pcie_rp *pp)
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>  	struct qcom_pcie *pcie = to_qcom_pcie(pci);
>  
> +	/*
> +	 * During system suspend, the Qcom RC driver may turn off the analog
> +	 * circuitry of PHY and remove controller votes to save power. If the
> +	 * link is in L1ss and the endpoint asserts CLKREQ# to exit L1ss, the
> +	 * time required to wake the system and restore the PHY/refclk will
> +	 * exceed the strict L1ss exit timing (L10_REFCLK_ON + T_COMMONMODE),
> +	 * resulting in Link Down (LDn) condition. Set this flag to indicate
> +	 * this limitation to client drivers so that they can avoid relying on
> +	 * L1ss during system suspend.
> +	 */
> +	pp->bridge->broken_l1ss_resume = true;
> +
>  	if (pcie->cfg->ops->host_post_init)
>  		pcie->cfg->ops->host_post_init(pcie);
>  }

Does this initialization order introduce a regression?

Since qcom_pcie_host_post_init() acts as the post_init callback for the
DesignWare host operations, it is called after pci_host_probe(bridge) in
dw_pcie_host_init():

drivers/pci/controller/dwc/pcie-designware-host.c:dw_pcie_host_init() {
    ...
	ret = pci_host_probe(bridge);
	if (ret)
		goto err_stop_link;

	if (pp->ops->post_init)
		pp->ops->post_init(pp);
    ...
}

Because pci_host_probe() handles device enumeration and triggers endpoint
drivers' probe() routines, any driver that checks the broken_l1ss_resume
flag during its probe sequence might read an uninitialized false value.

Would it be safer to set broken_l1ss_resume inside the init callback
instead, before enumeration begins?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-l1ss-fix-v2-0-b2c3a4bdeb15@oss.qualcomm.com?part=3

Re: [PATCH v2 4/4] nvme-pci: Use pci_suspend_retains_context() API during suspend

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Unused header `<linux/suspend.h>` is left in `drivers/nvme/host/pci.c`.
--

commit 626756383798c29170f6f0069d54faf950cd1526
Author: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

    nvme-pci: Use pci_suspend_retains_context() API during suspend

The patch replaces pm_suspend_via_firmware() with the newly introduced
pci_suspend_retains_context() API to check if the platform can retain
device context during suspend. This dictates whether the NVMe controller
should be fully shut down or kept in a low power mode.

> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index db5fc9bf66272..a6664983ce5da 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -3915,6 +3915,7 @@ static int nvme_suspend(struct device *dev)
>  	 * use host managed nvme power settings for lowest idle power if
>  	 * possible. This should have quicker resume latency than a full device
>  	 * shutdown.  But if the firmware is involved after the suspend or the
> +	 * platform has any limitation in waking from low power states or the
>  	 * device does not support any non-default power states, shut down the
>  	 * device fully.
>  	 *
> @@ -3923,7 +3924,7 @@ static int nvme_suspend(struct device *dev)
>  	 * down, so as to allow the platform to achieve its minimum low-power
>  	 * state (which may not be possible if the link is up).
>  	 */
> -	if (pm_suspend_via_firmware() || !ctrl->npss ||
> +	if (!pci_suspend_retains_context(pdev) || !ctrl->npss ||

This isn't a bug, but since the final usage of pm_suspend_via_firmware()
was replaced here, should the linux/suspend.h include also be removed?

drivers/nvme/host/pci.c:
    ...
    #include <linux/pci.h>
    #include <linux/suspend.h>
    #include <linux/t10-pi.h>
    ...

Removing it might maintain clean dependencies and avoid unnecessary
compilation overhead.

>  	    !pcie_aspm_enabled(pdev) ||
>  	    (ndev->ctrl.quirks & NVME_QUIRK_SIMPLE_SUSPEND))
>  		return nvme_disable_prepare_reset(ndev, true);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-l1ss-fix-v2-0-b2c3a4bdeb15@oss.qualcomm.com?part=4