Re: [PATCH v2 1/4] soc: fsl: qmc: Only set completion interrupt when needed

[Christophe Leroy]

Hi Hervé,

Le 14/08/2025 à 09:34, Herve Codina a écrit :
> Hi Christophe,
>
> On Wed, 13 Aug 2025 12:06:51 +0200
> Herve Codina <herve.codina@xxxxxxxxxxx> wrote:
>
> > Hi Christophe,
> >
> > On Tue, 12 Aug 2025 12:50:55 +0200
> > Christophe Leroy <christophe.leroy@xxxxxxxxxx> wrote:
> >
> > > When no post-completion processing is expected, don't waste time
> > > handling useless interrupts.
> > >
> > > Only set QMC_BD_[R/T]X_I when a completion function is passed in,
> > > and perform seamless completion on submit for interruptless buffers.
> > >
> > > Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
> > > ---
> > > v2: Keep the UB flag to mark not completed buffers and seamlessly flag them as completed during next submit.
> > > ---
> > >   drivers/soc/fsl/qe/qmc.c | 44 ++++++++++++++++++++++++++++++----------
> > >   1 file changed, 33 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
> > > index 36c0ccc06151f..8f76b9a5e385d 100644
> > > --- a/drivers/soc/fsl/qe/qmc.c
> > > +++ b/drivers/soc/fsl/qe/qmc.c
> > > @@ -461,9 +461,16 @@ int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,
> > >   
> > >   	ctrl = qmc_read16(&bd->cbd_sc);
> > >   	if (ctrl & (QMC_BD_TX_R | QMC_BD_TX_UB)) {
> > > -		/* We are full ... */
> > > -		ret = -EBUSY;
> > > -		goto end;
> > > +		if (!(ctrl & QMC_BD_TX_I) && bd == chan->txbd_done) {
> > > +			if (ctrl & QMC_BD_TX_W)
> > > +				chan->txbd_done = chan->txbds;
> > > +			else
> > > +				chan->txbd_done++;
> > > +		} else {
> > > +			/* We are full ... */
> > > +			ret = -EBUSY;
> > > +			goto end;
> > > +		}
> > >   	}
> > >   
> > >   	qmc_write16(&bd->cbd_datlen, length);
> > > @@ -475,6 +482,10 @@ int qmc_chan_write_submit(struct qmc_chan *chan, dma_addr_t addr, size_t length,
> > >   
> > >   	/* Activate the descriptor */
> > >   	ctrl |= (QMC_BD_TX_R | QMC_BD_TX_UB);
> > > +	if (complete)
> > > +		ctrl |= QMC_BD_TX_I;
> > > +	else
> > > +		ctrl &= ~QMC_BD_TX_I;
> > >   	wmb(); /* Be sure to flush the descriptor before control update */
> > >   	qmc_write16(&bd->cbd_sc, ctrl);
> > >     
> >
> > You try to purge one descriptor for which the transfer is done but you do that
> > when you have no more free descriptors.
> >
> > You end up with all descriptor "used". I think a better way to do that is
> > to purge all "done" descriptor configured to work without interrupts until a
> > descriptor with interrupt is found.
>
> I have looked again at your code and looking for a free descriptor only when it
> is needed is sufficient. You can forget my previous proposal.
>
> Back to your code, I think you need to be sure that the descriptor you want to
> re-use is really available and so you need to check the 'R' bit to be sure
> that we are not with 'R' = 1 and 'UB' = 1 which means "BD is used, waiting for
> a transfer".

Fixed in v3.

Thanks
Christophe