From: Marcus Glocker <marcus@nazgul.ch>
Subject: Re: qwz: enable suspend/resume
To: Mark Kettenis <mark.kettenis@xs4all.nl>
Cc: tech@openbsd.org
Date: Sun, 7 Jun 2026 12:02:44 +0200

On Sun, Jun 07, 2026 at 10:34:58AM +0200, Mark Kettenis wrote:

> > Date: Sun, 7 Jun 2026 08:39:07 +0200
> > From: Marcus Glocker <marcus@nazgul.ch>
> > 
> > This makes suspend/resume with qwz(4) work on my Samsung Galaxy Book4
> > Edge.
> > 
> > Ok?
> 
> So qwx(4) calls _core_deinit() from _stop(), whereas qwz(4) has:
> 
>         /*
>          * Firmware stays running across ifconfig down/up; the chip is
>          * only released on driver detach.  Do not clear pdevs_active
>          * or call qwz_core_deinit() here.
>          */
> 
> In that context, this diff makes sense.  But I do wonder whether
> qwx(4) should do the same thing.  Things to consider are:
> 
> 1. Are there any power savings to be had by stopping the firmware on
>    ifconfig down?

I didn't measure it.  I think it would be low power savings.  But
anyway, I think we haven't much options, see the explanation for your
second questions.
 
> 2. Are there firmware differences that justify that qwz(4) diverges
>    from qwx(4) here.

Yes, this is the load-bearing reason.  The WCN7850 ath12k firmware
empirically does not tolerate repeated full core_deinit /
core_init cycles -- state accumulates inside the chip across cycles
even with the documented M0-M3 flush and PCI sw_reset, and after a few
iterations WMI PDEV commands get silently dropped.  Therefore, we have
adapted the ath12k Linux driver life-cycle model, were the firmware
stays up for the lifetime of the driver attachment.

WCN6855 ath11k firmware apparently does not have this bug, since qwx
with its current pattern has been working reliably.  So qwx should not
follow qwz here.  Conversely qwz can't follow qwx because of the
firmware bug -- that's why we diverged.

> > Index: sys/dev/ic/qwz.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/ic/qwz.c,v
> > diff -u -p -u -p -r1.38 qwz.c
> > --- sys/dev/ic/qwz.c	26 May 2026 14:55:16 -0000	1.38
> > +++ sys/dev/ic/qwz.c	7 Jun 2026 06:01:46 -0000
> > @@ -24857,8 +24857,14 @@ qwz_activate(struct device *self, int ac
> >  			qwz_stop(ifp);
> >  			rw_exit(&sc->ioctl_rwl);
> >  		}
> > +		if (sc->fw_initialized)
> > +			qwz_core_deinit(sc);
> >  		break;
> >  	case DVACT_RESUME:
> > +		err = qwz_hal_srng_init(sc);
> > +		if (err)
> > +			printf("%s: could not initialize hal\n",
> > +			    sc->sc_dev.dv_xname);
> >  		break;
> >  	case DVACT_WAKEUP:
> >  		if ((ifp->if_flags & (IFF_UP | IFF_RUNNING)) == IFF_UP) {
> > 
> > 
>