Re: [PATCH v28] dmaengine: qcom: bam_dma: free interrupt before the clock in error path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dmaengine: qcom: bam_dma: IRQ storm and PM usage leak due to early request_irq
--

commit 6d143e987eb1a2023f5265042274ca78fbb524c0
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

dmaengine: qcom: bam_dma: free interrupt before the clock in error path

The BAM interrupt is requested with a devres helper and freed after probe()
returns, but the clock is disabled before freeing it, which can lead to
reading BAM registers with the clock disabled. This patch stops using the
devres helper and manually frees the interrupt before disabling the clock
in the error path and in remove().

Closes: https://sashiko.dev/#/patchset/20260427-qcom-qce-cmd-descr-v16-0-945fd1cafbbc%40oss.qualcomm.com?part=2

> diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> index 94cb957d4b187..f67a93a37d48c 100644
> --- a/drivers/dma/qcom/bam_dma.c
> +++ b/drivers/dma/qcom/bam_dma.c
> @@ -1332,8 +1332,8 @@ static int bam_dma_probe(struct platform_device *pdev)
>  	for (i = 0; i < bdev->num_channels; i++)
>  		bam_channel_init(bdev, &bdev->channels[i], i);
>  
> -	ret = devm_request_irq(bdev->dev, bdev->irq, bam_dma_irq,
> -			IRQF_TRIGGER_NONE, "bam_dma", bdev);
> +	ret = request_irq(bdev->irq, bam_dma_irq, IRQF_TRIGGER_NONE,
> +			  "bam_dma", bdev);

[Severity: High]
Could this request_irq() lead to an IRQ storm if an interrupt fires
immediately?

Looking at bam_dma_probe(), request_irq() is called here, but runtime PM is
configured and enabled much later in the function:

drivers/dma/qcom/bam_dma.c:bam_dma_probe() {
    ...
	pm_runtime_set_active(&pdev->dev);
	pm_runtime_enable(&pdev->dev);
    ...
}

If the BAM hardware asserts an interrupt before PM is enabled,
bam_dma_irq() will execute and call pm_runtime_get_sync():

drivers/dma/qcom/bam_dma.c:bam_dma_irq() {
    ...
	ret = pm_runtime_get_sync(bdev->dev);
	if (ret < 0)
		return IRQ_NONE;
    ...
}

Since PM is not active yet, pm_runtime_get_sync() will fail. The handler
returns IRQ_NONE without clearing the pending hardware interrupt, leaving
the level-triggered interrupt asserted and causing an IRQ storm.

Additionally, does this early return leak the PM runtime usage counter?
pm_runtime_get_sync() increments the usage counter even on failure, so
returning early without putting the counter would leak it on every
spurious execution.

>  	if (ret)
>  		goto err_bam_channel_exit;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-bam-dma-free-irq-v28-1-5e5b10de441e@oss.qualcomm.com?part=1