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

[sashiko-bot@kernel.org]

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