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

[Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>]

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