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
