Re: [PATCH 1/1] mtd: nand: raw: use dmaengine accessor API functions

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

On Wed, Nov 11, 2026 at 08:51:17PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] mtd: rawnand: cadence: unmap device mismatch in
>   cadence_nand_remove()
> It also noticed 1 related pre-existing issue this patch does not
> introduce:
> - [Critical] mtd: rawnand: fsmc: missing reinit_completion() in
>   dma_xfer() [1]
> --
>
> commit 9eda52238f6d5af7214b32eeb0870d65126ccf3d
> Author: Frank Li <Frank.Li@nxp.com>
>
> mtd: nand: raw: use dmaengine accessor API functions
>
> This commit replaces direct chan->device and device_prep_dma_memcpy()
> accesses in the Cadence and FSMC raw NAND drivers with
> dmaengine_get_dma_device() and dmaengine_prep_dma_memcpy().
>
> > diff --git a/drivers/mtd/nand/raw/cadence-nand-controller.c b/drivers/mtd/nand/raw/cadence-nand-controller.c
> > index 2085632631c02..5e3d34f24f9d9 100644
> > --- a/drivers/mtd/nand/raw/cadence-nand-controller.c
> > +++ b/drivers/mtd/nand/raw/cadence-nand-controller.c
> [ ... ]
> > @@ -3170,12 +3170,12 @@ static int cadence_nand_init(struct cdns_nand_ctrl *cdns_ctrl)
> >  		}
> >  	}
> >
> > -	dma_dev = cdns_ctrl->dmac->device;
> > -	cdns_ctrl->io.iova_dma = dma_map_resource(dma_dev->dev, cdns_ctrl->io.dma,
> > +	dma_dev = dmaengine_get_dma_device(cdns_ctrl->dmac);
> > +	cdns_ctrl->io.iova_dma = dma_map_resource(dma_dev, cdns_ctrl->io.dma,
> >  						  cdns_ctrl->io.size,
> >  						  DMA_BIDIRECTIONAL, 0);
> >
> > -	ret = dma_mapping_error(dma_dev->dev, cdns_ctrl->io.iova_dma);
> > +	ret = dma_mapping_error(dma_dev, cdns_ctrl->io.iova_dma);
> >  	if (ret) {
> >  		dev_err(cdns_ctrl->dev, "Failed to map I/O resource to DMA\n");
> >  		goto dma_release_chnl;
> > @@ -3204,7 +3204,7 @@ static int cadence_nand_init(struct cdns_nand_ctrl *cdns_ctrl)
> >  	return 0;
> >
> >  unmap_dma_resource:
> > -	dma_unmap_resource(dma_dev->dev, cdns_ctrl->io.iova_dma,
> > +	dma_unmap_resource(dma_dev, cdns_ctrl->io.iova_dma,
> >  			   cdns_ctrl->io.size, DMA_BIDIRECTIONAL, 0);
> >
> >  dma_release_chnl:
>
> [Severity: Medium]
> Should cadence_nand_remove() also use dmaengine_get_dma_device() when
> unmapping cdns_ctrl->io.iova_dma?
>
> drivers/mtd/nand/raw/cadence-nand-controller.c:cadence_nand_remove() {
> 	...
> 	if (cdns_ctrl->dmac)
> 		dma_unmap_resource(cdns_ctrl->dmac->device->dev,
> 				   cdns_ctrl->io.iova_dma, cdns_ctrl->io.size,
> 				   DMA_BIDIRECTIONAL, 0);

Already fixed at

commit a018524a2ec96da5bb1965053967a20e0069ed9e
Author: Frank Li <Frank.Li@nxp.com>
Date:   Thu Sep 17 16:47:06 2026 -0400

    mtd: use dmaengine_get_dma_device() instead of chan->device->dev

    Replace direct dma_chan::device::dev access with the proper
    dmaengine_get_dma_device() for consumer API

Previous script have not recongize this pattern.

> 	...
> }
>
> When chan->dev->chan_dma_dev is set, dmaengine_get_dma_device() returns
> &chan->dev->device rather than chan->device->dev, so cadence_nand_remove()
> unmaps the resource against a different struct device than
> cadence_nand_init() used to map it, leaking the mapping on driver removal.
> It also leaves a direct dereference of struct dma_device.
>
> > diff --git a/drivers/mtd/nand/raw/fsmc_nand.c b/drivers/mtd/nand/raw/fsmc_nand.c
> > index b13b2b0c3f300..831293a5bd814 100644
> > --- a/drivers/mtd/nand/raw/fsmc_nand.c
> > +++ b/drivers/mtd/nand/raw/fsmc_nand.c
> > @@ -486,7 +486,7 @@ static int dma_xfer(struct fsmc_nand_data *host, void *buffer, int len,
> >  		    enum dma_data_direction direction)
> >  {
> >  	struct dma_chan *chan;
> > -	struct dma_device *dma_dev;
> > +	struct device *dma_dev;
> >  	struct dma_async_tx_descriptor *tx;
> >  	dma_addr_t dma_dst, dma_src, dma_addr;
> >  	dma_cookie_t cookie;
> > @@ -501,9 +501,9 @@ static int dma_xfer(struct fsmc_nand_data *host, void *buffer, int len,
> >  	else
> >  		return -EINVAL;
> >
> > -	dma_dev = chan->device;
> > -	dma_addr = dma_map_single(dma_dev->dev, buffer, len, direction);
> > -	if (dma_mapping_error(dma_dev->dev, dma_addr))
> > +	dma_dev = dmaengine_get_dma_device(chan);
> > +	dma_addr = dma_map_single(dma_dev, buffer, len, direction);
> > +	if (dma_mapping_error(dma_dev, dma_addr))
> >  		return -EINVAL;
> >
> >  	if (direction == DMA_TO_DEVICE) {
> > @@ -514,8 +514,7 @@ static int dma_xfer(struct fsmc_nand_data *host, void *buffer, int len,
> >  		dma_dst = dma_addr;
> >  	}
> >
> > -	tx = dma_dev->device_prep_dma_memcpy(chan, dma_dst, dma_src,
> > -			len, flags);
> > +	tx = dmaengine_prep_dma_memcpy(chan, dma_dst, dma_src, len, flags);
> >  	if (!tx) {
> >  		dev_err(host->dev, "device_prep_dma_memcpy error\n");
> >  		ret = -EIO;
> > @@ -547,7 +546,7 @@ static int dma_xfer(struct fsmc_nand_data *host, void *buffer, int len,
> >  	ret = 0;
> >
> >  unmap_dma:
> > -	dma_unmap_single(dma_dev->dev, dma_addr, len, direction);
> > +	dma_unmap_single(dma_dev, dma_addr, len, direction);
> >
> >  	return ret;
> >  }
>
> [Severity: Critical]
> This problem wasn't introduced by this patch, but can dma_xfer() unmap
> dma_addr while the DMA transfer is still in flight [1]?

Existing problem.

Frank

>
> If an earlier transfer times out and its callback fires late,
> host->dma_access_complete is left signaled.  Because dma_xfer() does not
> call reinit_completion(&host->dma_access_complete) before submitting a new
> transfer, wait_for_completion_timeout() consumes the stale completion and
> returns immediately, so dma_unmap_single() runs while hardware DMA is
> still active on buffer.
>
> [1] https://sashiko.dev/#/bug/linux-836e69de-023a-4bd2-8b41-03e05feb8137
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20261111202502.3860647-1-Frank.Li@oss.nxp.com?part=1