Vinod Koul <vinod.koul@xxxxxxxxx> writes:
> dmaengine device should explicitly call devm_free_irq() when using
> devm_reqister_irq().
>
> The irq is still ON when devices remove is executed and irq should be
> quiesced before remove is completed.
Hi Vinod,
I understand this patch, but I'd like to know if this is the best fix.
Let me explain how I see a race in remove() path :
- as long as there is one dma channel requested, ie. dma_chan_get() but no
dma_chan_put(), the try_module_get() of dma_chan_get() prevents the remove()
routine from running
- when the last channel is released, ie. the last dma_chan_put() is called, if
there is a running DMA, a "mess-up" window appears
=> this is because pxa_dma doesn't implement device_synchronise()
=> this is where I think an improvement could be made
- if dmaengine_synchronize() is implemented, and the last descriptor is
finished, the DMA irq cannot fire anymore on pxa (phy_disable() done on all
phys implies DMA irq cannot fire anymore).
So I'd rather have pxad_device.slave.dmaengine_synchronize() implemented to
solve the race issue, and I do volunteer for that.
Now there might be something I'm not seeing that you've observed on other dma
IPs, so I'd like to know if I'm not missing another usecase where a race might
appear.
Please tell me your thoughts.
Cheers.
--
Robert
PS: I'm not very proud of myself about for loop and the interrupts allocation,
so seeing it another time in the release routine is a bit painful :)
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
