Am 16.11.2010 14:22, schrieb Ohad Ben-Cohen: > On Mon, Nov 1, 2010 at 10:27 AM, Ohad Ben-Cohen <ohad@xxxxxxxxxx> wrote: > >> On Sun, Oct 31, 2010 at 9:06 PM, Ohad Ben-Cohen <ohad@xxxxxxxxxx> 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@xxxxxxxxxx> >> 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@xxxxxxxxxx> >> --- >> 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 >> >> -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
