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

[Chen Yu]

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.
> 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. 
> Signed-off-by: Libing Lei <libing.lei@xxxxxxxxxxxxxxx>
The patch looks good to me.

thanks,
Chenyu