On 02/05/13 14:09, Ulf Hansson wrote:
> On 2 May 2013 11:57, Asutosh Das <asuto...@codeaurora.org> wrote:
>> On 5/2/2013 3:22 PM, Ulf Hansson wrote:
>>>
>>> On 2 May 2013 10:58, Adrian Hunter <adrian.hun...@intel.com> wrote:
>>>>
>>>> On 16/04/13 13:00, Ulf Hansson wrote:
>>>>>
>>>>> From: Ulf Hansson <ulf.hans...@linaro.org>
>>>>>
>>>>> Once the mmc blkdevice is being probed, runtime pm will be enabled.
>>>>> By using runtime autosuspend, the power save operations can be done
>>>>> when request inactivity occurs for a certain time. Right now the
>>>>> selected timeout value is set to 3 s. Obviously this value will likely
>>>>> need to be configurable somehow since it needs to be trimmed depending
>>>>> on the power save algorithm.
>>>>
>>>> Already is configurable in sysfs "power/autosuspend_delay_ms"
>>>
>>> Yes you are right - it is already configurable from sysfs point of view.
>>>
>>> What I had in mind was the default values, it could very well be
>>> depending on what power save operation that is to be scheduled. For
>>> example the aggressive power gating could likely use a longer timeout
>>> than when about to schedule idle BKOPS. Anyway, I think it is fair to
>>> leave this to a future patch to handle.
>>>
>>>> Another issue is that re-initialization consumes power - possibly more
>>>> than
>>>> is being saved by powering off. I wonder if the default value of 3
>>>> seconds
>>>> is realistic. Do you have any numbers to compare idle power consumption
>>>> with the power consumed by re-initialization?
>>>
>>> This is very card specific data.
>>>
>>> - I have not done a measurement for the power consumed during a
>>> re-init, but only the time it takes to do the re-init. Typically we
>>> are talking about hundreds of ms.
>>>
>>> - I have done measurements for a handful of pretty new SD-cards and
>>> for some eMMCs (preventing them from putting them to "sleep"). uSD:
>>> ~100-400 uA, eMMC: ~10-150 uA - in both cases I expect the card to not
>>> do any internal house keeping operations, since then we would likely
>>> be talking about tens of mA instead.
>>>
>>> The above numbers should also be possible to be fetched from vendors
>>> data sheets.
>>>
>>> Maybe some in the mmc code aurora team can elaborate more, since they
>>> have been using this feature for a while now in end user products I
>>> believe. Would be interesting to know what timeout they have chosen
>>> here.
>>
>> Currently, we have a default idle-timeout of 10 seconds. However, we haven't
>> pulled in and tested Ulf's patch series yet.
>
> That sounds reasonable!
>
> Although, I suggest we stick with the 3 s timeout in this patchset,
> since it's to be considered as the generic default timeout. Then we
> can send a new patch which possibly consider what power save
> operations that are to be scheduled before changing timeout. Is that a
> way forward for you?
OK
>
>>>
>>>
>>> Kind regards
>>> Ulf Hansson
>>>
>>>>> For SD-combo cards, we are still leaving the enablement of runtime PM
>>>>> to the SDIO init sequence since it depends on the capabilities of the
>>>>> SDIO func driver.
>>>>>
>>>>> Moreover, when the blk device is being suspended, we make sure the
>>>>> device
>>>>> will be runtime resumed. The reason for doing this is that we want the
>>>>> host suspend sequence to be unaware of any runtime power save operations
>>>>> done for the card in this phase. Thus it can just handle the suspend as
>>>>> the card is fully powered from a runtime perspective.
>>>>>
>>>>> Finally, this patch prepares to make it possible to move BKOPS handling
>>>>> into the runtime callbacks for the mmc bus_ops. Thus IDLE BKOPS can be
>>>>> accomplished.
>>>>>
>>>>> Signed-off-by: Ulf Hansson <ulf.hans...@linaro.org>
>>>>> Cc: Maya Erez <me...@codeaurora.org>
>>>>> Cc: Subhash Jadavani <subha...@codeaurora.org>
>>>>> Cc: Arnd Bergmann <a...@arndb.de>
>>>>> Cc: Kevin Liu <kl...@marvell.com>
>>>>> Cc: Adrian Hunter <adrian.hun...@intel.com>
>>>>> Cc: Daniel Drake <d...@laptop.org>
>>>>> Cc: Ohad Ben-Cohen <o...@wizery.com>
>>>>> ---
>>>>> drivers/mmc/card/block.c | 32 ++++++++++++++++++++++++++------
>>>>> drivers/mmc/core/core.c | 23 +++++++++++++++++++++++
>>>>> drivers/mmc/core/debugfs.c | 8 ++++----
>>>>> drivers/mmc/core/mmc.c | 4 ++--
>>>>> drivers/mmc/core/sd.c | 4 ++--
>>>>> include/linux/mmc/core.h | 3 +++
>>>>> 6 files changed, 60 insertions(+), 14 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>>> index e12a03c..360a41a 100644
>>>>> --- a/drivers/mmc/card/block.c
>>>>> +++ b/drivers/mmc/card/block.c
>>>>> @@ -34,6 +34,7 @@
>>>>> #include <linux/delay.h>
>>>>> #include <linux/capability.h>
>>>>> #include <linux/compat.h>
>>>>> +#include <linux/pm_runtime.h>
>>>>>
>>>>> #include <linux/mmc/ioctl.h>
>>>>> #include <linux/mmc/card.h>
>>>>> @@ -222,7 +223,7 @@ static ssize_t power_ro_lock_store(struct device
>>>>> *dev,
>>>>> md = mmc_blk_get(dev_to_disk(dev));
>>>>> card = md->queue.card;
>>>>>
>>>>> - mmc_claim_host(card->host);
>>>>> + mmc_get_card(card);
>>>>>
>>>>> ret = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BOOT_WP,
>>>>> card->ext_csd.boot_ro_lock |
>>>>> @@ -233,7 +234,7 @@ static ssize_t power_ro_lock_store(struct device
>>>>> *dev,
>>>>> else
>>>>> card->ext_csd.boot_ro_lock |=
>>>>> EXT_CSD_BOOT_WP_B_PWR_WP_EN;
>>>>>
>>>>> - mmc_release_host(card->host);
>>>>> + mmc_put_card(card);
>>>>>
>>>>> if (!ret) {
>>>>> pr_info("%s: Locking boot partition ro until next power
>>>>> on\n",
>>>>> @@ -492,7 +493,7 @@ static int mmc_blk_ioctl_cmd(struct block_device
>>>>> *bdev,
>>>>>
>>>>> mrq.cmd = &cmd;
>>>>>
>>>>> - mmc_claim_host(card->host);
>>>>> + mmc_get_card(card);
>>>>>
>>>>> err = mmc_blk_part_switch(card, md);
>>>>> if (err)
>>>>> @@ -559,7 +560,7 @@ static int mmc_blk_ioctl_cmd(struct block_device
>>>>> *bdev,
>>>>> }
>>>>>
>>>>> cmd_rel_host:
>>>>> - mmc_release_host(card->host);
>>>>> + mmc_put_card(card);
>>>>>
>>>>> cmd_done:
>>>>> mmc_blk_put(md);
>>>>> @@ -1896,7 +1897,7 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq,
>>>>> struct request *req)
>>>>>
>>>>> if (req && !mq->mqrq_prev->req)
>>>>> /* claim host only for the first request */
>>>>> - mmc_claim_host(card->host);
>>>>> + mmc_get_card(card);
>>>>>
>>>>> ret = mmc_blk_part_switch(card, md);
>>>>> if (ret) {
>>>>> @@ -1940,7 +1941,7 @@ out:
>>>>> * In case sepecial request, there is no reentry to
>>>>> * the 'mmc_blk_issue_rq' with 'mqrq_prev->req'.
>>>>> */
>>>>> - mmc_release_host(card->host);
>>>>> + mmc_put_card(card);
>>>>> return ret;
>>>>> }
>>>>>
>>>>> @@ -2337,6 +2338,19 @@ static int mmc_blk_probe(struct mmc_card *card)
>>>>> if (mmc_add_disk(part_md))
>>>>> goto out;
>>>>> }
>>>>> +
>>>>> + pm_runtime_set_autosuspend_delay(&card->dev, 3000);
>>>>> + pm_runtime_use_autosuspend(&card->dev);
>>>>> +
>>>>> + /*
>>>>> + * Don't enable runtime PM for SD-combo cards here. Leave that
>>>>> + * decision to be taken during the SDIO init sequence instead.
>>>>> + */
>>>>> + if (card->type != MMC_TYPE_SD_COMBO) {
>>>>> + pm_runtime_set_active(&card->dev);
>>>>> + pm_runtime_enable(&card->dev);
>>>>> + }
>>>>> +
>>>>> return 0;
>>>>>
>>>>> out:
>>>>> @@ -2350,9 +2364,13 @@ static void mmc_blk_remove(struct mmc_card *card)
>>>>> struct mmc_blk_data *md = mmc_get_drvdata(card);
>>>>>
>>>>> mmc_blk_remove_parts(card, md);
>>>>> + pm_runtime_get_sync(&card->dev);
>>>>> mmc_claim_host(card->host);
>>>>> mmc_blk_part_switch(card, md);
>>>>> mmc_release_host(card->host);
>>>>> + if (card->type != MMC_TYPE_SD_COMBO)
>>>>> + pm_runtime_disable(&card->dev);
>>>>> + pm_runtime_put_noidle(&card->dev);
>>>>> mmc_blk_remove_req(md);
>>>>> mmc_set_drvdata(card, NULL);
>>>>> }
>>>>> @@ -2364,6 +2382,7 @@ static int mmc_blk_suspend(struct mmc_card *card)
>>>>> struct mmc_blk_data *md = mmc_get_drvdata(card);
>>>>>
>>>>> if (md) {
>>>>> + pm_runtime_get_sync(&card->dev);
>>>>> mmc_queue_suspend(&md->queue);
>>>>> list_for_each_entry(part_md, &md->part, part) {
>>>>> mmc_queue_suspend(&part_md->queue);
>>>>> @@ -2387,6 +2406,7 @@ static int mmc_blk_resume(struct mmc_card *card)
>>>>> list_for_each_entry(part_md, &md->part, part) {
>>>>> mmc_queue_resume(&part_md->queue);
>>>>> }
>>>>> + pm_runtime_put(&card->dev);
>>>>> }
>>>>> return 0;
>>>>> }
>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>> index b16b64a..602db00 100644
>>>>> --- a/drivers/mmc/core/core.c
>>>>> +++ b/drivers/mmc/core/core.c
>>>>> @@ -952,6 +952,29 @@ void mmc_release_host(struct mmc_host *host)
>>>>> EXPORT_SYMBOL(mmc_release_host);
>>>>>
>>>>> /*
>>>>> + * This is a helper function, which fetches a runtime pm reference for
>>>>> the
>>>>> + * card device and also claims the host.
>>>>> + */
>>>>> +void mmc_get_card(struct mmc_card *card)
>>>>> +{
>>>>> + pm_runtime_get_sync(&card->dev);
>>>>> + mmc_claim_host(card->host);
>>>>> +}
>>>>> +EXPORT_SYMBOL(mmc_get_card);
>>>>> +
>>>>> +/*
>>>>> + * This is a helper function, which releases the host and drops the
>>>>> runtime
>>>>> + * pm reference for the card device.
>>>>> + */
>>>>> +void mmc_put_card(struct mmc_card *card)
>>>>> +{
>>>>> + mmc_release_host(card->host);
>>>>> + pm_runtime_mark_last_busy(&card->dev);
>>>>> + pm_runtime_put_autosuspend(&card->dev);
>>>>> +}
>>>>> +EXPORT_SYMBOL(mmc_put_card);
>>>>> +
>>>>> +/*
>>>>> * Internal function that does the actual ios call to the host driver,
>>>>> * optionally printing some debug output.
>>>>> */
>>>>> diff --git a/drivers/mmc/core/debugfs.c b/drivers/mmc/core/debugfs.c
>>>>> index 35c2f85..54829c0 100644
>>>>> --- a/drivers/mmc/core/debugfs.c
>>>>> +++ b/drivers/mmc/core/debugfs.c
>>>>> @@ -258,13 +258,13 @@ static int mmc_dbg_card_status_get(void *data, u64
>>>>> *val)
>>>>> u32 status;
>>>>> int ret;
>>>>>
>>>>> - mmc_claim_host(card->host);
>>>>> + mmc_get_card(card);
>>>>>
>>>>> ret = mmc_send_status(data, &status);
>>>>> if (!ret)
>>>>> *val = status;
>>>>>
>>>>> - mmc_release_host(card->host);
>>>>> + mmc_put_card(card);
>>>>>
>>>>> return ret;
>>>>> }
>>>>> @@ -291,9 +291,9 @@ static int mmc_ext_csd_open(struct inode *inode,
>>>>> struct file *filp)
>>>>> goto out_free;
>>>>> }
>>>>>
>>>>> - mmc_claim_host(card->host);
>>>>> + mmc_get_card(card);
>>>>> err = mmc_send_ext_csd(card, ext_csd);
>>>>> - mmc_release_host(card->host);
>>>>> + mmc_put_card(card);
>>>>> if (err)
>>>>> goto out_free;
>>>>>
>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>> index 66a530e..bf19058 100644
>>>>> --- a/drivers/mmc/core/mmc.c
>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>> @@ -1380,14 +1380,14 @@ static void mmc_detect(struct mmc_host *host)
>>>>> BUG_ON(!host);
>>>>> BUG_ON(!host->card);
>>>>>
>>>>> - mmc_claim_host(host);
>>>>> + mmc_get_card(host->card);
>>>>>
>>>>> /*
>>>>> * Just check if our card has been removed.
>>>>> */
>>>>> err = _mmc_detect_card_removed(host);
>>>>>
>>>>> - mmc_release_host(host);
>>>>> + mmc_put_card(host->card);
>>>>>
>>>>> if (err) {
>>>>> mmc_remove(host);
>>>>> diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
>>>>> index 9e645e1..30387d6 100644
>>>>> --- a/drivers/mmc/core/sd.c
>>>>> +++ b/drivers/mmc/core/sd.c
>>>>> @@ -1037,14 +1037,14 @@ static void mmc_sd_detect(struct mmc_host *host)
>>>>> BUG_ON(!host);
>>>>> BUG_ON(!host->card);
>>>>>
>>>>> - mmc_claim_host(host);
>>>>> + mmc_get_card(host->card);
>>>>>
>>>>> /*
>>>>> * Just check if our card has been removed.
>>>>> */
>>>>> err = _mmc_detect_card_removed(host);
>>>>>
>>>>> - mmc_release_host(host);
>>>>> + mmc_put_card(host->card);
>>>>>
>>>>> if (err) {
>>>>> mmc_sd_remove(host);
>>>>> diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
>>>>> index 39613b9..49fb132 100644
>>>>> --- a/include/linux/mmc/core.h
>>>>> +++ b/include/linux/mmc/core.h
>>>>> @@ -188,6 +188,9 @@ extern int __mmc_claim_host(struct mmc_host *host,
>>>>> atomic_t *abort);
>>>>> extern void mmc_release_host(struct mmc_host *host);
>>>>> extern int mmc_try_claim_host(struct mmc_host *host);
>>>>>
>>>>> +extern void mmc_get_card(struct mmc_card *card);
>>>>> +extern void mmc_put_card(struct mmc_card *card);
>>>>> +
>>>>> extern int mmc_flush_cache(struct mmc_card *);
>>>>>
>>>>> extern int mmc_detect_card_removed(struct mmc_host *host);
>>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>>
>> --
>> Sent by a consultant of the Qualcomm Innovation Center, Inc.
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
>>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
