regression: b43-sdio: probe of mmc0:0001:1 failed with error -16

Hi,

on todays Linus' git (v2.6.36-9871-g3985c7c) b43 sdio stopped working. It was working on (v2.6.36-6794-g12ba8d1).

b43-sdio: probe of mmc0:0001:1 failed with error -16

The error seems to originate from the following code in drivers/mmc/core/sdio_bus.c:sdio_bus_probe()

135 ret = pm_runtime_get_sync(dev);
136 if (ret < 0)
137 goto out;

so I suspect this has something to do with the following commit:

commit 40bba0c1ca83a370f749c8bc9afda71cf79ebd91

Author: Ohad Ben-Cohen <[email protected]>
Date:   Sat Oct 2 13:54:11 2010 +0200

mmc: sdio: enable runtime PM for SDIO functions

However, simply reverting 40bba0c1ca83a370f749c8bc9afda71cf79ebd91 does not work because then modprobe hangs.

Tested on AP4EVB (arm) with tmio_mmc. Any ideas?

Best regards,
Arnd

Hi Arnd,

We've been discussing this on the linux-mmc@ list:

http://thread.gmane.org/gmane.linux.kernel.mmc/4342

Until we have a fix, you should be able to get SDIO working again by reverting all nine commits between 1978fda85d..ed2a978594.

Thanks,

Chris Ball   <[email protected]>   <http://printf.net/>
One Laptop Per Child
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Am 31.10.2010 18:16, schrieb Arnd Hannemann:

FYI:

disabling CONFIG_PM_RUNTIME in my config seems to fix the issue for me.

Best regards,
Arnd

Hi Arnd,

It's exactly what Daniel is experiencing with the XO-1.5.

In Daniel's scenario, mmc_sdio_init_card() fails because mmc_send_relative_addr() returns -110.

Can you please check out if that's the same thing you have too ?

Thanks,
Ohad.

Hi Ohad,

Am 31.10.2010 20:14, schrieb Ohad Ben-Cohen:

No, it seems to be the pm_runtime_get_sync in sdio_bus.c::sdio_bus_probe().

If I test with CONFIG_PM_RUNTIME=y and the attached modifications, then I get this as output:

root@ap4evb:~# modprobe b43
[   29.859375] cfg80211: Calling CRDA to update world regulatory domain
[   30.226562] pm_runtime_get_sync: -16
[   30.234375] b43-sdio: probe of mmc0:0001:1 failed with error -16
[   30.242187] Broadcom 43xx driver loaded [ Features: LS, Firmware-ID: FW13 ]

If CONFIG_PM_RUNTIME is not set in .config, the problem goes away.

Regards,
Arnd

index 797cdb5..57b6dd4 100644
@@ -237,8 +237,10 @@ int mmc_send_relative_addr(struct mmc_host *host, unsigned int *rca)
 	cmd.flags = MMC_RSP_R6 | MMC_CMD_BCR;
 
 	err = mmc_wait_for_cmd(host, &cmd, MMC_CMD_RETRIES);
-	if (err)
+	if (err) {
+		printk(KERN_INFO "%s(): %d\n", __func__, err);
 		return err;
+	}
 
 	*rca = cmd.resp[0] >> 16;
 

index 2716c7a..102c163 100644
@@ -133,8 +133,10 @@ static int sdio_bus_probe(struct device *dev)
 	 * pm_runtime_get_noresume() in its remove routine.
 	 */
 	ret = pm_runtime_get_sync(dev);
-	if (ret < 0)
+	if (ret < 0) {
+		printk(KERN_INFO "pm_runtime_get_sync: %d\n", ret);
 		goto out;
+	}
 
 	/* Set the default block size so the driver is sure it's something
 	 * sensible. */

Yeah, this is just the symptom though - pm_runtime_get_sync tries to power up the card, and eventually mmc_sdio_init_card() is called (by mmc_sdio_resume which is invoked by mmc_resume_host).

On the XO-1.5 mmc_sdio_init_card() seems to fail due to mmc_send_relative_addr() returning -110; this may suggest that although mmc_power_off() and mmc_power_on() were called, we still didn't have a full power reset, and the card/controller are left at a weird state.

Can you please check out if that's the same failure in your case too ?

Thanks!
Ohad.

Am 01.11.2010 05:50, schrieb Ohad Ben-Cohen:

No, actually mmc_sdio_init_card() is called _before_ sdio_bus_probe and the failing pm_runtime_get_sync in my case.
Also mmd_sdio_resume is not called at all.

No mmc_send_relative_addr() does _not_ return -110, actually it is not called at all.

I inserted a WARN_ON() in sdio_bus_probe, and here is the backtrace I get:

[   32.351562] [<c0034754>] (warn_slowpath_null+0x0/0x2c) from [<c017ef48>] (sdio_bus_probe+0x5c/0xe8)
[   32.359375] [<c017eeec>] (sdio_bus_probe+0x0/0xe8) from [<c014e908>] (driver_probe_device+0xd0/0x18c)
[   32.367187]  r8:c00250c4 r7:bf12a210 r6:bf12a210 r5:c728dc3c r4:c728dc08
[   32.367187] r3:c017eeec
[   32.375000] [<c014e838>] (driver_probe_device+0x0/0x18c) from [<c014ea2c>] (__driver_attach+0x68/0x8c)
[   32.382812]  r7:00000000 r6:bf12a210 r5:c728dc3c r4:c728dc08
[   32.390625] [<c014e9c4>] (__driver_attach+0x0/0x8c) from [<c014e0c8>] (bus_for_each_dev+0x54/0x84)
[   32.398437]  r6:00000000 r5:c014e9c4 r4:bf12a210 r3:00000000
[   32.406250] [<c014e074>] (bus_for_each_dev+0x0/0x84) from [<c014e72c>] (driver_attach+0x20/0x28)
[   32.414062]  r6:c02b9440 r5:c72bf2c0 r4:bf12a210
[   32.414062] [<c014e70c>] (driver_attach+0x0/0x28) from [<c014d9b4>] (bus_add_driver+0xa8/0x21c)
[   32.421875] [<c014d90c>] (bus_add_driver+0x0/0x21c) from [<c014ed64>] (driver_register+0xb0/0x140)
[   32.429687] [<c014ecb4>] (driver_register+0x0/0x140) from [<c017f144>] (sdio_register_driver+0x24/0x2c)
[   32.437500]  r8:c00250c4 r7:00000000 r6:0001cfe0 r5:bf132000 r4:bf12a714
[   32.445312] r3:c02b9440
[   32.445312] [<c017f120>] (sdio_register_driver+0x0/0x2c) from [<bf120a0c>] (b43_sdio_init+0x14/0x1c [b43])
[   32.453125] [<bf1209f8>] (b43_sdio_init+0x0/0x1c [b43]) from [<bf132018>] (b43_init+0x18/0x88 [b43])
[   32.460937] [<bf132000>] (b43_init+0x0/0x88 [b43]) from [<c002441c>] (do_one_initcall+0xd4/0x1a8)
[   32.468750]  r4:bf12a714
[   32.468750] [<c0024348>] (do_one_initcall+0x0/0x1a8) from [<c005c564>] (sys_init_module+0x9c/0x1b4)
[   32.476562] [<c005c4c8>] (sys_init_module+0x0/0x1b4) from [<c0024f40>] (ret_fast_syscall+0x0/0x30)
[   32.484375]  r7:00000080 r6:00000000 r5:00000000 r4:0001b070
[   32.492187] ---[ end trace cb2ed4a07515af72 ]---

Regards,
Arnd

Yes, it is called once at boot while the card is mmc_rescan()ed.

But that's not interesting, because the card is then powered down, and will only be powered on again when it is probed with a driver.

Sorry, I meant mmc_sdio_restore_host.

You should see something like this:

sdio_bus_probe - >

    ... (runtime PM function calls) ... ->
           mmc_power_restore_host ->
                 mmc_sdio_power_restore ->
                        mmc_sdio_init_card -> ...

We have 1 report where the latter mmc_sdio_init_card fails at this point, and I'm interested to know whether it fails for you, too (and if yes, where). If it is not even called, I'd appreciate if you can check out where does this flow break in your case.

OK.

The interesting part is actually what happens after sdio_bus_probe(), not before it.

Is mmc_power_restore_host being called ? mmc_sdio_power_restore ? mmc_sdio_init_card ? etc...

I'm also attaching a patch that requires hosts to explicitly indicate whether they support powering off/on their cards after boot (which would prevent SDIO core from powering off you card after boot since your host doesn't indicate that capability).

Can you please see if the problem goes away with it ?

Thanks,
Ohad.

From 6b5691bdd8184cc0876d209c69d38844ea10f678 Mon Sep 17 00:00:00 2001
From: Ohad Ben-Cohen <[email protected]>
Date: Mon, 1 Nov 2010 09:41:44 +0200
Subject: [PATCH] mmc: add MMC_CAP_RUNTIME_PM

Some board/card/host configurations are not capable of powering off/on on the card during runtime.

To support such configurations, and to allow smoother transition to runtime PM behavior, MMC_CAP_RUNTIME_PM is added, so hosts need to explicitly indicate that it's OK to power off their cards after boot.

This will prevent sdio_bus_probe() from failing to power on the card when the driver is loaded on such setups.

---
 drivers/mmc/core/sdio.c     |   37 +++++++++++++++++++++++--------------
 drivers/mmc/core/sdio_bus.c |   33 ++++++++++++++++++++++-----------
 include/linux/mmc/host.h    |    1 +
 3 files changed, 46 insertions(+), 25 deletions(-)

index 6a9ad40..373d56d 100644
@@ -547,9 +547,11 @@ static void mmc_sdio_detect(struct mmc_host *host)
 	BUG_ON(!host->card);
 
 	/* Make sure card is powered before detecting it */
-	err = pm_runtime_get_sync(&host->card->dev);
-	if (err < 0)
-		goto out;
+	if (host->caps & MMC_CAP_RUNTIME_PM) {
+		err = pm_runtime_get_sync(&host->card->dev);
+		if (err < 0)
+			goto out;
+	}
 
 	mmc_claim_host(host);
 

@@ -570,7 +572,8 @@ out:
 	}
 
 	/* Tell PM core that we're done */
-	pm_runtime_put(&host->card->dev);
+	if (host->caps & MMC_CAP_RUNTIME_PM)
+		pm_runtime_put(&host->card->dev);
 }
 
 /*

@@ -720,16 +723,21 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
 	card = host->card;
 
 	/*
-	 * Let runtime PM core know our card is active
+	 * Enable runtime PM only if supported by host+card+board
 	 */
-	err = pm_runtime_set_active(&card->dev);
-	if (err)
-		goto remove;
+	if (host->caps & MMC_CAP_RUNTIME_PM) {
+		/*
+		 * Let runtime PM core know our card is active
+		 */
+		err = pm_runtime_set_active(&card->dev);
+		if (err)
+			goto remove;
 
-	/*
-	 * Enable runtime PM for this card
-	 */
-	pm_runtime_enable(&card->dev);
+		/*
+		 * Enable runtime PM for this card
+		 */
+		pm_runtime_enable(&card->dev);
+	}
 
 	/*
 	 * The number of functions on the card is encoded inside

@@ -747,9 +755,10 @@ int mmc_attach_sdio(struct mmc_host *host, u32 ocr)
 			goto remove;
 
 		/*
-		 * Enable Runtime PM for this func
+		 * Enable Runtime PM for this func (if supported)
 		 */
-		pm_runtime_enable(&card->sdio_func[i]->dev);
+		if (host->caps & MMC_CAP_RUNTIME_PM)
+			pm_runtime_enable(&card->sdio_func[i]->dev);
 	}
 
 	mmc_release_host(host);

index 2716c7a..f3ce21b 100644
@@ -17,6 +17,7 @@
 #include <linux/pm_runtime.h>
 
 #include <linux/mmc/card.h>
+#include <linux/mmc/host.h>
 #include <linux/mmc/sdio_func.h>
 
 #include "sdio_cis.h"

@@ -132,9 +133,11 @@ static int sdio_bus_probe(struct device *dev)
 	 * it should call pm_runtime_put_noidle() in its probe routine and
 	 * pm_runtime_get_noresume() in its remove routine.
 	 */
-	ret = pm_runtime_get_sync(dev);
-	if (ret < 0)
-		goto out;
+	if (func->card->host->caps & MMC_CAP_RUNTIME_PM) {
+		ret = pm_runtime_get_sync(dev);
+		if (ret < 0)
+			goto out;
+	}
 
 	/* Set the default block size so the driver is sure it's something
 	 * sensible. */

@@ -151,7 +154,8 @@ static int sdio_bus_probe(struct device *dev)
 	return 0;
 
 disable_runtimepm:
-	pm_runtime_put_noidle(dev);
+	if (func->card->host->caps & MMC_CAP_RUNTIME_PM)
+		pm_runtime_put_noidle(dev);
 out:
 	return ret;
 }

@@ -160,12 +164,14 @@ static int sdio_bus_remove(struct device *dev)
 {
 	struct sdio_driver *drv = to_sdio_driver(dev->driver);
 	struct sdio_func *func = dev_to_sdio_func(dev);
-	int ret;
+	int ret = 0;
 
 	/* Make sure card is powered before invoking ->remove() */
-	ret = pm_runtime_get_sync(dev);
-	if (ret < 0)
-		goto out;
+	if (func->card->host->caps & MMC_CAP_RUNTIME_PM) {
+		ret = pm_runtime_get_sync(dev);
+		if (ret < 0)
+			goto out;
+	}
 
 	drv->remove(func);
 

@@ -178,10 +184,12 @@ static int sdio_bus_remove(struct device *dev)
 	}
 
 	/* First, undo the increment made directly above */
-	pm_runtime_put_noidle(dev);
+	if (func->card->host->caps & MMC_CAP_RUNTIME_PM)
+		pm_runtime_put_noidle(dev);
 
 	/* Then undo the runtime PM settings in sdio_bus_probe() */
-	pm_runtime_put_noidle(dev);
+	if (func->card->host->caps & MMC_CAP_RUNTIME_PM)
+		pm_runtime_put_noidle(dev);
 
 out:
 	return ret;

@@ -191,6 +199,8 @@ out:
 
 static int sdio_bus_pm_prepare(struct device *dev)
 {
+	struct sdio_func *func = dev_to_sdio_func(dev);
+
 	/*
 	 * Resume an SDIO device which was suspended at run time at this
 	 * point, in order to allow standard SDIO suspend/resume paths

@@ -212,7 +222,8 @@ static int sdio_bus_pm_prepare(struct device *dev)
 	 * since there is little point in failing system suspend if a
 	 * device can't be resumed.
 	 */
-	pm_runtime_resume(dev);
+	if (func->card->host->caps & MMC_CAP_RUNTIME_PM)
+		pm_runtime_resume(dev);
 
 	return 0;
 }

index c3ffad8..e5eee0e 100644
@@ -167,6 +167,7 @@ struct mmc_host {
 						/* DDR mode at 1.8V */
 #define MMC_CAP_1_2V_DDR	(1 << 12)	/* can support */
 						/* DDR mode at 1.2V */
+#define MMC_CAP_RUNTIME_PM	(1 << 13)	/* Can power off/on in runtime */
 
 	mmc_pm_flag_t		pm_caps;	/* supported pm features */
 

1.7.0.4

Am 01.11.2010 10:02, schrieb Ohad Ben-Cohen:

It looks like this:

root@ap4evb:~# modprobe b43
[   36.023437] cfg80211: Calling CRDA to update world regulatory domain
[   36.460937] sdio_bus_probe()
[   36.468750] sdio_bus_probe() calling pm_runtime_get_sync()
[   36.468750] mmc_power_restore_host() begin
[   36.570312] mmc_sdio_power_restore() begin
[   36.570312] mmc_sdio_init_card()
[   36.578125] mmc_sdio_init_card() mmc_send_io_op_cond ret=-110
[   36.585937] mmc_sdio_power_restore() end ret=-110
[   36.593750] mmc_power_restore_host() end ret=-110
[   36.593750] sdio_bus_probe() pm_runtime_get_sync ret=-16
[   36.601562] b43-sdio: probe of mmc0:0001:1 failed with error -16

So it seems to fail in mmc_sdio_init_card at the mmc_send_io_op_cond() call.

Actually nothing special happens after sdio_bus_probe()

With your patch the problem goes away, thanks.

Regards,
Arnd

OK, thanks. CMD5 time outs. I suspect it's similar to what we have with the XO-1.5: despite software calling mmc_power_off/on, the hardware does not go through full power off/on cycle.

The actual failure happens after it (in mmc_sdio_init_card).

Thank you for testing this,
Ohad.

Hi

I'm facing the issue of "wlan_sdio: probe of mmc0:0001:1 failed with error -5"
I have put debug logs and found the problem is occuring in
drv->probe(func,id) in drivers/mmc/core/sdio_bus.c, Since I'm a newbie i'm not sure where to proceed. So any guidance is heartly appreciated.

Regards
Arumugam