On some boards D3cold cuts a discrete module's power rail and the device never comes back. The link does not train and config space reads all ones until reboot. The product reset the driver already implements recovers it, but the vendor DSM that selects the product reset mode reads the device's PCI ID out of config space first, so it can only be armed while the device still answers. iwl_trans_pcie_removal_wk(), the only place that arms it, is too late. Arm it in .suspend and disarm in .resume. If the disarm fails and CSR_HW_REV reads all ones, run the product reset. Skip integrated CNVi, whose DSM takes an AML path I cannot test. The DSM failure log drops to debug, since .suspend would otherwise hit it on every suspend on boards without the DSM. Cc: stable+noautosel@kernel.org # 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@nikableh.moe/ Link: https://lore.kernel.org/all/20260829093922.37103-1-navonjohnlukose@gmail.com/ Signed-off-by: Navon John Lukose --- The bugzilla and the first lore Link: are other BE200/GL reports of the same 0xffffffff-until-reboot, on machines I do not have. The second 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. Before the patch s2idle left the device dead until reboot. With it the device comes back, both with wifi connected at suspend and with the radio down, the case where .suspend runs with no op_mode. Skipping just the _RST call and leaving the remove and rescan in place left it absent. 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 (-ENODEV) device not responding after resume scheduling reset (mode=6) iwl_trans_pcie_set_product_reset can't enable product reset via DSM (-ENODEV) iwl_trans_pcie_call_reset called _RST on _PRR object iwl_trans_pcie_set_product_reset Disabled product reset via DSM mode=6 is IWL_RESET_MODE_PROD_RESET, so the request was not downgraded, and the last line is the probe deselect patch 3 adds. Wifi is usable about 5 s after .resume returns, 7 s end to end: 4.4 s of that is the platform's _RST and ~2 s is the PCI core retrying the link before .resume runs. What is untested or untestable with one machine: - .suspend and .resume are untouched on integrated/CNVi, where the arming helper returns early and prod_reset_set is never set. Getting the CNVi case working needs someone with the hardware. - This box has no CSME, so the ME path is untested. Where me_present is non-zero the request is downgraded to IWL_RESET_MODE_FUNC_RESET, but the device should still come back, since the mode is already armed and _RST does the product reset regardless. Bluetooth is not torn down first, which is the pre-existing hazard the previous patch describes. - The failure looks specific to s2idle. One real hibernate cycle armed the mode in .freeze and cleared it again on restore, both confirmed by the DSM readback, and the device came back without needing the reset at all. Hibernate cold-boots to resume, so the part gets a genuine power-on reset, where s2idle cuts the rail and leaves it wedged. - A device that dies at runtime is still unrecoverable. That needs the same thing hooked to runtime PM, which iwlwifi does not implement. drivers/net/wireless/intel/iwlwifi/pcie/drv.c | 24 ++++++++++++++++++ .../intel/iwlwifi/pcie/gen1_2/internal.h | 4 +++ .../intel/iwlwifi/pcie/gen1_2/trans.c | 25 ++++++++++++++----- 3 files changed, 47 insertions(+), 6 deletions(-) diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c index a3e6c9e..96fb7fc 100644 --- a/drivers/net/wireless/intel/iwlwifi/pcie/drv.c +++ b/drivers/net/wireless/intel/iwlwifi/pcie/drv.c @@ -1204,11 +1204,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. */ + /* Arming has to happen while the device still answers, because the + * AML gates this DSM on reading the device's PCI ID out of config + * space. It does not touch the NIC. + */ + iwl_trans_pcie_arm_product_reset(trans, true); + return 0; } @@ -1230,6 +1238,22 @@ static int _iwl_pci_resume(struct device *device, bool restore) */ pci_write_config_byte(pdev, PCI_CFG_RETRY_TIMEOUT, 0x00); + /* Two signals that the device 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. This runs before + * the op_mode test because 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 && + PCI_POSSIBLE_ERROR(iwl_read32(trans, CSR_HW_REV))) { + 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 d93d8f9..a764c6d 100644 --- a/drivers/net/wireless/intel/iwlwifi/pcie/gen1_2/trans.c +++ b/drivers/net/wireless/intel/iwlwifi/pcie/gen1_2/trans.c @@ -5,6 +5,7 @@ * Copyright (C) 2016-2017 Intel Deutschland GmbH */ #include +#include #include #include #include @@ -2075,7 +2076,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 +2090,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 %s product reset via DSM (%pe)\n", + str_enable_disable(enable), 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)