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

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

[Bartosz Golaszewski]

The BAM interrupt is requested with a devres helper and so on error it's
freed after probe() returns. We disable the clock before freeing or
masking it so it may still fire and we may end up reading BAM registers
with clock disabled.

Stop using devres for interrupts as we free it in remove() manually
anyway. Add an appropriate label and free the interrupt before disabling
the clock in error path and in remove().

Cc: stable@vger.kernel.org
Fixes: e7c0fe2a5c84 ("dmaengine: add Qualcomm BAM dma driver")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260427-qcom-qce-cmd-descr-v16-0-945fd1cafbbc%40oss.qualcomm.com?part=2
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
---
Sorry for the noise with v27, I didn't notice the change in dma/next.

This used to be part of the larger BAM DMA pipe locking series and never
got picked up despite months on the list. I'm resending it separately.
---
Changes in v28:
- Revert to using IRQF_TRIGGER_NONE when requesting the irq
- Link to v27: https://patch.msgid.link/20261008-bam-dma-free-irq-v27-1-660e70223e3c@oss.qualcomm.com

Changes in v27:
- Rabase on top of current dma/next
- Link to v26: https://patch.msgid.link/20261006-bam-dma-free-irq-v26-1-eef5c713d831@oss.qualcomm.com

Changes in v26:
- Rebased on top of current linux-next to address conflicts
- Link to v25: https://patch.msgid.link/20261002-bam-dma-free-irq-v25-1-f39e01d19910@oss.qualcomm.com

Changes in v25:
- Don't touch remove(), it's not wrong in its current version
- Link to v24: https://patch.msgid.link/20260723-qcom-qce-cmd-descr-v24-0-4f87bb4d9938@oss.qualcomm.com
---
 drivers/dma/qcom/bam_dma.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 94cb957d4b187704e207361d629fde00e3e090bf..f67a93a37d48cc23dcef04fb434d2613a1f22a77 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);
 	if (ret)
 		goto err_bam_channel_exit;
 
@@ -1366,7 +1366,7 @@ static int bam_dma_probe(struct platform_device *pdev)
 	ret = dma_async_device_register(&bdev->common);
 	if (ret) {
 		dev_err(bdev->dev, "failed to register dma async device\n");
-		goto err_bam_channel_exit;
+		goto err_free_irq;
 	}
 
 	ret = of_dma_controller_register(pdev->dev.of_node, bam_dma_xlate,
@@ -1385,6 +1385,8 @@ static int bam_dma_probe(struct platform_device *pdev)
 
 err_unregister_dma:
 	dma_async_device_unregister(&bdev->common);
+err_free_irq:
+	free_irq(bdev->irq, bdev);
 err_bam_channel_exit:
 	for (i = 0; i < bdev->num_channels; i++)
 		tasklet_kill(&bdev->channels[i].vc.task);
@@ -1410,7 +1412,7 @@ static void bam_dma_remove(struct platform_device *pdev)
 	/* mask all interrupts for this execution environment */
 	writel_relaxed(0, bam_addr(bdev, 0,  BAM_IRQ_SRCS_MSK_EE));
 
-	devm_free_irq(bdev->dev, bdev->irq, bdev);
+	free_irq(bdev->irq, bdev);
 
 	for (i = 0; i < bdev->num_channels; i++) {
 		bam_dma_terminate_all(&bdev->channels[i].vc.chan);

---
base-commit: 53b32375c9caf793ff392f762236c90f2a062ae5
change-id: 20261002-bam-dma-free-irq-cac4b268c465

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

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

[sashiko-bot]

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

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

[Vinod Koul]

On Thu, 08 Oct 2026 11:54:35 +0200, Bartosz Golaszewski wrote:
> The BAM interrupt is requested with a devres helper and so on error it's
> freed after probe() returns. We disable the clock before freeing or
> masking it so it may still fire and we may end up reading BAM registers
> with clock disabled.
> 
> Stop using devres for interrupts as we free it in remove() manually
> anyway. Add an appropriate label and free the interrupt before disabling
> the clock in error path and in remove().
> 
> [...]

Applied, thanks!

[1/1] dmaengine: qcom: bam_dma: free interrupt before the clock in error path
      commit: 82eed160cfa7a296ae4eb5acb8dd8f493323eea7

Best regards,
-- 
~Vinod