Re: [PATCH 3/3] mmc: block: Enable runtime pm for mmc blkdevice

[Asutosh Das <asutoshd@codeaurora.org>]

On 3/1/2013 6:17 PM, Ulf Hansson wrote:
> From: Ulf Hansson <ulf.hansson@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.
>
> 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 what the
> host suspend sequence to be unaware of any runtime power save operations,
> so it can just handle the suspend as the device is fully powerered from a
> runtime perspective.
>
> This patch is preparing 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.hansson@linaro.org>
> ---
>   drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
>   1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 5bab73b..89d1c39 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,6 +223,7 @@ static ssize_t power_ro_lock_store(struct device *dev,
>   	md = mmc_blk_get(dev_to_disk(dev));
>   	card = md->queue.card;
>   
> +	pm_runtime_get_sync(&card->dev);
>   	mmc_claim_host(card->host);
>   
>   	ret = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL, EXT_CSD_BOOT_WP,
> @@ -234,6 +236,8 @@ static ssize_t power_ro_lock_store(struct device *dev,
>   		card->ext_csd.boot_ro_lock |= EXT_CSD_BOOT_WP_B_PWR_WP_EN;
>   
>   	mmc_release_host(card->host);
> +	pm_runtime_mark_last_busy(&card->dev);
> +	pm_runtime_put_autosuspend(&card->dev);
>   
>   	if (!ret) {
>   		pr_info("%s: Locking boot partition ro until next power on\n",
> @@ -492,6 +496,7 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
>   
>   	mrq.cmd = &cmd;
>   
> +	pm_runtime_get_sync(&card->dev);
>   	mmc_claim_host(card->host);
>   
>   	err = mmc_blk_part_switch(card, md);
> @@ -560,6 +565,8 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
>   
>   cmd_rel_host:
>   	mmc_release_host(card->host);
> +	pm_runtime_mark_last_busy(&card->dev);
> +	pm_runtime_put_autosuspend(&card->dev);
>   
>   cmd_done:
>   	mmc_blk_put(md);
> @@ -1894,9 +1901,11 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
>   	struct mmc_host *host = card->host;
>   	unsigned long flags;
>   
> -	if (req && !mq->mqrq_prev->req)
> +	if (req && !mq->mqrq_prev->req) {
> +		pm_runtime_get_sync(&card->dev);
>   		/* claim host only for the first request */
>   		mmc_claim_host(card->host);
> +	}
>   
>   	ret = mmc_blk_part_switch(card, md);
>   	if (ret) {
> @@ -1932,9 +1941,13 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
>   	}
>   
>   out:
> -	if (!req && !(mq->flags & MMC_QUEUE_NEW_REQUEST))
> +	if (!req && !(mq->flags & MMC_QUEUE_NEW_REQUEST)) {
>   		/* release host only when there are no more requests */
>   		mmc_release_host(card->host);
> +		pm_runtime_mark_last_busy(&card->dev);
> +		pm_runtime_put_autosuspend(&card->dev);
> +	}
> +
>   	return ret;
>   }
>   
> @@ -2331,6 +2344,12 @@ static int mmc_blk_probe(struct mmc_card *card)
>   		if (mmc_add_disk(part_md))
>   			goto out;
>   	}
> +
> +	pm_runtime_set_active(&card->dev);
> +	pm_runtime_set_autosuspend_delay(&card->dev, 3000);
> +	pm_runtime_use_autosuspend(&card->dev);
> +	pm_runtime_enable(&card->dev);
> +
>   	return 0;
>   
>    out:
> @@ -2344,9 +2363,12 @@ 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);
> +	pm_runtime_disable(&card->dev);
> +	pm_runtime_put_noidle(&card->dev);
>   	mmc_blk_remove_req(md);
>   	mmc_set_drvdata(card, NULL);
>   }
> @@ -2358,6 +2380,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);
> @@ -2381,6 +2404,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;
>   }
Hi Ulf
Currently, I am implementing a patch that facilitates each device to 
manage is runtime pm on its own.
I am using the parent-child relationship that is already established in 
the mmc stack for this implementation. In this case,
mmc_card is a child of mmc_host which is in turn is the child of 
platform-device.
The following contexts exist which would have to invoke get_sync and 
put_sync on the mmc_card device:
1. mmcqd
2. bkops
3. mmc_rescan

The get_sync on card device would resume all the 3 devices starting from 
the platform-device and the put-sync would suspend all the 3 devices 
starting from the card-device.
pm_auto_suspend/pm_schedule_suspend may be used to trigger the suspend 
from the above contexts.
I believe this would simplify the runtime pm functionality.

I can put up the patch for review in a couple of days.

Please let me know your opinion about this approach.

--
Thanks
Asutosh

-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.