Hi Daniel, 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. Thanks, Ohad. >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
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