Re: [PATCH 24/32] dmaengine: pxa_dm: explicitly freeup irq

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

 



On Wed, Jul 06, 2016 at 08:52:12PM +0200, Robert Jarzmik wrote:
> 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.

Please do, patch is always welcome :)

> 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.

Suprious irqs. In case your irq fires somehow when you are not prepared can
least to nasty crashes...

> 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 :)

yeah had to so it for few ones :(

-- 
~Vinod
--
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



[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux