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

[Frank Li <Frank.li@oss.nxp.com>]

On Fri, Oct 02, 2026 at 10:51:47AM +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.

It is less possible to happen. I see other methods to fix similar issues

https://lore.kernel.org/all/20260608001128.80090-1-dennylin0707@gmail.com/
https://lore.kernel.org/dmaengine/20260927-dma40-fixes-v7-6-89f595e8851d@kernel.org/

>
> 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().

Just want to avoid bounce in future, change back to devm version.

>
> 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 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 05a3b1f9e0c23dc5f861488fae03494867970523..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_HIGH, "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);

Can you update it use dmaenginem_sync_device_register() to update current
base?

>  	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,

devm_of_dma_controller_register()

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

Actually there are problem

pm_runtime_force_suspend(&pdev->dev);  it will call suspend, which disable
clk,

...
writel_relaxed(0, bam_addr(bdev, 0,  BAM_IRQ_SRCS_MSK_EE));
access register

clk_disable_unprepare(bdev->bamclk);

Maybe cause refcount overflow because previous pm_runtime_force_suspend().

Actually DMA driver seldom remove. It has another issue if still have
consumer acquire channel because miss dev link between consumer and
provider.

Anyways, if you resolve above runtime pm problem, irq problem may not
existing.

Frank

>
> -	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: 9f24d789f03b22941b905ded43cb5ff8eea9ce62
> change-id: 20261002-bam-dma-free-irq-cac4b268c465
>
> Best regards,
> --
> Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
>