Re: [PATCH v4 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 2 potential issue(s) to consider:
- [High] The return value of dw_edma_device_config() is ignored, leading to silent preparation failures and corrupted state.
- [High] Lockless modification of shared channel configuration causes a race condition during concurrent descriptor preparation.
--

commit 72239068fd086ad95d773ab05d33c4df60a82fea
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: dw-edma: Use new .device_prep_config_sg() callback

This commit introduces the .device_prep_config_sg() callback for the
dw-edma driver. It combines configuration and scatter-gather descriptor
preparation into a single step.

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> index c2feb3adc79fa..f7f58b0010e26 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);

dw_edma_device_config() can return an error code like -EINVAL if the
configuration is invalid. Does ignoring this return value cause the transfer
to be scheduled with a corrupted or stale state? Should this return NULL if
the configuration fails?

Also, the new .device_prep_config_sg callback is often used to allow lockless
descriptor preparation. However, dw_edma_device_config() modifies shared
channel state:

dw_edma_device_config() {
	...
	chan->non_ll = false;
	...
	memcpy(&chan->config, config, sizeof(*config));
	chan->configured = true;
	...
}

If multiple threads prepare transfers concurrently, can this cause a data
race resulting in corrupted descriptors?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260506-dma_prep_config-v4-0-85b3d22babff@nxp.com?part=4