On Thu, Feb 26, 2026 at 10:23:27AM -0500, Frank Li wrote:
> On Thu, Feb 26, 2026 at 11:22:41AM +0900, Koichiro Den wrote:
> > On Wed, Feb 25, 2026 at 10:50:41AM -0500, Frank Li wrote:
> > > On Wed, Feb 25, 2026 at 05:26:02PM +0900, Koichiro Den wrote:
> > > > 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.
> > >
> > > This patch serial is actually straight forwards. we can ask vnod pick
> > > first
> > > one in case have other problems. put together to reduce patch's
> > > dependency.
> >
> > Yes, I see.
> >
> > My understanding is that the originally planned dependency chain was:
> >
> > #1->#2->#3
> >
> > #1 [PATCH v2 0/8] dmaengine: Add new API to combine onfiguration and
> > descriptor preparation
> >
> > https://lore.kernel.org/dmaengine/[email protected]/
> > #2 (this series)
> > #3 [PATCH RFT 0/5] dmaengine: dw-edma: support dynamtic add link entry
> > during dma engine running
> >
> > https://lore.kernel.org/dmaengine/[email protected]/
> >
> > I'm not sure whether #1 will proceed, as the thread appears to have
> > stalled. I
>
> Vnod said he will review #1 in this week. If not progress, I will create
> new one without dependent #1.
Thanks for letting me know that.
Best regards,
Koichiro
>
> Frank
>
> > might be missing something, though. In any case, #1 is semantically
> > orthogonal
> > to #2, so I believe #2 can be considered independently.
> >
> > Thanks,
> > Koichiro
> >
> > >
> > > Frank
> > > >
> > > > 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
> > > > >
