* MMC runtime PM patches break libertas probe @ 2010-10-31 14:29 Daniel Drake 2010-10-31 14:42 ` Ohad Ben-Cohen 0 siblings, 1 reply; 48+ messages in thread From: Daniel Drake @ 2010-10-31 14:29 UTC (permalink / raw) To: Ohad Ben-Cohen, linux-mmc Hi, I'm running linus master and can't load the libertas module for the sd8686 wifi hardware in the OLPC XO-1.5 laptop. Probe fails with -16. dmesg and config: http://dev.laptop.org/~dsd/20101031/dmesg-sdio-probe-fail.txt http://dev.laptop.org/~dsd/20101031/config-sdio-probe-fail.txt I've done some initial investigation; in sdio_bus_probe() we do this: ret = pm_runtime_get_sync(dev); if (ret < 0) goto out; and pm_runtime_get_sync() returns -16 Daniel ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-10-31 14:29 MMC runtime PM patches break libertas probe Daniel Drake @ 2010-10-31 14:42 ` Ohad Ben-Cohen 2010-10-31 14:55 ` Daniel Drake 0 siblings, 1 reply; 48+ messages in thread From: Ohad Ben-Cohen @ 2010-10-31 14:42 UTC (permalink / raw) To: Daniel Drake; +Cc: linux-mmc Hi Daniel, On Sun, Oct 31, 2010 at 4:29 PM, Daniel Drake <dsd@laptop.org> wrote: > I'm running linus master and can't load the libertas module for the > sd8686 wifi hardware in the OLPC XO-1.5 laptop. > Probe fails with -16. That's why I asked you if this scenario works in the other thread. It's probably the same issue (both scenarios try to power up the chip by calling mmc_sdio_power_restore). I guess the error comes from mmc_sdio_init_card() - can you please check out what exactly triggers it inside that function (just put some printk's there..) ? Thanks! Ohad. > > dmesg and config: > http://dev.laptop.org/~dsd/20101031/dmesg-sdio-probe-fail.txt > http://dev.laptop.org/~dsd/20101031/config-sdio-probe-fail.txt > > I've done some initial investigation; in sdio_bus_probe() we do this: > ret = pm_runtime_get_sync(dev); > if (ret < 0) > goto out; > > and pm_runtime_get_sync() returns -16 > > Daniel > ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-10-31 14:42 ` Ohad Ben-Cohen @ 2010-10-31 14:55 ` Daniel Drake 2010-10-31 15:08 ` Ohad Ben-Cohen 0 siblings, 1 reply; 48+ messages in thread From: Daniel Drake @ 2010-10-31 14:55 UTC (permalink / raw) To: Ohad Ben-Cohen; +Cc: linux-mmc On 31 October 2010 14:42, Ohad Ben-Cohen <ohad@wizery.com> wrote: > I guess the error comes from mmc_sdio_init_card() - can you please > check out what exactly triggers it inside that function (just put some > printk's there..) ? /* * For native busses: set card RCA and quit open drain mode. */ if (!powered_resume && !mmc_host_is_spi(host)) { err = mmc_send_relative_addr(host, &card->rca); This returns -110 Thanks, Daniel ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-10-31 14:55 ` Daniel Drake @ 2010-10-31 15:08 ` Ohad Ben-Cohen 2010-10-31 15:10 ` Daniel Drake 0 siblings, 1 reply; 48+ messages in thread From: Ohad Ben-Cohen @ 2010-10-31 15:08 UTC (permalink / raw) To: Daniel Drake; +Cc: linux-mmc On Sun, Oct 31, 2010 at 4:55 PM, Daniel Drake <dsd@laptop.org> wrote: > /* > * For native busses: set card RCA and quit open drain mode. > */ > if (!powered_resume && !mmc_host_is_spi(host)) { > err = mmc_send_relative_addr(host, &card->rca); > > This returns -110 Quick question - how did you manage the power to the sd8686 ? Was it possible to power it down after boot or was it always kept high ? Thanks, Ohad. > > Thanks, > Daniel > ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-10-31 15:08 ` Ohad Ben-Cohen @ 2010-10-31 15:10 ` Daniel Drake 2010-10-31 15:16 ` Ohad Ben-Cohen 0 siblings, 1 reply; 48+ messages in thread From: Daniel Drake @ 2010-10-31 15:10 UTC (permalink / raw) To: Ohad Ben-Cohen; +Cc: linux-mmc On 31 October 2010 15:08, Ohad Ben-Cohen <ohad@wizery.com> wrote: > Quick question - how did you manage the power to the sd8686 ? Was it > possible to power it down after boot or was it always kept high ? I didn't do anything except boot then try and load the module. Does that answer the question? Daniel ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-10-31 15:10 ` Daniel Drake @ 2010-10-31 15:16 ` Ohad Ben-Cohen 2010-10-31 15:21 ` Daniel Drake 2010-10-31 15:21 ` Daniel Drake 0 siblings, 2 replies; 48+ messages in thread From: Ohad Ben-Cohen @ 2010-10-31 15:16 UTC (permalink / raw) To: Daniel Drake; +Cc: linux-mmc On Sun, Oct 31, 2010 at 5:10 PM, Daniel Drake <dsd@laptop.org> wrote: > On 31 October 2010 15:08, Ohad Ben-Cohen <ohad@wizery.com> wrote: >> Quick question - how did you manage the power to the sd8686 ? Was it >> possible to power it down after boot or was it always kept high ? > > I didn't do anything except boot then try and load the module. Does > that answer the question? No, I'm asking a more general question about the sd8686 and the XO-1.5. How can one control the power to the sd8686 ? Is it always on and can't be controlled ? Is there a GPIO that controls it ? or any other mean to control its power ? I'm almost sure I understand what's going on, but your answer can confirm my speculation. Thanks, Ohad. ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-10-31 15:16 ` Ohad Ben-Cohen @ 2010-10-31 15:21 ` Daniel Drake 2010-10-31 15:21 ` Daniel Drake 1 sibling, 0 replies; 48+ messages in thread From: Daniel Drake @ 2010-10-31 15:21 UTC (permalink / raw) To: Ohad Ben-Cohen; +Cc: linux-mmc On 31 October 2010 15:16, Ohad Ben-Cohen <ohad@wizery.com> wrote: > No, I'm asking a more general question about the sd8686 and the XO-1.5. > How can one control the power to the sd8686 ? Is it always on and > can't be controlled ? Is there a GPIO that controls it ? or any other > mean to control its power ? The power can be controlled by the regular ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-10-31 15:16 ` Ohad Ben-Cohen 2010-10-31 15:21 ` Daniel Drake @ 2010-10-31 15:21 ` Daniel Drake 2010-10-31 15:27 ` Ohad Ben-Cohen 1 sibling, 1 reply; 48+ messages in thread From: Daniel Drake @ 2010-10-31 15:21 UTC (permalink / raw) To: Ohad Ben-Cohen; +Cc: linux-mmc On 31 October 2010 15:16, Ohad Ben-Cohen <ohad@wizery.com> wrote: > No, I'm asking a more general question about the sd8686 and the XO-1.5. > How can one control the power to the sd8686 ? Is it always on and > can't be controlled ? Is there a GPIO that controls it ? or any other > mean to control its power ? The power can be controlled by the regular SD power pin. There is no GPIO to control it. Daniel ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-10-31 15:21 ` Daniel Drake @ 2010-10-31 15:27 ` Ohad Ben-Cohen 2010-10-31 15:57 ` Daniel Drake 0 siblings, 1 reply; 48+ messages in thread From: Ohad Ben-Cohen @ 2010-10-31 15:27 UTC (permalink / raw) To: Daniel Drake; +Cc: linux-mmc On Sun, Oct 31, 2010 at 5:21 PM, Daniel Drake <dsd@laptop.org> wrote: > The power can be controlled by the regular SD power pin. There is no > GPIO to control it. OK, Good. Can you please tell me the output of the following line (after boot, but before you load the driver): cat /sys/kernel/debug/mmc1/ios (obviously you will have to mount -t debugfs none /sys/kernel/debug) ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-10-31 15:27 ` Ohad Ben-Cohen @ 2010-10-31 15:57 ` Daniel Drake 2010-10-31 16:16 ` Ohad Ben-Cohen 0 siblings, 1 reply; 48+ messages in thread From: Daniel Drake @ 2010-10-31 15:57 UTC (permalink / raw) To: Ohad Ben-Cohen; +Cc: linux-mmc On 31 October 2010 15:27, Ohad Ben-Cohen <ohad@wizery.com> wrote: > Can you please tell me the output of the following line > (after boot, but before you load the driver): > > cat /sys/kernel/debug/mmc1/ios clock: 0 Hz vdd: 0 (invalid) bus mode: 1 (open drain) chip select: 0 (don't care) power mode: 0 (off) bus width: 0 (1 bits) timing spec: 0 (legacy) ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-10-31 15:57 ` Daniel Drake @ 2010-10-31 16:16 ` Ohad Ben-Cohen 2010-10-31 16:24 ` Daniel Drake 0 siblings, 1 reply; 48+ messages in thread From: Ohad Ben-Cohen @ 2010-10-31 16:16 UTC (permalink / raw) To: Daniel Drake; +Cc: linux-mmc On Sun, Oct 31, 2010 at 5:57 PM, Daniel Drake <dsd@laptop.org> wrote: >> cat /sys/kernel/debug/mmc1/ios > > clock: 0 Hz > vdd: 0 (invalid) > bus mode: 1 (open drain) > chip select: 0 (don't care) > power mode: 0 (off) > bus width: 0 (1 bits) > timing spec: 0 (legacy) Good. Power is taken down after the mmc+sdio devices are added to the device tree. Just to make sure - on an older kernel (that doesn't have SDIO runtime pm), the card is powered on at this stage (this info will help me rule out some corner cases) ? ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-10-31 16:16 ` Ohad Ben-Cohen @ 2010-10-31 16:24 ` Daniel Drake 2010-10-31 19:06 ` Ohad Ben-Cohen 0 siblings, 1 reply; 48+ messages in thread From: Daniel Drake @ 2010-10-31 16:24 UTC (permalink / raw) To: Ohad Ben-Cohen; +Cc: linux-mmc On 31 October 2010 16:16, Ohad Ben-Cohen <ohad@wizery.com> wrote: > Just to make sure - on an older kernel (that doesn't have SDIO runtime > pm), the card is powered on at this stage (this info will help me rule > out some corner cases) ? Looks that way. Same test, after reverting your patches: clock: 25000000 Hz vdd: 20 (3.2 ~ 3.3 V) bus mode: 2 (push-pull) chip select: 0 (don't care) power mode: 2 (on) bus width: 2 (4 bits) timing spec: 0 (legacy) ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-10-31 16:24 ` Daniel Drake @ 2010-10-31 19:06 ` Ohad Ben-Cohen 2010-11-01 8:27 ` Ohad Ben-Cohen 2011-05-29 16:21 ` Daniel Drake 0 siblings, 2 replies; 48+ messages in thread From: Ohad Ben-Cohen @ 2010-10-31 19:06 UTC (permalink / raw) To: Daniel Drake; +Cc: linux-mmc On Sun, Oct 31, 2010 at 6:24 PM, Daniel Drake <dsd@laptop.org> wrote: > On 31 October 2010 16:16, Ohad Ben-Cohen <ohad@wizery.com> wrote: >> Just to make sure - on an older kernel (that doesn't have SDIO runtime >> pm), the card is powered on at this stage (this info will help me rule >> out some corner cases) ? > > Looks that way. Same test, after reverting your patches: > > clock: 25000000 Hz > vdd: 20 (3.2 ~ 3.3 V) > bus mode: 2 (push-pull) > chip select: 0 (don't care) > power mode: 2 (on) > bus width: 2 (4 bits) > timing spec: 0 (legacy) OK, as expected. So to summarize: 1. Card is powered up at boot, and successfully initialized 2. After mmc + sdio devices are added to the device tree, power is (seemingly) taken down by runtime PM 3. When the driver is loaded, card is powered up again, but doesn't respond to CMD3 The only explanation I can think of why the card doesn't respond to CMD3, after its power is brought up again, is that we didn't have a full reset here (i.e. mmc_power_off() didn't completely power off everything). It's interesting to understand exactly what happens in the context of the XO-1.5 (there can be several possible culprits, including a controller quirk), but even if that's not exactly the case here, it is pretty clear that we need to support boards with controllers/cards which we can't power off in runtime. On such boards, the right thing to do would be to disable runtime PM altogether. Luckily I'll have the XO-1.5 later on this week to cook a fix and test it. Thanks, Ohad. ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-10-31 19:06 ` Ohad Ben-Cohen @ 2010-11-01 8:27 ` Ohad Ben-Cohen 2010-11-06 21:19 ` Daniel Drake 2010-11-16 13:22 ` Ohad Ben-Cohen 2011-05-29 16:21 ` Daniel Drake 1 sibling, 2 replies; 48+ messages in thread From: Ohad Ben-Cohen @ 2010-11-01 8:27 UTC (permalink / raw) To: Daniel Drake; +Cc: linux-mmc, Chris Ball [-- Attachment #1: Type: text/plain, Size: 6725 bytes --] Hi Daniel, On Sun, Oct 31, 2010 at 9:06 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote: ... > we need to support boards with controllers/cards > which we can't power off in runtime. > > On such boards, the right thing to do would be to disable runtime PM altogether. The patch below (/attached) should trivially fix the problem - can you please check it out ? It's very simple: it just requires hosts to explicitly indicate they support runtime powering off/on their cards (by means of MMC_CAP_RUNTIME_PM). There is no way around this I'm afraid: it is legitimate for a board/host/card configuration not to support powering off the card after boot, and we must allow it. In addition having an explicit MMC_CAP_RUNTIME_PM will also allow us smoother transition to runtime PM behavior. Developers will have to explicitly turn it on, and will not be surprised if things won't immediately work. Please note that this cap is not interchangeable, and can't be replace with CONFIG_PM_RUNTIME. Having said that, I still think we need to understand why CMD3 timed out on the XO-1.5. Thanks, Ohad. >From 6b5691bdd8184cc0876d209c69d38844ea10f678 Mon Sep 17 00:00:00 2001 From: Ohad Ben-Cohen <ohad@wizery.com> 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. Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com> --- 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(-) diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index 6a9ad40..373d56d 100644 --- a/drivers/mmc/core/sdio.c +++ b/drivers/mmc/core/sdio.c @@ -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); diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c index 2716c7a..f3ce21b 100644 --- a/drivers/mmc/core/sdio_bus.c +++ b/drivers/mmc/core/sdio_bus.c @@ -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; } diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index c3ffad8..e5eee0e 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -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 [-- Attachment #2: 0001-mmc-add-MMC_CAP_RUNTIME_PM.patch --] [-- Type: text/x-patch, Size: 5656 bytes --] From 6b5691bdd8184cc0876d209c69d38844ea10f678 Mon Sep 17 00:00:00 2001 From: Ohad Ben-Cohen <ohad@wizery.com> 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. Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com> --- 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(-) diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index 6a9ad40..373d56d 100644 --- a/drivers/mmc/core/sdio.c +++ b/drivers/mmc/core/sdio.c @@ -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); diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c index 2716c7a..f3ce21b 100644 --- a/drivers/mmc/core/sdio_bus.c +++ b/drivers/mmc/core/sdio_bus.c @@ -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; } diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h index c3ffad8..e5eee0e 100644 --- a/include/linux/mmc/host.h +++ b/include/linux/mmc/host.h @@ -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 ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-01 8:27 ` Ohad Ben-Cohen @ 2010-11-06 21:19 ` Daniel Drake 2010-11-07 1:48 ` Nicolas Pitre 2010-11-07 10:42 ` Ohad Ben-Cohen 2010-11-16 13:22 ` Ohad Ben-Cohen 1 sibling, 2 replies; 48+ messages in thread From: Daniel Drake @ 2010-11-06 21:19 UTC (permalink / raw) To: Ohad Ben-Cohen; +Cc: linux-mmc, Chris Ball On 1 November 2010 08:27, Ohad Ben-Cohen <ohad@wizery.com> wrote: >> On such boards, the right thing to do would be to disable runtime PM altogether. > > The patch below (/attached) should trivially fix the problem - can you > please check it out ? Thanks. It solves the problem. But, what you point out is quite interesting. We currently have a (not-yet-upstream) rfkill driver for the XO-1.5, which we also put in place for power saving reasons. It operates by calling mmc_stop_host(). In light of your work, I guess that wasn't really turning off the card. So, it would be great if you could get runtime PM working on this combo, and then later we could make a real rfkill driver. I guess you have an XO-1.5 now. I'm happy to help you get started, I guess this is your first time working with an XO. Drop by irc.oftc.net #olpc-devel and I'll happily help you get started booting your own kernel etc. Thanks! Daniel ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-06 21:19 ` Daniel Drake @ 2010-11-07 1:48 ` Nicolas Pitre 2010-11-07 10:19 ` Daniel Drake 2010-11-07 10:42 ` Ohad Ben-Cohen 1 sibling, 1 reply; 48+ messages in thread From: Nicolas Pitre @ 2010-11-07 1:48 UTC (permalink / raw) To: Daniel Drake; +Cc: Ohad Ben-Cohen, linux-mmc, Chris Ball On Sat, 6 Nov 2010, Daniel Drake wrote: > On 1 November 2010 08:27, Ohad Ben-Cohen <ohad@wizery.com> wrote: > >> On such boards, the right thing to do would be to disable runtime PM altogether. > > > > The patch below (/attached) should trivially fix the problem - can you > > please check it out ? > > Thanks. > It solves the problem. > > But, what you point out is quite interesting. > We currently have a (not-yet-upstream) rfkill driver for the XO-1.5, > which we also put in place for power saving reasons. It operates by > calling mmc_stop_host(). Couldn't you implement this any other way? This seems to be a really bad strategy for this IMHO. Nicolas ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-07 1:48 ` Nicolas Pitre @ 2010-11-07 10:19 ` Daniel Drake 2010-11-07 15:12 ` Nicolas Pitre 0 siblings, 1 reply; 48+ messages in thread From: Daniel Drake @ 2010-11-07 10:19 UTC (permalink / raw) To: Nicolas Pitre; +Cc: Ohad Ben-Cohen, linux-mmc, Chris Ball On 7 November 2010 01:48, Nicolas Pitre <nico@fluxnic.net> wrote: > Couldn't you implement this any other way? This seems to be a really > bad strategy for this IMHO. What's so bad about it? Any suggestions? Daniel ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-07 10:19 ` Daniel Drake @ 2010-11-07 15:12 ` Nicolas Pitre 0 siblings, 0 replies; 48+ messages in thread From: Nicolas Pitre @ 2010-11-07 15:12 UTC (permalink / raw) To: Daniel Drake; +Cc: Ohad Ben-Cohen, linux-mmc, Chris Ball [-- Attachment #1: Type: TEXT/PLAIN, Size: 456 bytes --] On Sun, 7 Nov 2010, Daniel Drake wrote: > On 7 November 2010 01:48, Nicolas Pitre <nico@fluxnic.net> wrote: > > Couldn't you implement this any other way? This seems to be a really > > bad strategy for this IMHO. > > What's so bad about it? > Any suggestions? If you want rfkill functionality, then maybe that would be a good idea to implement it _within_ the libertas driver instead of hacking something around it in an unrelated fashion. Nicolas ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-06 21:19 ` Daniel Drake 2010-11-07 1:48 ` Nicolas Pitre @ 2010-11-07 10:42 ` Ohad Ben-Cohen 2010-11-07 10:51 ` Daniel Drake 1 sibling, 1 reply; 48+ messages in thread From: Ohad Ben-Cohen @ 2010-11-07 10:42 UTC (permalink / raw) To: Daniel Drake; +Cc: linux-mmc, Chris Ball On Sat, Nov 6, 2010 at 11:19 PM, Daniel Drake <dsd@laptop.org> wrote: > Thanks. > It solves the problem. Good. Did it also solve the 1st issue you had ? Anyway I'm still interested to reproduce that crash. No reason to get a kernel panic whatsoever. > > But, what you point out is quite interesting. > We currently have a (not-yet-upstream) rfkill driver for the XO-1.5, > which we also put in place for power saving reasons. Can you please elaborate ? What power saving ? How do you exactly plan to use it ? Do you plan to power off the card without taking the wlan interface down first ? > It operates by > calling mmc_stop_host(). In light of your work, I guess that wasn't > really turning off the card. Sounds like it. Chris and I briefly discussed this in LPC, and he mentioned that you might have an external power source to that card, that is not controlled by the mmc regulator, and which you disable/enable from user space just before suspending the host ? > > So, it would be great if you could get runtime PM working on this > combo, and then later we could make a real rfkill driver. The XO-1.5 problem is orthogonal to runtime PM. If your regulator really powered down your card, then everything would just have worked. > > I guess you have an XO-1.5 now. I'm happy to help you get started, I > guess this is your first time working with an XO. Drop by irc.oftc.net > #olpc-devel and I'll happily help you get started booting your own > kernel etc. Thanks. This would allow me to test generic core code on a platform which is completely different from those I have. Thanks, Ohad. > > Thanks! > Daniel > ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-07 10:42 ` Ohad Ben-Cohen @ 2010-11-07 10:51 ` Daniel Drake 2010-11-07 13:17 ` Ohad Ben-Cohen 0 siblings, 1 reply; 48+ messages in thread From: Daniel Drake @ 2010-11-07 10:51 UTC (permalink / raw) To: Ohad Ben-Cohen; +Cc: linux-mmc, Chris Ball On 7 November 2010 10:42, Ohad Ben-Cohen <ohad@wizery.com> wrote: > Good. Did it also solve the 1st issue you had ? Yes. >> But, what you point out is quite interesting. >> We currently have a (not-yet-upstream) rfkill driver for the XO-1.5, >> which we also put in place for power saving reasons. > > Can you please elaborate ? What power saving ? How do you exactly plan > to use it ? Do you plan to power off the card without taking the wlan > interface down first ? If you take the interface down, the card is still powered up in some way. We were looking for a way to cut as much power as possible. It can be used by the "rfkill block wifi" command, and theres a checkbox available in the sugar UI which calls that. > Chris and I briefly discussed this in LPC, and he mentioned that you > might have an external power source to that card, that is not > controlled by the mmc regulator, and which you disable/enable from > user space just before suspending the host ? Not sure what he's referring to there. Perhaps he can clarify. When I asked the hardware guy he said it was entirely controlled by the normal SD power pin, but a full card reset will be needed after cycling the power. Daniel ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-07 10:51 ` Daniel Drake @ 2010-11-07 13:17 ` Ohad Ben-Cohen 0 siblings, 0 replies; 48+ messages in thread From: Ohad Ben-Cohen @ 2010-11-07 13:17 UTC (permalink / raw) To: Daniel Drake; +Cc: linux-mmc, Chris Ball On Sun, Nov 7, 2010 at 12:51 PM, Daniel Drake <dsd@laptop.org> wrote: > If you take the interface down, the card is still powered up in some > way. With SDIO runtime pm support, it shouldn't be. > When I asked the hardware guy he said it was entirely controlled by > the normal SD power pin, but a full card reset will be needed after > cycling the power. We need to understand why the wlan card on the XO-1.5 does not go through full power off/on cycle, despite software calling mmc_power_off/on. If this is solved, I don't think you will still need that rfkill driver. ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-01 8:27 ` Ohad Ben-Cohen 2010-11-06 21:19 ` Daniel Drake @ 2010-11-16 13:22 ` Ohad Ben-Cohen 2010-11-16 14:29 ` Daniel Drake 2010-11-16 17:17 ` Arnd Hannemann 1 sibling, 2 replies; 48+ messages in thread From: Ohad Ben-Cohen @ 2010-11-16 13:22 UTC (permalink / raw) To: Daniel Drake; +Cc: linux-mmc, Chris Ball, Arnd Hannemann On Mon, Nov 1, 2010 at 10:27 AM, Ohad Ben-Cohen <ohad@wizery.com> wrote: > On Sun, Oct 31, 2010 at 9:06 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote: > ... >> we need to support boards with controllers/cards >> which we can't power off in runtime. >> >> On such boards, the right thing to do would be to disable runtime PM altogether. > > The patch below (/attached) should trivially fix the problem - can you > please check it out ? > > It's very simple: it just requires hosts to explicitly indicate they > support runtime powering off/on their cards (by means of > MMC_CAP_RUNTIME_PM). > > There is no way around this I'm afraid: it is legitimate for a > board/host/card configuration not to support powering off the card > after boot, and we must allow it. > > In addition having an explicit MMC_CAP_RUNTIME_PM will also allow us > smoother transition to runtime PM behavior. Developers will have to > explicitly turn it on, and will not be surprised if things won't > immediately work. > > Please note that this cap is not interchangeable, and can't be replace > with CONFIG_PM_RUNTIME. > > Having said that, I still think we need to understand why CMD3 timed > out on the XO-1.5. Just to update the list, the problem with the XO-1.5 was because the sd8686 has an external reset gpio line which is currently being manipulated manually by an out-of-tree kernel patch: http://dev.laptop.org/git/olpc-2.6/commit/?h=olpc-2.6.35&id=e9bee721fb0cc303286d1fe5df4930ce79b0b1e0 ... which makes me wonder whether we really want to take that MMC_CAP_RUNTIME_PM road. I'm not sure anymore. We need a demonstrated hardware limitation before we take that approach (or a setup which worked using vanilla kernels and now doesn't), because frankly this MMC_CAP_RUNTIME_PM approach is cumbersome and involving. I would not like to introduce it just to fix out-of-tree software issues, and it seems that at least the XO-1.5 case can be cleanly solved by software (e.g. by letting the regulator handle that sd8686 quirk). I'm looping in Arnd, who reported similar problems with b43-sdio on AP4EVB (arm) with tmio_mmc. Arnd, We're trying to exactly understand the reasons behind the reported SDIO runtime pm failures (we had two, yours, and OLPC). So far it seems that the OLPC had a software issue, and I'm wondering about yours. Can you please supply additional information about your board ? where does your wifi card gets its power from ? is there an external gpio involved ? Were you able to work with vanilla kernels (prior to 2.6.37) or do you carry some out-of-tree patches ? Thanks, Ohad. > > From 6b5691bdd8184cc0876d209c69d38844ea10f678 Mon Sep 17 00:00:00 2001 > From: Ohad Ben-Cohen <ohad@wizery.com> > 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. > > Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com> > --- > 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(-) > > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c > index 6a9ad40..373d56d 100644 > --- a/drivers/mmc/core/sdio.c > +++ b/drivers/mmc/core/sdio.c > @@ -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); > diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c > index 2716c7a..f3ce21b 100644 > --- a/drivers/mmc/core/sdio_bus.c > +++ b/drivers/mmc/core/sdio_bus.c > @@ -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; > } > diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h > index c3ffad8..e5eee0e 100644 > --- a/include/linux/mmc/host.h > +++ b/include/linux/mmc/host.h > @@ -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 > ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-16 13:22 ` Ohad Ben-Cohen @ 2010-11-16 14:29 ` Daniel Drake 2010-11-16 14:49 ` Ohad Ben-Cohen 2010-11-16 17:17 ` Arnd Hannemann 1 sibling, 1 reply; 48+ messages in thread From: Daniel Drake @ 2010-11-16 14:29 UTC (permalink / raw) To: Ohad Ben-Cohen; +Cc: linux-mmc, Chris Ball, Arnd Hannemann On 16 November 2010 13:22, Ohad Ben-Cohen <ohad@wizery.com> wrote: > Just to update the list, the problem with the XO-1.5 was because the > sd8686 has an external reset gpio line which is currently being > manipulated manually by an out-of-tree kernel patch: > > http://dev.laptop.org/git/olpc-2.6/commit/?h=olpc-2.6.35&id=e9bee721fb0cc303286d1fe5df4930ce79b0b1e0 > > ... which makes me wonder whether we really want to take that > MMC_CAP_RUNTIME_PM road. I'm not sure anymore. OLPC is not the only user of the sd8686. Every other user will face the same problem. Other users may not have the luxury of having a GPIO hooked up to the reset line. It's not like Marvell made this limitation very widely known. Daniel ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-16 14:29 ` Daniel Drake @ 2010-11-16 14:49 ` Ohad Ben-Cohen 2010-11-17 6:46 ` Mike Rapoport 0 siblings, 1 reply; 48+ messages in thread From: Ohad Ben-Cohen @ 2010-11-16 14:49 UTC (permalink / raw) To: Daniel Drake Cc: linux-mmc, Chris Ball, Arnd Hannemann, libertas-dev, Nicolas Pitre On Tue, Nov 16, 2010 at 4:29 PM, Daniel Drake <dsd@laptop.org> wrote: > On 16 November 2010 13:22, Ohad Ben-Cohen <ohad@wizery.com> wrote: >> Just to update the list, the problem with the XO-1.5 was because the >> sd8686 has an external reset gpio line which is currently being >> manipulated manually by an out-of-tree kernel patch: >> >> http://dev.laptop.org/git/olpc-2.6/commit/?h=olpc-2.6.35&id=e9bee721fb0cc303286d1fe5df4930ce79b0b1e0 >> >> ... which makes me wonder whether we really want to take that >> MMC_CAP_RUNTIME_PM road. I'm not sure anymore. > > OLPC is not the only user of the sd8686. > Every other user will face the same problem. > > Other users may not have the luxury of having a GPIO hooked up to the > reset line. Agree; those users will need a MMC_CAP_RUNTIME_PM (or maybe call it with the capability it really stands for which is something like MMC_CAP_POWER_OFF_CARD). But I want to be positively sure we have such users (or is it that obvious?). How is the sd8686's reset line manipulated on other platforms ? Or is the sd8686 usually just kept powered on after boot ? I'm looping in libertas-dev. Thanks, Ohad. ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-16 14:49 ` Ohad Ben-Cohen @ 2010-11-17 6:46 ` Mike Rapoport 2010-11-17 7:29 ` Ohad Ben-Cohen 0 siblings, 1 reply; 48+ messages in thread From: Mike Rapoport @ 2010-11-17 6:46 UTC (permalink / raw) To: Ohad Ben-Cohen Cc: Daniel Drake, Arnd Hannemann, Chris Ball, linux-mmc, Nicolas Pitre, libertas-dev On 11/16/10 16:49, Ohad Ben-Cohen wrote: > On Tue, Nov 16, 2010 at 4:29 PM, Daniel Drake <dsd@laptop.org> wrote: >> On 16 November 2010 13:22, Ohad Ben-Cohen <ohad@wizery.com> wrote: >>> Just to update the list, the problem with the XO-1.5 was because the >>> sd8686 has an external reset gpio line which is currently being >>> manipulated manually by an out-of-tree kernel patch: >>> >>> http://dev.laptop.org/git/olpc-2.6/commit/?h=olpc-2.6.35&id=e9bee721fb0cc303286d1fe5df4930ce79b0b1e0 >>> >>> ... which makes me wonder whether we really want to take that >>> MMC_CAP_RUNTIME_PM road. I'm not sure anymore. >> >> OLPC is not the only user of the sd8686. >> Every other user will face the same problem. >> >> Other users may not have the luxury of having a GPIO hooked up to the >> reset line. > > Agree; those users will need a MMC_CAP_RUNTIME_PM (or maybe call it > with the capability it really stands for which is something like > MMC_CAP_POWER_OFF_CARD). > > But I want to be positively sure we have such users (or is it that obvious?). > > How is the sd8686's reset line manipulated on other platforms ? Or is > the sd8686 usually just kept powered on after boot ? On our platforms we just keep it powered on after boot with the reset line held high. (e.g. arch/arm/mach-pxa/cm-x300.c, arch/arm/mach-omap/board-cm-t35.c). We don't bother much for power savings, though. > I'm looping in libertas-dev. > > Thanks, > Ohad. > > _______________________________________________ > libertas-dev mailing list > libertas-dev@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/libertas-dev -- Sincerely yours, Mike. ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-17 6:46 ` Mike Rapoport @ 2010-11-17 7:29 ` Ohad Ben-Cohen 2010-11-17 14:54 ` Nicolas Pitre 0 siblings, 1 reply; 48+ messages in thread From: Ohad Ben-Cohen @ 2010-11-17 7:29 UTC (permalink / raw) To: Mike Rapoport Cc: Daniel Drake, Arnd Hannemann, Chris Ball, linux-mmc, Nicolas Pitre, libertas-dev On Wed, Nov 17, 2010 at 8:46 AM, Mike Rapoport <mike@compulab.co.il> wrote: > On our platforms we just keep it powered on after boot with the reset line held > high. (e.g. arch/arm/mach-pxa/cm-x300.c, arch/arm/mach-omap/board-cm-t35.c). > We don't bother much for power savings, though. Thanks, Mike ! This paves the road for MMC_CAP_POWER_OFF_CARD. SDIO core will enable runtime PM for the card only if that cap is set. As a result, the card will be powered down after boot, and will only be powered up again when a driver is loaded (and then it's up to the driver whether power will be kept or not). ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-17 7:29 ` Ohad Ben-Cohen @ 2010-11-17 14:54 ` Nicolas Pitre 0 siblings, 0 replies; 48+ messages in thread From: Nicolas Pitre @ 2010-11-17 14:54 UTC (permalink / raw) To: Ohad Ben-Cohen Cc: Mike Rapoport, Daniel Drake, Arnd Hannemann, Chris Ball, linux-mmc, libertas-dev On Wed, 17 Nov 2010, Ohad Ben-Cohen wrote: > On Wed, Nov 17, 2010 at 8:46 AM, Mike Rapoport <mike@compulab.co.il> wrote: > > On our platforms we just keep it powered on after boot with the reset line held > > high. (e.g. arch/arm/mach-pxa/cm-x300.c, arch/arm/mach-omap/board-cm-t35.c). > > We don't bother much for power savings, though. > > Thanks, Mike ! > > This paves the road for MMC_CAP_POWER_OFF_CARD. > > SDIO core will enable runtime PM for the card only if that cap is set. > As a result, the card will be powered down after boot, and will only > be powered up again when a driver is loaded (and then it's up to the > driver whether power will be kept or not). Makes sense. Nicolas ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-16 13:22 ` Ohad Ben-Cohen 2010-11-16 14:29 ` Daniel Drake @ 2010-11-16 17:17 ` Arnd Hannemann 2010-11-16 20:58 ` Ohad Ben-Cohen 1 sibling, 1 reply; 48+ messages in thread From: Arnd Hannemann @ 2010-11-16 17:17 UTC (permalink / raw) To: Ohad Ben-Cohen; +Cc: Daniel Drake, linux-mmc, Chris Ball Am 16.11.2010 14:22, schrieb Ohad Ben-Cohen: > On Mon, Nov 1, 2010 at 10:27 AM, Ohad Ben-Cohen <ohad@wizery.com> wrote: > >> On Sun, Oct 31, 2010 at 9:06 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote: >> ... >> >>> we need to support boards with controllers/cards >>> which we can't power off in runtime. >>> >>> On such boards, the right thing to do would be to disable runtime PM altogether. >>> >> The patch below (/attached) should trivially fix the problem - can you >> please check it out ? >> >> It's very simple: it just requires hosts to explicitly indicate they >> support runtime powering off/on their cards (by means of >> MMC_CAP_RUNTIME_PM). >> >> There is no way around this I'm afraid: it is legitimate for a >> board/host/card configuration not to support powering off the card >> after boot, and we must allow it. >> >> In addition having an explicit MMC_CAP_RUNTIME_PM will also allow us >> smoother transition to runtime PM behavior. Developers will have to >> explicitly turn it on, and will not be surprised if things won't >> immediately work. >> >> Please note that this cap is not interchangeable, and can't be replace >> with CONFIG_PM_RUNTIME. >> >> Having said that, I still think we need to understand why CMD3 timed >> out on the XO-1.5. >> > Just to update the list, the problem with the XO-1.5 was because the > sd8686 has an external reset gpio line which is currently being > manipulated manually by an out-of-tree kernel patch: > > http://dev.laptop.org/git/olpc-2.6/commit/?h=olpc-2.6.35&id=e9bee721fb0cc303286d1fe5df4930ce79b0b1e0 > > ... which makes me wonder whether we really want to take that > MMC_CAP_RUNTIME_PM road. I'm not sure anymore. > > We need a demonstrated hardware limitation before we take that > approach (or a setup which worked using vanilla kernels and now > doesn't), because frankly this MMC_CAP_RUNTIME_PM approach is > cumbersome and involving. I would not like to introduce it just to fix > out-of-tree software issues, and it seems that at least the XO-1.5 > case can be cleanly solved by software (e.g. by letting the regulator > handle that sd8686 quirk). > > I'm looping in Arnd, who reported similar problems with b43-sdio on > AP4EVB (arm) with tmio_mmc. > > Arnd, > > We're trying to exactly understand the reasons behind the reported > SDIO runtime pm failures (we had two, yours, and OLPC). So far it > seems that the OLPC had a software issue, and I'm wondering about > yours. > > Can you please supply additional information about your board ? where > does your wifi card gets its power from ? is there an external gpio > involved ? Were you able to work with vanilla kernels (prior to > 2.6.37) or do you carry some out-of-tree patches ? > Its an AP4 (SH7372) evaluation board from renesas. It has an SD-Slot, where you plug the SDIO card into it. No special wiring or something like this. So I doubt some external GPIOs are involved. I have no idea how the wifi card gets its power, but I hope its over some well defined pins within the SD slot... I was able to work with 2.6.35 and .36 plus some out-of-tree patches for the sh_mobile_sdhi mfd, which are now in mainline: mmc: Allow 2 byte requests in 4-bit mode for tmio_mmc tmio_mmc: don't clear unhandled pending interrupts tmio_mmc: don't clear unhandled pending interrupts >> From 6b5691bdd8184cc0876d209c69d38844ea10f678 Mon Sep 17 00:00:00 2001 >> From: Ohad Ben-Cohen <ohad@wizery.com> >> 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. >> >> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com> >> --- >> 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(-) >> >> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c >> index 6a9ad40..373d56d 100644 >> --- a/drivers/mmc/core/sdio.c >> +++ b/drivers/mmc/core/sdio.c >> @@ -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); >> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c >> index 2716c7a..f3ce21b 100644 >> --- a/drivers/mmc/core/sdio_bus.c >> +++ b/drivers/mmc/core/sdio_bus.c >> @@ -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; >> } >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >> index c3ffad8..e5eee0e 100644 >> --- a/include/linux/mmc/host.h >> +++ b/include/linux/mmc/host.h >> @@ -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 >> >> ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-16 17:17 ` Arnd Hannemann @ 2010-11-16 20:58 ` Ohad Ben-Cohen 2010-11-16 21:16 ` Arnd Hannemann 0 siblings, 1 reply; 48+ messages in thread From: Ohad Ben-Cohen @ 2010-11-16 20:58 UTC (permalink / raw) To: Arnd Hannemann; +Cc: Daniel Drake, linux-mmc, Chris Ball On Tue, Nov 16, 2010 at 7:17 PM, Arnd Hannemann <hannemann@nets.rwth-aachen.de> >> > Its an AP4 (SH7372) evaluation board from renesas. It has an SD-Slot, > where you plug the SDIO card into it. No special wiring or something > like this. So I doubt some external GPIOs are involved. > I have no idea how the wifi card gets its power, but I hope its over > some well defined pins within the SD slot... Can you please specify what was the wlan card you used ? is it a commercial card or an evaluation board of some sort ? You said it, but just to be sure - I assume there is no external power supply involved with that card, right ? it's just plugged into the SD slot with no extra wire whatsoever ? Thanks! Ohad. > I was able to work with 2.6.35 and .36 plus some out-of-tree patches > for the sh_mobile_sdhi mfd, which are now in mainline: > mmc: Allow 2 byte requests in 4-bit mode for tmio_mmc > tmio_mmc: don't clear unhandled pending interrupts > tmio_mmc: don't clear unhandled pending interrupts > >>> From 6b5691bdd8184cc0876d209c69d38844ea10f678 Mon Sep 17 00:00:00 2001 >>> From: Ohad Ben-Cohen <ohad@wizery.com> >>> 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. >>> >>> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com> >>> --- >>> 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(-) >>> >>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c >>> index 6a9ad40..373d56d 100644 >>> --- a/drivers/mmc/core/sdio.c >>> +++ b/drivers/mmc/core/sdio.c >>> @@ -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); >>> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c >>> index 2716c7a..f3ce21b 100644 >>> --- a/drivers/mmc/core/sdio_bus.c >>> +++ b/drivers/mmc/core/sdio_bus.c >>> @@ -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; >>> } >>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >>> index c3ffad8..e5eee0e 100644 >>> --- a/include/linux/mmc/host.h >>> +++ b/include/linux/mmc/host.h >>> @@ -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 >>> >>> > > ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-16 20:58 ` Ohad Ben-Cohen @ 2010-11-16 21:16 ` Arnd Hannemann 2010-11-16 22:26 ` Ohad Ben-Cohen 0 siblings, 1 reply; 48+ messages in thread From: Arnd Hannemann @ 2010-11-16 21:16 UTC (permalink / raw) To: Ohad Ben-Cohen; +Cc: Daniel Drake, linux-mmc, Chris Ball Am 16.11.2010 21:58, schrieb Ohad Ben-Cohen: > On Tue, Nov 16, 2010 at 7:17 PM, Arnd Hannemann > <hannemann@nets.rwth-aachen.de> >> > Its an AP4 (SH7372) evaluation > board from renesas. It has an SD-Slot, > >> where you plug the SDIO card into it. No special wiring or something >> like this. So I doubt some external GPIOs are involved. >> I have no idea how the wifi card gets its power, but I hope its over >> some well defined pins within the SD slot... >> > Can you please specify what was the wlan card you used ? is it a > commercial card or an evaluation board of some sort ? > It says: c-guys sd link11b driver is b43. It's an commercial card. > You said it, but just to be sure - I assume there is no external power > supply involved with that card, right ? it's just plugged into the SD > slot with no extra wire whatsoever ? > Yes, just plugged it in. No extra wire whatsover. Regards, Arnd > Thanks! > Ohad. > > >> I was able to work with 2.6.35 and .36 plus some out-of-tree patches >> for the sh_mobile_sdhi mfd, which are now in mainline: >> mmc: Allow 2 byte requests in 4-bit mode for tmio_mmc >> tmio_mmc: don't clear unhandled pending interrupts >> tmio_mmc: don't clear unhandled pending interrupts >> >> >>>> From 6b5691bdd8184cc0876d209c69d38844ea10f678 Mon Sep 17 00:00:00 2001 >>>> From: Ohad Ben-Cohen <ohad@wizery.com> >>>> 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. >>>> >>>> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com> >>>> --- >>>> 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(-) >>>> >>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c >>>> index 6a9ad40..373d56d 100644 >>>> --- a/drivers/mmc/core/sdio.c >>>> +++ b/drivers/mmc/core/sdio.c >>>> @@ -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); >>>> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c >>>> index 2716c7a..f3ce21b 100644 >>>> --- a/drivers/mmc/core/sdio_bus.c >>>> +++ b/drivers/mmc/core/sdio_bus.c >>>> @@ -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; >>>> } >>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >>>> index c3ffad8..e5eee0e 100644 >>>> --- a/include/linux/mmc/host.h >>>> +++ b/include/linux/mmc/host.h >>>> @@ -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 >>>> >>>> >>>> >> >> > ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-11-16 21:16 ` Arnd Hannemann @ 2010-11-16 22:26 ` Ohad Ben-Cohen 0 siblings, 0 replies; 48+ messages in thread From: Ohad Ben-Cohen @ 2010-11-16 22:26 UTC (permalink / raw) To: Arnd Hannemann; +Cc: Daniel Drake, linux-mmc, Chris Ball On Tue, Nov 16, 2010 at 11:16 PM, Arnd Hannemann <arnd@arndnet.de> wrote: > Yes, just plugged it in. No extra wire whatsover. I wonder - when you suspend/resume the host, with that card plugged in, do you see any errors (particularly in the resume phase) ? After suspend/resume was completed, can you still work with that card (wlan is still functional) ? I'm asking because SDIO suspend/resume is very similar to what the new 2.6.37-rc1 SDIO runtime pm code does. Thanks, Ohad. > > Regards, > Arnd > >> Thanks! >> Ohad. >> >> >>> I was able to work with 2.6.35 and .36 plus some out-of-tree patches >>> for the sh_mobile_sdhi mfd, which are now in mainline: >>> mmc: Allow 2 byte requests in 4-bit mode for tmio_mmc >>> tmio_mmc: don't clear unhandled pending interrupts >>> tmio_mmc: don't clear unhandled pending interrupts >>> >>> >>>>> From 6b5691bdd8184cc0876d209c69d38844ea10f678 Mon Sep 17 00:00:00 2001 >>>>> From: Ohad Ben-Cohen <ohad@wizery.com> >>>>> 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. >>>>> >>>>> Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com> >>>>> --- >>>>> 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(-) >>>>> >>>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c >>>>> index 6a9ad40..373d56d 100644 >>>>> --- a/drivers/mmc/core/sdio.c >>>>> +++ b/drivers/mmc/core/sdio.c >>>>> @@ -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); >>>>> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c >>>>> index 2716c7a..f3ce21b 100644 >>>>> --- a/drivers/mmc/core/sdio_bus.c >>>>> +++ b/drivers/mmc/core/sdio_bus.c >>>>> @@ -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; >>>>> } >>>>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >>>>> index c3ffad8..e5eee0e 100644 >>>>> --- a/include/linux/mmc/host.h >>>>> +++ b/include/linux/mmc/host.h >>>>> @@ -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 >>>>> >>>>> >>>>> >>> >>> >> > > ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2010-10-31 19:06 ` Ohad Ben-Cohen 2010-11-01 8:27 ` Ohad Ben-Cohen @ 2011-05-29 16:21 ` Daniel Drake 2011-05-30 6:52 ` Ohad Ben-Cohen 1 sibling, 1 reply; 48+ messages in thread From: Daniel Drake @ 2011-05-29 16:21 UTC (permalink / raw) To: Ohad Ben-Cohen; +Cc: linux-mmc [-- Attachment #1: Type: text/plain, Size: 2601 bytes --] Hi Ohad, On 31 October 2010 19:06, Ohad Ben-Cohen <ohad@wizery.com> wrote: > OK, as expected. > > So to summarize: > 1. Card is powered up at boot, and successfully initialized > 2. After mmc + sdio devices are added to the device tree, power is > (seemingly) taken down by runtime PM > 3. When the driver is loaded, card is powered up again, but doesn't > respond to CMD3 > > The only explanation I can think of why the card doesn't respond to > CMD3, after its power is brought up again, is that we didn't have a > full reset here (i.e. mmc_power_off() didn't completely power off > everything). I have investigated this again, as we'd like runtime PM to work. It's certainly possible that there's something weird about the hardware in question, but we *are* able to successfully power down and up the card with a hacky rfkill driver that calls mmc_stop_host / mmc_start_host. So I went around finding out what the difference was between these functions and the runtime PM implementation. Through this comparison I think mmc_power_save_host() does almost exactly the same as mmc_stop_host() (good), but mmc_power_restore_host() lacks some steps which would otherwise be taken by mmc_start_host(). These are: in mmc_rescan_try_freq(): /* * sdio_reset sends CMD52 to reset card. Since we do not know * if the card is being re-initialized, just send it. CMD52 * should be ignored by SD/eMMC cards. */ sdio_reset(host); mmc_go_idle(host); mmc_send_if_cond(host, host->ocr_avail); In mmc_attach_sdio(): err = mmc_send_io_op_cond(host, 0, &ocr); if (err) return err; mmc_attach_bus(host, &mmc_sdio_ops); if (host->ocr_avail_sdio) host->ocr_avail = host->ocr_avail_sdio; /* * Sanity check the voltages that the card claims to * support. */ if (ocr & 0x7F) { printk(KERN_WARNING "%s: card claims to support voltages " "below the defined range. These will be ignored.\n", mmc_hostname(host)); ocr &= ~0x7F; } host->ocr = mmc_select_voltage(host, ocr); /* * Can we support the voltage(s) of the card(s)? */ if (!host->ocr) { err = -EINVAL; goto err; } Should anything in those code snippets be running during runtime PM resume? I went ahead and ran the obvious test by putting those bits of code in the runtime PM resume path... the first snippet doesn't seem to improve or hurt anything, but the second snippet fixes the problem. At least it means I can boot, the card gets powered down during boot, then I load the module and it powers up and initialises correctly. Patch attached for clarity. Any thoughts? Thanks, Daniel [-- Attachment #2: fix-sdio-power-restore.txt --] [-- Type: text/plain, Size: 1166 bytes --] diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index 4d0c15b..3b06379 100644 --- a/drivers/mmc/core/sdio.c +++ b/drivers/mmc/core/sdio.c @@ -691,15 +691,47 @@ static int mmc_sdio_resume(struct mmc_host *host) static int mmc_sdio_power_restore(struct mmc_host *host) { int ret; + u32 ocr; BUG_ON(!host); BUG_ON(!host->card); mmc_claim_host(host); + + ret = mmc_send_io_op_cond(host, 0, &ocr); + if (ret) + goto out; + + if (host->ocr_avail_sdio) + host->ocr_avail = host->ocr_avail_sdio; + + /* + * Sanity check the voltages that the card claims to + * support. + */ + if (ocr & 0x7F) { + printk(KERN_WARNING "%s: card claims to support voltages " + "below the defined range. These will be ignored.\n", + mmc_hostname(host)); + ocr &= ~0x7F; + } + + host->ocr = mmc_select_voltage(host, ocr); + + /* + * Can we support the voltage(s) of the card(s)? + */ + if (!host->ocr) { + ret = -EINVAL; + goto out; + } + ret = mmc_sdio_init_card(host, host->ocr, host->card, mmc_card_keep_power(host)); if (!ret && host->sdio_irqs) mmc_signal_sdio_irq(host); + +out: mmc_release_host(host); return ret; ^ permalink raw reply related [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2011-05-29 16:21 ` Daniel Drake @ 2011-05-30 6:52 ` Ohad Ben-Cohen 2011-05-30 7:01 ` Daniel Drake 0 siblings, 1 reply; 48+ messages in thread From: Ohad Ben-Cohen @ 2011-05-30 6:52 UTC (permalink / raw) To: Daniel Drake; +Cc: linux-mmc Hi Daniel, On Sun, May 29, 2011 at 7:21 PM, Daniel Drake <dsd@laptop.org> wrote: > It's certainly possible that there's something weird about the > hardware in question, but we *are* able to successfully power down and > up the card with a hacky rfkill driver that calls mmc_stop_host / > mmc_start_host. Are we talking about the XO-1.5 and the sd8686 ? Last we talked, we found out runtime PM didn't work because the sd8686 required an additional manipulation of an external reset gpio line, and that the only reason OLPC could power it down/up was this patch: http://dev.laptop.org/git/olpc-2.6/commit/?h=olpc-2.6.35&id=e9bee721fb0cc303286d1fe5df4930ce79b0b1e0 One of the suggested solutions back then was to use a regulator for this. Has anything changed since then ? does mmc_stop_host+mmc_start_host work for you without manipulating that reset gpio ? > err = mmc_send_io_op_cond(host, 0, &ocr); > if (err) > return err; This part actually makes sense. While sending a CMD5 arg=0 is not mandatory when initializing embedded SDIO cards (like the wl12xx is), some cards might still require it, and it is actually anyway required by the spec for removal cards. I just tried it briefly with the wl12xx, and things seems ok. > printk(KERN_WARNING "%s: card claims to support voltages " > "below the defined range. These will be ignored.\n", > mmc_hostname(host)); Please drop this warning though. It was already displayed when mmc_attach_sdio() executed, so the user already knows his card has this thing. With the wl12xx, you'd see this everytime you bring up the interface, so it's really unnecessarily too noisy. > Any thoughts? One other thing to consider is system resume. when wol is disabled, and your chip is powered off during system suspend, you'd need this CMD5 arg=0 in the resume path as well. Some code refactoring should be considered to avoid duplicating this snippet three times though. Otherwise, submit :) Thanks, Ohad. ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2011-05-30 6:52 ` Ohad Ben-Cohen @ 2011-05-30 7:01 ` Daniel Drake 2011-05-30 7:32 ` Ohad Ben-Cohen 0 siblings, 1 reply; 48+ messages in thread From: Daniel Drake @ 2011-05-30 7:01 UTC (permalink / raw) To: Ohad Ben-Cohen; +Cc: linux-mmc On 30 May 2011 07:52, Ohad Ben-Cohen <ohad@wizery.com> wrote: > On Sun, May 29, 2011 at 7:21 PM, Daniel Drake <dsd@laptop.org> wrote: >> It's certainly possible that there's something weird about the >> hardware in question, but we *are* able to successfully power down and >> up the card with a hacky rfkill driver that calls mmc_stop_host / >> mmc_start_host. > > Are we talking about the XO-1.5 and the sd8686 ? Yes. > Last we talked, we found out runtime PM didn't work because the sd8686 > required an additional manipulation of an external reset gpio line, > and that the only reason OLPC could power it down/up was this patch: > > http://dev.laptop.org/git/olpc-2.6/commit/?h=olpc-2.6.35&id=e9bee721fb0cc303286d1fe5df4930ce79b0b1e0 My further investigation here suggests that this change is not necessary. It was added in response to a separate (hard-to-reproduce) issue and it was never known if it would actually fix that issue, it was more of a guess. We don't have any convincing evidence that it helps, so it will be dropped in future. Anyway, just to be sure, I tried combining this hack with runtime PM, and also as a regulator, and it didn't help anything. runtime PM still fails to power up the card. Sorry for leading you down the wrong path there. > does mmc_stop_host+mmc_start_host > work for you without manipulating that reset gpio ? Yes. ... > Otherwise, submit :) Thanks for reviewing, I'll go ahead and clean it up. You didn't comment on the added mmc_select_voltage() call. Is that one also sensible? Thanks, Daniel ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2011-05-30 7:01 ` Daniel Drake @ 2011-05-30 7:32 ` Ohad Ben-Cohen 2011-05-30 11:04 ` zhangfei gao 0 siblings, 1 reply; 48+ messages in thread From: Ohad Ben-Cohen @ 2011-05-30 7:32 UTC (permalink / raw) To: Daniel Drake; +Cc: linux-mmc, Mike Rapoport, Zhangfei Gao On Mon, May 30, 2011 at 10:01 AM, Daniel Drake <dsd@laptop.org> wrote: > On 30 May 2011 07:52, Ohad Ben-Cohen <ohad@wizery.com> wrote: >> Last we talked, we found out runtime PM didn't work because the sd8686 >> required an additional manipulation of an external reset gpio line, >> and that the only reason OLPC could power it down/up was this patch: >> >> http://dev.laptop.org/git/olpc-2.6/commit/?h=olpc-2.6.35&id=e9bee721fb0cc303286d1fe5df4930ce79b0b1e0 > > My further investigation here suggests that this change is not > necessary. It was added in response to a separate (hard-to-reproduce) > issue and it was never known if it would actually fix that issue, it > was more of a guess. We don't have any convincing evidence that it > helps, so it will be dropped in future. > > Anyway, just to be sure, I tried combining this hack with runtime PM, > and also as a regulator, and it didn't help anything. runtime PM still > fails to power up the card. > > Sorry for leading you down the wrong path there. ... >> does mmc_stop_host+mmc_start_host >> work for you without manipulating that reset gpio ? > > Yes. Hm. I still don't entirely get it, because we had others (Mike, cc'd) saying too that the sd8686 requires manipulating an external reset gpio after bringing the power back up. Maybe someone from Marvell can comment on this (cc'ing Zhangfei Gao) ? > You didn't comment on the added mmc_select_voltage() call. Is that one > also sensible? I guess. if we're reading the I/O OCR, might as well use it. This way our runtime PM power up path will also be identical to the one induced by mmc_attach_sdio. ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2011-05-30 7:32 ` Ohad Ben-Cohen @ 2011-05-30 11:04 ` zhangfei gao 2011-05-30 11:16 ` Ohad Ben-Cohen 0 siblings, 1 reply; 48+ messages in thread From: zhangfei gao @ 2011-05-30 11:04 UTC (permalink / raw) To: Ohad Ben-Cohen Cc: Daniel Drake, linux-mmc, Mike Rapoport, Zhangfei Gao, Bing Zhao On Mon, May 30, 2011 at 3:32 AM, Ohad Ben-Cohen <ohad@wizery.com> wrote: > On Mon, May 30, 2011 at 10:01 AM, Daniel Drake <dsd@laptop.org> wrote: >> On 30 May 2011 07:52, Ohad Ben-Cohen <ohad@wizery.com> wrote: >>> Last we talked, we found out runtime PM didn't work because the sd8686 >>> required an additional manipulation of an external reset gpio line, >>> and that the only reason OLPC could power it down/up was this patch: >>> >>> http://dev.laptop.org/git/olpc-2.6/commit/?h=olpc-2.6.35&id=e9bee721fb0cc303286d1fe5df4930ce79b0b1e0 >> >> My further investigation here suggests that this change is not >> necessary. It was added in response to a separate (hard-to-reproduce) >> issue and it was never known if it would actually fix that issue, it >> was more of a guess. We don't have any convincing evidence that it >> helps, so it will be dropped in future. >> >> Anyway, just to be sure, I tried combining this hack with runtime PM, >> and also as a regulator, and it didn't help anything. runtime PM still >> fails to power up the card. >> >> Sorry for leading you down the wrong path there. > ... >>> does mmc_stop_host+mmc_start_host >>> work for you without manipulating that reset gpio ? >> >> Yes. > > Hm. I still don't entirely get it, because we had others (Mike, cc'd) > saying too that the sd8686 requires manipulating an external reset > gpio after bringing the power back up. sd8686 requires two pins to control power, PDn and RESETn. To enable sd8686, PDn and RESETn should both set high. To disable sd8686, PDn should be set low. We also have plans to use runtime PM to optimize these two pins, suggested by Ohad before. However currently we still use rfkill method to control the two pins. In the stress test, we found wlan card may not be probed successfully even the two pins are set correctly, so we manually detect whether mmc->card is allocated or not, not sure whether runtime PM to handle this case. > > Maybe someone from Marvell can comment on this (cc'ing Zhangfei Gao) ? > >> You didn't comment on the added mmc_select_voltage() call. Is that one >> also sensible? > > I guess. if we're reading the I/O OCR, might as well use it. This way > our runtime PM power up path will also be identical to the one induced > by mmc_attach_sdio. > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2011-05-30 11:04 ` zhangfei gao @ 2011-05-30 11:16 ` Ohad Ben-Cohen 2011-06-02 8:39 ` Bing Zhao 0 siblings, 1 reply; 48+ messages in thread From: Ohad Ben-Cohen @ 2011-05-30 11:16 UTC (permalink / raw) To: zhangfei gao Cc: Daniel Drake, linux-mmc, Mike Rapoport, Zhangfei Gao, Bing Zhao Hi Zhangfei, Bing, On Mon, May 30, 2011 at 2:04 PM, zhangfei gao <zhangfei.gao@gmail.com> wrote: > sd8686 requires two pins to control power, PDn and RESETn. > To enable sd8686, PDn and RESETn should both set high. > To disable sd8686, PDn should be set low. If we power down the sd8686, and power it up again, without toggling the reset pin at all (e.g. keep it always high), will the card work ? Thanks, Ohad. ^ permalink raw reply [flat|nested] 48+ messages in thread
* RE: MMC runtime PM patches break libertas probe 2011-05-30 11:16 ` Ohad Ben-Cohen @ 2011-06-02 8:39 ` Bing Zhao 2011-06-02 18:25 ` Ohad Ben-Cohen 0 siblings, 1 reply; 48+ messages in thread From: Bing Zhao @ 2011-06-02 8:39 UTC (permalink / raw) To: Ohad Ben-Cohen, zhangfei gao Cc: Daniel Drake, linux-mmc, Mike Rapoport, Zhangfei Gao Hi Ohad, > If we power down the sd8686, and power it up again, without toggling > the reset pin at all (e.g. keep it always high), will the card work ? Yes. The card should work without toggling the reset pin. Regards, Bing ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2011-06-02 8:39 ` Bing Zhao @ 2011-06-02 18:25 ` Ohad Ben-Cohen 2011-06-03 22:28 ` Bing Zhao 0 siblings, 1 reply; 48+ messages in thread From: Ohad Ben-Cohen @ 2011-06-02 18:25 UTC (permalink / raw) To: Bing Zhao Cc: zhangfei gao, Daniel Drake, linux-mmc, Mike Rapoport, Zhangfei Gao On Thu, Jun 2, 2011 at 11:39 AM, Bing Zhao <bzhao@marvell.com> wrote: >> If we power down the sd8686, and power it up again, without toggling >> the reset pin at all (e.g. keep it always high), will the card work ? > > Yes. The card should work without toggling the reset pin. Thanks. Just for closure-sake, can you please confirm that the sd8686 requires sending a CMD5 arg=0 as part of the initialization sequence after powering it on ? (we'd probably add it anyway, but it'd be nice to get a confirmation about this from you guys) Btw, it will be nice to allow cards to avoid sending this if not required; a simple card quirk will do. wl12xx will definitely use such a quirk. ^ permalink raw reply [flat|nested] 48+ messages in thread
* RE: MMC runtime PM patches break libertas probe 2011-06-02 18:25 ` Ohad Ben-Cohen @ 2011-06-03 22:28 ` Bing Zhao 2011-06-03 22:52 ` Ohad Ben-Cohen 0 siblings, 1 reply; 48+ messages in thread From: Bing Zhao @ 2011-06-03 22:28 UTC (permalink / raw) To: Ohad Ben-Cohen Cc: zhangfei gao, Daniel Drake, linux-mmc, Mike Rapoport, Zhangfei Gao > On Thu, Jun 2, 2011 at 11:39 AM, Bing Zhao <bzhao@marvell.com> wrote: >>> If we power down the sd8686, and power it up again, without toggling >>> the reset pin at all (e.g. keep it always high), will the card work ? >> >> Yes. The card should work without toggling the reset pin. > > Thanks. > > Just for closure-sake, can you please confirm that the sd8686 requires > sending a CMD5 arg=0 as part of the initialization sequence after > powering it on ? "CMD5 Arg=0" refers to the very first CMD5 sent from host during initialization sequence. This is required because our state machine always expects two CMD5 from host (5, 5, 3, 7, ...). Regards, Bing > (we'd probably add it anyway, but it'd be nice to get a confirmation > about this from you guys) > > Btw, it will be nice to allow cards to avoid sending this if not > required; a simple card quirk will do. wl12xx will definitely use such > a quirk. ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2011-06-03 22:28 ` Bing Zhao @ 2011-06-03 22:52 ` Ohad Ben-Cohen 2011-06-07 14:34 ` Arnd Hannemann 2011-06-08 14:34 ` Ohad Ben-Cohen 0 siblings, 2 replies; 48+ messages in thread From: Ohad Ben-Cohen @ 2011-06-03 22:52 UTC (permalink / raw) To: Bing Zhao, Arnd Hannemann Cc: zhangfei gao, Daniel Drake, linux-mmc, Mike Rapoport, Zhangfei Gao (cc'ing Arnd Hannermann) On Sat, Jun 4, 2011 at 1:28 AM, Bing Zhao <bzhao@marvell.com> wrote: > "CMD5 Arg=0" refers to the very first CMD5 sent from host during initialization sequence. > This is required because our state machine always expects two CMD5 from host (5, 5, 3, 7, ...). Great, thanks for confirming this ! Arnd, you have reported SDIO runtime pm issues with the b43 in the past. If you're still interested and have some free cycles, you might want to check out Daniel's patch and see if that fixes the issues you had too. With Daniel's patch we're now following the spec more strictly, and the change is particularly relevant for removable SDIO cards like the one you were using. Daniel, please go ahead and submit your patch when you're ready. Thanks, Ohad. ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2011-06-03 22:52 ` Ohad Ben-Cohen @ 2011-06-07 14:34 ` Arnd Hannemann 2011-06-07 14:45 ` Ohad Ben-Cohen 2011-06-08 14:34 ` Ohad Ben-Cohen 1 sibling, 1 reply; 48+ messages in thread From: Arnd Hannemann @ 2011-06-07 14:34 UTC (permalink / raw) To: Ohad Ben-Cohen Cc: Bing Zhao, zhangfei gao, Daniel Drake, linux-mmc, Mike Rapoport, Zhangfei Gao Hi Ohad, Am 04.06.2011 00:52, schrieb Ohad Ben-Cohen: > (cc'ing Arnd Hannermann) > > On Sat, Jun 4, 2011 at 1:28 AM, Bing Zhao <bzhao@marvell.com> wrote: >> "CMD5 Arg=0" refers to the very first CMD5 sent from host during initialization sequence. >> This is required because our state machine always expects two CMD5 from host (5, 5, 3, 7, ...). > > Great, thanks for confirming this ! > > Arnd, you have reported SDIO runtime pm issues with the b43 in the > past. If you're still interested and have some free cycles, you might > want to check out Daniel's patch and see if that fixes the issues you > had too. With Daniel's patch we're now following the spec more > strictly, and the change is particularly relevant for removable SDIO > cards like the one you were using. Sorry, I don't have the hardware anymore so I'm not able to check this. Best regards Arnd ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2011-06-07 14:34 ` Arnd Hannemann @ 2011-06-07 14:45 ` Ohad Ben-Cohen 0 siblings, 0 replies; 48+ messages in thread From: Ohad Ben-Cohen @ 2011-06-07 14:45 UTC (permalink / raw) To: Arnd Hannemann Cc: Bing Zhao, zhangfei gao, Daniel Drake, linux-mmc, Mike Rapoport, Zhangfei Gao On Tue, Jun 7, 2011 at 5:34 PM, Arnd Hannemann <arnd@arndnet.de> wrote: > Sorry, I don't have the hardware anymore so I'm not able to check this. Ok, thanks for the reply ! ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2011-06-03 22:52 ` Ohad Ben-Cohen 2011-06-07 14:34 ` Arnd Hannemann @ 2011-06-08 14:34 ` Ohad Ben-Cohen 2011-06-10 2:02 ` zhangfei gao 1 sibling, 1 reply; 48+ messages in thread From: Ohad Ben-Cohen @ 2011-06-08 14:34 UTC (permalink / raw) To: Bing Zhao Cc: zhangfei gao, Daniel Drake, linux-mmc, Mike Rapoport, Zhangfei Gao Hi Bing, On Sat, Jun 4, 2011 at 1:52 AM, Ohad Ben-Cohen <ohad@wizery.com> wrote: > On Sat, Jun 4, 2011 at 1:28 AM, Bing Zhao <bzhao@marvell.com> wrote: >> "CMD5 Arg=0" refers to the very first CMD5 sent from host during initialization sequence. >> This is required because our state machine always expects two CMD5 from host (5, 5, 3, 7, ...). > > Great, thanks for confirming this ! I have another question please. Does the sd8686 require an SDIO I/O reset (CMD52 setting bit 3 of address 0x6 of the CCCR) to it after powering it on ? Daniel (cc'ed) is trying to power it off and back on, and it does seem to work in the first time, without sending a reset. In the second time, though, the card doesn't answer CMD5 anymore, unless Daniel is sending it an SDIO I/O reset. I was wondering whether this is an sd8686 requirement, or whether we have some other issue at hand. Thanks, Ohad. ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2011-06-08 14:34 ` Ohad Ben-Cohen @ 2011-06-10 2:02 ` zhangfei gao 2011-06-10 4:28 ` Ohad Ben-Cohen 0 siblings, 1 reply; 48+ messages in thread From: zhangfei gao @ 2011-06-10 2:02 UTC (permalink / raw) To: Ohad Ben-Cohen Cc: Bing Zhao, Daniel Drake, linux-mmc, Mike Rapoport, Zhangfei Gao On Wed, Jun 8, 2011 at 10:34 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote: > Hi Bing, > > On Sat, Jun 4, 2011 at 1:52 AM, Ohad Ben-Cohen <ohad@wizery.com> wrote: >> On Sat, Jun 4, 2011 at 1:28 AM, Bing Zhao <bzhao@marvell.com> wrote: >>> "CMD5 Arg=0" refers to the very first CMD5 sent from host during initialization sequence. >>> This is required because our state machine always expects two CMD5 from host (5, 5, 3, 7, ...). >> >> Great, thanks for confirming this ! > > I have another question please. > > Does the sd8686 require an SDIO I/O reset (CMD52 setting bit 3 of > address 0x6 of the CCCR) to it after powering it on ? > > Daniel (cc'ed) is trying to power it off and back on, and it does seem > to work in the first time, without sending a reset. In the second > time, though, the card doesn't answer CMD5 anymore, unless Daniel is > sending it an SDIO I/O reset. I was wondering whether this is an > sd8686 requirement, or whether we have some other issue at hand. Hi, Ohad Here is answer got from the sd8686 maintainer. For 8686, the SDIO state machine can only handle init sequence (CMD5, 5, 3, 7) from host once. If host sends another init sequence, it will not be able to handle CMD5 and causes the SDIO block to hang. Chips that are newer than 8686 will be able to handle multiple init sequence from host. So yes, for 8686, an IO reset is needed before host can send a new set of init sequence. > > Thanks, > Ohad. > ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2011-06-10 2:02 ` zhangfei gao @ 2011-06-10 4:28 ` Ohad Ben-Cohen 2011-06-11 2:33 ` zhangfei gao 0 siblings, 1 reply; 48+ messages in thread From: Ohad Ben-Cohen @ 2011-06-10 4:28 UTC (permalink / raw) To: zhangfei gao Cc: Bing Zhao, Daniel Drake, linux-mmc, Mike Rapoport, Zhangfei Gao Hi Zhangfei, On Fri, Jun 10, 2011 at 5:02 AM, zhangfei gao <zhangfei.gao@gmail.com> wrote: > Here is answer got from the sd8686 maintainer. > > For 8686, the SDIO state machine can only handle init sequence (CMD5, > 5, 3, 7) from host once. If host sends another init sequence, it will > not be able to handle CMD5 and causes the SDIO block to hang. Chips > that are newer than 8686 will be able to handle multiple init sequence > from host. Thanks for the reply ! > So yes, for 8686, an IO reset is needed before host can send a new set > of init sequence. But if we're powering down and up the device first, then the init sequence is considered the first one, and then we don't need an IO reset, right ? That was what we wondered about. Thanks, Ohad. ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2011-06-10 4:28 ` Ohad Ben-Cohen @ 2011-06-11 2:33 ` zhangfei gao 2011-06-11 9:03 ` Ohad Ben-Cohen 0 siblings, 1 reply; 48+ messages in thread From: zhangfei gao @ 2011-06-11 2:33 UTC (permalink / raw) To: Ohad Ben-Cohen Cc: Bing Zhao, Daniel Drake, linux-mmc, Mike Rapoport, Zhangfei Gao, Benson Chau On Fri, Jun 10, 2011 at 12:28 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote: > Hi Zhangfei, > > On Fri, Jun 10, 2011 at 5:02 AM, zhangfei gao <zhangfei.gao@gmail.com> wrote: >> Here is answer got from the sd8686 maintainer. >> >> For 8686, the SDIO state machine can only handle init sequence (CMD5, >> 5, 3, 7) from host once. If host sends another init sequence, it will >> not be able to handle CMD5 and causes the SDIO block to hang. Chips >> that are newer than 8686 will be able to handle multiple init sequence >> from host. > > Thanks for the reply ! > >> So yes, for 8686, an IO reset is needed before host can send a new set >> of init sequence. > > But if we're powering down and up the device first, then the init > sequence is considered the first one, and then we don't need an IO > reset, right ? That was what we wondered about. Hi Ohad, If you power down and up the device, then IO reset is not needed and 8686 can process host init sequence correctly. CC Benson. > > Thanks, > Ohad. > ^ permalink raw reply [flat|nested] 48+ messages in thread
* Re: MMC runtime PM patches break libertas probe 2011-06-11 2:33 ` zhangfei gao @ 2011-06-11 9:03 ` Ohad Ben-Cohen 0 siblings, 0 replies; 48+ messages in thread From: Ohad Ben-Cohen @ 2011-06-11 9:03 UTC (permalink / raw) To: zhangfei gao Cc: Bing Zhao, Daniel Drake, linux-mmc, Mike Rapoport, Zhangfei Gao, Benson Chau On Sat, Jun 11, 2011 at 5:33 AM, zhangfei gao <zhangfei.gao@gmail.com> wrote: > If you power down and up the device, then IO reset is not needed and > 8686 can process host init sequence correctly. Thanks, that's what I thought. Daniel, this ultimately means that whenever sending a reset is required to re-init the 8686, we can surely say that the chip wasn't really powered off beforehand, and that something went wrong in the software, leading us to think the chip is powered off when it is really not. But I think we also demonstrated this with the simple insmod/rmmod/insmod scenario, where every insmod successfully re-initialized the chip without sending an sdio reset. Thanks, Ohad. ^ permalink raw reply [flat|nested] 48+ messages in thread
end of thread, other threads:[~2011-06-11 9:03 UTC | newest] Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-10-31 14:29 MMC runtime PM patches break libertas probe Daniel Drake 2010-10-31 14:42 ` Ohad Ben-Cohen 2010-10-31 14:55 ` Daniel Drake 2010-10-31 15:08 ` Ohad Ben-Cohen 2010-10-31 15:10 ` Daniel Drake 2010-10-31 15:16 ` Ohad Ben-Cohen 2010-10-31 15:21 ` Daniel Drake 2010-10-31 15:21 ` Daniel Drake 2010-10-31 15:27 ` Ohad Ben-Cohen 2010-10-31 15:57 ` Daniel Drake 2010-10-31 16:16 ` Ohad Ben-Cohen 2010-10-31 16:24 ` Daniel Drake 2010-10-31 19:06 ` Ohad Ben-Cohen 2010-11-01 8:27 ` Ohad Ben-Cohen 2010-11-06 21:19 ` Daniel Drake 2010-11-07 1:48 ` Nicolas Pitre 2010-11-07 10:19 ` Daniel Drake 2010-11-07 15:12 ` Nicolas Pitre 2010-11-07 10:42 ` Ohad Ben-Cohen 2010-11-07 10:51 ` Daniel Drake 2010-11-07 13:17 ` Ohad Ben-Cohen 2010-11-16 13:22 ` Ohad Ben-Cohen 2010-11-16 14:29 ` Daniel Drake 2010-11-16 14:49 ` Ohad Ben-Cohen 2010-11-17 6:46 ` Mike Rapoport 2010-11-17 7:29 ` Ohad Ben-Cohen 2010-11-17 14:54 ` Nicolas Pitre 2010-11-16 17:17 ` Arnd Hannemann 2010-11-16 20:58 ` Ohad Ben-Cohen 2010-11-16 21:16 ` Arnd Hannemann 2010-11-16 22:26 ` Ohad Ben-Cohen 2011-05-29 16:21 ` Daniel Drake 2011-05-30 6:52 ` Ohad Ben-Cohen 2011-05-30 7:01 ` Daniel Drake 2011-05-30 7:32 ` Ohad Ben-Cohen 2011-05-30 11:04 ` zhangfei gao 2011-05-30 11:16 ` Ohad Ben-Cohen 2011-06-02 8:39 ` Bing Zhao 2011-06-02 18:25 ` Ohad Ben-Cohen 2011-06-03 22:28 ` Bing Zhao 2011-06-03 22:52 ` Ohad Ben-Cohen 2011-06-07 14:34 ` Arnd Hannemann 2011-06-07 14:45 ` Ohad Ben-Cohen 2011-06-08 14:34 ` Ohad Ben-Cohen 2011-06-10 2:02 ` zhangfei gao 2011-06-10 4:28 ` Ohad Ben-Cohen 2011-06-11 2:33 ` zhangfei gao 2011-06-11 9:03 ` Ohad Ben-Cohen
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.