Re: [PATCH 3/3] dmaengine: imx-sdma: Use __assign_bit() for ownership configuration

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unsynchronized read-modify-write on global SDMA hardware registers across multiple channels.
--

commit 4ddf0614a2ae2b19c08cb7e599c4ee441c688f95
Author: Peng Fan <peng.fan@nxp.com>

dmaengine: imx-sdma: Use __assign_bit() for ownership configuration

This commit refactors sdma_config_ownership() by replacing multiple
if/else blocks containing __set_bit() and __clear_bit() with single
calls to __assign_bit().

> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 63261ab6402b9..da565409beaaa 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -710,20 +710,9 @@ static int sdma_config_ownership(struct sdma_channel *sdmac,
>  	mcu = readl_relaxed(sdma->regs + SDMA_H_HOSTOVR);
>  	dsp = readl_relaxed(sdma->regs + SDMA_H_DSPOVR);
>  
> -	if (dsp_override)
> -		__clear_bit(channel, &dsp);
> -	else
> -		__set_bit(channel, &dsp);
> -
> -	if (event_override)
> -		__clear_bit(channel, &evt);
> -	else
> -		__set_bit(channel, &evt);
> -
> -	if (mcu_override)
> -		__clear_bit(channel, &mcu);
> -	else
> -		__set_bit(channel, &mcu);
> +	__assign_bit(channel, &dsp, !dsp_override);
> +	__assign_bit(channel, &evt, !event_override);
> +	__assign_bit(channel, &mcu, !mcu_override);
>  
>  	writel_relaxed(evt, sdma->regs + SDMA_H_EVTOVR);
>  	writel_relaxed(mcu, sdma->regs + SDMA_H_HOSTOVR);

[Severity: High]
This isn't a bug introduced by this patch, but does this read-modify-write
sequence require an engine-wide lock?

If multiple DMA clients (such as serial or audio drivers) concurrently call
functions like dmaengine_slave_config() for different channels, could they
race while modifying the global SDMA_H_EVTOVR, SDMA_H_HOSTOVR, and
SDMA_H_DSPOVR hardware registers?

Since the registers are read into local variables, modified, and written back
without serialization, could concurrent calls to sdma_config_ownership() cause
one channel to inadvertently overwrite the configuration of another?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903-dma-mask-v1-0-211ffd12c236@nxp.com?part=3