On 2 May 2013 12:38, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: > On 16/04/13 13:00, Ulf Hansson wrote: >> Aggressive power management is suitable when saving power is >> essential. At request inactivity timeout, aka pm runtime >> autosuspend timeout, the card will be suspended. >> >> Once a new request arrives, the card will be re-initalized and >> thus the first request will suffer from a latency. This latency >> is card-specific, experiments has shown in general that SD-cards >> has quite poor initialization time, around 300ms-1100ms. eMMC is >> not surprisingly far better but still a couple of hundreds of ms >> has been observed. >> >> Except for the request latency, it is important to know that >> suspending the card will also prevent the card from executing >> internal house-keeping operations in idle mode. This could mean >> degradation in performance. >> >> To use this feature make sure the request inactivity timeout is >> chosen carefully. This has not been done as a part of this patch. >> >> Enable this feature by using host cap MMC_CAP_AGGRESSIVE_PM and >> by setting CONFIG_MMC_UNSAFE_RESUME. >> >> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx> >> Cc: Maya Erez <merez@xxxxxxxxxxxxxx> >> Cc: Subhash Jadavani <subhashj@xxxxxxxxxxxxxx> >> Cc: Arnd Bergmann <arnd@xxxxxxxx> >> Cc: Kevin Liu <kliu5@xxxxxxxxxxx> >> Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx> >> Cc: Daniel Drake <dsd@xxxxxxxxxx> >> Cc: Ohad Ben-Cohen <ohad@xxxxxxxxxx> >> --- >> drivers/mmc/core/mmc.c | 43 +++++++++++++++++++++++++++++++++++++++++++ >> drivers/mmc/core/sd.c | 42 ++++++++++++++++++++++++++++++++++++++++++ >> include/linux/mmc/host.h | 2 +- >> 3 files changed, 86 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c >> index bf19058..8dfbc84 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -1454,6 +1454,47 @@ static int mmc_resume(struct mmc_host *host) >> return err; >> } >> >> + >> +/* >> + * Callback for runtime_suspend. >> + */ >> +static int mmc_runtime_suspend(struct mmc_host *host) >> +{ >> + int err; >> + >> + if (!(host->caps & MMC_CAP_AGGRESSIVE_PM)) >> + return 0; >> + > > mmc_power_off() needs to be within mmc_claim_host() / mmc_release_host(). > > Claiming is nested, so you can out put mmc_claim_host() here: > > mmc_claim_host(host); OK > > >> + err = mmc_suspend(host); >> + if (err) { >> + pr_err("%s: error %d doing aggessive suspend\n", >> + mmc_hostname(host), err); >> + return err; > > goto out; > >> + } >> + >> + mmc_power_off(host); > > out: > mmc_release_host(host); > OK >> + return err; >> +} >> + >> +/* >> + * Callback for runtime_resume. >> + */ >> +static int mmc_runtime_resume(struct mmc_host *host) >> +{ >> + int err; >> + >> + if (!(host->caps & MMC_CAP_AGGRESSIVE_PM)) >> + return 0; >> + >> + mmc_power_up(host); > > As above > OK >> + err = mmc_resume(host); >> + if (err) >> + pr_err("%s: error %d doing aggessive resume\n", >> + mmc_hostname(host), err); >> + >> + return err; > > The power is on - leaving the device in a RPM_SUSPENDED state does not seem > useful so better to return zero here. OK > >> +} >> + >> static int mmc_power_restore(struct mmc_host *host) >> { >> int ret; >> @@ -1514,6 +1555,8 @@ static const struct mmc_bus_ops mmc_ops_unsafe = { >> .detect = mmc_detect, >> .suspend = mmc_suspend, >> .resume = mmc_resume, >> + .runtime_suspend = mmc_runtime_suspend, >> + .runtime_resume = mmc_runtime_resume, >> .power_restore = mmc_power_restore, >> .alive = mmc_alive, >> }; >> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c >> index 30387d6..e0458f9 100644 >> --- a/drivers/mmc/core/sd.c >> +++ b/drivers/mmc/core/sd.c >> @@ -1095,6 +1095,46 @@ static int mmc_sd_resume(struct mmc_host *host) >> return err; >> } >> >> +/* >> + * Callback for runtime_suspend. >> + */ >> +static int mmc_sd_runtime_suspend(struct mmc_host *host) >> +{ >> + int err; >> + >> + if (!(host->caps & MMC_CAP_AGGRESSIVE_PM)) >> + return 0; >> + >> + err = mmc_sd_suspend(host); >> + if (err) { >> + pr_err("%s: error %d doing aggessive suspend\n", >> + mmc_hostname(host), err); >> + return err; >> + } >> + >> + mmc_power_off(host); > > As above OK > >> + return err; >> +} >> + >> +/* >> + * Callback for runtime_resume. >> + */ >> +static int mmc_sd_runtime_resume(struct mmc_host *host) >> +{ >> + int err; >> + >> + if (!(host->caps & MMC_CAP_AGGRESSIVE_PM)) >> + return 0; >> + >> + mmc_power_up(host); > > As above OK > >> + err = mmc_sd_resume(host); >> + if (err) >> + pr_err("%s: error %d doing aggessive resume\n", >> + mmc_hostname(host), err); >> + >> + return err; > > As above OK > > return 0 > >> +} >> + >> static int mmc_sd_power_restore(struct mmc_host *host) >> { >> int ret; >> @@ -1119,6 +1159,8 @@ static const struct mmc_bus_ops mmc_sd_ops = { >> static const struct mmc_bus_ops mmc_sd_ops_unsafe = { >> .remove = mmc_sd_remove, >> .detect = mmc_sd_detect, >> + .runtime_suspend = mmc_sd_runtime_suspend, >> + .runtime_resume = mmc_sd_runtime_resume, >> .suspend = mmc_sd_suspend, >> .resume = mmc_sd_resume, >> .power_restore = mmc_sd_power_restore, >> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h >> index 17d7148..cec6684 100644 >> --- a/include/linux/mmc/host.h >> +++ b/include/linux/mmc/host.h >> @@ -239,7 +239,7 @@ struct mmc_host { >> #define MMC_CAP_SPI (1 << 4) /* Talks only SPI protocols */ >> #define MMC_CAP_NEEDS_POLL (1 << 5) /* Needs polling for card-detection */ >> #define MMC_CAP_8_BIT_DATA (1 << 6) /* Can the host do 8 bit transfers */ >> - >> +#define MMC_CAP_AGGRESSIVE_PM (1 << 7) /* Suspend (e)MMC/SD at idle */ > > Using a "cap" is not ideal here - it should really be under the control of > user space. Several other caps could be debated whether these should exist here as well, but let's leave that to a separate discussion. Until there are a solution for how to add new mmc features per host, which should be runtime configurable, I believe this is the only way we can do it. Do note, that runtime pm can be disabled from user space, which indirectly will prevent this feature from being used. > >> #define MMC_CAP_NONREMOVABLE (1 << 8) /* Nonremovable e.g. eMMC */ >> #define MMC_CAP_WAIT_WHILE_BUSY (1 << 9) /* Waits while card is busy */ >> #define MMC_CAP_ERASE (1 << 10) /* Allow erase/trim commands */ >> > -- 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