Re: [PATCH] ARM OMAP: Fix race in OMAP2/3 DMA IRQ handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Shilimkar, Santosh wrote:
--- a/arch/arm/plat-omap/dma.c
+++ b/arch/arm/plat-omap/dma.c
@@ -1898,11 +1898,11 @@ static int omap2_dma_handle_ch(int ch)
 		status = dma_read(CSR(ch));
 	}
+ dma_write(status, CSR(ch));
This is not necessary. Refers line  "dma_write(OMAP2_DMA_CSR_CLEAR_MASK, CSR(ch));" just above.

Yes, the current DMA code is full of inconsistencies and illogic. In general, clearing a hard-coded mask of bits in an IRQ status register is a nice way to enter a race with the machine. And that's a race you cannot win every time, so you'll miss IRQs that you haven't handled yet.

A major cleanup should be done to the DMA code, but that's no reason not to fix bugs now.

It will any way do the job of clearing. In a way, clear done after
the  callback has no effect since the status reg and global IRQ_enable for
the particular channel is already disabled before that.

Yes, in a way that completely ignores the code and hardware behaviour.

If you write a 1 to, say, the FRAME bit of the CSR *after* a transfer has been completed, *before* handling the event, you lose the CSR value, so the channel handling function complains (correctly) about a spurious IRQ and refuses to do anything more productive.

If you start a quick transfer from the callback function, the FRAME bit *will* get set before control returns from the callback function.

So dma library is safe from the problem you have described.

Sounds like you're in denial, man. I didn't just randomly send a patch, I actually ran into the problem, fixed it, verified it indeed is fixed, and only after that did I send it.

Cheers,
Juha
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux