mmc: pxamci: Fix passing NULL to PTR_ERR() in pxamci_probe()

Smatch reported:
drivers/mmc/host/pxamci.c:709 pxamci_probe() warn: passing zero to 'PTR_ERR'

Case 1:
When dma_request_chan() fails, host->dma_chan_rx is an ERR_PTR(), but it is reset to NULL before using PTR_ERR(), resulting in PTR_ERR(0). This mistakenly returns 0 instead of the real error code.

Case 2:
When devm_clk_get() fails, host->clk is an ERR_PTR() resulting in the similar issue like case 1.

Store the error code before nullifying the pointers in both the cases.

Reported-by kernel test robot <[email protected]>

Reported-by Dan Carpenter <[email protected]>

Closes https://lore.kernel.org/r/[email protected]/ open here

Fixes 58c40f3faf742c ("mmc: pxamci: Use devm_mmc_alloc_host() helper")

Signed-off-by Rakuram Eswaran <[email protected]>

---

Smatch reported:
drivers/mmc/host/pxamci.c:709 pxamci_probe() warn: passing zero to 'PTR_ERR'

Case 1:
When dma_request_chan() fails, host->dma_chan_rx is an ERR_PTR(), but it is reset to NULL before using PTR_ERR(), resulting in PTR_ERR(0). This mistakenly returns 0 instead of the real error code.

Case 2:
When devm_clk_get() fails, host->clk is an ERR_PTR() resulting in the similar issue like case 1.

Store the error code before nullifying the pointers in both the cases.

Reported-by kernel test robot <[email protected]>

Reported-by Dan Carpenter <[email protected]>

Closes https://lore.kernel.org/r/[email protected]/ open here

Fixes 58c40f3faf742c ("mmc: pxamci: Use devm_mmc_alloc_host() helper")

Signed-off-by Rakuram Eswaran <[email protected]>

---

Build and Analysis:
This patch was compiled against the configuration file reported by 0day CI in the above link (config: s390-randconfig-r071-20251004) using s390x-linux-gnu-gcc (Ubuntu 14.2.0-19ubuntu2) 14.2.0.

Static analysis was performed with Smatch to ensure the reported warning no longer reproduces after applying this fix.

Command used for verification:

   ARCH=s390 CROSS_COMPILE=s390x-linux-gnu- \
   ~/project/smatch/smatch_scripts/kchecker ./drivers/mmc/host/pxamci.c

  drivers/mmc/host/pxamci.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)

drivers/mmc/host/pxamci.c +2 -1

index 26d03352af63..4fab693d3b32 100644
@@ -653,8 +653,9 @@ static int pxamci_probe(struct platform_device *pdev)
 
  	host->clk = devm_clk_get(dev, NULL);
  	if (IS_ERR(host->clk)) {
+		ret = PTR_ERR(host->clk);
  		host->clk = NULL;
-		return PTR_ERR(host->clk);
+		return ret;
  	}
 
  	host->clkrate = clk_get_rate(host->clk);

Smatch reported:
drivers/mmc/host/pxamci.c:709 pxamci_probe() warn: passing zero to 'PTR_ERR'

Case 1:
When dma_request_chan() fails, host->dma_chan_rx is an ERR_PTR(), but it is reset to NULL before using PTR_ERR(), resulting in PTR_ERR(0). This mistakenly returns 0 instead of the real error code.

Case 2:
When devm_clk_get() fails, host->clk is an ERR_PTR() resulting in the similar issue like case 1.

Store the error code before nullifying the pointers in both the cases.

Reported-by kernel test robot <[email protected]>

Reported-by Dan Carpenter <[email protected]>

Closes https://lore.kernel.org/r/[email protected]/ open here

Fixes 58c40f3faf742c ("mmc: pxamci: Use devm_mmc_alloc_host() helper")

Signed-off-by Rakuram Eswaran <[email protected]>

---

Build and Analysis:
This patch was compiled against the configuration file reported by 0day CI in the above link (config: s390-randconfig-r071-20251004) using s390x-linux-gnu-gcc (Ubuntu 14.2.0-19ubuntu2) 14.2.0.

Static analysis was performed with Smatch to ensure the reported warning no longer reproduces after applying this fix.

Command used for verification:

   ARCH=s390 CROSS_COMPILE=s390x-linux-gnu- \
   ~/project/smatch/smatch_scripts/kchecker ./drivers/mmc/host/pxamci.c

  drivers/mmc/host/pxamci.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)

drivers/mmc/host/pxamci.c +2 -1

index 26d03352af63..4fab693d3b32 100644
@@ -653,8 +653,9 @@ static int pxamci_probe(struct platform_device *pdev)
 
  	host->clk = devm_clk_get(dev, NULL);
  	if (IS_ERR(host->clk)) {
+		ret = PTR_ERR(host->clk);
  		host->clk = NULL;
-		return PTR_ERR(host->clk);
+		return ret;
  	}
 
  	host->clkrate = clk_get_rate(host->clk);

Smatch reported:
drivers/mmc/host/pxamci.c:709 pxamci_probe() warn: passing zero to 'PTR_ERR'

Case 1:
When dma_request_chan() fails, host->dma_chan_rx is an ERR_PTR(), but it is reset to NULL before using PTR_ERR(), resulting in PTR_ERR(0). This mistakenly returns 0 instead of the real error code.

Case 2:
When devm_clk_get() fails, host->clk is an ERR_PTR() resulting in the similar issue like case 1.

Store the error code before nullifying the pointers in both the cases.

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?

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.

Hello Rakuram,

Quoted 10 lines · Rakuram Eswaran

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:

drivers/mmc/host/pxamci.c +1 -1

index 26d03352af63..e5068cc55fb2 100644
@@ -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;
 

Quoted 11 lines

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

Quoted 37 lines · Uwe Kleine-König

Hello Rakuram,

Quoted 10 lines · Rakuram Eswaran

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:

drivers/mmc/host/pxamci.c +2 -3

index 26d03352af63..e5068cc55fb2 100644
@@ -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:

drivers/mmc/host/pxamci.c static int pxamci_probe(struct platform_device *pdev) · line 774

Quoted 4 lines

drivers/mmc/host/pxamci.c +2 -0

+	if (host->dma_chan_rx)
+		dma_release_channel(host->dma_chan_rx);
  	return ret;
  }

I do not see the need for this code change. "if (host->dma_chan_tx)" will skip "dma_release_channel(host->dma_chan_tx)" since dma_chan_tx is already NULL. This code change does not add anything.

Quoted 4 lines

I do not see the need for this code change. "if (host->dma_chan_tx)" will skip "dma_release_channel(host->dma_chan_tx)" since dma_chan_tx is already NULL. This code change does not add anything.

Yes, stand alone this change doesn't make sense, but if we want to drop

        host->dma_chan_tx = NULL

in the error path above, this change is needed. Maybe then even

        if (host->dma_chan_rx)

and

        if (host->dma_chan_rx)

can be dropped.

I am not sure that sounds right. Looking at the code for __devm_clk_get(), if devres_alloc() fails, it returns -ENOMEM. If any of the other steps after a successful devres_alloc() fail, code goes through possibly clk_put() if needed and then devres_free(). So the resources are already freed at this point before the return to pxamci_probe(). The only thing left to do is to set host->clk to NULL since it would be set to an error pointer at this point.

Quoted 11 lines

Quoted 4 lines

I do not see the need for this code change. "if (host->dma_chan_tx)" will skip "dma_release_channel(host->dma_chan_tx)" since dma_chan_tx is already NULL. This code change does not add anything.

Yes, stand alone this change doesn't make sense, but if we want to drop

        host->dma_chan_tx = NULL

in the error path above, this change is needed. Maybe then even

Quoted 7 lines

        if (host->dma_chan_rx)

and

        if (host->dma_chan_rx)

can be dropped.

Hello Uwe,

I had one quick follow-up before sending v2.

Regarding the devm_clk_get() error path —
you mentioned that setting host->clk = NULL; is redundant since host is devm-managed and the function returns immediately afterward.

Quoted 7 lines

I am not sure that sounds right. Looking at the code for __devm_clk_get(), if devres_alloc() fails, it returns -ENOMEM. If any of the other steps after a successful devres_alloc() fail, code goes through possibly clk_put() if needed and then devres_free(). So the resources are already freed at this point before the return to pxamci_probe(). The only thing left to do is to set host->clk to NULL since it would be set to an error pointer at this point.

Khalid pointed out that when __devm_clk_get() fails after allocating a devres entry, the internal cleanup (clk_put() + devres_free()) ensures resources are released, but host->clk would still hold an ERR_PTR() value at that point.

His suggestion was that setting it to NULL might be a harmless defensive step to avoid any accidental later dereference.

For now, I have dropped the redundant NULL assignment from host->dma_chan_rx = NULL and directly returning the ERR_PTR instead of storing in a return variable.

Below I have appended proposed changes for v2.

drivers/mmc/host/pxamci.c +2 -2

index 26d03352af63..eb46a4861dbe 100644
@@ -653,8 +653,9 @@ static int pxamci_probe(struct platform_device *pdev)
 
 	host->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(host->clk)) {
+		ret = PTR_ERR(host->clk);
 		host->clk = NULL;
-		return PTR_ERR(host->clk);
+		return ret;
 	}
 
 	host->clkrate = clk_get_rate(host->clk);

@@ -705,7 +706,6 @@ static int pxamci_probe(struct platform_device *pdev)
 
 	host->dma_chan_rx = dma_request_chan(dev, "rx");
 	if (IS_ERR(host->dma_chan_rx)) {
-		host->dma_chan_rx = NULL;
 		return dev_err_probe(dev, PTR_ERR(host->dma_chan_rx),
 				     "unable to request rx dma channel\n");
 	}

Would you prefer that I:

1. Remove the host->clk = NULL; assignment for consistency (as you initially suggested), or

2. Keep it in v2 for defensive clarity, as Khalid reasoned?

I just wanted to confirm your preference before resending, to keep v2 aligned.

Hello Rakuram,

Quoted 35 lines · Rakuram Eswaran

Quoted 11 lines

Quoted 4 lines

I do not see the need for this code change. "if (host->dma_chan_tx)" will skip "dma_release_channel(host->dma_chan_tx)" since dma_chan_tx is already NULL. This code change does not add anything.

Yes, stand alone this change doesn't make sense, but if we want to drop

        host->dma_chan_tx = NULL

in the error path above, this change is needed. Maybe then even

Quoted 7 lines

        if (host->dma_chan_rx)

and

        if (host->dma_chan_rx)

can be dropped.

Hello Uwe,

I had one quick follow-up before sending v2.

Regarding the devm_clk_get() error path —
you mentioned that setting host->clk = NULL; is redundant since host is devm-managed and the function returns immediately afterward.

Quoted 7 lines

I am not sure that sounds right. Looking at the code for __devm_clk_get(), if devres_alloc() fails, it returns -ENOMEM. If any of the other steps after a successful devres_alloc() fail, code goes through possibly clk_put() if needed and then devres_free(). So the resources are already freed at this point before the return to pxamci_probe(). The only thing left to do is to set host->clk to NULL since it would be set to an error pointer at this point.

Quoted 7 lines

Khalid pointed out that when __devm_clk_get() fails after allocating a devres entry, the internal cleanup (clk_put() + devres_free()) ensures resources are released, but host->clk would still hold an ERR_PTR() value at that point.

His suggestion was that setting it to NULL might be a harmless defensive step to avoid any accidental later dereference.

Why is NULL better than an error pointer? (Spoiler: It isn't.)

Quoted 30 lines

For now, I have dropped the redundant NULL assignment from host->dma_chan_rx = NULL and directly returning the ERR_PTR instead of storing in a return variable.

Below I have appended proposed changes for v2.

drivers/mmc/host/pxamci.c +2 -2

index 26d03352af63..eb46a4861dbe 100644
@@ -653,8 +653,9 @@ static int pxamci_probe(struct platform_device *pdev)
 
  	host->clk = devm_clk_get(dev, NULL);
  	if (IS_ERR(host->clk)) {
+		ret = PTR_ERR(host->clk);
  		host->clk = NULL;
-		return PTR_ERR(host->clk);
+		return ret;
  	}
 
  	host->clkrate = clk_get_rate(host->clk);

@@ -705,7 +706,6 @@ static int pxamci_probe(struct platform_device *pdev)
 
  	host->dma_chan_rx = dma_request_chan(dev, "rx");
  	if (IS_ERR(host->dma_chan_rx)) {
-		host->dma_chan_rx = NULL;
  		return dev_err_probe(dev, PTR_ERR(host->dma_chan_rx),
  				     "unable to request rx dma channel\n");
  	}

Quoted 8 lines

Would you prefer that I:

1. Remove the host->clk = NULL; assignment for consistency (as you initially suggested), or

2. Keep it in v2 for defensive clarity, as Khalid reasoned?

I just wanted to confirm your preference before resending, to keep v2 aligned.

Note that in the end it's not me who decides, but Ulf (= mmc maintainer).

If you ask me however, I'd say the right thing to do there is like the following:

drivers/mmc/host/pxamci.c +0 -0

index 26d03352af63..ce896b3f697b 100644

drivers/mmc/host/pxamci.c +6 -5

+	/*
+	 * XXX: Note that the return value of clk_get_rate() is only valid if
+	 * the clock is enabled.
+	 */
  	host->clkrate = clk_get_rate(host->clk);
 
  	/*

@@ -703,20 +705,15 @@ static int pxamci_probe(struct platform_device *pdev)
 
  	platform_set_drvdata(pdev, mmc);
 
-	host->dma_chan_rx = dma_request_chan(dev, "rx");
-	if (IS_ERR(host->dma_chan_rx)) {
-		host->dma_chan_rx = NULL;
+	host->dma_chan_rx = devm_dma_request_chan(dev, "rx");
+	if (IS_ERR(host->dma_chan_rx))
  		return dev_err_probe(dev, PTR_ERR(host->dma_chan_rx),
  				     "unable to request rx dma channel\n");
-	}
 
-	host->dma_chan_tx = dma_request_chan(dev, "tx");

drivers/mmc/host/pxamci.c +1 -8

+
  		if (!ret)
  			host->use_ro_gpio = true;
 

@@ -759,16 +752,8 @@ static int pxamci_probe(struct platform_device *pdev)
  	if (ret) {
  		if (host->pdata && host->pdata->exit)
  			host->pdata->exit(dev, mmc);
-		goto out;
  	}
 
-	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);
  	return ret;
  }
 

Best regards
Uwe

Quoted 11 lines

drivers/mmc/host/pxamci.c +2 -5

@@ -703,20 +705,15 @@ static int pxamci_probe(struct platform_device *pdev)
  	platform_set_drvdata(pdev, mmc);
-	host->dma_chan_rx = dma_request_chan(dev, "rx");
-	if (IS_ERR(host->dma_chan_rx)) {
-		host->dma_chan_rx = NULL;
+	host->dma_chan_rx = devm_dma_request_chan(dev, "rx");
+	if (IS_ERR(host->dma_chan_rx))
  		return dev_err_probe(dev, PTR_ERR(host->dma_chan_rx),
  				     "unable to request rx dma channel\n");
-	}
-	host->dma_chan_tx = dma_request_chan(dev, "tx");

drivers/mmc/host/pxamci.c static int pxamci_probe(struct platform_device *pdev) · line 713

Quoted 10 lines

drivers/mmc/host/pxamci.c +4 -6

-	if (IS_ERR(host->dma_chan_tx)) {
-		dev_err(dev, "unable to request tx dma channel\n");
-		ret = PTR_ERR(host->dma_chan_tx);
-		host->dma_chan_tx = NULL;
-		goto out;
-	}
+	host->dma_chan_tx = devm_dma_request_chan(dev, "tx");
+	if (IS_ERR(host->dma_chan_tx))
+		return dev_err_probe(dev, PTR_ERR(host->dma_chan_tx),
+				     "unable to request tx dma channel\n");

We should still release DMA rx channel before returning here.

drivers/mmc/host/pxamci.c static int pxamci_probe(struct platform_device *pdev) · line 724

Quoted 9 lines

drivers/mmc/host/pxamci.c +1 -2

  	if (host->pdata) {
  		host->detect_delay_ms = host->pdata->detect_delay_ms;

@@ -724,25 +721,21 @@ static int pxamci_probe(struct platform_device *pdev)
  		host->power = devm_gpiod_get_optional(dev, "power", GPIOD_OUT_LOW);
  		if (IS_ERR(host->power)) {
  			ret = PTR_ERR(host->power);
-			dev_err(dev, "Failed requesting gpio_power\n");
-			goto out;
+			return dev_err_probe(dev, ret, "Failed requesting gpio_power\n");

Don't we need to release DMA Rx and Tx channels before we return from here?

drivers/mmc/host/pxamci.c static int pxamci_probe(struct platform_device *pdev) · line 730

Quoted 9 lines

drivers/mmc/host/pxamci.c +2 -4

  		}
  		/* FIXME: should we pass detection delay to debounce? */
  		ret = mmc_gpiod_request_cd(mmc, "cd", 0, false, 0);
-		if (ret && ret != -ENOENT) {
-			dev_err(dev, "Failed requesting gpio_cd\n");
-			goto out;
-		}
+		if (ret && ret != -ENOENT)
+			return dev_err_probe(dev, ret, "Failed requesting gpio_cd\n");

Same here

drivers/mmc/host/pxamci.c static int pxamci_probe(struct platform_device *pdev) · line 737

Quoted 9 lines

drivers/mmc/host/pxamci.c +2 -4

  		if (!host->pdata->gpio_card_ro_invert)
  			mmc->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
  		ret = mmc_gpiod_request_ro(mmc, "wp", 0, 0);
-		if (ret && ret != -ENOENT) {
-			dev_err(dev, "Failed requesting gpio_ro\n");
-			goto out;
-		}
+		if (ret && ret != -ENOENT)
+			return dev_err_probe(dev, ret, "Failed requesting gpio_ro\n");

and here.

Looking at Documentation/driver-api/driver-model/devres.rst, dma_request_chan() is not devres managed interface and thus will not be released automatically. Do you agree?

Quoted 60 lines · Khalid Aziz

Quoted 11 lines

drivers/mmc/host/pxamci.c +2 -5

@@ -703,20 +705,15 @@ static int pxamci_probe(struct platform_device *pdev)
   	platform_set_drvdata(pdev, mmc);
-	host->dma_chan_rx = dma_request_chan(dev, "rx");
-	if (IS_ERR(host->dma_chan_rx)) {
-		host->dma_chan_rx = NULL;
+	host->dma_chan_rx = devm_dma_request_chan(dev, "rx");
+	if (IS_ERR(host->dma_chan_rx))
   		return dev_err_probe(dev, PTR_ERR(host->dma_chan_rx),
   				     "unable to request rx dma channel\n");
-	}
-	host->dma_chan_tx = dma_request_chan(dev, "tx");

drivers/mmc/host/pxamci.c static int pxamci_probe(struct platform_device *pdev) · line 713

Quoted 10 lines

drivers/mmc/host/pxamci.c +4 -6

-	if (IS_ERR(host->dma_chan_tx)) {
-		dev_err(dev, "unable to request tx dma channel\n");
-		ret = PTR_ERR(host->dma_chan_tx);
-		host->dma_chan_tx = NULL;
-		goto out;
-	}
+	host->dma_chan_tx = devm_dma_request_chan(dev, "tx");
+	if (IS_ERR(host->dma_chan_tx))
+		return dev_err_probe(dev, PTR_ERR(host->dma_chan_tx),
+				     "unable to request tx dma channel\n");

We should still release DMA rx channel before returning here.

drivers/mmc/host/pxamci.c static int pxamci_probe(struct platform_device *pdev) · line 724

Quoted 9 lines

drivers/mmc/host/pxamci.c +1 -2

   	if (host->pdata) {
   		host->detect_delay_ms = host->pdata->detect_delay_ms;

@@ -724,25 +721,21 @@ static int pxamci_probe(struct platform_device *pdev)
   		host->power = devm_gpiod_get_optional(dev, "power", GPIOD_OUT_LOW);
   		if (IS_ERR(host->power)) {
   			ret = PTR_ERR(host->power);
-			dev_err(dev, "Failed requesting gpio_power\n");
-			goto out;
+			return dev_err_probe(dev, ret, "Failed requesting gpio_power\n");

Don't we need to release DMA Rx and Tx channels before we return from here?

drivers/mmc/host/pxamci.c static int pxamci_probe(struct platform_device *pdev) · line 730

Quoted 9 lines

drivers/mmc/host/pxamci.c +2 -4

   		}
   		/* FIXME: should we pass detection delay to debounce? */
   		ret = mmc_gpiod_request_cd(mmc, "cd", 0, false, 0);
-		if (ret && ret != -ENOENT) {
-			dev_err(dev, "Failed requesting gpio_cd\n");
-			goto out;
-		}
+		if (ret && ret != -ENOENT)
+			return dev_err_probe(dev, ret, "Failed requesting gpio_cd\n");

Same here

drivers/mmc/host/pxamci.c static int pxamci_probe(struct platform_device *pdev) · line 737

Quoted 9 lines

drivers/mmc/host/pxamci.c +2 -4

   		if (!host->pdata->gpio_card_ro_invert)
   			mmc->caps2 |= MMC_CAP2_RO_ACTIVE_HIGH;
   		ret = mmc_gpiod_request_ro(mmc, "wp", 0, 0);
-		if (ret && ret != -ENOENT) {
-			dev_err(dev, "Failed requesting gpio_ro\n");
-			goto out;
-		}
+		if (ret && ret != -ENOENT)
+			return dev_err_probe(dev, ret, "Failed requesting gpio_ro\n");

and here.

Quoted 4 lines

Looking at Documentation/driver-api/driver-model/devres.rst, dma_request_chan() is not devres managed interface and thus will not be released automatically. Do you agree?

The patch changes dma_request_chan() to devm_dma_request_chan().

regards,
dan carpenter

Quoted 8 lines · Rakuram Eswaran

Smatch reported:
drivers/mmc/host/pxamci.c:709 pxamci_probe() warn: passing zero to 'PTR_ERR'

Case 1:
When dma_request_chan() fails, host->dma_chan_rx is an ERR_PTR(), but it is reset to NULL before using PTR_ERR(), resulting in PTR_ERR(0). This mistakenly returns 0 instead of the real error code.

Quoted 5 lines

Case 2:
When devm_clk_get() fails, host->clk is an ERR_PTR() resulting in the similar issue like case 1.

Store the error code before nullifying the pointers in both the cases.

Why is the pointer set to NULL at all? This is in both cases memory that is freed directly afterwards (as host is devm managed). So I'd claim

drivers/mmc/host/pxamci.c +2 -6

index 26d03352af63..404f78198252 100644
@@ -652,10 +652,8 @@ static int pxamci_probe(struct platform_device *pdev)
  	host->clkrt = CLKRT_OFF;
 
  	host->clk = devm_clk_get(dev, NULL);
-	if (IS_ERR(host->clk)) {
-		host->clk = NULL;
+	if (IS_ERR(host->clk))
  		return PTR_ERR(host->clk);
-	}
 
  	host->clkrate = clk_get_rate(host->clk);
 

@@ -704,11 +702,9 @@ static int pxamci_probe(struct platform_device *pdev)
  	platform_set_drvdata(pdev, mmc);
 
  	host->dma_chan_rx = dma_request_chan(dev, "rx");
-	if (IS_ERR(host->dma_chan_rx)) {
-		host->dma_chan_rx = NULL;
+	if (IS_ERR(host->dma_chan_rx))
  		return dev_err_probe(dev, PTR_ERR(host->dma_chan_rx),
  				     "unable to request rx dma channel\n");
-	}
 
  	host->dma_chan_tx = dma_request_chan(dev, "tx");
  	if (IS_ERR(host->dma_chan_tx)) {

is a superior patch.

Best regards
Uwe