[RFC PATCH] PCI: dwc: Power the controller down when hibernating

[RFC PATCH] PCI: dwc: Power the controller down when hibernating

[Birk Skyum]

dw_pcie_suspend_noirq() leaves the controller powered, with
pci->suspended false, when a device below it cannot go to D3cold.
dw_pcie_resume_noirq() then returns early and the driver carries on
with the controller as it was.

That is fine for suspend. Drivers use the same two functions for
hibernation, where they run as freeze_noirq and restore_noirq. Devices
are normally still in D0 at freeze, so a controller with a device
behind it is left powered and the image records it as running. That
image is resumed after a power cycle, by a kernel that may never have
brought the controller up.

On a Lenovo Yoga Slim 7x (X1E80100) this resets the machine on every
resume from hibernation. The Wi-Fi card is behind pcie4, whose link
only comes up once the power sequencing drivers have been loaded from
the root file system. The kernel that loads the image resumes before
that, so it never powers pcie4, and the restored kernel then reads the
DBI registers of that controller:

  qcom_pcie_resume_noirq()
    qcom_pcie_icc_opp_update()
      dw_pcie_find_capability()
        dw_pcie_read_dbi()

The machine resets at that read. pcie6a, which holds the NVMe drive and
is brought up by both kernels, gets through the same path.

Take the shortcut only while a suspend is in progress. A hibernation
image then always holds a controller that is down, and thaw and restore
bring it up through dw_pcie_resume_noirq(), like after any other loss
of power.

Signed-off-by: Birk Skyum <birk.skyum@pm.me>
---
RFC because this changes the freeze path of every driver that uses
dw_pcie_suspend_noirq(), and I can only test qcom. If you would rather
have it in pcie-qcom alone, or solved on the restore side, I am happy
to redo it.

Tested on a Lenovo Yoga Slim 7x with Arch Linux ARM's v7.2 kernel plus
this patch, booted both for the image and for the resume.
dw_pcie_suspend_noirq() is the same in v7.3-rc6 and in pci/next, which
this is based on.

- A hibernation with a real power-off resumes (one run). Before, every
  attempt reset the machine.
- pm_test=core and test_resume pass.
- pcie4 and pcie6a both go down at freeze and their links come back at
  thaw and at restore. NVMe and Wi-Fi work afterwards.
- A suspend (deep) and wake still works, and neither link is retrained
  across it.

Two things to know about the test:

- I unloaded ath12k and the Bluetooth UART driver for the hibernation.
  With the Bluetooth UART open the resume stops later, in the GENI
  serial driver, and the remote processors do not come back after a
  resume either. Both are separate from this and still open.
- With ath12k unloaded the Wi-Fi card is in D0 at freeze, and the wait
  for L2 times out on pcie4 ("Timeout waiting for L2 entry! LTSSM:
  0x11"). pcie6a does not show that.

Nothing survives the reset, so to find the read a test module wrote
each step of the restored kernel to an EFI variable before it ran. The
last record is the entry into dw_pcie_read() for pcie4, after
gdsc_enable() and icc_enable() had returned.

 drivers/pci/controller/dwc/pcie-designware-host.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 77aa79ef0..458e4c6e7 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -19,6 +19,7 @@
 #include <linux/pci.h>
 #include <linux/pci_regs.h>
 #include <linux/platform_device.h>
+#include <linux/suspend.h>
 
 #include "../pci-host-common.h"
 #include "../../pci.h"
@@ -1231,7 +1232,14 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
 	if (!dw_pcie_link_up(pci))
 		goto stop_link;
 
-	if (!pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable))
+	/*
+	 * Leaving the controller powered is only safe if it keeps its state
+	 * until resume. A hibernation image is resumed after a power cycle, by
+	 * a kernel that may not have brought this controller up, so only a
+	 * suspend may leave it powered.
+	 */
+	if (pm_suspend_in_progress() &&
+	    !pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable))
 		return 0;
 
 	if (pci->pp.ops->pme_turn_off) {

base-commit: 591d89f23a7cbc668579521513c7f53b0968754c
-- 
2.53.0

Re: [RFC PATCH] PCI: dwc: Power the controller down when hibernating

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci: hibernation S4 WOL and thaw broken by short-circuited D3cold check
--

commit fefc3f1865b9c97f0642e5cbcd08bb6b0f27167f
Author: Birk Skyum <birk.skyum@pm.me>

PCI: dwc: Power the controller down when hibernating

This commit modifies dw_pcie_suspend_noirq() to power down the controller
during hibernation by checking pm_suspend_in_progress(). This ensures that a
hibernation image resumed after a power cycle does not crash when the kernel
reads uninitialized DBI registers.

> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 77aa79ef0ccf8..458e4c6e798b0 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
[ ... ]
> @@ -1231,7 +1232,14 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
>  	if (!dw_pcie_link_up(pci))
>  		goto stop_link;
>  
> -	if (!pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable))
> +	/*
> +	 * Leaving the controller powered is only safe if it keeps its state
> +	 * until resume. A hibernation image is resumed after a power cycle, by
> +	 * a kernel that may not have brought this controller up, so only a
> +	 * suspend may leave it powered.
> +	 */
> +	if (pm_suspend_in_progress() &&
> +	    !pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable))
>  		return 0;

[Severity: High]
Does this short-circuiting && operator cause issues during hibernation?

During hibernation phases (poweroff_noirq and freeze_noirq),
pm_suspend_in_progress() returns false. This causes the && operator to
short-circuit, skipping the call to pci_host_common_d3cold_possible().

Because of this, pme_capable remains at its default uninitialized value of
false. Later in dw_pcie_suspend_noirq(), pci->pp.skip_pwrctrl_off is assigned
the value of pme_capable:

stop_link:
	...
	pci->pp.skip_pwrctrl_off = pme_capable;
	dw_pcie_stop_link(pci);

Since pme_capable is unconditionally false, pci_pwrctrl_power_off_devices()
will force cutting auxiliary power. 

This completely breaks S4 Wake-on-LAN and wipes endpoint hardware state during
the freeze phase, violating the core PM contract that thaw operates on preserved
hardware state. Endpoints lacking D3cold support will crash or fail to resume
during the thaw phase.

>  
>  	if (pci->pp.ops->pme_turn_off) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/179157746908.73485.2343672865542690797@pm.me?part=1

Re: [RFC PATCH] PCI: dwc: Power the controller down when hibernating

[Birk Skyum]

> Does this short-circuiting && operator cause issues during hibernation?
> [...]
> Since pme_capable is unconditionally false, pci_pwrctrl_power_off_devices()
> will force cutting auxiliary power.

That part is right. At poweroff_noirq the devices are in D3hot, the
helper would have reported PME from D3cold, and with this patch a
wake-enabled device loses its power anyway. In v2 I will call the
helper first and ignore only its verdict, not its PME result:

	if (!pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable) &&
	    pm_suspend_in_progress())
		return 0;

At freeze_noirq the devices are in D0, so the helper stops at the first
one and pme_capable stays false, which is what the image needs. I ran
this version on the Yoga Slim 7x before writing, and a hibernation with
a power-off resumes as it does with v1.

> wipes endpoint hardware state during the freeze phase, violating the
> core PM contract that thaw operates on preserved hardware state.

The reset between freeze and thaw is real, and it is the price of this
approach. pci_pm_thaw_noirq() returns the device to D0 and restores its
config space before the driver's thaw runs, so that much is covered,
and a driver whose thaw is its resume copes. NVMe did here. A driver
with a lighter thaw that expects the device's own state to be intact
would not.

I did it at freeze because I do not see how to do it at restore. The
image says the clocks, regulators and PHY of the controller are on, so
restore_noirq cannot bring them up without first taking them down, and
that means writing to a PHY that may have no power, the same kind of
access that resets the machine today. If there is a way to do this on
the restore side, or if it belongs in pcie-qcom only, I would rather do
that.

I will hold v2 until a maintainer has had a look.

Re: [RFC PATCH] PCI: dwc: Power the controller down when hibernating

[Krishna Chaitanya Chundru]

On 10/10/2026 1:54 AM, Birk Skyum wrote:
> dw_pcie_suspend_noirq() leaves the controller powered, with
> pci->suspended false, when a device below it cannot go to D3cold.
> dw_pcie_resume_noirq() then returns early and the driver carries on
> with the controller as it was.
>
> That is fine for suspend. Drivers use the same two functions for
> hibernation, where they run as freeze_noirq and restore_noirq. Devices
> are normally still in D0 at freeze, so a controller with a device
> behind it is left powered and the image records it as running. That
> image is resumed after a power cycle, by a kernel that may never have
> brought the controller up.
>
> On a Lenovo Yoga Slim 7x (X1E80100) this resets the machine on every
> resume from hibernation. The Wi-Fi card is behind pcie4, whose link
> only comes up once the power sequencing drivers have been loaded from
> the root file system. The kernel that loads the image resumes before
> that, so it never powers pcie4, and the restored kernel then reads the
> DBI registers of that controller:
>
>   qcom_pcie_resume_noirq()
>     qcom_pcie_icc_opp_update()
>       dw_pcie_find_capability()
>         dw_pcie_read_dbi()
>
> The machine resets at that read. pcie6a, which holds the NVMe drive and
> is brought up by both kernels, gets through the same path.
>
> Take the shortcut only while a suspend is in progress. A hibernation
> image then always holds a controller that is down, and thaw and restore
> bring it up through dw_pcie_resume_noirq(), like after any other loss
> of power.
>
> Signed-off-by: Birk Skyum <birk.skyum@pm.me>
> ---
> RFC because this changes the freeze path of every driver that uses
> dw_pcie_suspend_noirq(), and I can only test qcom. If you would rather
> have it in pcie-qcom alone, or solved on the restore side, I am happy
> to redo it.
>
> Tested on a Lenovo Yoga Slim 7x with Arch Linux ARM's v7.2 kernel plus
> this patch, booted both for the image and for the resume.
> dw_pcie_suspend_noirq() is the same in v7.3-rc6 and in pci/next, which
> this is based on.
>
> - A hibernation with a real power-off resumes (one run). Before, every
>   attempt reset the machine.
> - pm_test=core and test_resume pass.
> - pcie4 and pcie6a both go down at freeze and their links come back at
>   thaw and at restore. NVMe and Wi-Fi work afterwards.
> - A suspend (deep) and wake still works, and neither link is retrained
>   across it.
>
> Two things to know about the test:
>
> - I unloaded ath12k and the Bluetooth UART driver for the hibernation.
>   With the Bluetooth UART open the resume stops later, in the GENI
>   serial driver, and the remote processors do not come back after a
>   resume either. Both are separate from this and still open.
> - With ath12k unloaded the Wi-Fi card is in D0 at freeze, and the wait
>   for L2 times out on pcie4 ("Timeout waiting for L2 entry! LTSSM:
>   0x11"). pcie6a does not show that.
>
> Nothing survives the reset, so to find the read a test module wrote
> each step of the restored kernel to an EFI variable before it ran. The
> last record is the entry into dw_pcie_read() for pcie4, after
> gdsc_enable() and icc_enable() had returned.
>
>  drivers/pci/controller/dwc/pcie-designware-host.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 77aa79ef0..458e4c6e7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -19,6 +19,7 @@
>  #include <linux/pci.h>
>  #include <linux/pci_regs.h>
>  #include <linux/platform_device.h>
> +#include <linux/suspend.h>
>  
>  #include "../pci-host-common.h"
>  #include "../../pci.h"
> @@ -1231,7 +1232,14 @@ int dw_pcie_suspend_noirq(struct dw_pcie *pci)
>  	if (!dw_pcie_link_up(pci))
>  		goto stop_link;
>  
> -	if (!pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable))
> +	/*
> +	 * Leaving the controller powered is only safe if it keeps its state
> +	 * until resume. A hibernation image is resumed after a power cycle, by
> +	 * a kernel that may not have brought this controller up, so only a
> +	 * suspend may leave it powered.
> +	 */
> +	if (pm_suspend_in_progress() &&
> +	    !pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable))
>  		return 0;

Birk Skyum,

can you add freeze_noirq and restore_noirq instead of reusing the suspend_noirq & resume_noirq. After adding them you can force d3cold similar to this patch[1]

[1] [PATCH v6 3/3] PCI: qcom: Implement shutdown() callback - Krishna Chaitanya
Chundru <https://lore.kernel.org/all/20260909-shutdown-v6-3-62b7bd123236@oss.qualcomm.com/>

- Krishna Chaitanya.

>  
>  	if (pci->pp.ops->pme_turn_off) {
>
> base-commit: 591d89f23a7cbc668579521513c7f53b0968754c