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 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 > > >
