Re: [PATCH v5 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 failures and state corruption.
- [High] The .device_prep_config_sg callback mutates shared channel state without locks, violating atomicity guarantees.
--

commit cc74cf96b36ed28ea1019078a61ff3eac8097031
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 to the dw-edma
driver 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..f7f58b0010e26 100644
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c
[ ... ]
> @@ -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);

Does this need to check the return value of dw_edma_device_config()?

If dw_edma_device_config() returns an error like -EINVAL, the driver will
silently proceed to dw_edma_device_transfer() with the old configuration.

Also, dw_edma_device_config() unconditionally clears chan->non_ll before
validating the new configuration:

static int dw_edma_device_config(...) {
	...
	chan->non_ll = false;
	if (chan->dw->chip->mf == EDMA_MF_HDMA_NATIVE) {
		if (config->peripheral_config &&
		    config->peripheral_size != sizeof(int)) {
			dev_err(dchan->device->dev, ...);
			return -EINVAL;
		}
	...
}

By ignoring the error, the transfer function might calculate bursts_max
incorrectly using a corrupted chan->non_ll flag. I see that subsequent
commits in this series do not appear to fix this missing error check.

Additionally, can this sequence result in a data race if called concurrently?

When a driver implements .device_prep_config_sg, the dmaengine framework
skips its internal spinlock in dmaengine_prep_config_sg_safe() and delegates
atomicity guarantees to the driver.

However, dw_edma_device_config() locklessly mutates shared channel state
such as chan->config and the chan->non_ll flag.

While a later commit in this series attempts to fix this by passing config
directly to the transfer function, dw_edma_device_config() still mutates
the shared chan->non_ll flag without locks, which is then read by
dw_edma_device_transfer() to determine bursts_max.

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512-dma_prep_config-v5-0-26865bf7d935@nxp.com?part=4