Re: [PATCH] mmc: pxamci: Fix passing NULL to PTR_ERR() in pxamci_probe()

Uwe Kleine-König
2025-10-10T09:59:27Z

[-- Attachment #1: Type: text/plain, Size: 2142 bytes --]

Hello Rakuram,

On Thu, Oct 09, 2025 at 08:57:38PM +0530, Rakuram Eswaran wrote:
> Your suggestion makes perfect sense — since host is devm-managed,
> explicitly assigning its members to NULL has no effect.
> I’ll remove those two redundant lines in v2 as you suggested.
>
> I had one small clarification regarding the remaining host->dma_chan_tx = NULL;
> in the TX DMA error path. Since that branch uses goto out,
> the cleanup section below may call dma_release_channel() on both RX
> and TX pointers. Setting TX to NULL there seems like a defensive step
> to avoid accidentally passing an ERR_PTR() to dma_release_channel()
> — is that understanding correct?

Ah right, so either keep host->dma_chan_tx = NULL or improve the error
handling like:

diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
index 26d03352af63..e5068cc55fb2 100644
--- a/drivers/mmc/host/pxamci.c
+++ b/drivers/mmc/host/pxamci.c
@@ -715,7 +715,7 @@ static int pxamci_probe(struct platform_device *pdev)
dev_err(dev, "unable to request tx dma channel\n");
ret = PTR_ERR(host->dma_chan_tx);
host->dma_chan_tx = NULL;
- goto out;
+ goto out_dma_tx;
}

if (host->pdata) {
@@ -765,10 +765,11 @@ static int pxamci_probe(struct platform_device *pdev)
return 0;

out:
- if (host->dma_chan_rx)
- dma_release_channel(host->dma_chan_rx);
if (host->dma_chan_tx)
dma_release_channel(host->dma_chan_tx);
+out_dma_tx:
+ if (host->dma_chan_rx)
+ dma_release_channel(host->dma_chan_rx);
return ret;
}

> Also, I noticed that in the build configuration downloaded from the LKP report
> link (CONFIG_DMA_ENGINE isn’t defined), the kernel uses the stub inline
> version of dma_release_channel() from include/linux/dmaengine.h,
> which becomes a no-op.
>
> From what I understand, when the DMA engine framework isn’t enabled,
> these APIs compile as no-ops through their inline stubs.
> Please correct me if I’m misunderstanding how this works.
>
> Please let me know if this reasoning aligns with what you had in mind.

Sounds right.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]