[PATCH 0/4] wifi: ath12k: recover from a firmware crash during resume

[PATCH 0/4] wifi: ath12k: recover from a firmware crash during resume

[Vladyslav Tymoshchuk]

On my device the firmware sometimes crashes during the cold boot of a
resume from s2idle (no WoWLAN, so the device is powered down in
suspend). Without these patches the driver does not recover: the resume
takes 20 s longer, pci_pm_resume returns -110 and the device is dead
until a reboot. Patches 1 and 2 fix that. Patches 3 and 4 fix two
oopses I hit when the firmware crashed more than once in a row while
testing them.

Platform: OnePlus Pad 3 (SM8750) with a WCN7880 hw2.0 (PCI 17cb:110e),
firmware WLAN.GNG.1.0-03342-QCAGNGSWPL_V1.0_V2.0_SILICONZ-1.81206.153.
ath-next does not support this chip; I run it with an out-of-tree patch
that adds it as a WCN7850 derivative, on v7.3-rc5 plus board patches.
For that reason the patches carry no Tested-on tag. On ath-next
(ff4c7359e310) the series is only build tested.

The firmware crash itself is not addressed here; I am sending a
separate report about it. It happened in 1 of 64 and in 1 of 240
suspend cycles.

Tests, all on the tree described above:

- Patches 1-3, crash forced with ath12k_mhi_force_rddm() at the
  ATH12K_QMI_EVENT_FW_READY event of the resume boot: 6 of 6 resumes
  recovered, the link was back 16 s after the resume (6 s without a
  crash). The same build without patch 1 fails like the natural crash
  ("Service connect timeout", "failed to connect to HTT: -110").
- Patches 1-3, no forced crash: 240 suspend cycles, the firmware
  crashed once and the driver recovered, 16 s.
- Patch 4, two forced crashes in a row, the second 3 to 9 s after
  "pdev 0 successfully recovered", followed by "iw scan trigger":
  without the patch the oops in 2 of 2 runs, on the first pair; with
  it 12 pairs without one.

The test hook is not part of the series; I can post it if it is of
use.

Two things I would like comments on:

- Patch 2 lets ath12k_core_resume() return while the reset is still
  running. The alternative I tried, keeping the MHI devices over the
  reset as suspend does (ath12k_hif_power_down(ab, true)), does not
  work: QMI stays silent after the second power up.
- With the series a crash during resume still ends in "Hardware became
  unavailable upon resume" from ieee80211_reconfig(), as the device is
  not up yet when mac80211 resumes, and userspace has to bring the
  interface up again. That is where the 16 s come from. I have not
  tried to change that.

I used an AI assistant (Claude) for the analysis and to draft the
patches and their descriptions; I reviewed the result and ran the tests
on my device.

Vladyslav Tymoshchuk (4):
  wifi: ath12k: disable and enable the CE interrupts only once
  wifi: ath12k: do not wait in resume for a reset that cannot finish
    there
  wifi: ath12k: unregister the thermal devices of a radio only once
  wifi: ath12k: skip the links without a radio when a scan fails

 drivers/net/wireless/ath/ath12k/core.c    | 6 ++++++
 drivers/net/wireless/ath/ath12k/core.h    | 1 +
 drivers/net/wireless/ath/ath12k/mac.c     | 3 +++
 drivers/net/wireless/ath/ath12k/pci.c     | 6 ++++--
 drivers/net/wireless/ath/ath12k/thermal.c | 2 +-
 5 files changed, 15 insertions(+), 3 deletions(-)


base-commit: ff4c7359e310744a7abe53cf66d921fa2969eb19
-- 
2.43.0

[PATCH 1/4] wifi: ath12k: disable and enable the CE interrupts only once

[Vladyslav Tymoshchuk]

ath12k_pci_ce_irqs_disable() calls disable_irq_nosync() each time it is
called, and ath12k_pci_ce_irqs_enable() calls enable_irq() once, from
ath12k_pci_start().

Without WoWLAN, ath12k_core_suspend_late() disables the CE interrupts
and powers the device down, and ath12k_core_resume_early() boots the
firmware again. If the firmware crashes during that boot, before
ath12k_pci_start(), ath12k_core_reset() disables the interrupts a
second time. The start that follows the reset enables them once, they
stay masked and the HTC control response is never seen:

  ath12k_wifi7_pci 0000:01:00.0: Service connect timeout
  ath12k_wifi7_pci 0000:01:00.0: failed to connect to HTT: -110
  ath12k_wifi7_pci 0000:01:00.0: failed to start core: -110
  ath12k_wifi7_pci 0000:01:00.0: failed to reconfigure driver on crash recovery

The device is dead until a reboot.

Make ATH12K_FLAG_CE_IRQ_ENABLED guard both functions, as
ATH12K_FLAG_EXT_IRQ_ENABLED does for the ext interrupts.

Fixes: 8d5f4da8d70b ("wifi: ath12k: support suspend/resume")
Signed-off-by: Vladyslav Tymoshchuk <tvlad744@gmail.com>
---
 drivers/net/wireless/ath/ath12k/pci.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c
index 4bb14a95aec9..c92821153706 100644
--- a/drivers/net/wireless/ath/ath12k/pci.c
+++ b/drivers/net/wireless/ath/ath12k/pci.c
@@ -398,7 +398,8 @@ static void ath12k_pci_ce_irqs_disable(struct ath12k_base *ab)
 {
 	int i;
 
-	clear_bit(ATH12K_FLAG_CE_IRQ_ENABLED, &ab->dev_flags);
+	if (!test_and_clear_bit(ATH12K_FLAG_CE_IRQ_ENABLED, &ab->dev_flags))
+		return;
 
 	for (i = 0; i < ab->hw_params->ce_count; i++) {
 		if (ath12k_ce_get_attr_flags(ab, i) & CE_ATTR_DIS_INTR)
@@ -739,7 +740,8 @@ void ath12k_pci_ce_irqs_enable(struct ath12k_base *ab)
 {
 	int i;
 
-	set_bit(ATH12K_FLAG_CE_IRQ_ENABLED, &ab->dev_flags);
+	if (test_and_set_bit(ATH12K_FLAG_CE_IRQ_ENABLED, &ab->dev_flags))
+		return;
 
 	for (i = 0; i < ab->hw_params->ce_count; i++) {
 		if (ath12k_ce_get_attr_flags(ab, i) & CE_ATTR_DIS_INTR)
-- 
2.43.0

[PATCH 2/4] wifi: ath12k: do not wait in resume for a reset that cannot finish there

[Vladyslav Tymoshchuk]

ath12k_core_resume() waits up to 20 s for restart_completed. When the
firmware crashes during the cold boot started by
ath12k_core_resume_early(), ath12k_core_reset() powers MHI down, which
destroys the MHI channel devices, and up again. The driver core does
not probe the new devices before the system resume is over
(device_block_probing()), so QMI cannot come up and the restart cannot
complete while ath12k_core_resume() is waiting for it:

  mhi mhi0: Wait for device to enter SBL or Mission mode
  ath12k_wifi7_pci 0000:01:00.0: timeout while waiting for restart complete
  ath12k_wifi7_pci 0000:01:00.0: failed to resume core: -110
  ath12k_wifi7_pci 0000:01:00.0: PM: dpm_run_callback(): pci_pm_resume returns -110
  PM: suspend exit
  ath12k_wifi7_pci 0000:01:00.0: fw_version 0x10097fff ...

The resume takes 20 s longer and fails.

Let ath12k_core_resume() go on as soon as a reset starts during the
resume. The restart then completes after the resume, as it does for a
crash at any other time.

Fixes: 8d5f4da8d70b ("wifi: ath12k: support suspend/resume")
Signed-off-by: Vladyslav Tymoshchuk <tvlad744@gmail.com>
---
 drivers/net/wireless/ath/ath12k/core.c | 6 ++++++
 drivers/net/wireless/ath/ath12k/core.h | 1 +
 2 files changed, 7 insertions(+)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 42a9cd28c652..b1d1247a0dc6 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -224,6 +224,7 @@ int ath12k_core_resume_early(struct ath12k_base *ab)
 		return ret;
 
 	reinit_completion(&ab->restart_completed);
+	WRITE_ONCE(ab->resuming, true);
 	ret = ath12k_hif_power_up(ab);
 	if (ret)
 		ath12k_warn(ab, "failed to power up hif during resume: %d\n", ret);
@@ -243,6 +244,7 @@ int ath12k_core_resume(struct ath12k_base *ab)
 
 	time_left = wait_for_completion_timeout(&ab->restart_completed,
 						ATH12K_RESET_TIMEOUT_HZ);
+	WRITE_ONCE(ab->resuming, false);
 	if (time_left == 0) {
 		ath12k_warn(ab, "timeout while waiting for restart complete");
 		return -ETIMEDOUT;
@@ -1727,6 +1729,10 @@ static void ath12k_core_reset(struct work_struct *work)
 	int reset_count, fail_cont_count, i;
 	long time_left;
 
+	/* The new MHI devices are probed only after the system resume */
+	if (READ_ONCE(ab->resuming))
+		complete(&ab->restart_completed);
+
 	if (!(test_bit(ATH12K_FLAG_QMI_FW_READY_COMPLETE, &ab->dev_flags))) {
 		ath12k_warn(ab, "ignore reset dev flags 0x%lx\n", ab->dev_flags);
 		return;
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 3a92a4b03359..2d8d91afb66f 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -1072,6 +1072,7 @@ struct ath12k_base {
 	atomic_t reset_count;
 	atomic_t recovery_count;
 	bool is_reset;
+	bool resuming;
 	struct completion reset_complete;
 	/* continuous recovery fail count */
 	atomic_t fail_cont_count;
-- 
2.43.0

[PATCH 3/4] wifi: ath12k: unregister the thermal devices of a radio only once

[Vladyslav Tymoshchuk]

ath12k_core_reconfigure_on_crash() calls ath12k_thermal_unregister(),
and the devices are registered again only when the start that follows
reaches ath12k_core_pdev_create(). If it does not, because the firmware
crashes again or the start fails earlier, the next restart unregisters
the devices a second time:

  Unable to handle kernel NULL pointer dereference at virtual address 0000000000000050
  Workqueue: ath12k_wq ath12k_core_restart [ath12k]
  pc : hwmon_device_unregister+0x24/0x90
  lr : ath12k_thermal_unregister+0x7c/0xf4 [ath12k]
  Call trace:
   hwmon_device_unregister+0x24/0x90 (P)
   ath12k_thermal_unregister+0x7c/0xf4 [ath12k]
   ath12k_core_restart+0x3c/0x210 [ath12k]

Skip the radios that have nothing registered.

Fixes: 151322bccdbd ("wifi: ath12k: add basic hwmon temperature reporting")
Signed-off-by: Vladyslav Tymoshchuk <tvlad744@gmail.com>
---
 drivers/net/wireless/ath/ath12k/thermal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath12k/thermal.c b/drivers/net/wireless/ath/ath12k/thermal.c
index 97fc49c40ac1..7707ae4eb683 100644
--- a/drivers/net/wireless/ath/ath12k/thermal.c
+++ b/drivers/net/wireless/ath/ath12k/thermal.c
@@ -257,7 +257,7 @@ static void ath12k_thermal_cleanup_radio(struct ath12k_base *ab, int i)
 	struct ath12k *ar;
 
 	ar = ab->pdevs[i].ar;
-	if (!ar)
+	if (!ar || !ar->thermal.cdev)
 		return;
 
 	hwmon_device_unregister(ar->thermal.hwmon_dev);
-- 
2.43.0

[PATCH 4/4] wifi: ath12k: skip the links without a radio when a scan fails

[Vladyslav Tymoshchuk]

ath12k_mac_vdev_create() clears arvif->ar when the vdev creation fails,
and the link stays in ahvif->links_map. When a scan fails after that,
ath12k_mac_op_hw_scan() walks all the links of the vif to clean up and
dereferences the NULL radio:

  ath12k_wifi7_pci 0000:01:00.0: failed to create WMI vdev 1: -108
  ath12k_wifi7_pci 0000:01:00.0: failed to create vdev 00:03:7f:12:34:56 ret -108
  Unable to handle kernel paging request at virtual address 0000000000001508
  pc : ath12k_mac_op_hw_scan+0x9cc/0xb28 [ath12k]
  Call trace:
   ath12k_mac_op_hw_scan+0x9cc/0xb28 [ath12k] (P)
   __ieee80211_start_scan+0x4e8/0x6e0 [mac80211]
   ieee80211_request_scan+0x10/0x1c [mac80211]
   ieee80211_scan+0x98/0x14c [mac80211]
   cfg80211_scan+0x50/0x1f4 [cfg80211]
   nl80211_trigger_scan+0x694/0x6cc [cfg80211]

This happens when the firmware crashes a second time while mac80211 is
reconfiguring the device after a first crash: the WMI commands fail
with -ESHUTDOWN, and so do the vdev creation of the station link and
the scan that wpa_supplicant requests next. The oops leaves the wiphy
mutex locked, and everything that needs it hangs.

Such a link has neither a scan to stop nor a vdev to remove.

Fixes: feed05f1526e ("wifi: ath12k: Split scan request for split band device")
Signed-off-by: Vladyslav Tymoshchuk <tvlad744@gmail.com>
---
 drivers/net/wireless/ath/ath12k/mac.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 4cb41b8b1d3b..49cb9bd8dda1 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -5889,6 +5889,9 @@ int ath12k_mac_op_hw_scan(struct ieee80211_hw *hw,
 				continue;
 
 			ar = arvif->ar;
+			if (!ar)
+				continue;
+
 			if (ar->scan.arvif == arvif) {
 				wiphy_work_cancel(hw->wiphy, &ar->scan.vdev_clean_wk);
 				spin_lock_bh(&ar->data_lock);
-- 
2.43.0