Re: [PATCH 11/32] dmaengine: imx-dma: explicitly freeup irq

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

 



On Tue, Jul 05, 2016 at 08:24:17PM +0530, Vinod Koul wrote:
> 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.
> 
> Signed-off-by: Vinod Koul <vinod.koul@xxxxxxxxx>
> Cc: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>
> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> +static void imxdma_free_irq(struct platform_device *pdev, struct imxdma_engine *imxdma)
> +{
> +	int i;
> +
> +	if (is_imx1_dma(imxdma)) {
> +		devm_free_irq(&pdev->dev, imxdma->irq, imxdma);
> +		devm_free_irq(&pdev->dev, imxdma->irq_err, imxdma);
> +	}
> +
> +	for (i = 0; i < IMX_DMA_CHANNELS; i++) {
> +		struct imxdma_channel *imxdmac = &imxdma->channel[i];
> +
> +		if (!is_imx1_dma(imxdma))
> +			devm_free_irq(&pdev->dev, imxdmac->irq, imxdma);
> +	}
> +}
> +
>  static int imxdma_remove(struct platform_device *pdev)
>  {
>  	struct imxdma_engine *imxdma = platform_get_drvdata(pdev);
>  
> +	imxdma_free_irq(pdev, imxdma);
> +

I don't think this is a good idea for various reasons.

When it's not safe to rely on the interrupt being released by devm later
then shouldn't we explicitly release the interrupt in the probe error
path aswell? Then if we don't let devm release the irq what's the point
in using devm_request_irq at all?

Also what's the case in which we have to explicitly release the irq? The
imx-dma driver explicitly disables the interrupts for deactivated
channels and I assume that when the drivers remove() function is called
every channel *is* deactivated. If it's not then we are in big trouble
since then we must assume that also a DMA transfer might be ongoing which
will surely lead to trouble when the driver is removed.

I think that when this patch solves an issue we really have a problem
elsewhere, including that most users of devm_request_irq are wrong and
this function maybe shouldn't even exist.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
--
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