[PATCH 00/26] dmaengine: Remove redundant error messages on IRQ request failure

[PATCH 00/26] dmaengine: Remove redundant error messages on IRQ request failure

[Pan Chuang]

Commit 55b48e23f5c4b6f5ca9b7ab09599b17dcf501c10 ("genirq/devres: Add
error handling in devm_request_*_irq()") added automatic error logging
to devm_request_threaded_irq() and devm_request_any_context_irq() via
the new devm_request_result() helper. The helper prints device name,
IRQ number, handler functions, and error code on failure.

Since devm_request_irq() is a static inline wrapper around
devm_request_threaded_irq(), it also benefits from this automatic
logging.

This series removes the now-redundant dev_err() and dev_err_probe() calls
in dmaengine drivers that follow these devm_request_*_irq() functions,
as the core now provides more detailed diagnostic information on failure.

Pan Chuang (26):
  dmaengine: fsl-edma-main: Remove redundant dev_err()/dev_err_probe()
  dmaengine: fsl-qdma: Remove redundant dev_err()/dev_err_probe()
  dmaengine: loongson-loongson1-apb-dma: Remove redundant
    dev_err()/dev_err_probe()
  dmaengine: mediatek-mtk-cqdma: Remove redundant
    dev_err()/dev_err_probe()
  dmaengine: mediatek-mtk-hsdma: Remove redundant
    dev_err()/dev_err_probe()
  dmaengine: mmp_pdma: Remove redundant dev_err()/dev_err_probe()
  dmaengine: moxart-dma: Remove redundant dev_err()/dev_err_probe()
  dmaengine: owl-dma: Remove redundant dev_err()/dev_err_probe()
  dmaengine: pxa_dma: Remove redundant dev_err()/dev_err_probe()
  dmaengine: qcom-gpi: Remove redundant dev_err()/dev_err_probe()
  dmaengine: sf-pdma-sf-pdma: Remove redundant dev_err()/dev_err_probe()
  dmaengine: sh-rcar-dmac: Remove redundant dev_err()/dev_err_probe()
  dmaengine: sh-rz-dmac: Remove redundant dev_err()/dev_err_probe()
  dmaengine: sh-shdmac: Remove redundant dev_err()/dev_err_probe()
  dmaengine: sh-usb-dmac: Remove redundant dev_err()/dev_err_probe()
  dmaengine: sprd-dma: Remove redundant dev_err()/dev_err_probe()
  dmaengine: st_fdma: Remove redundant dev_err()/dev_err_probe()
  dmaengine: stm32-stm32-dma: Remove redundant dev_err()/dev_err_probe()
  dmaengine: stm32-stm32-dma3: Remove redundant
    dev_err()/dev_err_probe()
  dmaengine: stm32-stm32-mdma: Remove redundant
    dev_err()/dev_err_probe()
  dmaengine: sun4i-dma: Remove redundant dev_err()/dev_err_probe()
  dmaengine: sun6i-dma: Remove redundant dev_err()/dev_err_probe()
  dmaengine: tegra20-apb-dma: Remove redundant dev_err()/dev_err_probe()
  dmaengine: ti-edma: Remove redundant dev_err()/dev_err_probe()
  dmaengine: uniphier-xdmac: Remove redundant dev_err()/dev_err_probe()
  dmaengine: xgene-dma: Remove redundant dev_err()/dev_err_probe()

 drivers/dma/fsl-edma-main.c              | 23 +++++++----------------
 drivers/dma/fsl-qdma.c                   |  4 +---
 drivers/dma/loongson/loongson1-apb-dma.c |  4 +---
 drivers/dma/mediatek/mtk-cqdma.c         |  5 +----
 drivers/dma/mediatek/mtk-hsdma.c         |  5 +----
 drivers/dma/mmp_pdma.c                   |  4 +---
 drivers/dma/moxart-dma.c                 |  4 +---
 drivers/dma/owl-dma.c                    |  4 +---
 drivers/dma/pxa_dma.c                    |  6 +-----
 drivers/dma/qcom/gpi.c                   |  5 +----
 drivers/dma/sf-pdma/sf-pdma.c            |  8 ++------
 drivers/dma/sh/rcar-dmac.c               |  5 +----
 drivers/dma/sh/rz-dmac.c                 |  8 +-------
 drivers/dma/sh/shdmac.c                  |  6 +-----
 drivers/dma/sh/usb-dmac.c                |  5 +----
 drivers/dma/sprd-dma.c                   |  4 +---
 drivers/dma/st_fdma.c                    |  4 +---
 drivers/dma/stm32/stm32-dma.c            |  6 +-----
 drivers/dma/stm32/stm32-dma3.c           |  5 +----
 drivers/dma/stm32/stm32-mdma.c           |  4 +---
 drivers/dma/sun4i-dma.c                  |  2 +-
 drivers/dma/sun6i-dma.c                  |  4 +---
 drivers/dma/tegra20-apb-dma.c            |  6 +-----
 drivers/dma/ti/edma.c                    |  8 ++------
 drivers/dma/uniphier-xdmac.c             |  4 +---
 drivers/dma/xgene-dma.c                  |  5 +----
 26 files changed, 34 insertions(+), 114 deletions(-)

-- 
2.34.1

[PATCH 01/26] dmaengine: fsl-edma-main: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/fsl-edma-main.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
index 36155ab1602a..0881f4f36b3f 100644
--- a/drivers/dma/fsl-edma-main.c
+++ b/drivers/dma/fsl-edma-main.c
@@ -357,24 +357,18 @@ fsl_edma_irq_init(struct platform_device *pdev, struct fsl_edma_engine *fsl_edma
 	if (fsl_edma->txirq == fsl_edma->errirq) {
 		ret = devm_request_irq(&pdev->dev, fsl_edma->txirq,
 				fsl_edma_irq_handler, 0, "eDMA", fsl_edma);
-		if (ret) {
-			dev_err(&pdev->dev, "Can't register eDMA IRQ.\n");
+		if (ret)
 			return ret;
-		}
 	} else {
 		ret = devm_request_irq(&pdev->dev, fsl_edma->txirq,
 				fsl_edma_tx_handler, 0, "eDMA tx", fsl_edma);
-		if (ret) {
-			dev_err(&pdev->dev, "Can't register eDMA tx IRQ.\n");
+		if (ret)
 			return ret;
-		}
 
 		ret = devm_request_irq(&pdev->dev, fsl_edma->errirq,
 				fsl_edma_err_handler, 0, "eDMA err", fsl_edma);
-		if (ret) {
-			dev_err(&pdev->dev, "Can't register eDMA err IRQ.\n");
+		if (ret)
 			return ret;
-		}
 	}
 
 	return 0;
@@ -418,7 +412,7 @@ static int fsl_edma3_irq_init(struct platform_device *pdev, struct fsl_edma_engi
 		ret = devm_request_irq(&pdev->dev, fsl_edma->errirq, fsl_edma3_err_handler_shared,
 				       0, errirq_name, fsl_edma);
 		if (ret)
-			return dev_err_probe(&pdev->dev, ret, "Can't register eDMA err IRQ.\n");
+			return ret;
 	}
 
 	return 0;
@@ -445,24 +439,21 @@ static int fsl_edma3_or_irq_init(struct platform_device *pdev,
 			       fsl_edma3_tx_0_15_handler, 0, "eDMA tx0_15",
 			       fsl_edma);
 	if (ret)
-		return dev_err_probe(&pdev->dev, ret,
-			       "Can't register eDMA tx0_15 IRQ.\n");
+		return ret;
 
 	if (fsl_edma->n_chans > 16) {
 		ret = devm_request_irq(&pdev->dev, fsl_edma->txirq_16_31,
 				       fsl_edma3_tx_16_31_handler, 0,
 				       "eDMA tx16_31", fsl_edma);
 		if (ret)
-			return dev_err_probe(&pdev->dev, ret,
-					"Can't register eDMA tx16_31 IRQ.\n");
+			return ret;
 	}
 
 	ret = devm_request_irq(&pdev->dev, fsl_edma->errirq,
 			       fsl_edma3_or_err_handler, 0, "eDMA err",
 			       fsl_edma);
 	if (ret)
-		return dev_err_probe(&pdev->dev, ret,
-				     "Can't register eDMA err IRQ.\n");
+		return ret;
 
 	return 0;
 }
-- 
2.34.1

Re: [PATCH 01/26] dmaengine: fsl-edma-main: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The txirq_16_31 interrupt is not freed before vchan cleanup, leading to a race condition and potential use-after-free on device unbind.
- [High] Double disable/unprepare of devm-managed clocks during the probe failure path.
- [High] Probe deferral is permanently broken due to unconditionally overriding negative error codes from `platform_get_irq()` with `-EINVAL` or `-ENXIO`.
--

--- Patch [1]: [PATCH 01/26] dmaengine: fsl-edma-main: Remove redundant dev_err()/dev_err_probe() ---
commit 0e4d4c4909fc6ed60a790a7b9fecb3bf5b716c3b
Author: Pan Chuang <panchuang@vivo.com>

    dmaengine: fsl-edma-main: Remove redundant dev_err()/dev_err_probe()
    
    The devm_request_irq() now automatically logs detailed error messages on
    failure. This eliminates the need for driver-specific dev_err() and
    dev_err_probe() calls that previously printed generic messages.

> diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
> index 36155ab1602a9..0881f4f36b3f4 100644
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c
[ ... ]
> @@ -418,7 +412,7 @@ static int fsl_edma3_irq_init(struct platform_device *pdev, struct fsl_edma_engi
>  		ret = devm_request_irq(&pdev->dev, fsl_edma->errirq, fsl_edma3_err_handler_shared,
>  				       0, errirq_name, fsl_edma);
>  		if (ret)
> -			return dev_err_probe(&pdev->dev, ret, "Can't register eDMA err IRQ.\n");
> +			return ret;

[Severity: High]
This is a pre-existing issue, but probe deferral appears to be permanently
broken for the interrupt requests in this function and fsl_edma2_irq_init().

In fsl_edma3_irq_init(), if platform_get_irq() returns -EPROBE_DEFER,
the error code is discarded and replaced with -EINVAL:

    fsl_chan->txirq = platform_get_irq(pdev, i);
    if (fsl_chan->txirq < 0)
        return -EINVAL;

Similarly in fsl_edma2_irq_init(), it replaces the error with -ENXIO:

    irq = platform_get_irq(pdev, i);
    ret = 0;
    if (irq < 0)
        return -ENXIO;

Could this prevent the DMA engine from probing on systems where the
interrupt controller is not yet ready?

>  	}
>  
>  	return 0;
> @@ -445,24 +439,21 @@ static int fsl_edma3_or_irq_init(struct platform_device *pdev,
>  			       fsl_edma3_tx_0_15_handler, 0, "eDMA tx0_15",
>  			       fsl_edma);
>  	if (ret)
> -		return dev_err_probe(&pdev->dev, ret,
> -			       "Can't register eDMA tx0_15 IRQ.\n");
> +		return ret;
>  
>  	if (fsl_edma->n_chans > 16) {
>  		ret = devm_request_irq(&pdev->dev, fsl_edma->txirq_16_31,
>  				       fsl_edma3_tx_16_31_handler, 0,
>  				       "eDMA tx16_31", fsl_edma);
>  		if (ret)
> -			return dev_err_probe(&pdev->dev, ret,
> -					"Can't register eDMA tx16_31 IRQ.\n");
> +			return ret;

[Severity: High]
This isn't a bug introduced by this patch, but does the handling of
txirq_16_31 create a race condition leading to a potential use-after-free?

When the driver is unbound, fsl_edma_remove() executes:

    fsl_edma_irq_exit(pdev, fsl_edma);
    fsl_edma_cleanup_vchan(&fsl_edma->dma_dev);

However, fsl_edma_irq_exit() manually frees txirq and errirq to stop
interrupts before vchan cleanup, but it misses txirq_16_31:

    if (fsl_edma->txirq == fsl_edma->errirq) {
        if (fsl_edma->txirq >= 0)
            devm_free_irq(&pdev->dev, fsl_edma->txirq, fsl_edma);
    } else {
        if (fsl_edma->txirq >= 0)
            devm_free_irq(&pdev->dev, fsl_edma->txirq, fsl_edma);
        if (fsl_edma->errirq >= 0)
            devm_free_irq(&pdev->dev, fsl_edma->errirq, fsl_edma);
    }

Since txirq_16_31 is not freed, is it possible for the interrupt handler
to still run during or after fsl_edma_cleanup_vchan() destroys the vchan
tasklets?

>  	}
>  
>  	ret = devm_request_irq(&pdev->dev, fsl_edma->errirq,
>  			       fsl_edma3_or_err_handler, 0, "eDMA err",
>  			       fsl_edma);
>  	if (ret)
> -		return dev_err_probe(&pdev->dev, ret,
> -				     "Can't register eDMA err IRQ.\n");
> +		return ret;
>  
>  	return 0;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but is there a double disable of
devm-managed clocks during the probe failure path?

In fsl_edma_probe(), the mux clocks are acquired using the devm variant:

    fsl_edma->muxclk[i] = devm_clk_get_enabled(&pdev->dev, clkname);

If resource mapping fails for a subsequent mux base, the error path manually
disables the clocks:

    if (IS_ERR(fsl_edma->muxbase[i])) {
        /* on error: disable all previously enabled clks */
        fsl_disable_clocks(fsl_edma, i);
        return PTR_ERR(fsl_edma->muxbase[i]);
    }

Because the clocks were acquired with devm_clk_get_enabled(), won't devres
also try to disable them when cleaning up the probe failure, resulting in a
double-disable?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=1

Re: [PATCH 01/26] dmaengine: fsl-edma-main: Remove redundant dev_err()/dev_err_probe()

[Frank Li]

On Thu, Jul 09, 2026 at 09:58:05PM +0800, Pan Chuang wrote:
> The devm_request_irq() now automatically logs detailed error messages on
> failure. This eliminates the need for driver-specific dev_err() and
> dev_err_probe() calls that previously printed generic messages.
>
> Signed-off-by: Pan Chuang <panchuang@vivo.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/fsl-edma-main.c | 23 +++++++----------------
>  1 file changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/dma/fsl-edma-main.c b/drivers/dma/fsl-edma-main.c
> index 36155ab1602a..0881f4f36b3f 100644
> --- a/drivers/dma/fsl-edma-main.c
> +++ b/drivers/dma/fsl-edma-main.c
> @@ -357,24 +357,18 @@ fsl_edma_irq_init(struct platform_device *pdev, struct fsl_edma_engine *fsl_edma
>  	if (fsl_edma->txirq == fsl_edma->errirq) {
>  		ret = devm_request_irq(&pdev->dev, fsl_edma->txirq,
>  				fsl_edma_irq_handler, 0, "eDMA", fsl_edma);
> -		if (ret) {
> -			dev_err(&pdev->dev, "Can't register eDMA IRQ.\n");
> +		if (ret)
>  			return ret;
> -		}
>  	} else {
>  		ret = devm_request_irq(&pdev->dev, fsl_edma->txirq,
>  				fsl_edma_tx_handler, 0, "eDMA tx", fsl_edma);
> -		if (ret) {
> -			dev_err(&pdev->dev, "Can't register eDMA tx IRQ.\n");
> +		if (ret)
>  			return ret;
> -		}
>
>  		ret = devm_request_irq(&pdev->dev, fsl_edma->errirq,
>  				fsl_edma_err_handler, 0, "eDMA err", fsl_edma);
> -		if (ret) {
> -			dev_err(&pdev->dev, "Can't register eDMA err IRQ.\n");
> +		if (ret)
>  			return ret;
> -		}
>  	}
>
>  	return 0;
> @@ -418,7 +412,7 @@ static int fsl_edma3_irq_init(struct platform_device *pdev, struct fsl_edma_engi
>  		ret = devm_request_irq(&pdev->dev, fsl_edma->errirq, fsl_edma3_err_handler_shared,
>  				       0, errirq_name, fsl_edma);
>  		if (ret)
> -			return dev_err_probe(&pdev->dev, ret, "Can't register eDMA err IRQ.\n");
> +			return ret;
>  	}
>
>  	return 0;
> @@ -445,24 +439,21 @@ static int fsl_edma3_or_irq_init(struct platform_device *pdev,
>  			       fsl_edma3_tx_0_15_handler, 0, "eDMA tx0_15",
>  			       fsl_edma);
>  	if (ret)
> -		return dev_err_probe(&pdev->dev, ret,
> -			       "Can't register eDMA tx0_15 IRQ.\n");
> +		return ret;
>
>  	if (fsl_edma->n_chans > 16) {
>  		ret = devm_request_irq(&pdev->dev, fsl_edma->txirq_16_31,
>  				       fsl_edma3_tx_16_31_handler, 0,
>  				       "eDMA tx16_31", fsl_edma);
>  		if (ret)
> -			return dev_err_probe(&pdev->dev, ret,
> -					"Can't register eDMA tx16_31 IRQ.\n");
> +			return ret;
>  	}
>
>  	ret = devm_request_irq(&pdev->dev, fsl_edma->errirq,
>  			       fsl_edma3_or_err_handler, 0, "eDMA err",
>  			       fsl_edma);
>  	if (ret)
> -		return dev_err_probe(&pdev->dev, ret,
> -				     "Can't register eDMA err IRQ.\n");
> +		return ret;
>
>  	return 0;
>  }
> --
> 2.34.1
>
>

[PATCH 02/26] dmaengine: fsl-qdma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/fsl-qdma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma/fsl-qdma.c b/drivers/dma/fsl-qdma.c
index df843fad0ece..7f0d69b99289 100644
--- a/drivers/dma/fsl-qdma.c
+++ b/drivers/dma/fsl-qdma.c
@@ -818,10 +818,8 @@ fsl_qdma_irq_init(struct platform_device *pdev,
 	ret = devm_request_irq(&pdev->dev, fsl_qdma->error_irq,
 			       fsl_qdma_error_handler, 0,
 			       "qDMA error", fsl_qdma);
-	if (ret) {
-		dev_err(&pdev->dev, "Can't register qDMA controller IRQ.\n");
+	if (ret)
 		return  ret;
-	}
 
 	for (i = 0; i < fsl_qdma->block_number; i++) {
 		sprintf(irq_name, "qdma-queue%d", i);
-- 
2.34.1

Re: [PATCH 02/26] dmaengine: fsl-qdma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Medium] Incomplete Refactoring / Commit Message Mismatch

Pre-existing issues:
- [High] Driver fails to halt the hardware DMA engine on teardown and error paths
- [High] Inverted teardown order: driver destroys internal resources before unregistering the device
- [High] vchan tasklets are not killed on probe failure
--

--- Patch [2]: [PATCH 02/26] dmaengine: fsl-qdma: Remove redundant dev_err()/dev_err_probe() ---
commit a6f00a613e01411f16385ac0c92bca0027dc654d
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: fsl-qdma: Remove redundant dev_err()/dev_err_probe()

This commit removes a redundant error log following a call to
devm_request_irq, as the core already logs failures. It intends to clean up
the error paths in the qDMA driver.

> The devm_request_irq() now automatically logs detailed error messages on
> failure. This eliminates the need for driver-specific dev_err() and
> dev_err_probe() calls that previously printed generic messages.

[Severity: Medium]
Is the commit message accurate regarding dev_err_probe? Looking at the
patch, no dev_err_probe calls are actually removed.

Also, does this patch miss the second devm_request_irq in the same
function?

drivers/dma/fsl-qdma.c:fsl_qdma_irq_init() {
[ ... ]
        ret = devm_request_irq(&pdev->dev,
                               fsl_qdma->queue_irq[i],
                               fsl_qdma_queue_handler,
                               0,
                               "qDMA queue",
                               fsl_qdma);
        if (ret) {
                dev_err(&pdev->dev,
                        "Can't register qDMA queue IRQ.\n");
                return  ret;
        }
[ ... ]
}

> diff --git a/drivers/dma/fsl-qdma.c b/drivers/dma/fsl-qdma.c
> index df843fad0ece5..7f0d69b99289e 100644
> --- a/drivers/dma/fsl-qdma.c
> +++ b/drivers/dma/fsl-qdma.c
> @@ -818,10 +818,8 @@ fsl_qdma_irq_init(struct platform_device *pdev,
>  	ret = devm_request_irq(&pdev->dev, fsl_qdma->error_irq,
>  			       fsl_qdma_error_handler, 0,
>  			       "qDMA error", fsl_qdma);
> -	if (ret) {
> -		dev_err(&pdev->dev, "Can't register qDMA controller IRQ.\n");
> +	if (ret)
>  		return  ret;
> -	}
>  
>  	for (i = 0; i < fsl_qdma->block_number; i++) {
>  		sprintf(irq_name, "qdma-queue%d", i);

[Severity: High]
This is a pre-existing issue, but does the driver fail to halt the hardware
DMA engine on teardown and error paths?

fsl_qdma_reg_init() explicitly enables the DMA hardware and its interrupts.
If probe() fails later or when the driver is unbound, the driver never 
invokes fsl_qdma_halt() to stop the device:

drivers/dma/fsl-qdma.c:fsl_qdma_remove() {
        fsl_qdma_irq_exit(pdev, fsl_qdma);
        fsl_qdma_cleanup_vchan(&fsl_qdma->dma_dev);
        of_dma_controller_free(np);
        dma_async_device_unregister(&fsl_qdma->dma_dev);
}

Because the DMA buffers are allocated via devres (e.g., dmam_alloc_coherent),
they are freed immediately upon driver removal or probe failure. Can the
unhalted hardware continue to perform DMA operations into this freed memory?

[Severity: High]
This is a pre-existing issue, but does fsl_qdma_remove() have an inverted
teardown order?

drivers/dma/fsl-qdma.c:fsl_qdma_remove() {
        fsl_qdma_irq_exit(pdev, fsl_qdma);
        fsl_qdma_cleanup_vchan(&fsl_qdma->dma_dev);
        of_dma_controller_free(np);
        dma_async_device_unregister(&fsl_qdma->dma_dev);
}

fsl_qdma_cleanup_vchan() is called before dma_async_device_unregister(). This
creates a window where the DMA controller is still visible to clients via the
DMA engine API, but its backend is destroyed.

Furthermore, fsl_qdma_cleanup_vchan() removes channels from the channels
list. Will dma_async_device_unregister() then iterate over an empty list and
fail to deregister the channels from sysfs?

[Severity: High]
This is a pre-existing issue, but are vchan tasklets properly killed on
probe failure?

drivers/dma/fsl-qdma.c:fsl_qdma_probe() {
[ ... ]
        ret = fsl_qdma_irq_init(pdev, fsl_qdma);
        if (ret)
                return ret;

        ret = dma_async_device_register(&fsl_qdma->dma_dev);
        if (ret)
                return dev_err_probe(&pdev->dev, ret,
                                     "Can't register NXP Layerscape qDMA engine.\n");
[ ... ]
}

vchan_init() initializes the vchan tasklets for each channel. If a subsequent
step like fsl_qdma_irq_init() or dma_async_device_register() fails, probe()
returns the error code directly without calling tasklet_kill().

Because the hardware is already enabled, could an interrupt fire and schedule
the tasklet? When devres frees the fsl_qdma->chans array upon probe failure,
would the scheduled tasklet execute against freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=2

Re: [PATCH 02/26] dmaengine: fsl-qdma: Remove redundant dev_err()/dev_err_probe()

[Frank Li]

On Thu, Jul 09, 2026 at 09:58:06PM +0800, Pan Chuang wrote:
> The devm_request_irq() now automatically logs detailed error messages on
> failure. This eliminates the need for driver-specific dev_err() and
> dev_err_probe() calls that previously printed generic messages.
>
> Signed-off-by: Pan Chuang <panchuang@vivo.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/dma/fsl-qdma.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/dma/fsl-qdma.c b/drivers/dma/fsl-qdma.c
> index df843fad0ece..7f0d69b99289 100644
> --- a/drivers/dma/fsl-qdma.c
> +++ b/drivers/dma/fsl-qdma.c
> @@ -818,10 +818,8 @@ fsl_qdma_irq_init(struct platform_device *pdev,
>  	ret = devm_request_irq(&pdev->dev, fsl_qdma->error_irq,
>  			       fsl_qdma_error_handler, 0,
>  			       "qDMA error", fsl_qdma);
> -	if (ret) {
> -		dev_err(&pdev->dev, "Can't register qDMA controller IRQ.\n");
> +	if (ret)
>  		return  ret;
> -	}
>
>  	for (i = 0; i < fsl_qdma->block_number; i++) {
>  		sprintf(irq_name, "qdma-queue%d", i);
> --
> 2.34.1
>

[PATCH 03/26] dmaengine: loongson-loongson1-apb-dma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/loongson/loongson1-apb-dma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma/loongson/loongson1-apb-dma.c b/drivers/dma/loongson/loongson1-apb-dma.c
index 89786cbd20ab..812bd7c2068f 100644
--- a/drivers/dma/loongson/loongson1-apb-dma.c
+++ b/drivers/dma/loongson/loongson1-apb-dma.c
@@ -162,10 +162,8 @@ static int ls1x_dma_alloc_chan_resources(struct dma_chan *dchan)
 
 	ret = devm_request_irq(dev, chan->irq, ls1x_dma_irq_handler,
 			       IRQF_SHARED, dma_chan_name(dchan), chan);
-	if (ret) {
-		dev_err(dev, "failed to request IRQ %d\n", chan->irq);
+	if (ret)
 		return ret;
-	}
 
 	chan->lli_pool = dma_pool_create(dma_chan_name(dchan), dev,
 					 sizeof(struct ls1x_dma_lli),
-- 
2.34.1

Re: [PATCH 03/26] dmaengine: loongson-loongson1-apb-dma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing cleanup of devm-managed IRQ on error paths inside ls1x_dma_alloc_chan_resources() leading to persistent leaks.
- [High] Unsafe IRQF_SHARED handler lacks hardware status verification, risking data corruption.
- [High] API violation: non-atomic readl_poll_timeout macro used in atomic context risks hard lockups.
- [High] Incorrect use of the logical sysfs channel device for DMA API operations and devres allocations.
- [High] Bypassing the devres unregister mechanism causes a Use-After-Free and resource leaks.
- [High] Memory for the DMA descriptor is leaked if scatterlist allocation fails in ls1x_dma_prep_dma_cyclic().
--

--- Patch [3]: [PATCH 03/26] dmaengine: loongson-loongson1-apb-dma: Remove redundant dev_err()/dev_err_probe() ---
commit cc0e2dcbd30ac2febb8a1c6ef2b34cbde416aa33
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: loongson-loongson1-apb-dma: Remove redundant dev_err()/dev_err_probe()

This commit removes redundant dev_err() calls when devm_request_irq() fails in
ls1x_dma_alloc_chan_resources(). Since devm_request_irq() logs its own detailed
error messages, the driver-specific messages are no longer needed.

> diff --git a/drivers/dma/loongson/loongson1-apb-dma.c b/drivers/dma/loongson/loongson1-apb-dma.c
> index 89786cbd20ab5..812bd7c2068f9 100644
> --- a/drivers/dma/loongson/loongson1-apb-dma.c
> +++ b/drivers/dma/loongson/loongson1-apb-dma.c
> @@ -162,10 +162,8 @@ static int ls1x_dma_alloc_chan_resources(struct dma_chan *dchan)
>  
>  	ret = devm_request_irq(dev, chan->irq, ls1x_dma_irq_handler,
>  			       IRQF_SHARED, dma_chan_name(dchan), chan);
> -	if (ret) {
> -		dev_err(dev, "failed to request IRQ %d\n", chan->irq);
> +	if (ret)
>  		return ret;
> -	}
>  
>  	chan->lli_pool = dma_pool_create(dma_chan_name(dchan), dev,
>  					 sizeof(struct ls1x_dma_lli),
>  					 __alignof__(struct ls1x_dma_lli), 0);
>  	if (!chan->lli_pool)
>  		return -ENOMEM;
>  
>  	/* allocate memory for querying the current lli */
>  	dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
>  	chan->curr_lli = dma_alloc_coherent(dev, sizeof(struct ls1x_dma_lli),
>  					    &phys, GFP_KERNEL);
>  	if (!chan->curr_lli) {
>  		dma_pool_destroy(chan->lli_pool);
>  		return -ENOMEM;
>  	}

[Severity: High]
This is a pre-existing issue, but is the devm-managed IRQ leaked on this
error path?

If dma_pool_create() or dma_alloc_coherent() fails, we return -ENOMEM without
freeing the IRQ requested via devm_request_irq(). 

The devm-managed cleanup won't happen because the channel's sysfs device is not
destroyed, leading to local clients hitting -EBUSY on subsequent allocation
attempts.

[ ... ]

> static inline struct device *chan2dev(struct dma_chan *chan)
> {
> 	return &chan->dev->device;
> }

[Severity: High]
This isn't a bug introduced by this patch, but should this helper be returning
the hardware platform device instead?

This macro returns the logical sysfs channel device instead of the hardware
platform device (dchan->device->dev). Passing this logical device to DMA API
operations like dma_alloc_coherent() or dma_pool_create() can cause allocation
failures or incorrect mappings on platforms with strict DMA tracking or an
IOMMU.

[ ... ]

> static inline int ls1x_dma_start(struct ls1x_dma_chan *chan,
> 				 dma_addr_t *lli_phys)
> {
[ ... ]
> 	ret = readl_poll_timeout(chan->reg_base + LS1X_DMA_CTRL, val,
> 				 !(val & LS1X_DMA_START), 0, 1000);

[Severity: High]
This is a pre-existing issue, but does using readl_poll_timeout() here risk
hard lockups?

ls1x_dma_start() is called from ls1x_dma_resume() and ls1x_dma_issue_pending()
with the channel's lock held and interrupts disabled:

ls1x_dma_resume()
    guard(spinlock_irqsave)(&chan->vc.lock);
    return ls1x_dma_start(chan, &chan->curr_lli->phys);

Using the non-atomic readl_poll_timeout() macro in an atomic context is unsafe
because timekeeping might be suspended, causing the internal ktime_get() to
stall or return invalid values.

[ ... ]

> static struct dma_async_tx_descriptor *
> ls1x_dma_prep_dma_cyclic(struct dma_chan *dchan, dma_addr_t buf_addr,
[ ... ]
> 	desc = ls1x_dma_alloc_desc();
> 	if (!desc)
> 		return NULL;
> 
> 	/* allocate the scatterlist */
> 	sg_len = buf_len / period_len;
> 	sgl = kmalloc_objs(*sgl, sg_len, GFP_NOWAIT);
> 	if (!sgl)
> 		return NULL;

[Severity: High]
This isn't a bug introduced by this patch, but what happens to the allocated
descriptor if kmalloc_objs() fails?

If we fail to allocate the scatterlist, the function returns NULL without
freeing the desc allocated just above it, resulting in a memory leak.

[ ... ]

> static irqreturn_t ls1x_dma_irq_handler(int irq, void *data)
> {
[ ... ]
> 	scoped_guard(spinlock, &chan->vc.lock) {
> 		vd = vchan_next_desc(&chan->vc);
> 		if (!vd) {
> 			dev_warn(dev,
> 				 "IRQ %d with no active desc on channel %d\n",
> 				 irq, dchan->chan_id);
> 			return IRQ_NONE;
> 		}

[Severity: High]
This is a pre-existing issue, but is it safe for a shared IRQ handler to
assume ownership of the interrupt without checking hardware status?

The handler is registered with IRQF_SHARED, but it processes active descriptors
purely based on whether one is queued in software. If another device sharing
the IRQ line triggers an interrupt, this could prematurely complete the current
in-flight DMA transfer and cause data corruption.

[ ... ]

> static void ls1x_dma_chan_remove(struct ls1x_dma *dma)
> {
> 	int id;
> 
> 	for (id = 0; id < dma->nr_chans; id++) {
> 		struct ls1x_dma_chan *chan = &dma->chan[id];
> 
> 		if (chan->vc.chan.device == &dma->ddev) {
> 			list_del(&chan->vc.chan.device_node);
> 			tasklet_kill(&chan->vc.task);
> 		}
> 	}
> }

[Severity: High]
This isn't a bug introduced by this patch, but will deleting the channel from
the list here bypass the devres unregister mechanism?

During module removal or probe failure, manually calling list_del() orphans
the channel from the dmaengine core. When devres later runs
dma_async_device_unregister(), it iterates over an empty list and fails to
unregister the channels.

This permanently leaks the sysfs channel devices and percpu memory, and will
trigger a Use-After-Free when the underlying struct ls1x_dma is freed by
devm_kzalloc cleanup.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=3

[PATCH 04/26] dmaengine: mediatek-mtk-cqdma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/mediatek/mtk-cqdma.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
index 80791e30aec2..4dbe84954de6 100644
--- a/drivers/dma/mediatek/mtk-cqdma.c
+++ b/drivers/dma/mediatek/mtk-cqdma.c
@@ -828,11 +828,8 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
 		err = devm_request_irq(&pdev->dev, cqdma->pc[i]->irq,
 				       mtk_cqdma_irq, 0, dev_name(&pdev->dev),
 				       cqdma);
-		if (err) {
-			dev_err(&pdev->dev,
-				"request_irq failed with err %d\n", err);
+		if (err)
 			return -EINVAL;
-		}
 	}
 
 	/* allocate resource for VCs */
-- 
2.34.1

Re: [PATCH 04/26] dmaengine: mediatek-mtk-cqdma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

Pre-existing issues:
- [High] IRQ is requested and enabled before tasklets are initialized, leading to a crash.
- [High] The IRQ handler accesses unallocated channel structures, causing a NULL pointer dereference.
- [High] Device registers are accessed before clocks and power management are enabled, due to premature device registration.
- [High] Missing OF DMA controller deregistration on hardware initialization failure.
- [High] Missing cleanup for virtual channel tasklets on the probe error path leads to a potential Use-After-Free.
- [High] Teardown asymmetry in the device removal routine allows concurrent access to destroyed state.
--

--- Patch [4]: [PATCH 04/26] dmaengine: mediatek-mtk-cqdma: Remove redundant dev_err()/dev_err_probe() ---
commit 04c062c96653e2015152bd44faa0dd41edc7a398
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: mediatek-mtk-cqdma: Remove redundant dev_err()/dev_err_probe()

This commit removes redundant dev_err() calls when devm_request_irq() fails
in mtk_cqdma_probe(), relying on the core's built-in logging instead. It also
simplifies the error path by returning -EINVAL directly.

> diff --git a/drivers/dma/mediatek/mtk-cqdma.c b/drivers/dma/mediatek/mtk-cqdma.c
> index 80791e30aec24..4dbe84954de6b 100644
> --- a/drivers/dma/mediatek/mtk-cqdma.c
> +++ b/drivers/dma/mediatek/mtk-cqdma.c
> @@ -828,11 +828,8 @@ static int mtk_cqdma_probe(struct platform_device *pdev)
>  		err = devm_request_irq(&pdev->dev, cqdma->pc[i]->irq,
>  				       mtk_cqdma_irq, 0, dev_name(&pdev->dev),
>  				       cqdma);

[Severity: High]
This is a pre-existing issue, but is it safe to request and enable the IRQ
here before the required tasklet is initialized?

If a pending hardware interrupt fires before tasklet_setup() is called at
the end of the probe function, won't the interrupt handler mtk_cqdma_irq()
execute and call tasklet_schedule() on a zero-initialized tasklet structure?

[Severity: High]
This is also a pre-existing issue, but does the IRQ handler safely handle
unallocated channels?

Physical channels (cqdma->pc[i]) are allocated and their IRQs requested
sequentially in this loop. If an interrupt fires for an earlier channel
(e.g., i=0) before the loop allocates the remaining channels,
mtk_cqdma_irq() executes and blindly iterates over all channels
(0 to cqdma->dma_channels - 1) attempting to lock cqdma->pc[i]->lock.

Could this result in a NULL pointer dereference for the unallocated
channels?

> -		if (err) {
> -			dev_err(&pdev->dev,
> -				"request_irq failed with err %d\n", err);
> +		if (err)
>  			return -EINVAL;
> -		}
>  	}
>  
>  	/* allocate resource for VCs */

While reviewing this patch, I noticed a few other pre-existing issues in
this driver:

drivers/dma/mediatek/mtk-cqdma.c:mtk_cqdma_probe() {
    ...
    err = dma_async_device_register(dd);
    ...
    err = mtk_cqdma_hw_init(cqdma);
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but are device registers accessed
before clocks and power management are enabled?

dma_async_device_register() exposes the DMA engine to clients. If a client
immediately requests a channel, mtk_cqdma_alloc_chan_resources() is called,
which invokes mtk_cqdma_hard_reset() to write to device registers.

However, mtk_cqdma_hw_init(), which enables the device clocks and power
management, is called after registration.

Can the register writes occur with disabled clocks causing a synchronous
external abort?


drivers/dma/mediatek/mtk-cqdma.c:mtk_cqdma_probe() {
    ...
    err = of_dma_controller_register(pdev->dev.of_node,
                                     of_dma_xlate_by_chan_id, cqdma);
    ...
err_unregister:
    dma_async_device_unregister(dd);

    return err;
}

[Severity: High]
This isn't a bug introduced by this patch, but does the error path leak the
OF DMA controller registration?

If mtk_cqdma_hw_init() fails, the code jumps to err_unregister. The error
path calls dma_async_device_unregister() but fails to call
of_dma_controller_free(pdev->dev.of_node).

Could this leave the device registered in the OF DMA list with a freed
driver context?


drivers/dma/mediatek/mtk-cqdma.c:mtk_cqdma_probe() {
    ...
    for (i = 0; i < cqdma->dma_requests; i++) {
        vc = &cqdma->vc[i];
        vc->vc.desc_free = mtk_cqdma_vdesc_free;
        vchan_init(&vc->vc, dd);
        init_completion(&vc->issue_completion);
    }
    ...
err_unregister:
    dma_async_device_unregister(dd);

    return err;
}

[Severity: High]
This isn't a bug introduced by this patch, but is there missing cleanup for
virtual channel tasklets on the probe error path?

vchan_init() sets up virtual channel tasklets. If a subsequent
initialization step (like mtk_cqdma_hw_init) fails, the err_unregister path
does not call tasklet_kill() for these channels.

Because the device was temporarily exposed via dma_async_device_register(),
a client could have scheduled a transaction, leaving the tasklet active.

When devres frees the device memory, could the executing tasklet trigger a
use-after-free?


drivers/dma/mediatek/mtk-cqdma.c:mtk_cqdma_remove() {
    ...
    /* internal driver state torn down here */
    ...
    dma_async_device_unregister(&cqdma->ddev);
    of_dma_controller_free(pdev->dev.of_node);
}

[Severity: High]
This isn't a bug introduced by this patch, but does the teardown asymmetry
in the device removal routine allow concurrent access to destroyed state?

In mtk_cqdma_remove(), internal driver state (tasklets and hardware
clocks/interrupts) is torn down before the subsystem interfaces
(dma_async_device_unregister and of_dma_controller_free) are unregistered.

Does this teardown inversion allow clients to issue new DMA requests while
the driver is actively being destroyed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=4

[PATCH 05/26] dmaengine: mediatek-mtk-hsdma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/mediatek/mtk-hsdma.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/dma/mediatek/mtk-hsdma.c b/drivers/dma/mediatek/mtk-hsdma.c
index a43412ff5edd..6ee8911c12e0 100644
--- a/drivers/dma/mediatek/mtk-hsdma.c
+++ b/drivers/dma/mediatek/mtk-hsdma.c
@@ -988,11 +988,8 @@ static int mtk_hsdma_probe(struct platform_device *pdev)
 	err = devm_request_irq(&pdev->dev, hsdma->irq,
 			       mtk_hsdma_irq, 0,
 			       dev_name(&pdev->dev), hsdma);
-	if (err) {
-		dev_err(&pdev->dev,
-			"request_irq failed with err %d\n", err);
+	if (err)
 		goto err_free;
-	}
 
 	platform_set_drvdata(pdev, hsdma);
 
-- 
2.34.1

Re: [PATCH 05/26] dmaengine: mediatek-mtk-hsdma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The DMA controller is registered and exposed to clients before hardware initialization and IRQ setup, causing race conditions and Use-After-Free vulnerabilities on probe failure.
- [High] The return value of mtk_hsdma_hw_init() is ignored, masking clock initialization failures and leading to hardware lockups and leaked PM runtime references.
- [Medium] In mtk_hsdma_remove(), the DMA engine is unregistered after internal resources are disabled, creating a window where clients can access unclocked hardware.
--

--- Patch [5]: [PATCH 05/26] dmaengine: mediatek-mtk-hsdma: Remove redundant dev_err()/dev_err_probe() ---
commit 572ded362561924159b30085ea17ce96e1c47b86
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: mediatek-mtk-hsdma: Remove redundant dev_err()/dev_err_probe()

This commit removes redundant error messages after devm_request_irq() in
the MediaTek HSDMA driver. The core subsystem now handles logging detailed
error messages automatically on failure.

> diff --git a/drivers/dma/mediatek/mtk-hsdma.c b/drivers/dma/mediatek/mtk-hsdma.c
> index a43412ff5eddc..6ee8911c12e0f 100644
> --- a/drivers/dma/mediatek/mtk-hsdma.c
> +++ b/drivers/dma/mediatek/mtk-hsdma.c
> @@ -988,11 +988,8 @@ static int mtk_hsdma_probe(struct platform_device *pdev)
>  	err = devm_request_irq(&pdev->dev, hsdma->irq,
>  			       mtk_hsdma_irq, 0,
>  			       dev_name(&pdev->dev), hsdma);
> -	if (err) {
> -		dev_err(&pdev->dev,
> -			"request_irq failed with err %d\n", err);
> +	if (err)
>  		goto err_free;

[Severity: High]
This is a pre-existing issue, but is it safe to ignore the return value of
mtk_hsdma_hw_init() called just before devm_request_irq()?

If clk_prepare_enable() fails inside mtk_hsdma_hw_init(), it returns an
error but mtk_hsdma_probe() proceeds anyway. If devm_request_irq() then
fails here, the error path calls mtk_hsdma_hw_deinit(), which writes to a
DMA register with the clock disabled. Might this crash the system with a
bus fault?

[Severity: High]
This isn't a bug introduced by this patch, but does this error path leave
dangling pointers if it fails?

The DMA controller is registered globally via dma_async_device_register()
earlier in mtk_hsdma_probe(), making channels visible to clients. If a
client acquires a channel and then devm_request_irq() fails here, the probe
aborts and devres frees the mtk_hsdma_device structure.

Since dma_async_device_unregister() doesn't forcefully revoke channels held
by clients, could the client be left with a dangling pointer to freed
memory?

Looking at mtk_hsdma_remove() in the same file:

drivers/dma/mediatek/mtk-hsdma.c:mtk_hsdma_remove() {
        ...
        /* Disable hardware */
        mtk_hsdma_hw_deinit(hsdma);

        dma_async_device_unregister(&hsdma->ddev);
        ...
}

[Severity: Medium]
This is a pre-existing issue, but could there be a race condition during
device removal?

The hardware is disabled via mtk_hsdma_hw_deinit() before the DMA
controller is unregistered. If a client attempts to use a DMA channel
while the driver is being unbound, could it issue a request after clocks
are disabled but before the device is unregistered?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=5

[PATCH 06/26] dmaengine: mmp_pdma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/mmp_pdma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
index 386e85cd4882..c2a638d55488 100644
--- a/drivers/dma/mmp_pdma.c
+++ b/drivers/dma/mmp_pdma.c
@@ -1161,10 +1161,8 @@ static int mmp_pdma_chan_init(struct mmp_pdma_device *pdev, int idx, int irq)
 	if (irq) {
 		ret = devm_request_irq(pdev->dev, irq, mmp_pdma_chan_handler,
 				       IRQF_SHARED, "pdma", phy);
-		if (ret) {
-			dev_err(pdev->dev, "channel request irq fail!\n");
+		if (ret)
 			return ret;
-		}
 	}
 
 	spin_lock_init(&chan->desc_lock);
-- 
2.34.1

Re: [PATCH 06/26] dmaengine: mmp_pdma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential NULL pointer dereference in IRQ handlers (`mmp_pdma_chan_handler` / `mmp_pdma_int_handler`) when handling spurious or shared interrupts before initialization is complete or when the PHY is inactive.
- [High] Missing `tasklet_kill()` on device removal and probe error paths causes a use-after-free if the tasklet runs after `devres` frees the channel memory.
- [Medium] Negative IRQ error codes like `-EPROBE_DEFER` are incorrectly passed to `devm_request_irq()` and masked as `-EINVAL`.
--

--- Patch [6]: [PATCH 06/26] dmaengine: mmp_pdma: Remove redundant dev_err()/dev_err_probe() ---
commit f205944d00824c4ac830efbda0d25e06f33f54a9
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: mmp_pdma: Remove redundant dev_err()/dev_err_probe()

The devm_request_irq() function automatically logs detailed error messages on
failure. This commit eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

> diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c
> index 386e85cd4882a..c2a638d554883 100644
> --- a/drivers/dma/mmp_pdma.c
> +++ b/drivers/dma/mmp_pdma.c
> @@ -1161,10 +1161,8 @@ static int mmp_pdma_chan_init(struct mmp_pdma_device *pdev, int idx, int irq)
>  	if (irq) {

[Severity: Medium]
This is a pre-existing issue, but could irq be a negative error code here?

If platform_get_irq() returns -EPROBE_DEFER, it evaluates as true in this
condition. It would then be passed directly to devm_request_irq() below,
which implicitly casts it to an unsigned int and fails with -EINVAL, permanently
failing the probe instead of deferring it.

>  		ret = devm_request_irq(pdev->dev, irq, mmp_pdma_chan_handler,
>  				       IRQF_SHARED, "pdma", phy);

[Severity: High]
This isn't a bug introduced by this patch, but is there a risk of a NULL
pointer dereference with these shared IRQs?

In mmp_pdma_probe(), a shared interrupt is requested before the loop that
initializes phy->base in mmp_pdma_chan_init(). If a shared interrupt triggers
immediately, mmp_pdma_int_handler() calls mmp_pdma_chan_handler() and then
clear_chan_irq(), which dereferences phy->base:

drivers/dma/mmp_pdma.c:clear_chan_irq() {
    ...
    u32 dint = readl(phy->base + DINT);
    ...
}

Additionally, if a spurious interrupt fires while phy->vchan is NULL,
clear_chan_irq() can still return 0. This causes mmp_pdma_chan_handler() to
unconditionally execute tasklet_schedule(&phy->vchan->tasklet), which
dereferences NULL.

> -		if (ret) {
> -			dev_err(pdev->dev, "channel request irq fail!\n");
> +		if (ret)
>  			return ret;
> -		}
>  	}
>  
>  	spin_lock_init(&chan->desc_lock);

[Severity: High]
This is a pre-existing issue, but does this driver leave tasklets running
after the channel memory is freed?

A few lines below here in mmp_pdma_chan_init(), a tasklet is set up via
tasklet_setup(). However, mmp_pdma_remove() does not appear to call
tasklet_kill(). 

Since free_irq() synchronizes with running interrupt handlers but not with
scheduled tasklets, if a tasklet was scheduled right before the IRQ was freed,
it could execute after devres has freed the chan memory, causing a
use-after-free.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=6

[PATCH 07/26] dmaengine: moxart-dma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/moxart-dma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma/moxart-dma.c b/drivers/dma/moxart-dma.c
index 442f5aa16031..16a14a5179c1 100644
--- a/drivers/dma/moxart-dma.c
+++ b/drivers/dma/moxart-dma.c
@@ -600,10 +600,8 @@ static int moxart_probe(struct platform_device *pdev)
 
 	ret = devm_request_irq(dev, irq, moxart_dma_interrupt, 0,
 			       "moxart-dma-engine", mdc);
-	if (ret) {
-		dev_err(dev, "devm_request_irq failed\n");
+	if (ret)
 		return ret;
-	}
 	mdc->irq = irq;
 
 	ret = dma_async_device_register(&mdc->dma_slave);
-- 
2.34.1

Re: [PATCH 07/26] dmaengine: moxart-dma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but there is a missing NULL pointer check for `txstate` in `moxart_tx_status()`, which will cause a kernel panic if a caller passes a NULL state.
- [High] This problem wasn't introduced by this patch, but there is a Time-Of-Check to Time-Of-Use (TOCTOU) race condition in the interrupt handler `moxart_dma_interrupt()` accessing `ch->desc` outside of its spinlock, leading to a NULL pointer dereference.
- [High] This problem wasn't introduced by this patch, but there is a lifecycle mismatch where `dma_device` is allocated with `devm_kzalloc`, leading to a Use-After-Free if the driver is unbound while clients hold channel references.
--

--- Patch [7]: [PATCH 07/26] dmaengine: moxart-dma: Remove redundant dev_err()/dev_err_probe() ---
commit 9d354e585688446b04a5e03b938d46845d466dab
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: moxart-dma: Remove redundant dev_err()/dev_err_probe()

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

> diff --git a/drivers/dma/moxart-dma.c b/drivers/dma/moxart-dma.c
> index 442f5aa16031f..16a14a5179c17 100644
> --- a/drivers/dma/moxart-dma.c
> +++ b/drivers/dma/moxart-dma.c
> @@ -600,10 +600,8 @@ static int moxart_probe(struct platform_device *pdev)
>  
>  	ret = devm_request_irq(dev, irq, moxart_dma_interrupt, 0,
                                         ^^^^^^^^^^^^^^^^^^^^

[Severity: High]
This isn't a bug introduced by this patch, but there is a Time-Of-Check to
Time-Of-Use (TOCTOU) race condition in moxart_dma_interrupt() accessing
ch->desc outside of its spinlock:

drivers/dma/moxart-dma.c:moxart_dma_interrupt() {
...
		if (ctrl & APB_DMA_FIN_INT_STS) {
			ctrl &= ~APB_DMA_FIN_INT_STS;
			if (ch->desc) {
				spin_lock(&ch->vc.lock);
				if (++ch->sgidx < ch->desc->sglen) {
...
}

If a concurrent call to moxart_terminate_all() on another CPU acquires the
lock, frees ch->desc, sets it to NULL, and releases the lock between the if
check and the spin_lock, the interrupt handler will unconditionally dereference
ch->desc via ch->desc->sglen.

Does this race condition lead to a NULL pointer dereference?

>  			       "moxart-dma-engine", mdc);
> -	if (ret) {
> -		dev_err(dev, "devm_request_irq failed\n");
> +	if (ret)
>  		return ret;
> -	}
>  	mdc->irq = irq;
>  
>  	ret = dma_async_device_register(&mdc->dma_slave);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
This isn't a bug introduced by this patch, but there is a lifecycle mismatch
where dma_device is allocated with devm_kzalloc.

Earlier in moxart_probe():
	mdc = devm_kzalloc(dev, sizeof(*mdc), GFP_KERNEL);

And during unbind:
drivers/dma/moxart-dma.c:moxart_remove() {
...
	dma_async_device_unregister(&m->dma_slave);
...
}

The DMA engine subsystem independently refcounts the dma_device via kref_init().
If the driver is unbound via sysfs while DMA clients hold references to the
channels, moxart_remove() returns and devres frees the mdc structure memory
immediately.

When the clients later drop their references, does this trigger a
Use-After-Free on the freed memory?

While reviewing the driver, I also noticed an issue in moxart_tx_status():

drivers/dma/moxart-dma.c:moxart_tx_status() {
...
	if (vd) {
		d = to_moxart_dma_desc(&vd->tx);
		txstate->residue = moxart_dma_desc_size(d, 0);
...
	} else if (ch->desc && ch->desc->vd.tx.cookie == cookie) {
		txstate->residue = moxart_dma_desc_size_in_flight(ch);
...
}

[Severity: High]
This isn't a bug introduced by this patch, but there is a missing NULL pointer
check for txstate in moxart_tx_status().

The dmaengine API allows callers to pass a NULL state argument to
dmaengine_tx_status() if they only want to query the completion status and
do not need the residue. However, this function unconditionally dereferences
txstate.

Will this cause a kernel panic if a caller passes a NULL state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=7

[PATCH 08/26] dmaengine: owl-dma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/owl-dma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma/owl-dma.c b/drivers/dma/owl-dma.c
index 7c80572fc71d..71d0b15968c6 100644
--- a/drivers/dma/owl-dma.c
+++ b/drivers/dma/owl-dma.c
@@ -1163,10 +1163,8 @@ static int owl_dma_probe(struct platform_device *pdev)
 	od->irq = platform_get_irq(pdev, 0);
 	ret = devm_request_irq(&pdev->dev, od->irq, owl_dma_interrupt, 0,
 			       dev_name(&pdev->dev), od);
-	if (ret) {
-		dev_err(&pdev->dev, "unable to request IRQ\n");
+	if (ret)
 		return ret;
-	}
 
 	/* Init physical channel */
 	od->pchans = devm_kcalloc(&pdev->dev, od->nr_pchans,
-- 
2.34.1

Re: [PATCH 08/26] dmaengine: owl-dma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver directly passes the signed return value of `platform_get_irq()` to `devm_request_irq()` without checking for errors, causing negative error codes like `-EPROBE_DEFER` to be cast to large unsigned integers and replaced by `-EINVAL`, permanently breaking probe deferral.
- [High] The IRQ handler is registered before critical data structures (`od->pchans`) are allocated and before the hardware clock is enabled (`clk_prepare_enable`), risking a system crash or NULL pointer dereference.
- [High] The return value of `clk_prepare_enable()` is not checked. If the clock fails to enable, the driver silently registers the DMA controller, leading to a system crash when clients later attempt to use it.
- [High] During probe error paths, the clock is manually disabled while the devres-managed IRQ remains registered and active, creating a race condition that can cause a system crash.
--

--- Patch [8]: [PATCH 08/26] dmaengine: owl-dma: Remove redundant dev_err()/dev_err_probe() ---
commit 8f8dd83b44c39b0ec6f44933f79e28cff3053a61
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: owl-dma: Remove redundant dev_err()/dev_err_probe()

This commit removes driver-specific error logging around devm_request_irq()
since the core now automatically logs detailed error messages on failure.

> diff --git a/drivers/dma/owl-dma.c b/drivers/dma/owl-dma.c
> index 7c80572fc71d1..71d0b15968c6d 100644
> --- a/drivers/dma/owl-dma.c
> +++ b/drivers/dma/owl-dma.c
> @@ -1163,10 +1163,8 @@ static int owl_dma_probe(struct platform_device *pdev)
>  	od->irq = platform_get_irq(pdev, 0);
>  	ret = devm_request_irq(&pdev->dev, od->irq, owl_dma_interrupt, 0,
>  			       dev_name(&pdev->dev), od);

[Severity: High]
This is a pre-existing issue, but is it safe to pass the unchecked return
value of platform_get_irq() directly to devm_request_irq()?

If platform_get_irq() returns a negative error code like -EPROBE_DEFER,
it gets cast to a large unsigned integer. This causes devm_request_irq()
to fail with -EINVAL, which permanently breaks probe deferral.

[Severity: High]
This is a pre-existing issue, but does requesting the IRQ here risk a crash?

The IRQ is unmasked before od->pchans is allocated and before the hardware
clock is enabled. If a spurious interrupt fires immediately,
owl_dma_interrupt() will attempt to access hardware registers and dereference
the uninitialized od->pchans pointer:

owl_dma_interrupt() {
	pending = dma_readl(od, OWL_DMA_IRQ_PD0);
	...
	for_each_set_bit(i, &pending, od->nr_pchans) {
		pchan = &od->pchans[i];
		pchan_update(pchan, OWL_DMAX_INT_STATUS, 0xff, false);
	}
}

> -	if (ret) {
> -		dev_err(&pdev->dev, "unable to request IRQ\n");
> +	if (ret)
>  		return ret;
> -	}
>  
>  	/* Init physical channel */
>  	od->pchans = devm_kcalloc(&pdev->dev, od->nr_pchans,

[Severity: High]
This is a pre-existing issue, but further down in owl_dma_probe(), is it
intended that the return value of clk_prepare_enable() is unchecked?

owl_dma_probe() {
	...
	clk_prepare_enable(od->clk);

	ret = dma_async_device_register(&od->dma);
	...
}

If the clock fails to enable, the driver silently registers the DMA
controller, which can lead to a system crash when clients later attempt
to use it and access unclocked hardware.

[Severity: High]
This is a pre-existing issue, but does the error path in owl_dma_probe()
leave a window for a crash?

owl_dma_probe() {
	...
err_pool_free:
	clk_disable_unprepare(od->clk);
	dma_pool_destroy(od->lli_pool);

	return ret;
}

The clock is manually disabled here, but the devres-managed IRQ remains
registered and active until the probe function returns and triggers devres
cleanup. If an interrupt fires during this window, the handler will access
unclocked hardware registers.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=8

[PATCH 09/26] dmaengine: pxa_dma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/pxa_dma.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
index fa2ee0b3e09f..7bdcc5e6a3d8 100644
--- a/drivers/dma/pxa_dma.c
+++ b/drivers/dma/pxa_dma.c
@@ -1260,12 +1260,8 @@ static int pxad_init_phys(struct platform_device *op,
 			ret = devm_request_irq(&op->dev, irq0,
 					       pxad_int_handler,
 					       IRQF_SHARED, "pxa-dma", pdev);
-		if (ret) {
-			dev_err(pdev->slave.dev,
-				"%s(): can't request irq %d:%d\n", __func__,
-				irq, ret);
+		if (ret)
 			return ret;
-		}
 	}
 
 	return 0;
-- 
2.34.1

Re: [PATCH 09/26] dmaengine: pxa_dma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The shared interrupt handler panics on `BUG_ON(!chan)` for spurious or shared interrupts.
- [High] Tasklets are not explicitly destroyed if `of_dma_controller_register()` fails, leading to a potential use-after-free.
--

--- Patch [9]: [PATCH 09/26] dmaengine: pxa_dma: Remove redundant dev_err()/dev_err_probe() ---
commit f2ce223752ea303da12586b72cd70a589be5a08b
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: pxa_dma: Remove redundant dev_err()/dev_err_probe()

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

> diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c
> index fa2ee0b3e09f8..7bdcc5e6a3d8f 100644
> --- a/drivers/dma/pxa_dma.c
> +++ b/drivers/dma/pxa_dma.c
> @@ -1260,12 +1260,8 @@ static int pxad_init_phys(struct platform_device *op,
>  			ret = devm_request_irq(&op->dev, irq0,
>  					       pxad_int_handler,
>  					       IRQF_SHARED, "pxa-dma", pdev);
> -		if (ret) {
> -			dev_err(pdev->slave.dev,
> -				"%s(): can't request irq %d:%d\n", __func__,
> -				irq, ret);
> +		if (ret)
>  			return ret;

[Severity: High]
This isn't a bug introduced by this patch, but registering the IRQ handlers
with IRQF_SHARED exposes a potential kernel panic.

If another device sharing the IRQ line generates an interrupt, or if a
spurious interrupt occurs, the kernel calls pxad_chan_handler():

drivers/dma/pxa_dma.c:pxad_chan_handler() {
    struct pxad_phy *phy = dev_id;
    struct pxad_chan *chan = phy->vchan;
    ...
    BUG_ON(!chan);
    ...
}

If no DMA transfers are currently active, phy->vchan is NULL, leading to a
deterministic kernel panic. 

Should the shared interrupt handler return IRQ_NONE instead of panicking
when the device has not generated an interrupt?


[Severity: High]
This is another pre-existing issue, but returning directly on failure here
or in the probe function can cause a use-after-free.

During probe, the driver exposes DMA channels by calling
dmaenginem_async_device_register() inside pxad_init_dmadev(). If the
subsequent of_dma_controller_register() call fails, it returns directly:

drivers/dma/pxa_dma.c:pxad_probe() {
    ...
    ret = pxad_init_dmadev(op, pdev, dma_channels, nb_requestors);
    if (ret) { ... }

    if (op->dev.of_node) {
        ret = of_dma_controller_register(op->dev.of_node,
                                         pxad_dma_xlate, pdev);
        if (ret < 0) {
            dev_err(pdev->slave.dev,
                "of_dma_controller_register failed\n");
            return ret;
        }
    ...
}

Because the DMA engine was temporarily registered, a concurrent non-DT
client could have requested a channel and scheduled its tasklet. When devres
automatically frees the struct pxad_chan memory upon probe failure, the
scheduled tasklet remains on the system's tasklet list.

When the tasklet executes, it will access the freed channel structure. 

Does the error path need an explicit call to pxad_free_channels() to kill
the tasklets, similar to what is done in pxad_remove()?

> -		}
>  	}
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=9

[PATCH 10/26] dmaengine: qcom-gpi: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/qcom/gpi.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
index a5055a6273af..af824682e64c 100644
--- a/drivers/dma/qcom/gpi.c
+++ b/drivers/dma/qcom/gpi.c
@@ -617,11 +617,8 @@ static int gpi_config_interrupts(struct gpii *gpii, enum gpii_irq_settings setti
 		ret = devm_request_irq(gpii->gpi_dev->dev, gpii->irq,
 				       gpi_handle_irq, IRQF_TRIGGER_HIGH,
 				       "gpi-dma", gpii);
-		if (ret < 0) {
-			dev_err(gpii->gpi_dev->dev, "error request irq:%d ret:%d\n",
-				gpii->irq, ret);
+		if (ret < 0)
 			return ret;
-		}
 	}
 
 	if (settings == MASK_IEOB_SETTINGS) {
-- 
2.34.1

Re: [PATCH 10/26] dmaengine: qcom-gpi: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The error cleanup paths in `gpi_ch_init()` use the wrong channel pointer when stopping and deallocating channels, leading to a leaked hardware state and redundant commands sent to the wrong channel.
- [High] The transfer ring memory (`gchan->ch_ring`) is permanently leaked if `gpi_ch_init()` fails during channel allocation in `gpi_alloc_chan_resources()`.
--

--- Patch [10]: [PATCH 10/26] dmaengine: qcom-gpi: Remove redundant dev_err()/dev_err_probe() ---
commit 2195d06ab56c88788ead88f1489a78636aee6d8b
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: qcom-gpi: Remove redundant dev_err()/dev_err_probe()

This patch removes driver-specific error logging when devm_request_irq()
fails. The core already logs detailed error messages automatically, making
these manual prints redundant.

> diff --git a/drivers/dma/qcom/gpi.c b/drivers/dma/qcom/gpi.c
> index a5055a6273af6..af824682e64c8 100644
> --- a/drivers/dma/qcom/gpi.c
> +++ b/drivers/dma/qcom/gpi.c
[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in the error
cleanup path of gpi_ch_init() while reviewing this file:

drivers/dma/qcom/gpi.c:gpi_ch_init() {
    ...
error_start_chan:
	for (i = i - 1; i >= 0; i--) {
		gpi_stop_chan(&gpii->gchan[i]);
		gpi_send_cmd(gpii, gchan, GPI_CH_CMD_RESET);
	}
	i = 2;
error_alloc_chan:
	for (i = i - 1; i >= 0; i--)
		gpi_reset_chan(gchan, GPI_CH_CMD_DE_ALLOC);
    ...
}

In both loops, the code calls gpi_send_cmd() and gpi_reset_chan() using the
fixed function argument gchan instead of the loop variable &gpii->gchan[i].

Does this leave the successfully initialized channels in an incorrect hardware
state and send duplicate reset and deallocate commands to the same channel?

[Severity: High]
This is also a pre-existing issue, but there appears to be a missing cleanup
step in the channel allocation path:

drivers/dma/qcom/gpi.c:gpi_alloc_chan_resources() {
    ...
	/* allocate memory for transfer ring */
	ret = gpi_alloc_ring(&gchan->ch_ring, CHAN_TRES,
			     sizeof(struct gpi_tre), gpii);
	if (ret)
		goto xfer_alloc_err;

	ret = gpi_ch_init(gchan);

	mutex_unlock(&gpii->ctrl_lock);

	return ret;
    ...
}

If gpi_ch_init() returns an error, the function returns directly without calling
gpi_free_ring() to free the newly allocated gchan->ch_ring.

Since the dmaengine core aborts the allocation on failure and will not invoke
device_free_chan_resources(), does this permanently leak the DMA coherent
memory allocated for the transfer ring?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=10

[PATCH 11/26] dmaengine: sf-pdma-sf-pdma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/sf-pdma/sf-pdma.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
index 6f79cc28703e..5e5694750c57 100644
--- a/drivers/dma/sf-pdma/sf-pdma.c
+++ b/drivers/dma/sf-pdma/sf-pdma.c
@@ -411,10 +411,8 @@ static int sf_pdma_irq_init(struct platform_device *pdev, struct sf_pdma *pdma)
 
 		r = devm_request_irq(&pdev->dev, irq, sf_pdma_done_isr, 0,
 				     dev_name(&pdev->dev), (void *)chan);
-		if (r) {
-			dev_err(&pdev->dev, "Fail to attach done ISR: %d\n", r);
+		if (r)
 			return -EINVAL;
-		}
 
 		chan->txirq = irq;
 
@@ -424,10 +422,8 @@ static int sf_pdma_irq_init(struct platform_device *pdev, struct sf_pdma *pdma)
 
 		r = devm_request_irq(&pdev->dev, irq, sf_pdma_err_isr, 0,
 				     dev_name(&pdev->dev), (void *)chan);
-		if (r) {
-			dev_err(&pdev->dev, "Fail to attach err ISR: %d\n", r);
+		if (r)
 			return -EINVAL;
-		}
 
 		chan->errirq = irq;
 	}
-- 
2.34.1

Re: [PATCH 11/26] dmaengine: sf-pdma-sf-pdma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver masks standard error codes with -EINVAL, permanently breaking probe deferral.
- [High] Interrupt handlers are registered before channel structures are initialized, causing an immediate kernel panic on spurious interrupts.
- [High] Tasklets are not killed on probe error paths, leading to use-after-free list corruption.
- [High] Tasklets access `chan->desc` without checking for NULL, leading to crashes when racing with channel termination.
--

--- Patch [11]: [PATCH 11/26] dmaengine: sf-pdma-sf-pdma: Remove redundant dev_err()/dev_err_probe() ---
commit 09662195d630e22b4643f58a4176a95df1d14255
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: sf-pdma-sf-pdma: Remove redundant dev_err()/dev_err_probe()

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

> diff --git a/drivers/dma/sf-pdma/sf-pdma.c b/drivers/dma/sf-pdma/sf-pdma.c
> index b3cba11b62032..93611e335a075 100644
> --- a/drivers/dma/sf-pdma/sf-pdma.c
> +++ b/drivers/dma/sf-pdma/sf-pdma.c
> @@ -412,10 +412,8 @@ static int sf_pdma_irq_init(struct platform_device *pdev, struct sf_pdma *pdma)
>  
>  		r = devm_request_irq(&pdev->dev, irq, sf_pdma_done_isr, 0,
>  				     dev_name(&pdev->dev), (void *)chan);
> -		if (r) {
> -			dev_err(&pdev->dev, "Fail to attach done ISR: %d\n", r);
> +		if (r)
>  			return -EINVAL;

[Severity: High]
This is a pre-existing issue, but does this mask standard error codes?

If platform_get_irq() or devm_request_irq() returns -EPROBE_DEFER, returning
-EINVAL will cause the driver core to treat the probe as a permanent failure
instead of deferring it. Should the original error code be propagated instead?

>  
>  		chan->txirq = irq;
>  
> @@ -425,10 +423,8 @@ static int sf_pdma_irq_init(struct platform_device *pdev, struct sf_pdma *pdma)
>  
>  		r = devm_request_irq(&pdev->dev, irq, sf_pdma_err_isr, 0,
>  				     dev_name(&pdev->dev), (void *)chan);
> -		if (r) {
> -			dev_err(&pdev->dev, "Fail to attach err ISR: %d\n", r);
> +		if (r)
>  			return -EINVAL;

[Severity: High]
This isn't a bug introduced by this patch, but does this also drop the
actual error code returned by devm_request_irq()?

Propagating the original error code 'r' would ensure probe deferrals and
other specific failure reasons are handled correctly by the driver core.

[ ... ]

[Severity: High]
This is a pre-existing issue, but can spurious interrupts cause a panic
during probe?

In drivers/dma/sf-pdma/sf-pdma.c, sf_pdma_probe() calls sf_pdma_irq_init()
before sf_pdma_setup_chans():

sf_pdma_probe() {
    ...
    ret = sf_pdma_irq_init(pdev, pdma);
    if (ret)
        return ret;

    sf_pdma_setup_chans(pdma);
    ...
}

If an interrupt fires immediately after devm_request_irq(),
sf_pdma_done_isr() will read from the uninitialized chan->regs.ctrl:

sf_pdma_done_isr() {
    ...
    writel((readl(regs->ctrl)) & ~PDMA_DONE_STATUS_MASK, regs->ctrl);
    ...
}

Would it be safer to call sf_pdma_setup_chans() before requesting interrupts?

[Severity: High]
This is a pre-existing issue, but do we leave tasklets running if the probe
fails later on?

If an interrupt fires during probe and schedules done_tasklet, and a
subsequent step like dma_async_device_register() fails:

sf_pdma_probe() {
    ...
err_unregister:
    dma_async_device_unregister(&pdma->dma_dev);

    return ret;
}

The devres framework will free the device memory and IRQs, but
devm_free_irq() does not kill pending tasklets. Could the scheduled tasklet
execute later using freed memory?

[Severity: High]
This isn't a bug introduced by this patch, but is there a potential NULL
pointer dereference if tasklets race with channel termination?

sf_pdma_terminate_all() frees chan->desc and sets it to NULL without
synchronizing with the tasklets:

sf_pdma_terminate_all() {
    ...
    kfree(chan->desc);
    chan->desc = NULL;
    ...
}

If done_tasklet runs immediately after, it acquires the lock and blindly
dereferences chan->desc:

sf_pdma_donebh_tasklet() {
    ...
    spin_lock_irqsave(&chan->vchan.lock, flags);
    list_del(&chan->desc->vdesc.node);
    ...
}

Should chan->desc be checked for NULL, or should the tasklets be killed
during termination?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=11

[PATCH 12/26] dmaengine: sh-rcar-dmac: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_threaded_irq() now automatically logs detailed error
messages on failure. This eliminates the need for driver-specific
dev_err() and dev_err_probe() calls that previously printed generic
messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/sh/rcar-dmac.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
index 44eab2d21d54..4cd7f0189bc7 100644
--- a/drivers/dma/sh/rcar-dmac.c
+++ b/drivers/dma/sh/rcar-dmac.c
@@ -1793,11 +1793,8 @@ static int rcar_dmac_chan_probe(struct rcar_dmac *dmac,
 					rcar_dmac_isr_channel,
 					rcar_dmac_isr_channel_thread, 0,
 					irqname, rchan);
-	if (ret) {
-		dev_err(dmac->dev, "failed to request IRQ %u (%d)\n",
-			rchan->irq, ret);
+	if (ret)
 		return ret;
-	}
 
 	return 0;
 }
-- 
2.34.1

Re: [PATCH 12/26] dmaengine: sh-rcar-dmac: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] System crash due to unclocked register access on spurious interrupt
--

--- Patch [12]: [PATCH 12/26] dmaengine: sh-rcar-dmac: Remove redundant dev_err()/dev_err_probe() ---
commit 910ca9859b95c7ea2133b72448deeefa62bebf2b
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: sh-rcar-dmac: Remove redundant dev_err()/dev_err_probe()

This commit removes driver-specific error logging when requesting the
channel interrupt. The logging is removed because
devm_request_threaded_irq() now automatically logs detailed error messages
on failure.

> diff --git a/drivers/dma/sh/rcar-dmac.c b/drivers/dma/sh/rcar-dmac.c
> index 44eab2d21d54f..4cd7f0189bc7b 100644
> --- a/drivers/dma/sh/rcar-dmac.c
> +++ b/drivers/dma/sh/rcar-dmac.c
> @@ -1793,11 +1793,8 @@ static int rcar_dmac_chan_probe(struct rcar_dmac *dmac,
>  					rcar_dmac_isr_channel,
>  					rcar_dmac_isr_channel_thread, 0,
>  					irqname, rchan);

[Severity: High]
This is a pre-existing issue, but is it safe to leave the interrupt active
when the device enters runtime suspend?

Since the IRQ is managed by devm_request_threaded_irq(), it remains
unmasked even when the power domain gates the clock (for example, after
rcar_dmac_probe() calls pm_runtime_put()).

If a spurious interrupt fires while the clock is gated, the hardirq
handler unconditionally accesses a device register:

rcar_dmac_isr_channel() {
        ...
        chcr = rcar_dmac_chan_read(chan, RCAR_DMACHCR);
        ...
}

Could this unclocked register read cause a synchronous external abort and
subsequent kernel panic?

A similar window appears to exist in rcar_dmac_remove(). It calls
pm_runtime_disable(), which leaves the clock disabled if the device was
already suspended, before devres unregisters the IRQ. Can a spurious
interrupt during this teardown phase also trigger the same panic?

> -	if (ret) {
> -		dev_err(dmac->dev, "failed to request IRQ %u (%d)\n",
> -			rchan->irq, ret);
> +	if (ret)
>  		return ret;
> -	}
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=12

Re: [PATCH 12/26] dmaengine: sh-rcar-dmac: Remove redundant dev_err()/dev_err_probe()

[Geert Uytterhoeven]

On Thu, 9 Jul 2026 at 15:59, Pan Chuang <panchuang@vivo.com> wrote:
> The devm_request_threaded_irq() now automatically logs detailed error
> messages on failure. This eliminates the need for driver-specific
> dev_err() and dev_err_probe() calls that previously printed generic
> messages.
>
> Signed-off-by: Pan Chuang <panchuang@vivo.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 13/26] dmaengine: sh-rz-dmac: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_threaded_irq() and devm_request_irq now automatically
logs detailed error messages on failure. This eliminates the need for
driver-specific dev_err() and dev_err_probe() calls that previously
printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/sh/rz-dmac.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index ca76f1bb45c4..b5ecab072be2 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -1335,9 +1335,6 @@ static int rz_dmac_chan_probe(struct rz_dmac *dmac,
 	ret = devm_request_threaded_irq(dmac->dev, irq, rz_dmac_irq_handler,
 					rz_dmac_irq_handler_thread, 0,
 					irqname, channel);
-	if (ret)
-		dev_err(dmac->dev, "failed to request IRQ %u (%d)\n", irq, ret);
-
 	return ret;
 }
 
@@ -1470,11 +1467,8 @@ static int rz_dmac_probe(struct platform_device *pdev)
 	if (irq > 0) {
 		ret = devm_request_irq(&pdev->dev, irq, rz_dmac_irq_handler, 0,
 				       irqname, NULL);
-		if (ret) {
-			dev_err(&pdev->dev, "failed to request IRQ %u (%d)\n",
-				irq, ret);
+		if (ret)
 			goto err;
-		}
 	}
 
 	/* Register the DMAC as a DMA provider for DT. */
-- 
2.34.1

Re: [PATCH 13/26] dmaengine: sh-rz-dmac: Remove redundant dev_err()/dev_err_probe()

[Geert Uytterhoeven]

On Thu, 9 Jul 2026 at 15:59, Pan Chuang <panchuang@vivo.com> wrote:
> The devm_request_threaded_irq() and devm_request_irq now automatically
> logs detailed error messages on failure. This eliminates the need for
> driver-specific dev_err() and dev_err_probe() calls that previously
> printed generic messages.
>
> Signed-off-by: Pan Chuang <panchuang@vivo.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 14/26] dmaengine: sh-shdmac: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/sh/shdmac.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/dma/sh/shdmac.c b/drivers/dma/sh/shdmac.c
index 603e15102e45..e0391f72384d 100644
--- a/drivers/dma/sh/shdmac.c
+++ b/drivers/dma/sh/shdmac.c
@@ -800,12 +800,8 @@ static int sh_dmae_probe(struct platform_device *pdev)
 
 		err = devm_request_irq(&pdev->dev, errirq, sh_dmae_err,
 				       irqflags, "DMAC Address Error", shdev);
-		if (err) {
-			dev_err(&pdev->dev,
-				"DMA failed requesting irq #%d, error %d\n",
-				errirq, err);
+		if (err)
 			goto eirq_err;
-		}
 	} else {
 		chanirq_res = errirq_res;
 	}
-- 
2.34.1

Re: [PATCH 14/26] dmaengine: sh-shdmac: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Possible use-after-free of `sdev->schan` in `sh_dmae_err` during device probe failure or driver removal.
- [High] Unsafe hardware register access in a shared interrupt handler while the device is runtime suspended.
- [High] Failure of pm_runtime_get_sync() is logged but execution continues, potentially causing hardware faults.
--

--- Patch [14]: [PATCH 14/26] dmaengine: sh-shdmac: Remove redundant dev_err()/dev_err_probe() ---
commit c205bd1feae8c44a14e3a6ace262ed40dc6c1ba6
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: sh-shdmac: Remove redundant dev_err()/dev_err_probe()

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

> diff --git a/drivers/dma/sh/shdmac.c b/drivers/dma/sh/shdmac.c
> index 603e15102e45e..e0391f72384d3 100644
> --- a/drivers/dma/sh/shdmac.c
> +++ b/drivers/dma/sh/shdmac.c
> @@ -800,12 +800,8 @@ static int sh_dmae_probe(struct platform_device *pdev)
>  
>  		err = devm_request_irq(&pdev->dev, errirq, sh_dmae_err,
>  				       irqflags, "DMAC Address Error", shdev);

[Severity: High]
This is a pre-existing issue, but since we are requesting a potentially
shared interrupt here, is there a risk of accessing memory-mapped registers
of an unclocked or powered-down device?

If the device is runtime suspended and a shared interrupt fires from another
device, the handlers sh_dmae_err() and sh_dmae_chan_irq() unconditionally
read registers like DMAOR and CHCR.

Could this result in a synchronous external abort or system hang if the
module clock is disabled?

> -		if (err) {
> -			dev_err(&pdev->dev,
> -				"DMA failed requesting irq #%d, error %d\n",
> -				errirq, err);
> +		if (err)
>  			goto eirq_err;

[Severity: High]
This isn't a bug introduced by this patch, but does the error handling path
have a use-after-free risk?

If a subsequent probe step fails and we take the eirq_err path, it calls
shdma_cleanup() which frees the sdev->schan array. However, the devm-managed
interrupt requested above is not freed until the probe function returns and
devres cleanup runs.

If the shared interrupt fires during this window, sh_dmae_err() executes. If
it reads a value with DMAOR_AE set, it calls sh_dmae_reset() -> shdma_reset()
which iterates over the freed sdev->schan array.

The same vulnerability appears to exist in sh_dmae_remove() where
shdma_cleanup() is called before the devm-managed interrupt is freed. Might
we need to explicitly free or disable the interrupt before cleaning up?

[Severity: High]
This is also a pre-existing issue, but I noticed earlier in sh_dmae_probe()
that we log a pm_runtime_get_sync() failure but continue execution:

drivers/dma/sh/shdmac.c:sh_dmae_probe() {
    ...
	pm_runtime_enable(&pdev->dev);
	err = pm_runtime_get_sync(&pdev->dev);
	if (err < 0)
		dev_err(&pdev->dev, "%s(): GET = %d\n", __func__, err);

	spin_lock_irq(&sh_dmae_lock);
	list_add_tail_rcu(&shdev->node, &sh_dmae_devices);
	spin_unlock_irq(&sh_dmae_lock);

	/* reset dma controller - only needed as a test */
	err = sh_dmae_rst(shdev);
    ...
}

Will this cause a fatal bus error? If the PM domain failed to power on the
device, accessing the memory-mapped registers in sh_dmae_rst() could cause a
crash. Should the probe process be aborted if pm_runtime_get_sync() fails?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=14

[PATCH 15/26] dmaengine: sh-usb-dmac: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/sh/usb-dmac.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/dma/sh/usb-dmac.c b/drivers/dma/sh/usb-dmac.c
index 16509be0d360..17b385d85793 100644
--- a/drivers/dma/sh/usb-dmac.c
+++ b/drivers/dma/sh/usb-dmac.c
@@ -726,11 +726,8 @@ static int usb_dmac_chan_probe(struct usb_dmac *dmac,
 
 	ret = devm_request_irq(dmac->dev, uchan->irq, usb_dmac_isr_channel,
 			       IRQF_SHARED, irqname, uchan);
-	if (ret) {
-		dev_err(dmac->dev, "failed to request IRQ %u (%d)\n",
-			uchan->irq, ret);
+	if (ret)
 		return ret;
-	}
 
 	uchan->vc.desc_free = usb_dmac_virt_desc_free;
 	vchan_init(&uchan->vc, &dmac->engine);
-- 
2.34.1

Re: [PATCH 15/26] dmaengine: sh-usb-dmac: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver unconditionally replaces the error code from `platform_get_irq_byname` with `-ENODEV`, breaking probe deferral.
- [High] Shared interrupt is requested before `vchan_init` initializes the channel's spinlock and lists.
- [High] Missing IRQ cleanup in the probe error path leaves shared interrupts registered while device clocks are disabled.
- [High] The driver fails to cancel the `vchan` tasklets during channel teardown and device removal, which can lead to a Use-After-Free if a tasklet executes after the device memory is freed.
- [High] If `pm_runtime_get_sync()` fails in `usb_dmac_alloc_chan_resources()`, the newly allocated DMA descriptors and the PM runtime usage counter are permanently leaked.
--

--- Patch [15]: [PATCH 15/26] dmaengine: sh-usb-dmac: Remove redundant dev_err()/dev_err_probe() ---
commit 26b5ad26e29c18063b36a1a20ac48c474a74b7b1
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: sh-usb-dmac: Remove redundant dev_err()/dev_err_probe()

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

> diff --git a/drivers/dma/sh/usb-dmac.c b/drivers/dma/sh/usb-dmac.c
> index 16509be0d3608..17b385d857930 100644
> --- a/drivers/dma/sh/usb-dmac.c
> +++ b/drivers/dma/sh/usb-dmac.c
> @@ -726,11 +726,8 @@ static int usb_dmac_chan_probe(struct usb_dmac *dmac,
>  
>  	ret = devm_request_irq(dmac->dev, uchan->irq, usb_dmac_isr_channel,
>  			       IRQF_SHARED, irqname, uchan);

[Severity: High]
This is a pre-existing issue, but is the shared interrupt requested too
early?

devm_request_irq() is called with IRQF_SHARED before vchan_init()
initializes the channel's spinlock and lists. If another device sharing
the IRQ line fires an interrupt immediately, the registered ISR
usb_dmac_isr_channel() will execute and unconditionally acquire the
uninitialized chan->vc.lock:

drivers/dma/sh/usb-dmac.c:usb_dmac_isr_channel() {
    ...
    spin_lock(&chan->vc.lock);
    ...
}

Could this lead to a lockdep splat or kernel panic on platforms where this
IRQ line is shared with another active device?

> -	if (ret) {
> -		dev_err(dmac->dev, "failed to request IRQ %u (%d)\n",
> -			uchan->irq, ret);
> +	if (ret)
>  		return ret;
> -	}
>  
>  	uchan->vc.desc_free = usb_dmac_virt_desc_free;
>  	vchan_init(&uchan->vc, &dmac->engine);

[Severity: High]
This is a pre-existing issue, but does the unconditional return of
-ENODEV earlier in usb_dmac_chan_probe() break probe deferral?

drivers/dma/sh/usb-dmac.c:usb_dmac_chan_probe() {
    ...
    uchan->irq = platform_get_irq_byname(pdev, pdev_irqname);
    if (uchan->irq < 0)
        return -ENODEV;
    ...
}

If the interrupt controller is probed after the DMA controller,
platform_get_irq_byname() would return -EPROBE_DEFER, which gets
incorrectly replaced with -ENODEV. Will this cause the driver to
permanently fail to probe?


[Severity: High]
This is a pre-existing issue, but does the probe error path leave shared
interrupts active while device clocks are disabled?

drivers/dma/sh/usb-dmac.c:usb_dmac_probe() {
    ...
error:
    of_dma_controller_free(pdev->dev.of_node);
error_pm:
    pm_runtime_put(&pdev->dev);
    pm_runtime_disable(&pdev->dev);
    return ret;
}

If dma_async_device_register() fails, the error path disables PM runtime.
However, the shared IRQs for successfully probed channels remain active
until devres cleans them up after the function returns. If a shared
interrupt fires in this window, could the ISR access device registers
with stopped clocks and cause an imprecise external abort?


[Severity: High]
This is a pre-existing issue, but does the channel teardown logic fail to
cancel the vchan tasklets?

drivers/dma/sh/usb-dmac.c:usb_dmac_chan_remove() {
    usb_dmac_chan_halt(uchan);
    devm_free_irq(dmac->dev, uchan->irq, uchan);
}

If the driver is unbound or fails during probe while a vchan tasklet is
scheduled, devres will free the DMA controller memory containing the
tasklet structure. If the tasklet subsequently executes, will it access
freed memory? Should tasklet_kill(&uchan->vc.task) be called here?


[Severity: High]
This is a pre-existing issue, but does this leak resources if
pm_runtime_get_sync() fails?

drivers/dma/sh/usb-dmac.c:usb_dmac_alloc_chan_resources() {
    ...
    return pm_runtime_get_sync(chan->device->dev);
}

If it returns a negative error, it leaves the PM runtime usage counter
incremented and the previously allocated DMA descriptors are not freed.
The dmaengine core abandons the channel without calling
device_free_chan_resources(), which appears to result in a permanent leak
preventing the device from entering runtime suspend.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=15

Re: [PATCH 15/26] dmaengine: sh-usb-dmac: Remove redundant dev_err()/dev_err_probe()

[Geert Uytterhoeven]

On Thu, 9 Jul 2026 at 15:59, Pan Chuang <panchuang@vivo.com> wrote:
> The devm_request_irq() now automatically logs detailed error messages on
> failure. This eliminates the need for driver-specific dev_err() and
> dev_err_probe() calls that previously printed generic messages.
>
> Signed-off-by: Pan Chuang <panchuang@vivo.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH 16/26] dmaengine: sprd-dma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/sprd-dma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
index 087fea3af2e4..dbf52ff88b47 100644
--- a/drivers/dma/sprd-dma.c
+++ b/drivers/dma/sprd-dma.c
@@ -1162,10 +1162,8 @@ static int sprd_dma_probe(struct platform_device *pdev)
 	if (sdev->irq > 0) {
 		ret = devm_request_irq(&pdev->dev, sdev->irq, dma_irq_handle,
 				       0, "sprd_dma", (void *)sdev);
-		if (ret < 0) {
-			dev_err(&pdev->dev, "request dma irq failed\n");
+		if (ret < 0)
 			return ret;
-		}
 	} else {
 		dev_warn(&pdev->dev, "no interrupts for the dma controller\n");
 	}
-- 
2.34.1

Re: [PATCH 16/26] dmaengine: sprd-dma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Incorrect Optional Resource Handling / Ignored Probe Deferral
- [High] Premature IRQ Request Causes Potential NULL Pointer Dereference
- [High] Missing Tasklet Cleanup on Probe Failure Leads to Use-After-Free
--

--- Patch [16]: [PATCH 16/26] dmaengine: sprd-dma: Remove redundant dev_err()/dev_err_probe() ---
commit e390b8344f4bfbd854daa3d432e12e9099d1828c
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: sprd-dma: Remove redundant dev_err()/dev_err_probe()

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

> diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
> index 087fea3af2e41..dbf52ff88b47e 100644
> --- a/drivers/dma/sprd-dma.c
> +++ b/drivers/dma/sprd-dma.c
> @@ -1162,10 +1162,8 @@ static int sprd_dma_probe(struct platform_device *pdev)
>  	if (sdev->irq > 0) {

[Severity: High]
This is a pre-existing issue, but does this correctly handle probe deferral?

If platform_get_irq() returns -EPROBE_DEFER, the check if (sdev->irq > 0) 
ignores it and the driver will start without its IRQ. 

A similar issue exists earlier in this function with the ashb_eb clock:

	sdev->ashb_clk = devm_clk_get(&pdev->dev, "ashb_eb");
	if (IS_ERR(sdev->ashb_clk))
		dev_warn(&pdev->dev, "no optional ashb eb clock\n");

If devm_clk_get() returns -EPROBE_DEFER, the driver merely prints a warning
and continues. If the hardware actually requires this clock, accessing the 
DMA registers unclocked could cause a bus error and freeze the system.

Should this code use devm_clk_get_optional() and platform_get_irq_optional(),
and explicitly propagate errors like -EPROBE_DEFER?

>  		ret = devm_request_irq(&pdev->dev, sdev->irq, dma_irq_handle,
>  				       0, "sprd_dma", (void *)sdev);

[Severity: High]
This is a pre-existing issue, but can this early IRQ request cause a NULL
pointer dereference?

The interrupt is requested and enabled here, but sdev->glb_base is mapped 
afterwards:

	sdev->glb_base = devm_platform_ioremap_resource(pdev, 0);

If the IRQ line is already active (e.g., from the bootloader or kexec),
dma_irq_handle() will trigger immediately and try to read from glb_base:

	struct sprd_dma_dev *sdev = (struct sprd_dma_dev *)dev_id;
	u32 irq_status = readl(sdev->glb_base + SPRD_DMA_GLB_INT_MSK_STS);

Since sdev->glb_base hasn't been mapped yet, this will dereference a NULL
pointer. Should the IRQ request be deferred until all hardware mappings and
internal channel structures are fully initialized?

> -		if (ret < 0) {
> -			dev_err(&pdev->dev, "request dma irq failed\n");
> +		if (ret < 0)
>  			return ret;
> -		}
>  	} else {
>  		dev_warn(&pdev->dev, "no interrupts for the dma controller\n");
>  	}

[Severity: High]
This is a pre-existing issue, but does the probe error path leak scheduled
tasklets leading to a use-after-free?

During vchan_init(), tasklets are set up for each channel. If a late probe 
step fails (like of_dma_controller_register()), the error path handles
cleanup:

err_of_register:
	dma_async_device_unregister(&sdev->dma_dev);
err_register:
	pm_runtime_put_noidle(&pdev->dev);
	pm_runtime_disable(&pdev->dev);
err_rpm:
	sprd_dma_disable(sdev);
	return ret;

However, this doesn't call tasklet_kill() for the channels. If an interrupt 
fired or a channel was used, the tasklet remains scheduled. When devres 
subsequently frees the sdev memory, the scheduled tasklet will execute and
access freed memory, causing a panic. 

Should the error rollback path manually call tasklet_kill() for all 
initialized channels?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=16

[PATCH 17/26] dmaengine: st_fdma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/st_fdma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma/st_fdma.c b/drivers/dma/st_fdma.c
index d9547017f3bd..05a2642601e6 100644
--- a/drivers/dma/st_fdma.c
+++ b/drivers/dma/st_fdma.c
@@ -771,10 +771,8 @@ static int st_fdma_probe(struct platform_device *pdev)
 
 	ret = devm_request_irq(&pdev->dev, fdev->irq, st_fdma_irq_handler, 0,
 			       dev_name(&pdev->dev), fdev);
-	if (ret) {
-		dev_err(&pdev->dev, "Failed to request irq (%d)\n", ret);
+	if (ret)
 		goto err;
-	}
 
 	fdev->slim_rproc = st_slim_rproc_alloc(pdev, fdev->fw_name);
 	if (IS_ERR(fdev->slim_rproc)) {
-- 
2.34.1

Re: [PATCH 17/26] dmaengine: st_fdma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The IRQ is requested before dependent resources are initialized, and the probe error path destroys resources before the devm-managed IRQ is disabled, risking NULL pointer dereferences, uninitialized spinlock access, and Use-After-Free.
- [High] Mismatch between devm-managed DMA registration and manual resource teardown causes sysfs leaks and use-after-free risks in both probe error and remove paths.
--

--- Patch [17]: [PATCH 17/26] dmaengine: st_fdma: Remove redundant dev_err()/dev_err_probe() ---
commit 15a299566b3b9735b597a7ab918fd3926002aefe
Author: Pan Chuang <panchuang@vivo.com>
dmaengine: st_fdma: Remove redundant dev_err()/dev_err_probe()

This commit removes driver-specific error logging for devm_request_irq()
failures in st_fdma_probe(), as the core now automatically logs detailed
error messages on failure.

> diff --git a/drivers/dma/st_fdma.c b/drivers/dma/st_fdma.c
> index d9547017f3bd6..05a2642601e61 100644
> --- a/drivers/dma/st_fdma.c
> +++ b/drivers/dma/st_fdma.c
> @@ -771,10 +771,8 @@ static int st_fdma_probe(struct platform_device *pdev)
>  
>  	ret = devm_request_irq(&pdev->dev, fdev->irq, st_fdma_irq_handler, 0,
>  			       dev_name(&pdev->dev), fdev);
> -	if (ret) {
> -		dev_err(&pdev->dev, "Failed to request irq (%d)\n", ret);
> +	if (ret)
>  		goto err;

[Severity: High]
This is a pre-existing issue, but is it safe to request the IRQ before
allocating fdev->slim_rproc and initializing the channels?

If an interrupt fires immediately after devm_request_irq() is called,
st_fdma_irq_handler() will use fdma_read(), which dereferences
fdev->slim_rproc->peri. Since fdev->slim_rproc is still NULL, would this
cause a crash?

Also, if probe fails later (e.g., during DMA registration), the error
path destroys slim_rproc and kills tasklets:

drivers/dma/st_fdma.c:st_fdma_probe() {
    ...
err_rproc:
	st_fdma_free(fdev);
	st_slim_rproc_put(fdev->slim_rproc);
err:
	return ret;
}

Since the IRQ is managed by devres, it remains active until the devres
cleanup runs. If an interrupt fires during this window, could it access
destroyed resources?

[Severity: High]
This is also a pre-existing issue, but does the error handling in
st_fdma_probe() and the driver remove path create a mismatch with
devm-managed DMA registration?

When probe fails in of_dma_controller_register(), the err_rproc path calls
st_fdma_free() before devres cleanup:

drivers/dma/st_fdma.c:st_fdma_free() {
    ...
	list_del(&fchan->vchan.chan.device_node);
    ...
}

This manual list_del() empties fdev->dma_device.channels. Will this cause
the devres dma_async_device_unregister() to skip unregistering the sysfs
devices, leaving dangling pointers to the freed fdev?

Similarly, in the remove path:

drivers/dma/st_fdma.c:st_fdma_remove() {
	struct st_fdma_dev *fdev = platform_get_drvdata(pdev);

	devm_free_irq(&pdev->dev, fdev->irq, fdev);
	st_slim_rproc_put(fdev->slim_rproc);
	of_dma_controller_free(pdev->dev.of_node);
}

st_fdma_remove() does not call st_fdma_free() to kill the tasklets, and it
puts slim_rproc manually. When devres later frees fdev, could any
scheduled but unkilled tasklet trigger a use-after-free, and could concurrent
clients terminating channels trigger a use-after-free on slim_rproc?

>  
>  	fdev->slim_rproc = st_slim_rproc_alloc(pdev, fdev->fw_name);
>  	if (IS_ERR(fdev->slim_rproc)) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=17

[PATCH 18/26] dmaengine: stm32-stm32-dma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/stm32/stm32-dma.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/dma/stm32/stm32-dma.c b/drivers/dma/stm32/stm32-dma.c
index d3ad78562a14..51cb2328a8e1 100644
--- a/drivers/dma/stm32/stm32-dma.c
+++ b/drivers/dma/stm32/stm32-dma.c
@@ -1669,12 +1669,8 @@ static int stm32_dma_probe(struct platform_device *pdev)
 		ret = devm_request_irq(&pdev->dev, chan->irq,
 				       stm32_dma_chan_irq, 0,
 				       dev_name(chan2dev(chan)), chan);
-		if (ret) {
-			dev_err(&pdev->dev,
-				"request_irq failed with err %d channel %d\n",
-				ret, i);
+		if (ret)
 			goto err_unregister;
-		}
 	}
 
 	ret = of_dma_controller_register(pdev->dev.of_node,
-- 
2.34.1

Re: [PATCH 18/26] dmaengine: stm32-stm32-dma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Dangling pointer in `irqaction->name` leads to a Use-After-Free read when accessing `/proc/interrupts` on probe failure due to premature device name deallocation.
--

--- Patch [18]: [PATCH 18/26] dmaengine: stm32-stm32-dma: Remove redundant dev_err()/dev_err_probe() ---
commit deae2b976db4b82d7a92a5e0ad3b7fc0fc39d45b
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: stm32-stm32-dma: Remove redundant dev_err()/dev_err_probe()

This commit removes redundant driver-specific error logging when
devm_request_irq() fails. The core now logs detailed error messages
automatically.

> diff --git a/drivers/dma/stm32/stm32-dma.c b/drivers/dma/stm32/stm32-dma.c
> index d3ad78562a140..51cb2328a8e19 100644
> --- a/drivers/dma/stm32/stm32-dma.c
> +++ b/drivers/dma/stm32/stm32-dma.c
> @@ -1669,12 +1669,8 @@ static int stm32_dma_probe(struct platform_device *pdev)
>  		ret = devm_request_irq(&pdev->dev, chan->irq,
>  				       stm32_dma_chan_irq, 0,
>  				       dev_name(chan2dev(chan)), chan);
> -		if (ret) {
> -			dev_err(&pdev->dev,
> -				"request_irq failed with err %d channel %d\n",
> -				ret, i);
> +		if (ret)
>  			goto err_unregister;

[Severity: High]
This is a pre-existing issue, but does this error path leave a dangling
pointer in the IRQ subsystem?

In stm32_dma_probe(), the devm_request_irq() call uses
dev_name(chan2dev(chan)) as the IRQ name. The IRQ subsystem stores this
pointer directly in irqaction->name without copying the string.

If the probe function fails and jumps to err_unregister,
dma_async_device_unregister() is called:

err_unregister:
	dma_async_device_unregister(dd);

This unregisters the channel devices and frees their kobject name strings.
Since the IRQs are devm-managed, they remain active until the probe function
completely returns and devres_release_all() cleans them up.

During this window, can a concurrent read of /proc/interrupts access the
freed device name string?

> -		}
>  	}
>  
>  	ret = of_dma_controller_register(pdev->dev.of_node,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=18

[PATCH 19/26] dmaengine: stm32-stm32-dma3: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/stm32/stm32-dma3.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/dma/stm32/stm32-dma3.c b/drivers/dma/stm32/stm32-dma3.c
index 4724e7fa0008..68ed1d695f1d 100644
--- a/drivers/dma/stm32/stm32-dma3.c
+++ b/drivers/dma/stm32/stm32-dma3.c
@@ -1893,11 +1893,8 @@ static int stm32_dma3_probe(struct platform_device *pdev)
 
 		ret = devm_request_irq(&pdev->dev, chan->irq, stm32_dma3_chan_irq, 0,
 				       dev_name(chan2dev(chan)), chan);
-		if (ret) {
-			dev_err_probe(&pdev->dev, ret, "Failed to request channel %s IRQ\n",
-				      dev_name(chan2dev(chan)));
+		if (ret)
 			goto err_clk_disable;
-		}
 	}
 
 	ret = of_dma_controller_register(np, stm32_dma3_of_xlate, ddata);
-- 
2.34.1

Re: [PATCH 19/26] dmaengine: stm32-stm32-dma3: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] The OF DMA controller is registered using the non-managed `of_dma_controller_register()`, but it is never freed in the driver's `remove` function, leading to a resource leak and a Use-After-Free vulnerability.
- [High] The DMA controller exposes its channels to clients before completing initialization, creating a fatal race condition if subsequent channel setups fail.
- [High] Disabling the clock manually before devres cleans up devm-managed IRQs leaves a window where active IRQs can access unclocked hardware.
--

--- Patch [19]: [PATCH 19/26] dmaengine: stm32-stm32-dma3: Remove redundant dev_err()/dev_err_probe() ---
commit 71644e471343978f46dd63e4905f5c779768528f
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: stm32-stm32-dma3: Remove redundant dev_err()/dev_err_probe()

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

> diff --git a/drivers/dma/stm32/stm32-dma3.c b/drivers/dma/stm32/stm32-dma3.c
> --- a/drivers/dma/stm32/stm32-dma3.c
> +++ b/drivers/dma/stm32/stm32-dma3.c
> @@ -1893,11 +1893,8 @@ static int stm32_dma3_probe(struct platform_device *pdev)
>  
>  		ret = devm_request_irq(&pdev->dev, chan->irq, stm32_dma3_chan_irq, 0,
>  				       dev_name(chan2dev(chan)), chan);
> -		if (ret) {
> -			dev_err_probe(&pdev->dev, ret, "Failed to request channel %s IRQ\n",
> -				      dev_name(chan2dev(chan)));
> +		if (ret)
>  			goto err_clk_disable;

[Severity: High]
This is a pre-existing issue, but by jumping to err_clk_disable here, could
we trigger a crash if an interrupt fires?

If stm32_dma3_probe() fails on a later iteration of this loop, we jump to
the manual clock disable cleanup:

err_clk_disable:
        clk_disable_unprepare(ddata->clk);

Since we used devm_request_irq() for previous channels, those IRQs remain
active until devres runs. If an interrupt fires during this window, the
handler would access unclocked registers.

[Severity: High]
This is also a pre-existing issue, but does failing here leave dangling
channel registrations?

Earlier in stm32_dma3_probe(), we called dmaenginem_async_device_register()
and dma_async_device_channel_register() before this channel initialization
loop completes. 

If a failure occurs in this loop and we jump to the error path, any client
that has already requested the successfully registered channels could be
exposed to an asynchronous teardown, potentially accessing unclocked hardware
or freed memory.

> -		}
>  	}
>  
>  	ret = of_dma_controller_register(np, stm32_dma3_of_xlate, ddata);

[Severity: High]
This is a pre-existing issue, but does this leak the DMA controller?

The of_dma_controller_register() function is used here, but looking at
stm32_dma3_remove(), there is no corresponding call to
of_dma_controller_free().

stm32_dma3_remove() simply disables runtime PM:

static void stm32_dma3_remove(struct platform_device *pdev)
{
        pm_runtime_disable(&pdev->dev);
}

If the driver is unbound, the devm-managed ddata will be freed, but the
controller remains registered. A subsequent DMA request via devicetree
would then invoke stm32_dma3_of_xlate() with a dangling ddata pointer,
causing a use-after-free.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=19

[PATCH 20/26] dmaengine: stm32-stm32-mdma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/stm32/stm32-mdma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma/stm32/stm32-mdma.c b/drivers/dma/stm32/stm32-mdma.c
index e3bbdc9ee36e..26e6c117f89d 100644
--- a/drivers/dma/stm32/stm32-mdma.c
+++ b/drivers/dma/stm32/stm32-mdma.c
@@ -1713,10 +1713,8 @@ static int stm32_mdma_probe(struct platform_device *pdev)
 
 	ret = devm_request_irq(&pdev->dev, dmadev->irq, stm32_mdma_irq_handler,
 			       0, dev_name(&pdev->dev), dmadev);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to request IRQ\n");
+	if (ret)
 		goto err_clk;
-	}
 
 	ret = dmaenginem_async_device_register(dd);
 	if (ret)
-- 
2.34.1

Re: [PATCH 20/26] dmaengine: stm32-stm32-mdma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing `.remove` callback causes resource leaks and use-after-free on driver unbind.
- [High] Devres leak of DMA pools on channel allocation error path.
- [High] Disabling the clock before freeing the IRQ on the probe error path can cause a synchronous external abort.
--

--- Patch [20]: [PATCH 20/26] dmaengine: stm32-stm32-mdma: Remove redundant dev_err()/dev_err_probe() ---
commit 6440956ea9b4ed6aa697c4eb90a3a35f158a8e26
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: stm32-stm32-mdma: Remove redundant dev_err()/dev_err_probe()

This patch removes redundant dev_err() calls after devm_request_irq() 
since the core now logs detailed error messages automatically.

> diff --git a/drivers/dma/stm32/stm32-mdma.c b/drivers/dma/stm32/stm32-mdma.c
> --- a/drivers/dma/stm32/stm32-mdma.c
> +++ b/drivers/dma/stm32/stm32-mdma.c
> @@ -1713,10 +1713,8 @@ static int stm32_mdma_probe(struct platform_device *pdev)
>  
>  	ret = devm_request_irq(&pdev->dev, dmadev->irq, stm32_mdma_irq_handler,
>  			       0, dev_name(&pdev->dev), dmadev);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to request IRQ\n");
> +	if (ret)
>  		goto err_clk;

[Severity: High]
This is a pre-existing issue, but does disabling the clock on the error path
before devres frees the IRQ risk a synchronous external abort?

If a spurious interrupt arrives after the goto err_clk path calls
clk_disable_unprepare() but before devres cleans up the IRQ handler,
stm32_mdma_irq_handler() will execute.

Could attempting to read STM32_MDMA_GISR0 while the APB clock is disabled
cause a bus fault and crash the kernel?

>  
>  	ret = dmaenginem_async_device_register(dd);
>  	if (ret)

I also noticed a couple of other pre-existing issues in this file:

drivers/dma/stm32/stm32-mdma.c:stm32_mdma_driver

static struct platform_driver stm32_mdma_driver = {
	.probe = stm32_mdma_probe,
	.driver = {
		.name = "stm32-mdma",
...

[Severity: High]
This is a pre-existing issue, but does this platform driver lack a .remove
callback?

Since suppress_bind_attrs is not set, a privileged user can unbind the
driver via sysfs. When devres frees the device memory for dmadev,
of_dma_controller_free() is never called, leaving a dangling pointer
in the global of_dma_list.

Will subsequent DMA requests by other devices dereference this freed
pointer? Also, could scheduled vchan tasklets cause a use-after-free if
they execute during unbind?


drivers/dma/stm32/stm32-mdma.c:stm32_mdma_alloc_chan_resources()

	...
	ret = pm_runtime_resume_and_get(dmadev->ddev.dev);
	if (ret < 0)
		return ret;

	ret = stm32_mdma_disable_chan(chan);
	if (ret < 0)
		goto err_pm;
	...
err_pm:
	pm_runtime_put(dmadev->ddev.dev);
	return ret;

[Severity: High]
This is a pre-existing issue, but does this error path leak the descriptor
pool?

Earlier in stm32_mdma_alloc_chan_resources(), dmam_pool_create() allocates
a new pool. If a subsequent step like pm_runtime_resume_and_get() or
stm32_mdma_disable_chan() fails, the function returns without destroying
the pool.

Since the DMA engine core does not call the free callback if allocation
fails, could repeated channel requests by userspace continuously accumulate
leaked DMA pools and lead to an out of memory condition?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=20

[PATCH 21/26] dmaengine: sun4i-dma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/sun4i-dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/sun4i-dma.c b/drivers/dma/sun4i-dma.c
index d2321f7287d2..303d71ba0ab6 100644
--- a/drivers/dma/sun4i-dma.c
+++ b/drivers/dma/sun4i-dma.c
@@ -1337,7 +1337,7 @@ static int sun4i_dma_probe(struct platform_device *pdev)
 	ret = devm_request_irq(&pdev->dev, priv->irq, sun4i_dma_interrupt,
 			       0, dev_name(&pdev->dev), priv);
 	if (ret)
-		return dev_err_probe(&pdev->dev, ret, "Cannot request IRQ\n");
+		return ret;
 
 	ret = dmaenginem_async_device_register(&priv->slave);
 	if (ret)
-- 
2.34.1

Re: [PATCH 21/26] dmaengine: sun4i-dma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing tasklet_kill() for virtual DMA channels in device teardown (remove and probe error paths) leads to Use-After-Free.
- [High] Ongoing DMA hardware transfers are not gracefully stopped in sun4i_dma_remove() before clock disable.
- [High] Race condition between sun4i_dma_interrupt() and sun4i_dma_terminate_all() leads to NULL pointer dereference and Use-After-Free.
- [Medium] Incorrect DMA residue calculation in sun4i_dma_tx_status() for queued (unstarted) contracts.
- [Low] sun4i_dma_of_xlate() strictly validates against hardcoded max DRQ type limits rather than the SoC-specific config.
--

--- Patch [21]: [PATCH 21/26] dmaengine: sun4i-dma: Remove redundant dev_err()/dev_err_probe() ---
commit 0e16200d5dd839cadb8ffc88ed67f75051cff988
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: sun4i-dma: Remove redundant dev_err()/dev_err_probe()

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

> diff --git a/drivers/dma/sun4i-dma.c b/drivers/dma/sun4i-dma.c
> index d2321f7287d20..303d71ba0ab61 100644
> --- a/drivers/dma/sun4i-dma.c
> +++ b/drivers/dma/sun4i-dma.c
> @@ -1337,7 +1337,7 @@ static int sun4i_dma_probe(struct platform_device *pdev)
>  	ret = devm_request_irq(&pdev->dev, priv->irq, sun4i_dma_interrupt,
>  			       0, dev_name(&pdev->dev), priv);
>  	if (ret)
> -		return dev_err_probe(&pdev->dev, ret, "Cannot request IRQ\n");
> +		return ret;
>  
>  	ret = dmaenginem_async_device_register(&priv->slave);
>  	if (ret)

[Severity: High]
This is a pre-existing issue, but does driver unbind safely tear down
the virtual channels?

drivers/dma/sun4i-dma.c:sun4i_dma_remove() {
    struct sun4i_dma_dev *priv = platform_get_drvdata(pdev);

    /* Disable IRQ so no more work is scheduled */
    disable_irq(priv->irq);

    of_dma_controller_free(pdev->dev.of_node);
}

If the driver is unbound while an interrupt has scheduled a tasklet,
dma_async_device_unregister() called by devres will free the DMA channels
and the associated priv->vchans memory.

Since there is no tasklet_kill(&vchan->vc.task) call to cancel pending
tasklets, could the scheduled tasklet execute after the memory is freed
and cause a use-after-free?


[Severity: High]
This is another pre-existing issue in the same function, but does this
safely stop active DMA transfers?

drivers/dma/sun4i-dma.c:sun4i_dma_remove() {
    struct sun4i_dma_dev *priv = platform_get_drvdata(pdev);

    /* Disable IRQ so no more work is scheduled */
    disable_irq(priv->irq);

    of_dma_controller_free(pdev->dev.of_node);
}

If the driver is unbound while a DMA transfer is actively executing, the
hardware configuration registers are not cleared to halt the channels.

When devres subsequently disables the DMA controller's clock mid-transaction,
can this forcefully lock up the AXI/AHB bus or corrupt memory?


[Severity: High]
This is a pre-existing issue, but is there a race condition between the
interrupt handler and channel termination?

drivers/dma/sun4i-dma.c:sun4i_dma_interrupt() {
    ...
    contract = vchan->contract;

    /*
     * Disable the IRQ and free the pchan if it's an end
     * interrupt (odd bit)
     */
    if (bit & 1) {
        spin_lock(&vchan->vc.lock);

        /*
         * Move the promise into the completed list now that
         * we're done with it
         */
        list_move_tail(&vchan->processing->list,
                       &contract->completed_demands);
    ...
}

The contract pointer is read before acquiring vchan->vc.lock. Concurrently,
sun4i_dma_terminate_all() on another CPU could acquire the lock, set
vchan->processing to NULL, free the active contract, and release the lock.

When sun4i_dma_interrupt() subsequently acquires the lock, it performs a
list_move_tail() using vchan->processing->list.

Could this cause a guaranteed NULL pointer dereference since
vchan->processing would be NULL, as well as a use-after-free when accessing
the freed contract structure?


[Severity: Medium]
This is a pre-existing issue, but does this calculate the residue correctly
for queued contracts?

drivers/dma/sun4i-dma.c:sun4i_dma_tx_status() {
    ...
    promise = list_first_entry_or_null(&contract->demands,
                                       struct sun4i_dma_promise, list);
    if (promise && pchan) {
        bytes -= promise->len;
        if (pchan->is_dedicated)
            bytes += readl(pchan->base + SUN4I_DDMA_BYTE_COUNT_REG);
        else
            bytes += readl(pchan->base + SUN4I_NDMA_BYTE_COUNT_REG);
    }
    ...
}

If vchan_find_desc() returns a pending, unstarted contract, and the virtual
channel is currently executing a different contract on a physical channel,
this code unconditionally assumes the hardware byte count belongs to the
queried contract.

By subtracting the queried contract's promise length and adding the executing
contract's hardware residue, can this return corrupted residue values for
queued transfers?


[Severity: Low]
This is a pre-existing issue, but does the validation correctly restrict
endpoints on all SoC variants?

drivers/dma/sun4i-dma.c:sun4i_dma_of_xlate() {
    ...
    /* Make sure the endpoint looks sane */
    if ((is_dedicated && endpoint >= SUN4I_DDMA_DRQ_TYPE_LIMIT) ||
        (!is_dedicated && endpoint >= SUN4I_NDMA_DRQ_TYPE_LIMIT))
        return NULL;
    ...
}

The validation uses hardcoded limits like SUN4I_DDMA_DRQ_TYPE_LIMIT (32)
regardless of the SoC variant.

If a device tree requests an endpoint of 20 on a suniv_f1c100s device, which
has a limit of 10, would this bypass the check and program out-of-bounds
DRQ indices into the hardware?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=21

[PATCH 22/26] dmaengine: sun6i-dma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/sun6i-dma.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index f47a326dd7ff..87ad7eae2069 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -1454,10 +1454,8 @@ static int sun6i_dma_probe(struct platform_device *pdev)
 
 	ret = devm_request_irq(&pdev->dev, sdc->irq, sun6i_dma_interrupt, 0,
 			       dev_name(&pdev->dev), sdc);
-	if (ret) {
-		dev_err(&pdev->dev, "Cannot request IRQ\n");
+	if (ret)
 		goto err_mbus_clk_disable;
-	}
 
 	ret = dma_async_device_register(&sdc->slave);
 	if (ret) {
-- 
2.34.1

[PATCH 23/26] dmaengine: tegra20-apb-dma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/tegra20-apb-dma.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c
index 640b8a218c9a..fcfd2f35c821 100644
--- a/drivers/dma/tegra20-apb-dma.c
+++ b/drivers/dma/tegra20-apb-dma.c
@@ -1496,12 +1496,8 @@ static int tegra_dma_probe(struct platform_device *pdev)
 		snprintf(tdc->name, sizeof(tdc->name), "apbdma.%d", i);
 		ret = devm_request_irq(&pdev->dev, irq, tegra_dma_isr, 0,
 				       tdc->name, tdc);
-		if (ret) {
-			dev_err(&pdev->dev,
-				"request_irq failed with err %d channel %d\n",
-				ret, i);
+		if (ret)
 			goto err_pm_disable;
-		}
 
 		tdc->dma_chan.device = &tdma->dma_dev;
 		dma_cookie_init(&tdc->dma_chan);
-- 
2.34.1

[PATCH 24/26] dmaengine: ti-edma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/ti/edma.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c
index d97db5af3555..e023ca9640de 100644
--- a/drivers/dma/ti/edma.c
+++ b/drivers/dma/ti/edma.c
@@ -2415,10 +2415,8 @@ static int edma_probe(struct platform_device *pdev)
 
 		ret = devm_request_irq(dev, irq, dma_irq_handler, 0, irq_name,
 				       ecc);
-		if (ret) {
-			dev_err(dev, "CCINT (%d) failed --> %d\n", irq, ret);
+		if (ret)
 			goto err_disable_pm;
-		}
 		ecc->ccint = irq;
 	}
 
@@ -2436,10 +2434,8 @@ static int edma_probe(struct platform_device *pdev)
 
 		ret = devm_request_irq(dev, irq, dma_ccerr_handler, 0, irq_name,
 				       ecc);
-		if (ret) {
-			dev_err(dev, "CCERRINT (%d) failed --> %d\n", irq, ret);
+		if (ret)
 			goto err_disable_pm;
-		}
 		ecc->ccerrint = irq;
 	}
 
-- 
2.34.1

Re: [PATCH 24/26] dmaengine: ti-edma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unhandled probe deferral from `platform_get_irq_byname` causes the driver to silently probe without interrupts.
--

--- Patch [24]: [PATCH 24/26] dmaengine: ti-edma: Remove redundant dev_err()/dev_err_probe() ---
commit 3f067543fc6aa46c8b00f2557e8220b1465e86e3
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: ti-edma: Remove redundant dev_err()/dev_err_probe()

The commit removes redundant dev_err() logging around devm_request_irq()
calls. The devm_request_irq() function now automatically logs detailed error
messages on failure, eliminating the need for driver-specific messages.

> diff --git a/drivers/dma/ti/edma.c b/drivers/dma/ti/edma.c
> index d97db5af35555..e023ca9640dec 100644
> --- a/drivers/dma/ti/edma.c
> +++ b/drivers/dma/ti/edma.c
> @@ -2415,10 +2415,8 @@ static int edma_probe(struct platform_device *pdev)
>  
>  	irq = platform_get_irq_byname(pdev, "edma3_ccint");
>  	if (irq < 0 && node)
>  		irq = irq_of_parse_and_map(node, 0);
>  
>  	if (irq > 0) {
>  		irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s_ccint",
>  					  dev_name(dev));
>  		if (!irq_name) {
>  			ret = -ENOMEM;
>  			goto err_disable_pm;
>  		}
>  
>  		ret = devm_request_irq(dev, irq, dma_irq_handler, 0, irq_name,
>  				       ecc);
> -		if (ret) {
> -			dev_err(dev, "CCINT (%d) failed --> %d\n", irq, ret);
> +		if (ret)
>  			goto err_disable_pm;
> -		}
>  		ecc->ccint = irq;
>  	}

[Severity: High]
This is a pre-existing issue, but does this logic handle probe deferrals
correctly?

If platform_get_irq_byname() returns -EPROBE_DEFER, the irq value is
either overwritten to 0 by irq_of_parse_and_map(), or remains -EPROBE_DEFER.
In both scenarios, the if (irq > 0) check fails, causing the driver to
silently skip interrupt setup and successfully complete the probe without
interrupts.

Could this lead to dependent devices hanging or experiencing timeouts
because DMA completions are never signaled?

[ ... ]

> @@ -2436,10 +2434,8 @@ static int edma_probe(struct platform_device *pdev)
>  
>  	irq = platform_get_irq_byname(pdev, "edma3_ccerrint");
>  	if (irq < 0 && node)
>  		irq = irq_of_parse_and_map(node, 2);
>  
>  	if (irq > 0) {
>  		irq_name = devm_kasprintf(dev, GFP_KERNEL, "%s_ccerrint",
>  					  dev_name(dev));
>  		if (!irq_name) {
>  			ret = -ENOMEM;
>  			goto err_disable_pm;
>  		}
>  
>  		ret = devm_request_irq(dev, irq, dma_ccerr_handler, 0, irq_name,
>  				       ecc);
> -		if (ret) {
> -			dev_err(dev, "CCERRINT (%d) failed --> %d\n", irq, ret);
> +		if (ret)
>  			goto err_disable_pm;
> -		}
>  		ecc->ccerrint = irq;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but the error interrupt setup
appears to suffer from the same problem.

Would an unhandled probe deferral from platform_get_irq_byname() result
in silently swallowed errors here as well?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=24

[PATCH 25/26] dmaengine: uniphier-xdmac: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/uniphier-xdmac.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma/uniphier-xdmac.c b/drivers/dma/uniphier-xdmac.c
index 120c0d4f12dd..b23d61bf7fd5 100644
--- a/drivers/dma/uniphier-xdmac.c
+++ b/drivers/dma/uniphier-xdmac.c
@@ -532,10 +532,8 @@ static int uniphier_xdmac_probe(struct platform_device *pdev)
 
 	ret = devm_request_irq(dev, irq, uniphier_xdmac_irq_handler,
 			       IRQF_SHARED, "xdmac", xdev);
-	if (ret) {
-		dev_err(dev, "Failed to request IRQ\n");
+	if (ret)
 		return ret;
-	}
 
 	ret = dma_async_device_register(ddev);
 	if (ret) {
-- 
2.34.1

Re: [PATCH 25/26] dmaengine: uniphier-xdmac: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Returning early from uniphier_xdmac_remove() if a channel fails to terminate leaves the device registered with the DMA engine while devres frees its underlying memory, causing a Use-After-Free.
- [Medium] The shared IRQ handler unconditionally returns IRQ_HANDLED, which breaks the kernel's spurious interrupt detection.
--

--- Patch [25]: [PATCH 25/26] dmaengine: uniphier-xdmac: Remove redundant dev_err()/dev_err_probe() ---
commit d83da39937d839c163b8d27546ec157138f20ec2
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: uniphier-xdmac: Remove redundant dev_err()/dev_err_probe()

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

> diff --git a/drivers/dma/uniphier-xdmac.c b/drivers/dma/uniphier-xdmac.c
> index 120c0d4f12dda..b23d61bf7fd5b 100644
> --- a/drivers/dma/uniphier-xdmac.c
> +++ b/drivers/dma/uniphier-xdmac.c
> @@ -532,10 +532,8 @@ static int uniphier_xdmac_probe(struct platform_device *pdev)
>  
>  	ret = devm_request_irq(dev, irq, uniphier_xdmac_irq_handler,
>  			       IRQF_SHARED, "xdmac", xdev);

[Severity: Medium]
This is a pre-existing issue, but since we are registering this handler with
IRQF_SHARED here, does unconditionally returning IRQ_HANDLED break the
kernel's spurious interrupt detection?

Looking at uniphier_xdmac_irq_handler():

static irqreturn_t uniphier_xdmac_irq_handler(int irq, void *dev_id)
{
	struct uniphier_xdmac_device *xdev = dev_id;
	int i;

	for (i = 0; i < xdev->nr_chans; i++)
		uniphier_xdmac_chan_irq(&xdev->channels[i]);

	return IRQ_HANDLED;
}

If a shared interrupt line experiences an interrupt storm or unhandled
interrupt from another device, and no channels here have pending interrupts
(uniphier_xdmac_chan_irq does nothing), this driver will still claim to have
handled it. Will this prevent the kernel from disabling the broken line,
ultimately leading to a system hang?

> -	if (ret) {
> -		dev_err(dev, "Failed to request IRQ\n");
> +	if (ret)
>  		return ret;
> -	}
>  
>  	ret = dma_async_device_register(ddev);

[Severity: High]
This is a pre-existing issue, but concerning the device registration here,
what happens during teardown if dmaengine_terminate_sync() fails?

drivers/dma/uniphier-xdmac.c:uniphier_xdmac_remove() {
	...
	list_for_each_entry(chan, &ddev->channels, device_node) {
		ret = dmaengine_terminate_sync(chan);
		if (ret) {
			dev_alert(&pdev->dev, "Failed to terminate...");
			return;
		}
		...
	}

	of_dma_controller_free(pdev->dev.of_node);
	dma_async_device_unregister(ddev);
}

If we return early because of a termination failure,
dma_async_device_unregister() is skipped. Because the xdev structure is
managed by devm_kzalloc, devres will automatically free the memory when the
unbind completes. Will this leave a dangling pointer in the global DMA
device list, leading to a use-after-free if the subsystem accesses the
freed memory?

>  	if (ret) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=25

[PATCH 26/26] dmaengine: xgene-dma: Remove redundant dev_err()/dev_err_probe()

[Pan Chuang]

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

Signed-off-by: Pan Chuang <panchuang@vivo.com>
---
 drivers/dma/xgene-dma.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/dma/xgene-dma.c b/drivers/dma/xgene-dma.c
index fa1173e49900..6797a3b84f1b 100644
--- a/drivers/dma/xgene-dma.c
+++ b/drivers/dma/xgene-dma.c
@@ -1435,11 +1435,8 @@ static int xgene_dma_request_irqs(struct xgene_dma *pdma)
 	/* Register DMA error irq */
 	ret = devm_request_irq(pdma->dev, pdma->err_irq, xgene_dma_err_isr,
 			       0, "dma_error", pdma);
-	if (ret) {
-		dev_err(pdma->dev,
-			"Failed to register error IRQ %d\n", pdma->err_irq);
+	if (ret)
 		return ret;
-	}
 
 	/* Register DMA channel rx irq */
 	for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++) {
-- 
2.34.1

Re: [PATCH 26/26] dmaengine: xgene-dma: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Medium] Incomplete removal of redundant error messages for `devm_request_irq()` failures.

Pre-existing issues:
- [High] Incorrect channel index used during IRQ rollback path in `xgene_dma_request_irqs()`, leading to multiple kernel warnings and failed rollback.
- [High] Race condition in probe sequence causes NULL pointer dereference via uninitialized tasklet.
- [High] Probe error path fails to disable DMA hardware, leading to potential memory corruption.
--

--- Patch [26]: [PATCH 26/26] dmaengine: xgene-dma: Remove redundant dev_err()/dev_err_probe() ---
commit b27e7ae82dbb69011f33590c87faea09bb044017
Author: Pan Chuang <panchuang@vivo.com>

dmaengine: xgene-dma: Remove redundant dev_err()/dev_err_probe()

The devm_request_irq() now automatically logs detailed error messages on
failure. This eliminates the need for driver-specific dev_err() and
dev_err_probe() calls that previously printed generic messages.

> diff --git a/drivers/dma/xgene-dma.c b/drivers/dma/xgene-dma.c
> --- a/drivers/dma/xgene-dma.c
> +++ b/drivers/dma/xgene-dma.c
> @@ -1436,11 +1436,8 @@ static int xgene_dma_request_irqs(struct xgene_dma *pdma)
>  	/* Register DMA error irq */
>  	ret = devm_request_irq(pdma->dev, pdma->err_irq, xgene_dma_err_isr,
>  			       0, "dma_error", pdma);
> -	if (ret) {
> -		dev_err(pdma->dev,
> -			"Failed to register error IRQ %d\n", pdma->err_irq);
> +	if (ret)
>  		return ret;
> -	}
>  
>  	/* Register DMA channel rx irq */
>  	for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++) {
>  		chan = &pdma->chan[i];
>  		irq_set_status_flags(chan->rx_irq, IRQ_DISABLE_UNLAZY);
>  		ret = devm_request_irq(chan->dev, chan->rx_irq,
>  				       xgene_dma_chan_ring_isr,
>  				       0, chan->name, chan);
>  		if (ret) {
>  			chan_err(chan, "Failed to register Rx IRQ %d\n",
>  				 chan->rx_irq);

[Severity: Medium]
The commit message states that the patch eliminates driver-specific error
logs for devm_request_irq failures because the core now logs them
automatically.

While the patch removes the dev_err for the global error IRQ above, it
appears to miss this chan_err macro for the Rx IRQ registration failure.

Should this chan_err also be removed to fully align with the commit message?

>  			devm_free_irq(pdma->dev, pdma->err_irq, pdma);
>  
>  			for (j = 0; j < i; j++) {
>  				chan = &pdma->chan[i];

[Severity: High]
This is a pre-existing issue, but there appears to be a typo in the rollback
loop in xgene_dma_request_irqs.

It assigns chan using the outer loop index i instead of the inner loop
index j. Will this cause devm_free_irq to be called multiple times on an
unregistered IRQ and fail to clear the IRQ_DISABLE_UNLAZY flag for the
successfully registered ones?

>  				irq_clear_status_flags(chan->rx_irq, IRQ_DISABLE_UNLAZY);
>  				devm_free_irq(chan->dev, chan->rx_irq, chan);
>  			}
>  
>  			return ret;
>  		}
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but looking at the probe sequence
in xgene_dma_probe:

drivers/dma/xgene-dma.c:xgene_dma_probe() {
    ...
    ret = xgene_dma_request_irqs(pdma);
    if (ret)
        goto err_request_irq;

    /* Configure and enable DMA engine */
    xgene_dma_init_hw(pdma);

    /* Register DMA device with linux async framework */
    ret = xgene_dma_init_async(pdma);
    ...
}

The function xgene_dma_request_irqs is called before the hardware is unmasked
in xgene_dma_init_hw. If a hardware interrupt fires immediately after
xgene_dma_init_hw, xgene_dma_chan_ring_isr will execute and unconditionally
schedule the tasklet:

drivers/dma/xgene-dma.c:xgene_dma_chan_ring_isr() {
    ...
    tasklet_schedule(&chan->tasklet);
}

However, chan->tasklet is not set up until xgene_dma_init_async runs later
in the probe sequence. Can this trigger a NULL pointer dereference by
scheduling an uninitialized tasklet?

[Severity: High]
This is also a pre-existing issue, but looking at the error handling path
in xgene_dma_probe:

drivers/dma/xgene-dma.c:xgene_dma_probe() {
    ...
err_async_init:
    xgene_dma_free_irqs(pdma);

err_request_irq:
    for (i = 0; i < XGENE_DMA_MAX_CHANNEL; i++)
        xgene_dma_delete_chan_rings(&pdma->chan[i]);
    ...
}

If xgene_dma_init_async fails, the code jumps to err_async_init and frees
software resources and DMA descriptor ring buffers.

This path does not appear to call xgene_dma_disable or
xgene_dma_mask_interrupts. Could this leave the DMA controller actively
running and pointing at freed memory, potentially causing memory corruption
if the hardware continues to process descriptors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709135846.97972-1-panchuang@vivo.com?part=26

Re: [PATCH 00/26] dmaengine: Remove redundant error messages on IRQ request failure

[Wolfram Sang]

On Thu, Jul 09, 2026 at 09:58:04PM +0800, Pan Chuang wrote:
> Commit 55b48e23f5c4b6f5ca9b7ab09599b17dcf501c10 ("genirq/devres: Add
> error handling in devm_request_*_irq()") added automatic error logging
> to devm_request_threaded_irq() and devm_request_any_context_irq() via
> the new devm_request_result() helper. The helper prints device name,
> IRQ number, handler functions, and error code on failure.
> 
> Since devm_request_irq() is a static inline wrapper around
> devm_request_threaded_irq(), it also benefits from this automatic
> logging.
> 
> This series removes the now-redundant dev_err() and dev_err_probe() calls
> in dmaengine drivers that follow these devm_request_*_irq() functions,
> as the core now provides more detailed diagnostic information on failure.
> 
> Pan Chuang (26):
>   dmaengine: fsl-edma-main: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: fsl-qdma: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: loongson-loongson1-apb-dma: Remove redundant
>     dev_err()/dev_err_probe()
>   dmaengine: mediatek-mtk-cqdma: Remove redundant
>     dev_err()/dev_err_probe()
>   dmaengine: mediatek-mtk-hsdma: Remove redundant
>     dev_err()/dev_err_probe()
>   dmaengine: mmp_pdma: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: moxart-dma: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: owl-dma: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: pxa_dma: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: qcom-gpi: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: sf-pdma-sf-pdma: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: sh-rcar-dmac: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: sh-rz-dmac: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: sh-shdmac: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: sh-usb-dmac: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: sprd-dma: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: st_fdma: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: stm32-stm32-dma: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: stm32-stm32-dma3: Remove redundant
>     dev_err()/dev_err_probe()
>   dmaengine: stm32-stm32-mdma: Remove redundant
>     dev_err()/dev_err_probe()
>   dmaengine: sun4i-dma: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: sun6i-dma: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: tegra20-apb-dma: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: ti-edma: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: uniphier-xdmac: Remove redundant dev_err()/dev_err_probe()
>   dmaengine: xgene-dma: Remove redundant dev_err()/dev_err_probe()

One patch per subsystem for such trivial changes, please.