On 10/19/22 19:49, Vinod Koul wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On 20-08-22, 15:57, Tudor Ambarus wrote:
>> IRQs requested with this function will be automatically freed on driver
>> detach. Use devm_request_irq() and make the code cleaner.
>
> That is not really good idea!
>
>>
>> Signed-off-by: Tudor Ambarus <tudor.ambarus@xxxxxxxxxxxxx>
>> ---
>> drivers/dma/at_hdmac.c | 11 ++++-------
>> 1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
>> index 9c414f167b62..96b885f83374 100644
>> --- a/drivers/dma/at_hdmac.c
>> +++ b/drivers/dma/at_hdmac.c
>> @@ -2241,6 +2241,10 @@ static int __init at_dma_probe(struct platform_device *pdev)
>> irq = platform_get_irq(pdev, 0);
>> if (irq < 0)
>> return irq;
>> + err = devm_request_irq(&pdev->dev, irq, at_dma_interrupt, 0,
>> + dev_name(&pdev->dev), atdma);
>> + if (err)
>> + return err;
>>
>> /* discover transaction capabilities */
>> atdma->dma_device.cap_mask = plat_dat->cap_mask;
>> @@ -2257,10 +2261,6 @@ static int __init at_dma_probe(struct platform_device *pdev)
>> /* force dma off, just in case */
>> at_dma_off(atdma);
>>
>> - err = request_irq(irq, at_dma_interrupt, 0, "at_hdmac", atdma);
>> - if (err)
>> - goto err_irq;
>
> here you are ready to receive the irq, but now you moved it early on..
> it may cause issues..
Oh, yes.
>
>> -
>> platform_set_drvdata(pdev, atdma);
>>
>> /* create a pool of consistent memory blocks for hardware descriptors */
>> @@ -2377,8 +2377,6 @@ static int __init at_dma_probe(struct platform_device *pdev)
>> err_memset_pool_create:
>> dma_pool_destroy(atdma->dma_desc_pool);
>> err_desc_pool_create:
>> - free_irq(platform_get_irq(pdev, 0), atdma);
>> -err_irq:
>> clk_disable_unprepare(atdma->clk);
>> err_clk_prepare:
>> clk_put(atdma->clk);
>> @@ -2397,7 +2395,6 @@ static int at_dma_remove(struct platform_device *pdev)
>>
>> dma_pool_destroy(atdma->memset_pool);
>> dma_pool_destroy(atdma->dma_desc_pool);
>> - free_irq(platform_get_irq(pdev, 0), atdma);
>
> Now we have an active irq while we are unrolling device, which can be
> fired and cause tasklet to also get scheduled...
>
> You need to make sure now irq cant fire here and tasklet is killed..
right, I should have disabled the irq here and I missed it. Thanks, will
respin.
>
>>
>> list_for_each_entry_safe(chan, _chan, &atdma->dma_device.channels,
>> device_node) {
>> --
>> 2.25.1
>
> --
> ~Vinod
--
Cheers,
ta
