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

[PATCH v27] 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>
---
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 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..a626746b5c93496e5c5e885b35a856d38c284448 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_HIGH,
+			  "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 v27] dmaengine: qcom: bam_dma: free interrupt before the clock in error path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] dmaengine: qcom: bam_dma: hardcoded IRQF_TRIGGER_HIGH overrides DT
- [High] dmaengine: qcom: unclocked register access and clock double-disable in remove()
- [High] qcom/bam_dma: interrupt storm if IRQ fires before pm_runtime_enable()
--

commit 066c83fa78866b6ea0bc75cfc8548e9a56be5f96
Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

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

This commit replaces devm_request_irq() with request_irq() to manually free
the interrupt before disabling the clock in the error path. It also aims to
apply the same ordering to the remove path to prevent reading BAM registers
with the clock disabled.

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..a626746b5c934 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_HIGH,
> +			  "bam_dma", bdev);

[Severity: High]
Does hardcoding IRQF_TRIGGER_HIGH override the platform-specific Device Tree
configuration?

The original implementation used IRQF_TRIGGER_NONE, which properly delegated
the trigger configuration to the firmware or Device Tree. If this interrupt
is wired as active-low or edge-triggered on some platforms, could hardcoding
this to HIGH misconfigure the interrupt controller and lead to missed
interrupts or failure to request the IRQ?

[Severity: High]
If a pending or spurious interrupt fires immediately after request_irq(), could
it cause an interrupt storm?

bam_dma_irq() calls pm_runtime_get_sync(bdev->dev). However, pm_runtime_enable()
is not called until later in probe:

drivers/dma/qcom/bam_dma.c:bam_dma_probe() {
    ...
    ret = request_irq(bdev->irq, bam_dma_irq, IRQF_TRIGGER_HIGH, ...);
    ...
    pm_runtime_enable(&pdev->dev);
    ...
}

If the interrupt fires before pm_runtime_enable() is called, pm_runtime_get_sync()
will fail, and the handler will return IRQ_NONE without clearing the hardware
interrupt registers. Since this is a level-triggered interrupt, won't the GIC
immediately re-enter the handler upon exit, causing a tight infinite interrupt
storm until the spurious interrupt detector permanently disables the IRQ line?

[ ... ]

> @@ -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);

[Severity: High]
The commit message states this patch will "free the interrupt before disabling
the clock in error path and in remove()".

However, in bam_dma_remove(), pm_runtime_force_suspend() is called before freeing
the interrupt and before accessing the hardware:

drivers/dma/qcom/bam_dma.c:bam_dma_remove() {
    pm_runtime_force_suspend(&pdev->dev);
    ...
    writel_relaxed(0, bam_addr(bdev, 0,  BAM_IRQ_SRCS_MSK_EE));
    free_irq(bdev->irq, bdev);
    ...
    clk_disable_unprepare(bdev->bamclk);
}

Since pm_runtime_force_suspend() turns off the BAM clock, does this
writel_relaxed() access the hardware with the clock disabled, potentially
triggering a fatal bus fault or SError? Additionally, won't the unconditional
clk_disable_unprepare() at the end of remove() cause a clock imbalance WARN
trace?

Could the commit message and code be out of sync here since the reordering in
remove() appears to have been reverted?

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

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

[Bartosz Golaszewski]

On Thu, 8 Oct 2026 11:30:56 +0200, Bartosz Golaszewski
<bartosz.golaszewski@oss.qualcomm.com> said:
> 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>
> ---
> 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 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..a626746b5c93496e5c5e885b35a856d38c284448 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_HIGH,
> +			  "bam_dma", bdev);

Please disregard it version, this should have remained IRQF_TRIGGER_NONE.

Bart