All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sheetal <sheetal@nvidia.com>
To: Vinod Koul <vkoul@kernel.org>, Laxman Dewangan <ldewangan@nvidia.com>
Cc: Jon Hunter <jonathanh@nvidia.com>, Frank Li <Frank.Li@kernel.org>,
	"Thierry Reding" <thierry.reding@kernel.org>,
	Sameer Pujar <spujar@nvidia.com>,
	"Mohan Kumar" <mkumard@nvidia.com>, <dmaengine@vger.kernel.org>,
	<linux-tegra@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	Sheetal <sheetal@nvidia.com>
Subject: [PATCH v6] dmaengine: tegra210-adma: Add error logging on failure paths
Date: Tue, 12 May 2026 09:25:08 +0000	[thread overview]
Message-ID: <20260512092508.1406119-1-sheetal@nvidia.com> (raw)

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

             reply	other threads:[~2026-05-12  9:25 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-12  9:25 Sheetal [this message]
2026-05-12 20:07 ` [PATCH v6] dmaengine: tegra210-adma: Add error logging on failure paths Frank Li
2026-06-08 10:29   ` Jon Hunter
2026-05-13 20:14 ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260512092508.1406119-1-sheetal@nvidia.com \
    --to=sheetal@nvidia.com \
    --cc=Frank.Li@kernel.org \
    --cc=dmaengine@vger.kernel.org \
    --cc=jonathanh@nvidia.com \
    --cc=ldewangan@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=mkumard@nvidia.com \
    --cc=spujar@nvidia.com \
    --cc=thierry.reding@kernel.org \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.