Re: [PATCH V3 4/4] mmc: core: Support aggressive power management for (e)MMC/SD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux