dmaengine: axi-dmac: Use devm_request_irq()
Subsystems (2)
PATCH 1/2 dmaengine: axi-dmac: Use devm_request_irq()
Use devm_request_irq() instead of request_irq() to request the IRQ.
---
Hi Lars,
was there a specific reason not to use devm_request_irq() ?
Cheers,
Moritz
--- drivers/dma/dma-axi-dmac.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/dma/dma-axi-dmac.c +3 -3
@@ -687,8 +687,9 @@ static int axi_dmac_probe(struct platform_device *pdev)if (ret) goto err_unregister_device; - ret =request_irq (dmac->irq, axi_dmac_interrupt_handler,0 , -dev_name (&pdev->dev), dmac); + ret =devm_request_irq (&pdev->dev, dmac->irq, + axi_dmac_interrupt_handler,0 , +dev_name (&pdev->dev), dmac);if (ret) goto err_unregister_of;
@@ -711,7 +712,6 @@ static int axi_dmac_remove(struct platform_device *pdev)struct axi_dmac *dmac =platform_get_drvdata (pdev);of_dma_controller_free (pdev->dev.of_node); -free_irq (dmac->irq, dmac);tasklet_kill (&dmac->chan.vchan.task);dma_async_device_unregister (&dmac->dma_dev);clk_disable_unprepare (dmac->clk);
PATCH 2/2 dmaengine: axi-dmac: Request IRQ with IRQF_SHARED
Request IRQ with IRQF_SHARED flag. This works since the interrupt handler already checks if there is an actual IRQ pending and returns IRQ_NONE otherwise.
--- drivers/dma/dma-axi-dmac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/dma/dma-axi-dmac.c +1 -1
@@ -688,7 +688,7 @@ static int axi_dmac_probe(struct platform_device *pdev) goto err_unregister_device; ret =devm_request_irq (&pdev->dev, dmac->irq, - axi_dmac_interrupt_handler,0 , + axi_dmac_interrupt_handler,IRQF_SHARED ,dev_name (&pdev->dev), dmac);if (ret) goto err_unregister_of;
Re: [PATCH 2/2] dmaengine: axi-dmac: Request IRQ with IRQF_SHARED
Quoted 5 lines · Moritz Fischer
Request IRQ with
IRQF_SHAREDflag. This works since the interrupt handler already checks if there is an actual IRQ pending and returnsIRQ_NONEotherwise.Signed-off-by Moritz Fischer <[email protected]>
Thanks.
Quoted 10 lines
--- drivers/dma/dma-axi-dmac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/dma/dma-axi-dmac.c +0
-0
drivers/dma/dma-axi-dmac.c static int axi_dmac_probe(struct platform_device *pdev) · line 694
Quoted 8 lines
drivers/dma/dma-axi-dmac.c +1
-1@@ -688,7 +688,7 @@ static int axi_dmac_probe(struct platform_device *pdev) goto err_unregister_device; ret =devm_request_irq (&pdev->dev, dmac->irq, - axi_dmac_interrupt_handler,0 , + axi_dmac_interrupt_handler,IRQF_SHARED ,dev_name (&pdev->dev), dmac);if (ret) goto err_unregister_of;
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Lars-Peter Clausen
maintainerQuoted 8 lines · Moritz Fischer
Use
devm_request_irq()instead ofrequest_irq()to request the IRQ.Signed-off-by Moritz Fischer <[email protected]>---
Hi Lars,
was there a specific reason not to use
devm_request_irq()?
Yes, it is wrong :)
This reorders the sequence in the remove() function. We really need to free the IRQ at the point in the sequence where it is at the moment. We need to call tasklet_kill() after the free_irq() function has been called, otherwise the tasklet can be re-scheduled and we might end up with a use after free.
But also if we disable the clock to the core before the IRQ is freed we might end up trying to access the register map while the clock is off and then never get a response and block the AXI bus.
This is all very very unlikely to ever happen, but still I'd like to have correct code.
Quoted 27 lines
Cheers,
Moritz
--- drivers/dma/dma-axi-dmac.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)drivers/dma/dma-axi-dmac.c +3
-2@@ -687,8 +687,9 @@ static int axi_dmac_probe(struct platform_device *pdev)if (ret) goto err_unregister_device; - ret =request_irq (dmac->irq, axi_dmac_interrupt_handler,0 , -dev_name (&pdev->dev), dmac); + ret =devm_request_irq (&pdev->dev, dmac->irq, + axi_dmac_interrupt_handler,0 , +dev_name (&pdev->dev), dmac);if (ret) goto err_unregister_of;
drivers/dma/dma-axi-dmac.c static int axi_dmac_remove(struct platform_device *pdev) · line 717
Quoted 7 lines
drivers/dma/dma-axi-dmac.c +0
-1@@ -711,7 +712,6 @@ static int axi_dmac_remove(struct platform_device *pdev)struct axi_dmac *dmac =platform_get_drvdata (pdev);of_dma_controller_free (pdev->dev.of_node); -free_irq (dmac->irq, dmac);tasklet_kill (&dmac->chan.vchan.task);dma_async_device_unregister (&dmac->dma_dev);clk_disable_unprepare (dmac->clk);
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Lars,
Quoted 17 lines · Lars-Peter Clausen
Quoted 8 lines · Moritz Fischer
Use
devm_request_irq()instead ofrequest_irq()to request the IRQ.Signed-off-by Moritz Fischer <[email protected]>---
Hi Lars,
was there a specific reason not to use
devm_request_irq()?Yes, it is wrong :)
This reorders the sequence in the
remove()function. We really need to free the IRQ at the point in the sequence where it is at the moment. We need to calltasklet_kill()after thefree_irq()function has been called, otherwise the tasklet can be re-scheduled and we might end up with a use after free.
Quoted 6 lines
But also if we disable the clock to the core before the IRQ is freed we might end up trying to access the register map while the clock is off and then never get a response and block the AXI bus.
This is all very very unlikely to ever happen, but still I'd like to have correct code.
Thanks for the explanation. I'll drop this and resubmit [2/2] without this change.
- Moritz
---
To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html