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

[Birk Skyum <birk.skyum@pm.me>]

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