From: Marcus Glocker <marcus@nazgul.ch>
Subject: Re: qcpas(4) mdt init: add retry
To: "Kirill A. Korinsky" <kirill@korins.ky>
Cc: tech@openbsd.org
Date: Sun, 17 May 2026 06:39:21 +0200

On Sat, May 16, 2026 at 10:39:48PM +0200, Kirill A. Korinsky wrote:

> On Sat, 16 May 2026 18:12:25 +0200,
> Marcus Glocker <marcus@nazgul.ch> wrote:
> > 
> > On the Samsung Galaxy Book4 Edge every since I am getting intermittent
> > mdt initialization errors from qcpas(4) looking like
> > 
> > 	qcpas0: failed to shutdown lite firmware
> > 	qcpas0: failed to receive ready signal
> > 	qcpas0: failed to boot coprocessor
> > 
> > leaving me without the qcpas(4) HW sensors (they anyway ship no data
> > on the Samsung currently, but that's a different story I want to tackle
> > next).
> > 
> > Implementing a retry loop will help to overcome this situation:
> > 
> > 	qcpas0: failed to shutdown lite firmware
> > 	qcpas0: failed to receive ready signal
> > 	qcpas0: coprocessor boot attempt 2
> > 	qcpas0: failed to receive ready signal
> > 	qcpas0: coprocessor boot attempt 3
> > 
> > Feedback, regression testers, OKs?
> >
> 
> Tested on my honor, it doesn't solve my issue, but it also do not introduce
> any regression. If it fixes your machine I think it make sense to commit and
> I OK with it, but I suggest to wait more tests and feedback.
> 
> On my device it errors like that:
> 
> qcpas0: failed to receive ready signal
> qcpas0: coprocessor boot attempt 2
> qcpas0: failed to receive ready signal
> qcpas0: coprocessor boot attempt 3
> qcpas0: failed to receive ready signal
> qcpas0: failed to boot coprocessor
> 
> I've tried increase number of attempts, add pause... nothing.

Thanks for testing.

But I will need to draw back this diff for now;  It turned out in the
meantime that while the HW sensors do come back after the retry, the
chip still isn't fully initialized.

Without this diff, what situation do you have with qcpas(4) on your
Honor?
 
> > Index: sys/dev/fdt/qcpas.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/fdt/qcpas.c,v
> > diff -u -p -u -p -r1.12 qcpas.c
> > --- sys/dev/fdt/qcpas.c	1 Aug 2025 08:55:09 -0000	1.12
> > +++ sys/dev/fdt/qcpas.c	16 May 2026 15:50:14 -0000
> > @@ -211,7 +211,7 @@ qcpas_mountroot(struct device *self)
> >  	char fwname[128];
> >  	size_t fwlen, dtb_fwlen;
> >  	u_char *fw, *dtb_fw;
> > -	int node, ret;
> > +	int i, node, ret;
> >  	int error;
> >  
> >  	if (qcpas_map_memory(sc) != 0)
> > @@ -272,7 +272,19 @@ qcpas_mountroot(struct device *self)
> >  		free(dtb_fw, M_DEVBUF, dtb_fwlen);
> >  	}
> >  
> > -	ret = qcpas_mdt_init(sc, sc->sc_pas_id, fw, fwlen);
> > +	/*
> > +	 * Coprocessor boot is intermittent on some platforms
> > +	 * (e.g. Samsung Galaxy Book4 Edge): all of the SCM calls
> > +	 * succeed but the firmware sometimes silently dies before
> > +	 * signalling ready.  Retry up to a few times -- when it
> > +	 * works, the ready signal arrives within a few hundred ms.
> > +	 */
> > +	for (ret = -1, i = 0; ret != 0 && i < 3; i++) {
> > +		if (i > 0)
> > +			printf("%s: coprocessor boot attempt %d\n",
> > +			    sc->sc_dev.dv_xname, i + 1);
> > +		ret = qcpas_mdt_init(sc, sc->sc_pas_id, fw, fwlen);
> > +	}
> >  	free(fw, M_DEVBUF, fwlen);
> >  	if (ret != 0) {
> >  		printf("%s: failed to boot coprocessor\n",
> > @@ -467,6 +479,7 @@ qcpas_mdt_init(struct qcpas_softc *sc, i
> >  	    QCPAS_DMA_DVA(sc->sc_metadata[idx])) != 0) {
> >  		printf("%s: init image failed\n", sc->sc_dev.dv_xname);
> >  		qcpas_dmamem_free(sc, sc->sc_metadata[idx]);
> > +		sc->sc_metadata[idx] = NULL;
> >  		return EINVAL;
> >  	}
> >  
> > @@ -474,6 +487,7 @@ qcpas_mdt_init(struct qcpas_softc *sc, i
> >  	    sc->sc_mem_phys[idx], maxpa - minpa) != 0) {
> >  		printf("%s: mem setup failed\n", sc->sc_dev.dv_xname);
> >  		qcpas_dmamem_free(sc, sc->sc_metadata[idx]);
> > +		sc->sc_metadata[idx] = NULL;
> >  		return EINVAL;
> >  	}
> >  
> > @@ -509,6 +523,7 @@ qcpas_mdt_init(struct qcpas_softc *sc, i
> >  	if (qcscm_pas_auth_and_reset(pas_id) != 0) {
> >  		printf("%s: auth and reset failed\n", sc->sc_dev.dv_xname);
> >  		qcpas_dmamem_free(sc, sc->sc_metadata[idx]);
> > +		sc->sc_metadata[idx] = NULL;
> >  		return EINVAL;
> >  	}
> >  
> > @@ -519,6 +534,14 @@ qcpas_mdt_init(struct qcpas_softc *sc, i
> >  	if (error) {
> >  		printf("%s: failed to receive ready signal\n",
> >  		    sc->sc_dev.dv_xname);
> > +		/*
> > +		 * The coprocessor was started but never signalled ready.
> > +		 * Shut it down and release the metadata buffer so the
> > +		 * caller can retry from scratch.
> > +		 */
> > +		qcscm_pas_shutdown(pas_id);
> > +		qcpas_dmamem_free(sc, sc->sc_metadata[idx]);
> > +		sc->sc_metadata[idx] = NULL;
> >  		return error;
> >  	}
> > 
> 
> -- 
> wbr, Kirill
>