Re: [PATCH v6] dmaengine: tegra210-adma: Add error logging on failure paths

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Race condition on probe error paths due to async pm_runtime_put() followed by pm_runtime_disable(), leaving hardware powered on.
--

commit 64b163be27d8e21c8b03c51eb8cec223bd8a75f4
Author: Sheetal <sheetal@nvidia.com>

dmaengine: tegra210-adma: Add error logging on failure paths

This commit adds error logging across failure paths to improve
debuggability of DMA errors during runtime and probe. It also cleans up the
probe path using devres managed helpers like devm_pm_runtime_enable() and
dmaenginem_async_device_register(), removing manual error unwind labels.

> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
> index 14e0c408ed1e6..6987bf6ec6487 100644
> --- a/drivers/dma/tegra210-adma.c
> +++ b/drivers/dma/tegra210-adma.c

[ ... ]

> @@ -1127,26 +1160,33 @@ static int tegra_adma_probe(struct platform_device *pdev)
>  					cdata->global_ch_config_base + (4 * i);
>  		}
>  
> -		tdc->irq = of_irq_get(pdev->dev.of_node, i);
> -		if (tdc->irq <= 0) {
> -			ret = tdc->irq ?: -ENXIO;
> -			goto irq_dispose;
> -		}
> +		ret = of_irq_get(pdev->dev.of_node, i);
> +		if (ret <= 0)
> +			return dev_err_probe(&pdev->dev, ret ?: -ENXIO,
> +					     "failed to get IRQ for channel %d\n", i);
> +		tdc->irq = ret;
>  
>  		vchan_init(&tdc->vc, &tdma->dma_dev);
>  		tdc->vc.desc_free = tegra_adma_desc_free;
>  		tdc->tdma = tdma;
>  	}
>  
> -	pm_runtime_enable(&pdev->dev);
> +	ret = devm_pm_runtime_enable(&pdev->dev);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "failed to enable runtime PM\n");
> +
> +	ACQUIRE(pm_runtime_active_try_enabled, pm)(&pdev->dev);
>  
> -	ret = pm_runtime_resume_and_get(&pdev->dev);
> +	ret = ACQUIRE_ERR(pm_runtime_active_try_enabled, &pm);
>  	if (ret < 0)
> -		goto rpm_disable;
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "runtime PM resume failed\n");
>  
>  	ret = tegra_adma_init(tdma);
>  	if (ret)
> -		goto rpm_put;
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "failed to initialize ADMA\n");

Does using a scoped guard here introduce a race condition on error paths?

The scoped guard drops its reference using an asynchronous pm_runtime_put()
when it goes out of scope on return. If the probe fails after this point,
the guard drops the reference asynchronously, and then the devres framework
executes pm_runtime_disable() to tear down the environment.

Calling pm_runtime_disable() immediately after an async pm_runtime_put() can
intercept and cancel the pending async suspend work, which leaves the
hardware powered on in an incorrect power state. The original code avoided
this by explicitly calling pm_runtime_put_sync() before disabling runtime PM.

Also, does this change the resource teardown order?

The scoped guard drops the PM reference immediately when the function
returns, which happens before devres executes
dmaenginem_async_device_unregister(). This causes the DMA engine to be
unregistered after the power reference has already been dropped,
breaking the LIFO teardown order from the original code where the PM
put happened after DMA engine unregistration.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512092508.1406119-1-sheetal@nvidia.com?part=1