Re: qwz: enable suspend/resume

Mark Kettenis <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]>
> Date: Sun, 7 Jun 2026 12:02:44 +0200
> From: Marcus Glocker <[email protected]>
> 
> 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 <[email protected]>
> > > 
> > > 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) {
> > > 
> > > 
> > 
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.