DMA Engine development
 help / color / mirror / Atom feed
* [PATCH v6] dmaengine: tegra210-adma: Add error logging on failure paths
@ 2026-05-12  9:25 Sheetal
  2026-05-12 20:07 ` Frank Li
  2026-05-13 20:14 ` sashiko-bot
  0 siblings, 2 replies; 3+ messages in thread
From: Sheetal @ 2026-05-12  9:25 UTC (permalink / raw)
  To: Vinod Koul, Laxman Dewangan
  Cc: Jon Hunter, Frank Li, Thierry Reding, Sameer Pujar, Mohan Kumar,
	dmaengine, linux-tegra, linux-kernel, Sheetal

Add dev_err/dev_err_probe logging across failure paths to improve
debuggability of DMA errors during runtime and probe.

Use return dev_err_probe() pattern consistently in the probe function,
and dev_err in non-probe functions. Also convert existing dev_err calls
in probe to dev_err_probe for consistency.

As part of the probe-path cleanup, use dmaenginem_async_device_register()
and devm_pm_runtime_enable(), and add tegra_adma_irq_dispose() for managed
IRQ mapping cleanup. These managed helpers remove the probe error unwind
labels while keeping the remove path minimal.

Signed-off-by: Sheetal <sheetal@nvidia.com>
---
Changes in v6:
- Document managed DMA registration, runtime PM enablement, and IRQ cleanup
  in the commit message.

 drivers/dma/tegra210-adma.c | 123 ++++++++++++++++++++++++--------------------
 1 file changed, 68 insertions(+), 55 deletions(-)

diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
index 14e0c408ed1e..6987bf6ec648 100644
--- a/drivers/dma/tegra210-adma.c
+++ b/drivers/dma/tegra210-adma.c
@@ -335,8 +335,16 @@ static int tegra_adma_request_alloc(struct tegra_adma_chan *tdc,
 	struct tegra_adma *tdma = tdc->tdma;
 	unsigned int sreq_index = tdc->sreq_index;
 
-	if (tdc->sreq_reserved)
-		return tdc->sreq_dir == direction ? 0 : -EINVAL;
+	if (tdc->sreq_reserved) {
+		if (tdc->sreq_dir != direction) {
+			dev_err(tdma->dev,
+				"DMA request direction mismatch: reserved=%s, requested=%s\n",
+				dmaengine_get_direction_text(tdc->sreq_dir),
+				dmaengine_get_direction_text(direction));
+			return -EINVAL;
+		}
+		return 0;
+	}
 
 	if (sreq_index > tdma->cdata->ch_req_max) {
 		dev_err(tdma->dev, "invalid DMA request\n");
@@ -665,8 +673,11 @@ static int tegra_adma_set_xfer_params(struct tegra_adma_chan *tdc,
 	const struct tegra_adma_chip_data *cdata = tdc->tdma->cdata;
 	unsigned int burst_size, adma_dir, fifo_size_shift;
 
-	if (desc->num_periods > ADMA_CH_CONFIG_MAX_BUFS)
+	if (desc->num_periods > ADMA_CH_CONFIG_MAX_BUFS) {
+		dev_err(tdc2dev(tdc), "invalid DMA periods %zu (max %u)\n",
+			desc->num_periods, ADMA_CH_CONFIG_MAX_BUFS);
 		return -EINVAL;
+	}
 
 	switch (direction) {
 	case DMA_MEM_TO_DEV:
@@ -1020,6 +1031,17 @@ static const struct of_device_id tegra_adma_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, tegra_adma_of_match);
 
+static void tegra_adma_irq_dispose(void *data)
+{
+	struct tegra_adma *tdma = data;
+	int i;
+
+	for (i = 0; i < tdma->nr_channels; ++i) {
+		if (tdma->channels[i].irq > 0)
+			irq_dispose_mapping(tdma->channels[i].irq);
+	}
+}
+
 static int tegra_adma_probe(struct platform_device *pdev)
 {
 	const struct tegra_adma_chip_data *cdata;
@@ -1029,8 +1051,8 @@ static int tegra_adma_probe(struct platform_device *pdev)
 
 	cdata = of_device_get_match_data(&pdev->dev);
 	if (!cdata) {
-		dev_err(&pdev->dev, "device match data not found\n");
-		return -ENODEV;
+		return dev_err_probe(&pdev->dev, -ENODEV,
+				     "device match data not found\n");
 	}
 
 	tdma = devm_kzalloc(&pdev->dev,
@@ -1056,7 +1078,8 @@ static int tegra_adma_probe(struct platform_device *pdev)
 			unsigned int ch_base_offset;
 
 			if (res_page->start < res_base->start)
-				return -EINVAL;
+				return dev_err_probe(&pdev->dev, -EINVAL,
+						     "invalid page/global resource order\n");
 			page_offset = res_page->start - res_base->start;
 			ch_base_offset = cdata->ch_base_offset;
 			if (!ch_base_offset)
@@ -1064,7 +1087,9 @@ static int tegra_adma_probe(struct platform_device *pdev)
 
 			page_no = div_u64(page_offset, ch_base_offset);
 			if (!page_no || page_no > INT_MAX)
-				return -EINVAL;
+				return dev_err_probe(&pdev->dev, -EINVAL,
+						     "invalid page number %llu\n",
+						     (unsigned long long)page_no);
 
 			tdma->ch_page_no = page_no - 1;
 			tdma->base_addr = devm_ioremap_resource(&pdev->dev, res_base);
@@ -1079,7 +1104,8 @@ static int tegra_adma_probe(struct platform_device *pdev)
 			if (IS_ERR(tdma->base_addr))
 				return PTR_ERR(tdma->base_addr);
 		} else {
-			return -ENODEV;
+			return dev_err_probe(&pdev->dev, -ENODEV,
+					     "failed to get memory resource\n");
 		}
 
 		tdma->ch_base_addr = tdma->base_addr + cdata->ch_base_offset;
@@ -1087,8 +1113,8 @@ static int tegra_adma_probe(struct platform_device *pdev)
 
 	tdma->ahub_clk = devm_clk_get(&pdev->dev, "d_audio");
 	if (IS_ERR(tdma->ahub_clk)) {
-		dev_err(&pdev->dev, "Error: Missing ahub controller clock\n");
-		return PTR_ERR(tdma->ahub_clk);
+		return dev_err_probe(&pdev->dev, PTR_ERR(tdma->ahub_clk),
+				     "failed to get ahub clock\n");
 	}
 
 	tdma->dma_chan_mask = devm_kzalloc(&pdev->dev,
@@ -1104,11 +1130,18 @@ static int tegra_adma_probe(struct platform_device *pdev)
 					 (u32 *)tdma->dma_chan_mask,
 					 BITS_TO_U32(tdma->nr_channels));
 	if (ret < 0 && (ret != -EINVAL)) {
-		dev_err(&pdev->dev, "dma-channel-mask is not complete.\n");
-		return ret;
+		return dev_err_probe(&pdev->dev, ret,
+				     "dma-channel-mask is not complete.\n");
 	}
 
 	INIT_LIST_HEAD(&tdma->dma_dev.channels);
+
+	ret = devm_add_action_or_reset(&pdev->dev, tegra_adma_irq_dispose,
+				       tdma);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "failed to add IRQ cleanup action\n");
+
 	for (i = 0; i < tdma->nr_channels; i++) {
 		struct tegra_adma_chan *tdc = &tdma->channels[i];
 
@@ -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");
 
 	dma_cap_set(DMA_SLAVE, tdma->dma_dev.cap_mask);
 	dma_cap_set(DMA_PRIVATE, tdma->dma_dev.cap_mask);
@@ -1170,53 +1210,26 @@ static int tegra_adma_probe(struct platform_device *pdev)
 	tdma->dma_dev.device_pause = tegra_adma_pause;
 	tdma->dma_dev.device_resume = tegra_adma_resume;
 
-	ret = dma_async_device_register(&tdma->dma_dev);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "ADMA registration failed: %d\n", ret);
-		goto rpm_put;
-	}
+	ret = dmaenginem_async_device_register(&tdma->dma_dev);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "ADMA registration failed\n");
 
 	ret = of_dma_controller_register(pdev->dev.of_node,
 					 tegra_dma_of_xlate, tdma);
-	if (ret < 0) {
-		dev_err(&pdev->dev, "ADMA OF registration failed %d\n", ret);
-		goto dma_remove;
-	}
-
-	pm_runtime_put(&pdev->dev);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "ADMA OF registration failed\n");
 
 	dev_info(&pdev->dev, "Tegra210 ADMA driver registered %d channels\n",
 		 tdma->nr_channels);
 
 	return 0;
-
-dma_remove:
-	dma_async_device_unregister(&tdma->dma_dev);
-rpm_put:
-	pm_runtime_put_sync(&pdev->dev);
-rpm_disable:
-	pm_runtime_disable(&pdev->dev);
-irq_dispose:
-	while (--i >= 0)
-		irq_dispose_mapping(tdma->channels[i].irq);
-
-	return ret;
 }
 
 static void tegra_adma_remove(struct platform_device *pdev)
 {
-	struct tegra_adma *tdma = platform_get_drvdata(pdev);
-	int i;
-
 	of_dma_controller_free(pdev->dev.of_node);
-	dma_async_device_unregister(&tdma->dma_dev);
-
-	for (i = 0; i < tdma->nr_channels; ++i) {
-		if (tdma->channels[i].irq)
-			irq_dispose_mapping(tdma->channels[i].irq);
-	}
-
-	pm_runtime_disable(&pdev->dev);
 }
 
 static const struct dev_pm_ops tegra_adma_dev_pm_ops = {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v6] dmaengine: tegra210-adma: Add error logging on failure paths
  2026-05-12  9:25 [PATCH v6] dmaengine: tegra210-adma: Add error logging on failure paths Sheetal
@ 2026-05-12 20:07 ` Frank Li
  2026-05-13 20:14 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: Frank Li @ 2026-05-12 20:07 UTC (permalink / raw)
  To: Sheetal
  Cc: Vinod Koul, Laxman Dewangan, Jon Hunter, Frank Li, Thierry Reding,
	Sameer Pujar, Mohan Kumar, dmaengine, linux-tegra, linux-kernel

On Tue, May 12, 2026 at 09:25:08AM +0000, Sheetal wrote:
> Add dev_err/dev_err_probe logging across failure paths to improve
> debuggability of DMA errors during runtime and probe.
>
> Use return dev_err_probe() pattern consistently in the probe function,
> and dev_err in non-probe functions. Also convert existing dev_err calls
> in probe to dev_err_probe for consistency.
>
> As part of the probe-path cleanup, use dmaenginem_async_device_register()
> and devm_pm_runtime_enable(), and add tegra_adma_irq_dispose() for managed
> IRQ mapping cleanup. These managed helpers remove the probe error unwind
> labels while keeping the remove path minimal.
>
> Signed-off-by: Sheetal <sheetal@nvidia.com>
> ---

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

> Changes in v6:
> - Document managed DMA registration, runtime PM enablement, and IRQ cleanup
>   in the commit message.
>
>  drivers/dma/tegra210-adma.c | 123 ++++++++++++++++++++++++--------------------
>  1 file changed, 68 insertions(+), 55 deletions(-)
>
> diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
> index 14e0c408ed1e..6987bf6ec648 100644
> --- a/drivers/dma/tegra210-adma.c
> +++ b/drivers/dma/tegra210-adma.c
> @@ -335,8 +335,16 @@ static int tegra_adma_request_alloc(struct tegra_adma_chan *tdc,
>  	struct tegra_adma *tdma = tdc->tdma;
>  	unsigned int sreq_index = tdc->sreq_index;
>
> -	if (tdc->sreq_reserved)
> -		return tdc->sreq_dir == direction ? 0 : -EINVAL;
> +	if (tdc->sreq_reserved) {
> +		if (tdc->sreq_dir != direction) {
> +			dev_err(tdma->dev,
> +				"DMA request direction mismatch: reserved=%s, requested=%s\n",
> +				dmaengine_get_direction_text(tdc->sreq_dir),
> +				dmaengine_get_direction_text(direction));
> +			return -EINVAL;
> +		}
> +		return 0;
> +	}
>
>  	if (sreq_index > tdma->cdata->ch_req_max) {
>  		dev_err(tdma->dev, "invalid DMA request\n");
> @@ -665,8 +673,11 @@ static int tegra_adma_set_xfer_params(struct tegra_adma_chan *tdc,
>  	const struct tegra_adma_chip_data *cdata = tdc->tdma->cdata;
>  	unsigned int burst_size, adma_dir, fifo_size_shift;
>
> -	if (desc->num_periods > ADMA_CH_CONFIG_MAX_BUFS)
> +	if (desc->num_periods > ADMA_CH_CONFIG_MAX_BUFS) {
> +		dev_err(tdc2dev(tdc), "invalid DMA periods %zu (max %u)\n",
> +			desc->num_periods, ADMA_CH_CONFIG_MAX_BUFS);
>  		return -EINVAL;
> +	}
>
>  	switch (direction) {
>  	case DMA_MEM_TO_DEV:
> @@ -1020,6 +1031,17 @@ static const struct of_device_id tegra_adma_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, tegra_adma_of_match);
>
> +static void tegra_adma_irq_dispose(void *data)
> +{
> +	struct tegra_adma *tdma = data;
> +	int i;
> +
> +	for (i = 0; i < tdma->nr_channels; ++i) {
> +		if (tdma->channels[i].irq > 0)
> +			irq_dispose_mapping(tdma->channels[i].irq);
> +	}
> +}
> +
>  static int tegra_adma_probe(struct platform_device *pdev)
>  {
>  	const struct tegra_adma_chip_data *cdata;
> @@ -1029,8 +1051,8 @@ static int tegra_adma_probe(struct platform_device *pdev)
>
>  	cdata = of_device_get_match_data(&pdev->dev);
>  	if (!cdata) {
> -		dev_err(&pdev->dev, "device match data not found\n");
> -		return -ENODEV;
> +		return dev_err_probe(&pdev->dev, -ENODEV,
> +				     "device match data not found\n");
>  	}
>
>  	tdma = devm_kzalloc(&pdev->dev,
> @@ -1056,7 +1078,8 @@ static int tegra_adma_probe(struct platform_device *pdev)
>  			unsigned int ch_base_offset;
>
>  			if (res_page->start < res_base->start)
> -				return -EINVAL;
> +				return dev_err_probe(&pdev->dev, -EINVAL,
> +						     "invalid page/global resource order\n");
>  			page_offset = res_page->start - res_base->start;
>  			ch_base_offset = cdata->ch_base_offset;
>  			if (!ch_base_offset)
> @@ -1064,7 +1087,9 @@ static int tegra_adma_probe(struct platform_device *pdev)
>
>  			page_no = div_u64(page_offset, ch_base_offset);
>  			if (!page_no || page_no > INT_MAX)
> -				return -EINVAL;
> +				return dev_err_probe(&pdev->dev, -EINVAL,
> +						     "invalid page number %llu\n",
> +						     (unsigned long long)page_no);
>
>  			tdma->ch_page_no = page_no - 1;
>  			tdma->base_addr = devm_ioremap_resource(&pdev->dev, res_base);
> @@ -1079,7 +1104,8 @@ static int tegra_adma_probe(struct platform_device *pdev)
>  			if (IS_ERR(tdma->base_addr))
>  				return PTR_ERR(tdma->base_addr);
>  		} else {
> -			return -ENODEV;
> +			return dev_err_probe(&pdev->dev, -ENODEV,
> +					     "failed to get memory resource\n");
>  		}
>
>  		tdma->ch_base_addr = tdma->base_addr + cdata->ch_base_offset;
> @@ -1087,8 +1113,8 @@ static int tegra_adma_probe(struct platform_device *pdev)
>
>  	tdma->ahub_clk = devm_clk_get(&pdev->dev, "d_audio");
>  	if (IS_ERR(tdma->ahub_clk)) {
> -		dev_err(&pdev->dev, "Error: Missing ahub controller clock\n");
> -		return PTR_ERR(tdma->ahub_clk);
> +		return dev_err_probe(&pdev->dev, PTR_ERR(tdma->ahub_clk),
> +				     "failed to get ahub clock\n");
>  	}
>
>  	tdma->dma_chan_mask = devm_kzalloc(&pdev->dev,
> @@ -1104,11 +1130,18 @@ static int tegra_adma_probe(struct platform_device *pdev)
>  					 (u32 *)tdma->dma_chan_mask,
>  					 BITS_TO_U32(tdma->nr_channels));
>  	if (ret < 0 && (ret != -EINVAL)) {
> -		dev_err(&pdev->dev, "dma-channel-mask is not complete.\n");
> -		return ret;
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "dma-channel-mask is not complete.\n");
>  	}
>
>  	INIT_LIST_HEAD(&tdma->dma_dev.channels);
> +
> +	ret = devm_add_action_or_reset(&pdev->dev, tegra_adma_irq_dispose,
> +				       tdma);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "failed to add IRQ cleanup action\n");
> +
>  	for (i = 0; i < tdma->nr_channels; i++) {
>  		struct tegra_adma_chan *tdc = &tdma->channels[i];
>
> @@ -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");
>
>  	dma_cap_set(DMA_SLAVE, tdma->dma_dev.cap_mask);
>  	dma_cap_set(DMA_PRIVATE, tdma->dma_dev.cap_mask);
> @@ -1170,53 +1210,26 @@ static int tegra_adma_probe(struct platform_device *pdev)
>  	tdma->dma_dev.device_pause = tegra_adma_pause;
>  	tdma->dma_dev.device_resume = tegra_adma_resume;
>
> -	ret = dma_async_device_register(&tdma->dma_dev);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "ADMA registration failed: %d\n", ret);
> -		goto rpm_put;
> -	}
> +	ret = dmaenginem_async_device_register(&tdma->dma_dev);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "ADMA registration failed\n");
>
>  	ret = of_dma_controller_register(pdev->dev.of_node,
>  					 tegra_dma_of_xlate, tdma);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "ADMA OF registration failed %d\n", ret);
> -		goto dma_remove;
> -	}
> -
> -	pm_runtime_put(&pdev->dev);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret,
> +				     "ADMA OF registration failed\n");
>
>  	dev_info(&pdev->dev, "Tegra210 ADMA driver registered %d channels\n",
>  		 tdma->nr_channels);
>
>  	return 0;
> -
> -dma_remove:
> -	dma_async_device_unregister(&tdma->dma_dev);
> -rpm_put:
> -	pm_runtime_put_sync(&pdev->dev);
> -rpm_disable:
> -	pm_runtime_disable(&pdev->dev);
> -irq_dispose:
> -	while (--i >= 0)
> -		irq_dispose_mapping(tdma->channels[i].irq);
> -
> -	return ret;
>  }
>
>  static void tegra_adma_remove(struct platform_device *pdev)
>  {
> -	struct tegra_adma *tdma = platform_get_drvdata(pdev);
> -	int i;
> -
>  	of_dma_controller_free(pdev->dev.of_node);
> -	dma_async_device_unregister(&tdma->dma_dev);
> -
> -	for (i = 0; i < tdma->nr_channels; ++i) {
> -		if (tdma->channels[i].irq)
> -			irq_dispose_mapping(tdma->channels[i].irq);
> -	}
> -
> -	pm_runtime_disable(&pdev->dev);
>  }
>
>  static const struct dev_pm_ops tegra_adma_dev_pm_ops = {
> --
> 2.17.1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v6] dmaengine: tegra210-adma: Add error logging on failure paths
  2026-05-12  9:25 [PATCH v6] dmaengine: tegra210-adma: Add error logging on failure paths Sheetal
  2026-05-12 20:07 ` Frank Li
@ 2026-05-13 20:14 ` sashiko-bot
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-05-13 20:14 UTC (permalink / raw)
  To: Sheetal; +Cc: Frank.Li, vkoul, dmaengine

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-13 20:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-12  9:25 [PATCH v6] dmaengine: tegra210-adma: Add error logging on failure paths Sheetal
2026-05-12 20:07 ` Frank Li
2026-05-13 20:14 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox