Re: [PATCH v2] mtd: rawnand: gpmi: terminate the DMA chain on completion timeout

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mtd: rawnand: gpmi: unsynchronized DMA termination in gpmi_nfc_exec_op()
--

commit f9aac792f98d50292f01d08b01dd1b06dcf84ac0
Author: Samarth Narang <samarth.colleges@gmail.com>

mtd: rawnand: gpmi: terminate the DMA chain on completion timeout

This commit calls dmaengine_terminate_sync() before unmapping DMA buffers
when a DMA completion timeout occurs in gpmi_nfc_exec_op(), intending to
stop the APBH DMA channel and prevent stale descriptors from running against
unmapped buffers.

> diff --git a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> index 527165ccc839d..86163a552aad8 100644
> --- a/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/raw/gpmi-nand/gpmi-nand.c
> @@ -2659,6 +2659,7 @@ static int gpmi_nfc_exec_op(struct nand_chip *chip,
>  	if (!to) {
>  		dev_err(this->dev, "DMA timeout, last DMA\n");
>  		gpmi_dump_info(this);
> +		dmaengine_terminate_sync(get_dma_chan(this));

[Severity: Medium]
Does dmaengine_terminate_sync() actually synchronize with the channel IRQ
and completion tasklet on mxs-dma?

In drivers/dma/mxs-dma.c, mxs_dma_probe() does not implement
device_synchronize, so dmaengine_synchronize() is a no-op, and
mxs_dma_terminate_all() resets the channel without killing mxs_chan->tasklet
or clearing mxs_chan->desc.callback:

drivers/dma/mxs-dma.c:mxs_dma_terminate_all() {
	mxs_dma_reset_chan(chan);
	mxs_dma_disable_chan(chan);

	return 0;
}

Furthermore, when mxs_dma_reset_chan() resets an active APBH channel (or if
a late completion interrupt races with the timeout), mxs_dma_int_handler()
fires for the termination error (err == 0x01) or completion and schedules
mxs_chan->tasklet:

drivers/dma/mxs-dma.c:mxs_dma_int_handler() {
	...
	if (err) {
		dev_dbg(mxs_dma->dma_device.dev,
			"%s: error in channel %d\n", __func__,
			chan);
		mxs_chan->status = DMA_ERROR;
		mxs_dma_reset_chan(&mxs_chan->chan);
	}
	...
	/* schedule tasklet on this channel */
	tasklet_schedule(&mxs_chan->tasklet);
}

Because mxs_chan->tasklet is not synchronized before
dmaengine_terminate_sync() returns, could mxs_dma_tasklet() run
dma_irq_callback(this) and complete this->dma_done after a subsequent
gpmi_nfc_exec_op() call has already reinitialized this->dma_done and
submitted a new transfer, causing the new operation to complete prematurely
and unmap its buffers while DMA is still active?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030150929.2944717-1-samarth.colleges@gmail.com?part=1