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

> Date: Sun, 7 Jun 2026 12:02:44 +0200
> From: Marcus Glocker <marcus@nazgul.ch>
> 
> 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.

Thanks for the explanation.  I suspect we'll want to revisit this at
some stage.  Because if there are issues with repeated core_deinit /
core_init cycles those could show up with suspend/resume as well.

But for now: ok kettenis@

> > > 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) {
> > > 
> > > 
> > 
>