[PATCH wireless v2 3/3] wifi: iwlwifi: pcie: recover a device that lost power in D3cold

[Navon John Lukose]

On a Lenovo Yoga Pro 7 14IAH10 (Arrow Lake-H) with a discrete BE200
(8086:272b), D3cold removes the module's power rail and the device does
not restart when the rail and PERST# are restored. After _ON the
power-enable and PERST# GPIO pad registers read correct and the link
still never trains; config space reads all ones until reboot.

The platform can recover it, with a WLAN-specific reset line driven by
the object _PRR returns - which is exactly the product reset the driver
already implements. The problem is ordering: AML only dispatches the
vendor DSM that selects that mode after reading the device's PCI ID back
out of config space,

    Method (WIST) { Switch (ToInteger (VDID)) { Case (0x272B8086) {...} } }
    Method (_DSM) { ... If (WIST ()) { ... Return (IFUN (...)) } ... }

so once the device is off the bus acpi_check_dsm() fails and the mode
stays deselected. iwl_trans_pcie_removal_wk() is the only place that
selects it today, and by then it is too late: _RST takes its other
branch and issues a function level reset to a device that is not there.

So arm the mode in .suspend, while the device still answers, and disarm
it again in .resume. Treat the device as gone only when two independent
signals agree: the mode we armed can no longer be disarmed (so the
platform cannot see the device either) and CSR_HW_REV reads all ones (so
neither can we). Either alone is not enough - a DSM can fail for
transient ACPI reasons on a healthy adapter, and a false positive costs a
remove, a platform reset and a rescan on every resume. The order of the
terms is load bearing: a device in D3hot answers config cycles but does
not decode its BARs, so the DSM would still work while CSR_HW_REV read
all ones. The disarm has to short-circuit.

Recovery goes through the existing iwl_trans_pcie_reset() path, which
only queues a work item, so the remove, the _RST and the rescan happen
after .resume has returned and the PM core has dropped the device lock.
The op_mode is not notified beyond the STATUS_TRANS_DEAD that
iwl_trans_pcie_reset() sets; as today, it finds out by having its own
resume fail against the dead device. Taking this path also skips the
handshake timeouts and the bogus ADVANCED_SYSASSERT dump the driver
otherwise produces against absent hardware, which on this machine cost
about two seconds on every failed resume.

Arming is confined to discrete modules: on integrated CNVi parts
iwl_trans_pcie_set_product_reset() sends EN_PROD_RESET on its own, which
lands in \_SB.PC00.CNVW.RSTT and is what the CNVi _RST branches on before
killing Bluetooth and issuing the PLDR. Arming that from .suspend on
hardware I cannot test is not worth it, so the integrated mask stays as
unexercised as it is today.

iwl_trans_pcie_set_product_reset() now reports whether the DSM took, and
its error on failure becomes a debug message: .suspend would otherwise log
an error on every suspend on every discrete machine without this DSM. So
iwl_trans_pcie_removal_wk() no longer logs at error level when it cannot
arm, which on the recovery path is every time, since the device is off the
bus by then. The cost is that a genuine product reset on a live device
with no DSM support is now silent at error level too.

One known limitation: where me_present is not 0, iwl_trans_pcie_reset()
downgrades the request to IWL_RESET_MODE_FUNC_RESET. The device still
comes back, because the mode is already armed and _RST does the product
reset regardless, but the Bluetooth function is not torn down first.

Cc: stable+noautosel@xxxxxxxxxx # new suspend/resume behaviour, one machine
Link: https://bugzilla.kernel.org/show_bug.cgi?id=221695
Link: https://lore.kernel.org/all/20260722021321.68902-1-nika@xxxxxxxxxxxx/
Link: https://lore.kernel.org/all/20260829093922.37103-1-navonjohnlukose@xxxxxxxxx/
Signed-off-by: Navon John Lukose <navonjohnlukose@xxxxxxxxx>
---
The bugzilla and the first lore Link: are other BE200/GL reports of the
same 0xffffffff-until-reboot, on machines I do not have; neither is
claimed as fixed, hence Link: and not Closes:. The assert in the bugzilla
report is the dump against absent hardware this patch skips, not a
firmware bug it fixes. The second lore Link: is my own analysis of this
machine's AML.

Lenovo Yoga Pro 7 14IAH10 (Arrow Lake-H, Core Ultra 9 285H), BIOS
QGCN35WW, discrete BE200 SUBSYS_00F48086, Bluetooth on USB, no CSME,
stock ACPI tables.

This machine ships a udev rule forcing d3cold_allowed to 0, which would
have made the test vacuous. It was moved aside for the whole run and
d3cold_allowed written back to 1 before every cycle, so the device really
did reach D3cold: detection fired on all five cycles, which cannot happen
otherwise. Before the patch every cycle left the device dead until reboot;
with it the device recovered on all five, four with wifi connected at
suspend and one with the radio down, the case where .suspend runs with no
op_mode. With the reset skipped and nothing else changed it stayed absent,
so it is the reset that recovers it and not the remove/rescan.

With debug=0x100 one cycle logs the intended path end to end:

    iwl_trans_pcie_set_product_reset Enabled product reset via DSM
    iwl_trans_pcie_check_product_reset_mode product reset mode is 0x1
    iwl_trans_pcie_set_product_reset can't disable product reset via DSM (-19)
    device not responding after resume
    scheduling reset (mode=6)
    iwl_trans_pcie_set_product_reset can't enable product reset via DSM (-19)
    iwl_trans_pcie_call_reset called _RST on _PRR object

mode=6 is IWL_RESET_MODE_PROD_RESET, so the request was not downgraded.
The link trained in 64-76 ms and the interface was usable 5.00-5.05 s after
.resume returned; end to end it is closer to 7 s, because the PCI core
spends ~2 s retraining a link that cannot train before .resume is called.
4.365 s of the rest is one _RST evaluation against a 4.320 s floor computed
from the Sleep() operators in the AML, so essentially all of it is platform
AML, and asking for a product reset is not what costs it: both arms of _RST
fall through to the same two 2000 ms sleeps and the product arm adds only
2 x RDLY (160 ms each here).

What is untested or untestable with one machine:

- .suspend and .resume are untouched on integrated/CNVi: the arming helper
  returns early there, so prod_reset_set is never set. (The error-level
  demotion does apply to integrated parts on the removal_wk() path.)
  Getting the CNVi case working needs someone with the hardware.
- Where me_present is non-zero - including the permanent -1 that
  iwl_pcie_check_me_status() leaves on everything below
  IWL_DEVICE_FAMILY_BZ, which is four of the five Intel IDs this AML
  accepts - the request is downgraded to IWL_RESET_MODE_FUNC_RESET. The
  device still comes back, because the mode is armed and _RST does the
  product reset anyway, but Bluetooth is not torn down first. That is the
  pre-existing hazard the previous patch describes, now reachable from
  resume. me_present is a real 0 here, so this is reasoned, not observed.
- If the disarm fails transiently on a live device, the code clears
  prod_reset_set and carries on while the platform's mode stays selected
  until the next probe, which re-opens that same hazard. Retrying the
  disarm would narrow it; I did not, because a retry loop around an AML
  method on the resume path needs a bound I cannot justify from one
  machine.
- Only s2idle was tested. The same callback is .freeze and .poweroff, so
  hibernate arms too and the image is snapshotted while armed, meaning a
  restore kernel disarms a mode a previous boot selected. Harmless as far
  as I can reason it, but unexercised. A device that dies at runtime is
  still unrecoverable; that needs the same thing on runtime PM, which
  iwlwifi does not implement.
- Only one BIOS. On mine the reset method branches solely on the mode
  variable, never on WIST()/VDID, which is what makes the downgrade above
  survivable. I cannot claim that for every implementation.
- On a platform with the arming DSM but no usable _PRR,
  iwl_trans_pcie_call_reset() falls back to pci_reset_function() against a
  device that is gone, and pci_dev_wait() polls config space for up to
  ~65 s per reset method with pci_lock_rescan_remove() held. That is the
  cost of a true positive, not a false one: a false positive still answers
  config cycles, so pci_dev_wait() returns on its first read.
- iwl_trans_pcie_removal_wk() holds pci_lock_rescan_remove() across the
  whole reset, so it is now held for ~4.3 s during system resume on a
  machine that also has Thunderbolt wanting it. Pre-existing, but this
  patch is what puts it on the resume path.

The arming mask is heavier than the recovery needs - only EN_PROD_RESET
drives the GPIO - but I kept it so the reset armed from .suspend is bit for
bit the one iwl_trans_pcie_removal_wk() already arms. Narrowing it is an
easy follow-up.

 drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 24 +++++++++++++++++++
 .../intel/iwlwifi/pcie/gen1_2/internal.h      |  4 ++++
 .../intel/iwlwifi/pcie/gen1_2/trans.c         | 24 ++++++++++++++-----
 3 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
index 7a7b101..5d01a4d 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c
@@ -1203,11 +1203,19 @@ static void iwl_pci_remove(struct pci_dev *pdev)
 
 static int iwl_pci_suspend(struct device *device)
 {
+	struct iwl_trans *trans = pci_get_drvdata(to_pci_dev(device));
+
 	/* Before you put code here, think about WoWLAN. You cannot check here
 	 * whether WoWLAN is enabled or not, and your code will run even if
 	 * WoWLAN is enabled - don't kill the NIC, someone may need it in Sx.
 	 */
 
+	/* Has to be here, while the device still answers: AML gates this DSM
+	 * on reading the device's PCI ID out of config space. It doesn't touch
+	 * the NIC.
+	 */
+	iwl_trans_pcie_arm_product_reset(trans, true);
+
 	return 0;
 }
 
@@ -1229,6 +1237,22 @@ static int _iwl_pci_resume(struct device *device, bool restore)
 	 */
 	pci_write_config_byte(pdev, PCI_CFG_RETRY_TIMEOUT, 0x00);
 
+	/* Two signals that it didn't come back from D3cold: the platform can't
+	 * deselect the mode armed in .suspend (so it can't see the device
+	 * either), and the device doesn't answer. Before the op_mode test: the
+	 * firmware may never have been loaded.
+	 */
+	if (trans_pcie->prod_reset_set) {
+		iwl_trans_pcie_arm_product_reset(trans, false);
+		if (trans_pcie->prod_reset_set &&
+		    iwl_read32(trans, CSR_HW_REV) == ~0U) {
+			IWL_ERR(trans, "device not responding after resume\n");
+			iwl_trans_pcie_reset(trans, IWL_RESET_MODE_PROD_RESET);
+			return 0;
+		}
+		trans_pcie->prod_reset_set = false;
+	}
+
 	if (!trans->op_mode)
 		return 0;
 
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/gen1_2/internal.h b/drivers/net/wireless/intel/iwlwifi/pcie/gen1_2/internal.h
index d84c7c1..1caaff9 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/gen1_2/internal.h
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/gen1_2/internal.h
@@ -495,6 +495,8 @@ struct iwl_pcie_txqs {
  * @isr_stats: interrupt statistics
  * @napi_dev: (fake) netdev for NAPI registration
  * @txqs: transport tx queues data.
+ * @prod_reset_set: the product reset mode is selected in the platform;
+ *	system suspend/resume only, so process context only
  * @me_present: WiAMT/CSME is detected as present (1), not present (0)
  *	or unknown (-1, so can still use it as a boolean safely)
  * @me_recheck_wk: worker to recheck WiAMT/CSME presence
@@ -605,6 +607,7 @@ struct iwl_trans_pcie {
 
 	struct iwl_pcie_txqs txqs;
 
+	bool prod_reset_set;
 	s8 me_present;
 	struct delayed_work me_recheck_wk;
 
@@ -657,6 +660,7 @@ bool _iwl_trans_pcie_grab_nic_access(struct iwl_trans *trans, bool silent);
 
 void iwl_trans_pcie_check_product_reset_status(struct pci_dev *pdev);
 void iwl_trans_pcie_check_product_reset_mode(struct pci_dev *pdev);
+void iwl_trans_pcie_arm_product_reset(struct iwl_trans *trans, bool arm);
 
 /*****************************************************
 * RX
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/gen1_2/trans.c b/drivers/net/wireless/intel/iwlwifi/pcie/gen1_2/trans.c
index df89fb3..56eb35d 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/gen1_2/trans.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/gen1_2/trans.c
@@ -2075,7 +2075,7 @@ void iwl_trans_pcie_check_product_reset_mode(struct pci_dev *pdev)
 	ACPI_FREE(res);
 }
 
-static void iwl_trans_pcie_set_product_reset(struct pci_dev *pdev, bool enable,
+static bool iwl_trans_pcie_set_product_reset(struct pci_dev *pdev, bool enable,
 					     bool integrated)
 {
 	union acpi_object *res;
@@ -2089,17 +2089,29 @@ static void iwl_trans_pcie_set_product_reset(struct pci_dev *pdev, bool enable,
 						 DSM_INTERNAL_PLDR_CMD_SET_MODE,
 						 mode);
 	if (IS_ERR(res)) {
-		if (enable)
-			IWL_ERR_DEV(&pdev->dev,
-				    "ACPI _DSM not available (%d), cannot do product reset\n",
-				    (int)PTR_ERR(res));
-		return;
+		IWL_DEBUG_DEV_POWER(&pdev->dev,
+				    "can't %sable product reset via DSM (%d)\n",
+				    enable ? "en" : "dis", (int)PTR_ERR(res));
+		return false;
 	}
 
 	ACPI_FREE(res);
 	IWL_DEBUG_DEV_POWER(&pdev->dev, "%sabled product reset via DSM\n",
 			    enable ? "En" : "Dis");
 	iwl_trans_pcie_check_product_reset_mode(pdev);
+	return true;
+}
+
+void iwl_trans_pcie_arm_product_reset(struct iwl_trans *trans, bool arm)
+{
+	struct iwl_trans_pcie *trans_pcie = IWL_TRANS_GET_PCIE_TRANS(trans);
+
+	/* discrete only: the integrated arming mask is untested */
+	if (trans->mac_cfg->integrated)
+		return;
+
+	if (iwl_trans_pcie_set_product_reset(trans_pcie->pci_dev, arm, false))
+		trans_pcie->prod_reset_set = arm;
 }
 
 void iwl_trans_pcie_check_product_reset_status(struct pci_dev *pdev)
-- 
2.55.0