Re: [PATCH v5 2/2] dmaengine: fsl-edma: Support dynamic scatter/gather chaining

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Jul 02, 2026 at 12:45:52PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] Double-execution of DMA descriptors due to state machine desync when interrupts coalesce.
> - [Critical] Channel deadlock due to dynamic descriptor linking bypassing `issue_pending()`.
> --
>
> commit a67c20eb8f20e15fe2cadb092322588069fe328e
> Author: Benoît Monin <benoit.monin@bootlin.com>
>
> dmaengine: fsl-edma: Support dynamic scatter/gather chaining
>
> Implement dynamic linking of scatter/gather transfers to enable
> chaining multiple DMA descriptors without stopping the channel.
> This avoids waiting for the channel to go idle if there is another
> transaction already issued.
>
> > diff --git a/drivers/dma/fsl-edma-common.c b/drivers/dma/fsl-edma-common.c
> > --- a/drivers/dma/fsl-edma-common.c
> > +++ b/drivers/dma/fsl-edma-common.c
> > @@ -58,7 +58,10 @@ void fsl_edma_tx_chan_handler(struct fsl_edma_chan *fsl_chan)
> >  		list_del(&fsl_chan->edesc->vdesc.node);
> >  		vchan_cookie_complete(&fsl_chan->edesc->vdesc);
> >  		fsl_chan->edesc = NULL;
> > -		fsl_chan->status = DMA_COMPLETE;
> > +		if (!(fsl_edma_drvflags(fsl_chan) & FSL_EDMA_DRV_SPLIT_REG) ||
> > +		    (edma_readl_chreg(fsl_chan, ch_csr) & EDMA_V3_CH_CSR_DONE)) {
> > +			fsl_chan->status = DMA_COMPLETE;
> > +		}
>
> [Severity: Critical]
> Does checking the channel-global DONE flag here cause chained descriptors
> to execute twice if interrupts coalesce?
>
> For example, if descriptors A and B are chained and the hardware finishes
> both before the interrupt handler runs, won't the handler complete A,
> see DONE is set, and mark the channel DMA_COMPLETE?
>
> If that happens, wouldn't fsl_edma_xfer_desc() then fetch B from the
> issued list and restart the hardware, executing B a second time?

This is problem, need go though issue list and compared current hardware
working pos. then move all finished item to compelete queue

A
B
C  <- hardware working this
D

Need move A and B to complete queue. Because eDMA have not write back
status to descript and no compelete queue, it needs some efforts to find
dma is working on which item in issue list

>
> [ ... ]
>
> > @@ -673,6 +676,68 @@ struct dma_async_tx_descriptor *fsl_edma_prep_dma_cyclic(
> > +static dma_cookie_t fsl_edma_tx_submit(struct dma_async_tx_descriptor *tx)
> > +{
> > +	struct virt_dma_desc *vd = container_of(tx, struct virt_dma_desc, tx);
> > +	struct fsl_edma_chan *fsl_chan = to_fsl_edma_chan(tx->chan);
> > +	struct fsl_edma_desc *fsl_desc = to_fsl_edma_desc(vd);
> > +	struct virt_dma_chan *vc = to_virt_chan(tx->chan);
> > +	dma_cookie_t cookie;
> > +
> > +	guard(spinlock_irqsave)(&fsl_chan->vchan.lock);
> > +
> > +	fsl_edma_link_sg(fsl_chan, fsl_desc);
> > +	cookie = dma_cookie_assign(tx);
> > +	list_move_tail(&vd->node, &vc->desc_submitted);
> > +
> > +	return cookie;
> > +}
>
> [Severity: Critical]
> Can this cause a channel deadlock by executing unissued descriptors?
>
> Since hardware linking happens in fsl_edma_tx_submit() rather than waiting
> for the issue_pending() callback, if the hardware completes the newly linked
> descriptor before the client calls issue_pending(), won't the interrupt
> handler fire but find edesc is NULL?

Yes, you can't link hardware queue in tx_submit

you can create hardware queue list,  N1->N2->N3.

at issue_pending() function, assume issue queue I1->I2->I3

link submit queue header N3 to issue queue tail I1.

Frank

>
> In fsl_edma_tx_chan_handler():
>
>     if (!fsl_chan->edesc) {
>         /* terminate_all called before */
>         spin_unlock(&fsl_chan->vchan.lock);
>         return;
>     }
>
> Wouldn't this drop the completion and leave the channel stuck when
> issue_pending() is eventually called, since the channel status is still left
> as DMA_IN_PROGRESS?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260702-fsl-edma-dyn-sg-v5-0-16787185be49@bootlin.com?part=2