On Fri, Jan 09, 2026 at 10:28:21AM -0500, Frank Li wrote:
> The DONE_INT_MASK and ABORT_INT_MASK registers are shared by all DMA
> channels, and modifying them requires a read-modify-write sequence.
> Because this operation is not atomic, concurrent calls to
> dw_edma_v0_core_start() can introduce race conditions if two channels
> update these registers simultaneously.
>
> Add a spinlock to serialize access to these registers and prevent race
> conditions.
>
> Signed-off-by: Frank Li <[email protected]>
> ---
> vc.lock protect should be another problem. This one just fix register
> access for difference DMA channel.
>
> Other improve defer to dynamtic append descriptor works later.
> ---
> drivers/dma/dw-edma/dw-edma-v0-core.c | 6 ++++++
> 1 file changed, 6 insertions(+)
Hi Frank,
I'm very interested in seeing the work toward the "dynamic append" series land,
but in my opinion this one can be submitted independently.
Even in the current mainline, under concurrent multi-channel load, this race can
already be triggered.
Also, with this patch, dw->lock is no longer "Only for legacy", so we should
probably update the comment in dw-edma-core.h.
Best regards,
Koichiro
>
> diff --git a/drivers/dma/dw-edma/dw-edma-v0-core.c
> b/drivers/dma/dw-edma/dw-edma-v0-core.c
> index
> b75fdaffad9a4ea6cd8d15e8f43bea550848b46c..2850a9df80f54d00789144415ed2dfe31dea3965
> 100644
> --- a/drivers/dma/dw-edma/dw-edma-v0-core.c
> +++ b/drivers/dma/dw-edma/dw-edma-v0-core.c
> @@ -364,6 +364,7 @@ static void dw_edma_v0_core_start(struct dw_edma_chunk
> *chunk, bool first)
> {
> struct dw_edma_chan *chan = chunk->chan;
> struct dw_edma *dw = chan->dw;
> + unsigned long flags;
> u32 tmp;
>
> dw_edma_v0_core_write_chunk(chunk);
> @@ -408,6 +409,8 @@ static void dw_edma_v0_core_start(struct dw_edma_chunk
> *chunk, bool first)
> }
> }
> /* Interrupt unmask - done, abort */
> + raw_spin_lock_irqsave(&dw->lock, flags);
> +
> tmp = GET_RW_32(dw, chan->dir, int_mask);
> tmp &= ~FIELD_PREP(EDMA_V0_DONE_INT_MASK, BIT(chan->id));
> tmp &= ~FIELD_PREP(EDMA_V0_ABORT_INT_MASK, BIT(chan->id));
> @@ -416,6 +419,9 @@ static void dw_edma_v0_core_start(struct dw_edma_chunk
> *chunk, bool first)
> tmp = GET_RW_32(dw, chan->dir, linked_list_err_en);
> tmp |= FIELD_PREP(EDMA_V0_LINKED_LIST_ERR_MASK, BIT(chan->id));
> SET_RW_32(dw, chan->dir, linked_list_err_en, tmp);
> +
> + raw_spin_unlock_irqrestore(&dw->lock, flags);
> +
> /* Channel control */
> SET_CH_32(dw, chan->dir, chan->id, ch_control1,
> (DW_EDMA_V0_CCS | DW_EDMA_V0_LLE));
>
> --
> 2.34.1
>
