Messages in this thread |  | | Date | Tue, 5 May 2026 13:08:35 +0530 | | Subject | Re: [PATCH v4] dmaengine: tegra210-adma: Add error logging on failure paths | | From | Mohan Kumar D <> |
| |
On 13-04-2026 12:15, 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. > > Signed-off-by: Sheetal <sheetal@nvidia.com> > --- > Changes in v4: > - Use return dev_err_probe() pattern in probe function instead of > dev_err() + return > - Use dev_err_probe consistently for all error paths in probe > - Convert existing dev_err in probe to dev_err_probe > > Changes in v3: > - Cast page_no to (unsigned long long) for %llu to fix -Wformat > warning on 32-bit builds where resource_size_t is unsigned int > - Remove redundant dev_err for devm_ioremap_resource failures since > the API already logs errors internally. > > Changes in v2: > - Fix format specifier for size_t: use %zu instead of %u for > desc->num_periods to resolve -Wformat warning with W=1 > > drivers/dma/tegra210-adma.c | 52 +++++++++++++++++++++++++------------ > 1 file changed, 36 insertions(+), 16 deletions(-) > > diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c > index 14e0c408ed1e..1ced5b37d0d8 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: > @@ -1029,8 +1040,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 +1067,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 +1076,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 +1093,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 +1102,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,8 +1119,8 @@ 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); > @@ -1130,6 +1145,7 @@ static int tegra_adma_probe(struct platform_device *pdev) > tdc->irq = of_irq_get(pdev->dev.of_node, i); > if (tdc->irq <= 0) { > ret = tdc->irq ?: -ENXIO; > + dev_err_probe(&pdev->dev, ret, "failed to get IRQ for channel %d\n", i); > goto irq_dispose; > } > > @@ -1141,12 +1157,16 @@ static int tegra_adma_probe(struct platform_device *pdev) > pm_runtime_enable(&pdev->dev); > > ret = pm_runtime_resume_and_get(&pdev->dev); > - if (ret < 0) > + if (ret < 0) { > + dev_err_probe(&pdev->dev, ret, "runtime PM resume failed\n"); > goto rpm_disable; > + } > > ret = tegra_adma_init(tdma); > - if (ret) > + if (ret) { > + dev_err_probe(&pdev->dev, ret, "failed to initialize ADMA\n"); > goto rpm_put; > + } > > dma_cap_set(DMA_SLAVE, tdma->dma_dev.cap_mask); > dma_cap_set(DMA_PRIVATE, tdma->dma_dev.cap_mask); > @@ -1172,14 +1192,14 @@ static int tegra_adma_probe(struct platform_device *pdev) > > ret = dma_async_device_register(&tdma->dma_dev); > if (ret < 0) { > - dev_err(&pdev->dev, "ADMA registration failed: %d\n", ret); > + dev_err_probe(&pdev->dev, ret, "ADMA registration failed\n"); > goto rpm_put; > } > > 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); > + dev_err_probe(&pdev->dev, ret, "ADMA OF registration failed\n"); > goto dma_remove; > } >
Reviewed-by: Mohan Kumar <mkumard@nvidia.com>
|  |