Re: [PATCH v7 5/9] dmaengine: dw-edma: Pass dma_slave_config to dw_edma_device_transfer()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch claims to support atomic configuration but continues to locklessly mutate and read global channel state, causing data races and broken atomicity.
--

commit e550cfd0855adb0abdce8c07ba7dc3b15ac63f86
Author: Frank Li <Frank.Li@nxp.com>

dmaengine: dw-edma: Pass dma_slave_config to dw_edma_device_transfer()

This commit updates dw_edma_device_transfer() to take a dma_slave_config
argument. It intends to support atomic configuration and descriptor
preparation when a non-NULL config is provided to device_prep_config_sg().

> diff --git a/drivers/dma/dw-edma/dw-edma-core.c b/drivers/dma/dw-edma/dw-edma-core.c
> --- a/drivers/dma/dw-edma/dw-edma-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-core.c

[ ... ]

> @@ -595,7 +610,7 @@ dw_edma_device_prep_config_sg(struct dma_chan *dchan, struct scatterlist *sgl,
>  	if (config && dw_edma_device_config(dchan, config))
>  		return NULL;
>  
> -	return dw_edma_device_transfer(&xfer);
> +	return dw_edma_device_transfer(&xfer, dw_edma_device_get_config(dchan, config));
>  }

[Severity: High]
This is a pre-existing issue, but does this actually achieve the intended
atomic configuration and descriptor preparation mentioned in the commit
message?

Looking at dw_edma_device_prep_config_sg(), it still explicitly calls
dw_edma_device_config() before delegating to dw_edma_device_transfer().

The dw_edma_device_config() function locklessly mutates global channel state:

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

If multiple threads use this API concurrently for the same channel, could they
race on this memcpy() and global state updates?

Additionally, dw_edma_device_transfer() continues to read the globally mutated
chan->non_ll state to determine burst limits:

static struct dma_async_tx_descriptor *
dw_edma_device_transfer(...)
{
    ...
    bursts_max = chan->non_ll ? 1 : chan->ll_max;
    ...
}

Because chan->non_ll is evaluated globally rather than from the locally passed
config, could a concurrent preparation silently alter the hardware burst
behavior of an ostensibly atomic and isolated descriptor preparation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-dma_prep_config-v7-0-1f73f4899883@nxp.com?part=5