Re: [PATCH v5 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 2 potential issue(s) to consider:
- [High] The `dw_edma_device_prep_config_sg` function ignores the return value of `dw_edma_device_config` and proceeds to use an explicitly rejected configuration for DMA transfers.
- [High] The implementation fails to achieve the claimed "atomic configuration" because it locklessly mutates shared channel state, corrupting concurrently active DMA transfers.
--

commit 8f2944930122fd18a6a04f03176bb9c6eddcd9ba
Author: Frank Li <Frank.Li@nxp.com>

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

This commit passes dma_slave_config to dw_edma_device_transfer() to
support atomic configuration and descriptor preparation when a non-NULL
configuration 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
> index f7f58b0010e26..ec6f6b1e48256 100644
> --- 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);

Should we check the return value of dw_edma_device_config() here?

If an invalid configuration is passed, dw_edma_device_config() returns
-EINVAL. By ignoring the return value, the code proceeds to extract the
source and destination addresses from the explicitly rejected configuration
via dw_edma_device_get_config(), potentially resulting in descriptors
prepared with unvalidated hardware parameters.

Additionally, the commit message states this change supports atomic
configuration, but dw_edma_device_config() mutates shared channel state:

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

Since the dmaengine API allows concurrent descriptor preparations when
device_prep_config_sg is provided, could this cause a data race on
chan->config and chan->non_ll?

If chan->non_ll is actively read by the interrupt handler to dictate
hardware execution mode, could modifying it while preparing a new descriptor
dynamically corrupt the execution mode of currently active transfers?

> -	return dw_edma_device_transfer(&xfer);
> +	return dw_edma_device_transfer(&xfer, dw_edma_device_get_config(dchan, config));
>  }

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