On Fri, Apr 25, 2025 at 07:17:31PM +0200, Mark Kettenis wrote:
> However, when suspending the laptop, I see some error messages upon
> resume.  The machine recovers and qwx(4) works fine afterwards.  But
> it suggests there is still something not quite right with the state
> machine?

> qwx0: scan stop timeout
> qwx0: failed to abort scan: 35

The scan errors are caused by a missing wakeup() in the code path
which stops scans. An obvious bug. The thread which sends the stop
command is never woken up.

> qwx0: peer create command timeout
> qwx0: Failed to add peer: 6c:71:d9:cd:39:76 for VDEV: 0

I am seeing this one, too. The best explanation I have is that the
newstate task gets scheduled because of a frame received in parallel
to qwx_stop(), after the FLUSH_CRASH flag is cleared (stupid flag name,
I know -- this flag from the Linux driver signals crashed firmware. Very
early during porting of this driver I repurposed this flag to prevent
scheduling of new tasks as well, and never got around to making this pretty).

This error can be prevented by checking for the IFF_RUNNING flag before
scheduling the newstate task.

Both fixes combined below, on top of previous diffs.
I don't see any more errors now.

M  sys/dev/ic/qwx.c  |  8+  6-

1 file changed, 8 insertions(+), 6 deletions(-)

commit - a9a3e6e76dd5e088a6557077d5b215526a7848e3
commit + aad1f217b0653b67795cd712afa39450765c96fd
blob - 731d85bacb2e762cb57c5abb8f0ed4bf9843b6d2
blob + c3bffe842e8ff319f1c7f5de4a859a8cd4d2a4ef
--- sys/dev/ic/qwx.c
+++ sys/dev/ic/qwx.c
@@ -338,13 +338,13 @@ qwx_stop(struct ifnet *ifp)
 
 	qwx_setkey_clear(sc);
 
-	clear_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags);
-
 	ifp->if_timer = sc->sc_tx_timer = 0;
 
 	ifp->if_flags &= ~IFF_RUNNING;
 	ifq_clr_oactive(&ifp->if_snd);
 
+	clear_bit(ATH11K_FLAG_CRASH_FLUSH, sc->sc_flags);
+
 	/*
 	 * Manually run the newstate task's code for switching to INIT state.
 	 * This reconfigures firmware state to stop scanning, or disassociate
@@ -846,6 +846,10 @@ qwx_newstate(struct ieee80211com *ic, enum ieee80211_s
 	struct ifnet *ifp = &ic->ic_if;
 	struct qwx_softc *sc = ifp->if_softc;
 
+	/* We may get triggered by received frames during qwx_stop(). */
+	if (!(ifp->if_flags & IFF_RUNNING))
+		return 0;
+
 	/*
 	 * Prevent attempts to transition towards the same state, unless
 	 * we are scanning in which case a SCAN -> SCAN transition
@@ -12976,7 +12980,6 @@ qwx_wmi_event_scan_start_failed(struct qwx_softc *sc)
 		    qwx_scan_state_str(sc->scan.state), sc->scan.state);
 		break;
 	case ATH11K_SCAN_STARTING:
-		wakeup(&sc->scan.state);
 		qwx_mac_scan_finish(sc);
 		break;
 	}
@@ -23573,9 +23576,8 @@ qwx_mac_scan_finish(struct qwx_softc *sc)
 		timeout_del(&sc->scan.timeout);
 		if (!sc->scan.is_roc)
 			ieee80211_end_scan(ifp);
-#if 0
-		complete_all(&ar->scan.completed);
-#endif
+
+		wakeup(&sc->scan.state);
 		break;
 	}
 }