Vinod Koul <vinod.koul@xxxxxxxxx> writes: > On Sat, Jul 16, 2016 at 07:38:38PM +0200, Robert Jarzmik wrote: >> Vinod Koul <vinod.koul@xxxxxxxxx> writes: >> >> > On Tue, Jul 05, 2016 at 08:24:06PM +0530, Vinod Koul wrote: >> >> This series attempts to do few cleanup for drivers. My aim was to fix irq >> >> cleanup but on the way found few more things to be fixed as well. >> >> o explicitly freeup irq: Many drivers nowadays use devm variant for >> >> requesting irq, which is fine from cleanup POV, but on remove you leave irq >> >> enabled and able to fire. This can lead to nasty races, so better idea is >> >> to explicitly freeup. >> >> o same situation is with tasklets. They need to be killed before we exit, >> >> so that has been fixed too >> >> o while at it noticed few drivers still set .owner, removed them as well. >> >> o also fixed few sparse complaints >> > >> > Merging this now with the ACKs and changes as discussed >> >> Hi Vinod, >> >> I thought that we agreed that commit "dmaengine: pxa_dma: implement >> device_synchronize" superseeded "dmaengine: pxa_dm: explicitly freeup irq" which >> wasn't necessary anymore. > > sync does help but it won't help you when you channel is free and hw decides > to send you an interrupt for no reason... while you are removing. As stated in commit "dmaengine: pxa_dma: implement device_synchronize", hardware is not entitled to generate interrupts for that channel anymore once dma_synchronize() is terminated by construct. If it does, then we assume hardware is faulty. If it's faulty, then "disabling" the interrupt might not work, and generate an interrupt anyway. Therefore the commit "dmaengine: pxa_dm: explicitly freeup irq" won't help anyway IMHO. > The irq will trigger, probably your tasklet as well and while you are > figuring this out some resources are removed. So I do prefer disabling or > freeing irqs explicitly before we do anything else... I must add I did extensive testing with a loop of pxa_dma modprobe/rmmod and a user driver modprobe/removal for one night, with the dma_synchronize() patch but not the disable_irq() one. No error occurrence. Would that all convince you that disabling irq is not necessary on pxa architecture ? Cheers. -- Robert -- 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
