Re: [PATCH v6 4/9] dmaengine: dw-edma: Use new .device_prep_config_sg() callback

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly added `dw_edma_device_prep_config_sg` callback modifies shared channel state locklessly, violating dmaengine API atomicity guarantees and potentially corrupting active hardware transfers.
--

commit 3f734236aec2497e34a2baf98f60bcd0cd1c6525
Author: Frank Li <Frank.Li@nxp.com>
dmaengine: dw-edma: Use new .device_prep_config_sg() callback

This commit replaces the device_prep_slave_sg callback with the new
device_prep_config_sg callback to combine configuration and descriptor
preparation.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index c2feb3adc79fa..92572dd8131e6 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -577,10 +577,11 @@ dw_edma_device_transfer(struct dw_edma_transfer *xfer)
>  }
>  
>  static struct dma_async_tx_descriptor *
> -dw_edma_device_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
> -			     unsigned int len,
> -			     enum dma_transfer_direction direction,
> -			     unsigned long flags, void *context)
> +dw_edma_device_prep_config_sg(struct dma_chan *dchan, struct scatterlist *sgl,
> +			      unsigned int len,
> +			      enum dma_transfer_direction direction,
> +			      unsigned long flags,
> +			      struct dma_slave_config *config)
>  {
>  	struct dw_edma_transfer xfer;
>  
> @@ -591,6 +592,9 @@ dw_edma_device_prep_slave_sg(struct dma_chan *dchan, struct scatterlist *sgl,
>  	xfer.flags = flags;
>  	xfer.type = EDMA_XFER_SCATTER_GATHER;
>  
> +	if (config && dw_edma_device_config(dchan, config))
> +		return NULL;

[Severity: High]
Does this introduce a data race on the shared channel state?

By implementing the device_prep_config_sg callback, the driver bypasses
the dmaengine core's per-channel spinlock, which normally protects
configuration and preparation.

However, dw_edma_device_config() modifies global channel state locklessly:

static int dw_edma_device_config(...)
{
        ...
        chan->non_ll = false;
        ...
        if (cfg_non_ll || non_ll)
                chan->non_ll = true;
        ...
        memcpy(&chan->config, config, sizeof(*config));
        ...
}

If multiple threads queue transfers concurrently, could this corrupt the
descriptor configuration?

More importantly, if a new transfer is prepared while the channel is actively
executing, the interrupt handler might read the momentarily cleared or
overwritten chan->non_ll.

Could this cause the interrupt handler to invoke the wrong hardware routine
and desynchronize the hardware state?

> +
>  	return dw_edma_device_transfer(&xfer);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520-dma_prep_config-v6-0-06e49b7acb38@nxp.com?part=4