RE: [PATCH V3] dmaengine: dw-axi-dmac: add spinlock for multi-channel DMA

[Libing Lei]

On 2023-04-07 at 14:48:56 +0800, Chen Yu <yu.c.chen@xxxxxxxxx> wrote:
> On 2023-04-04 at 23:39:45 +0800, Libing Lei wrote:
> > When multiple channels operate simultaneously, the common register shared
> > by all channels could be set to an unexpected value due to race conditions
> Does the channel mean the "chip" in the following patch? I mean, each channel
> has 1 chip plugged in? I asked this question because I'm trying to figure out the
> protection scope of the introduced spinlock.
No,chip doesn't means "channel" but the dmac chip,which points to some
common registers for all channels.
> > on SMP machines, resulting in a disaster that DMAC cannot work from then
> > on:
> >
> > cpu0                            cpu1
> >  dw_axi_dma_interrupt()
> >   axi_dma_irq_disable(chip)
> >   ...
> >                                 axi_chan_block_xfer_start()
> >                                  axi_dma_enable()
> >                                   val = axi_dma_ioread32(chip, DMAC_CFG)
> >   axi_dma_irq_enable(chip)
> >                                   axi_dma_iowrite32(chip, DMAC_CFG, val)
> >
> > As a result, the global interrupt enable bit INT_EN in the DMAC_CFG
> > register is eventually cleared and the DMAC will no longer generates
> > interrupts.
> >
> > The error scenario is as follows:
> >
> > [ 63.483688] dmatest: dma0chan1-copy: result #18: 'test timed out' with
> > src_off=0xc2 dst_off=0x27b len=0x3a54 (0)
> > [ 63.483693] dmatest: dma0chan2-copy: result #18: 'test timed out' with
> > src_off=0x239 dst_off=0xfc9 len=0x213a (0)
> > [ 63.483696] dmatest: dma0chan0-copy: result #19: 'test timed out' with
> > src_off=0x5d1 dst_off=0x231 len=0x395e (0)
> >
> > a spinlock is added to fix it up.
> >
> Although it is unlikely to introduce ABBA lock, maybe enabling the
> CONFIG_LOCKDEP to have a double check on this.
Thanks for your good suggestion!I have enabled the CONFIG_LOCKDEP in my
testcase:
/ # ls /proc/lockdep*
/proc/lockdep         /proc/lockdep_chains  /proc/lockdep_stats
> > Signed-off-by: Libing Lei <libing.lei@xxxxxxxxxxxxxxx>
> The patch looks good to me.
> 
> thanks,
> Chenyu
> 
It's my pleasure.

Best Regards,
Libing.Lei