> Date: Sat, 26 Apr 2025 15:07:00 +0200
> From: Stefan Sperling <stsp@stsp.name>
> 
> 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.

Me neither.  I don't think I understand the driver well enough to give
you an ok, but what you say makes sense.

> 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;
>  	}
>  }
> 
>