All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 0/2] mmc: Use runtime pm for blkdevice
@ 2013-04-04 16:41 Ulf Hansson
  2013-04-04 16:41 ` [PATCH V2 1/2] mmc: core: Add bus_ops for runtime pm callbacks Ulf Hansson
  2013-04-04 16:41 ` [PATCH V2 2/2] mmc: block: Enable runtime pm for mmc blkdevice Ulf Hansson
  0 siblings, 2 replies; 12+ messages in thread
From: Ulf Hansson @ 2013-04-04 16:41 UTC (permalink / raw)
  To: linux-mmc, Chris Ball; +Cc: Ulf Hansson

From: Ulf Hansson <ulf.hansson@linaro.org>

SDIO has been using runtime pm for a while to handle runtime power save
operations. This patchset is enabling the option to make the sd/mmc
blockdevices to use runtime pm as well.

The runtime pm implementation for the block device will make use of
autosuspend to defer power save operation to after request inactivty for
a certain time.

To actually perform some power save operations the corresponding bus ops
for mmc and sd shall be implemented. Typically it could make sense to do
BKOPS for eMMC in here.


Changes in v2:
        - Removed the stated patch below from this patchset. It can be handled
        separately. "mmc: core: Remove power_restore bus_ops for mmc and sd"
        - Rebased patches on latest mmc-next.
        - Added Acks.

Ulf Hansson (2):
  mmc: core: Add bus_ops for runtime pm callbacks
  mmc: block: Enable runtime pm for mmc blkdevice

 drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
 drivers/mmc/core/bus.c   |   14 ++++++++++++--
 drivers/mmc/core/core.h  |    2 ++
 drivers/mmc/core/sdio.c  |   20 ++++++++++++++++++++
 4 files changed, 60 insertions(+), 4 deletions(-)

-- 
1.7.10


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 1/2] mmc: core: Add bus_ops for runtime pm callbacks
  2013-04-04 16:41 [PATCH V2 0/2] mmc: Use runtime pm for blkdevice Ulf Hansson
@ 2013-04-04 16:41 ` Ulf Hansson
  2013-04-05 20:10   ` merez
  2013-04-04 16:41 ` [PATCH V2 2/2] mmc: block: Enable runtime pm for mmc blkdevice Ulf Hansson
  1 sibling, 1 reply; 12+ messages in thread
From: Ulf Hansson @ 2013-04-04 16:41 UTC (permalink / raw)
  To: linux-mmc, Chris Ball; +Cc: Ulf Hansson

From: Ulf Hansson <ulf.hansson@linaro.org>

SDIO is the only protocol that uses runtime pm for the card device
right now. To provide the option for sd and mmc to use runtime pm as
well the bus_ops callback are extended with two new functions. One for
runtime_suspend and one for runtime_resume.

This patch will also implement the callbacks for SDIO to make sure
existing functionallity is maintained.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Maya Erez <merez@codeaurora.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Kevin Liu <kliu5@marvell.com>
---
 drivers/mmc/core/bus.c  |   14 ++++++++++++--
 drivers/mmc/core/core.h |    2 ++
 drivers/mmc/core/sdio.c |   20 ++++++++++++++++++++
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index e219c97..d9e8c2b 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -151,15 +151,25 @@ static int mmc_bus_resume(struct device *dev)
 static int mmc_runtime_suspend(struct device *dev)
 {
 	struct mmc_card *card = mmc_dev_to_card(dev);
+	struct mmc_host *host = card->host;
+	int ret = 0;
+
+	if (host->bus_ops->runtime_suspend)
+		ret = host->bus_ops->runtime_suspend(host);
 
-	return mmc_power_save_host(card->host);
+	return ret;
 }
 
 static int mmc_runtime_resume(struct device *dev)
 {
 	struct mmc_card *card = mmc_dev_to_card(dev);
+	struct mmc_host *host = card->host;
+	int ret = 0;
+
+	if (host->bus_ops->runtime_resume)
+		ret = host->bus_ops->runtime_resume(host);
 
-	return mmc_power_restore_host(card->host);
+	return ret;
 }
 
 static int mmc_runtime_idle(struct device *dev)
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index b9f18a2..9445519 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -22,6 +22,8 @@ struct mmc_bus_ops {
 	void (*detect)(struct mmc_host *);
 	int (*suspend)(struct mmc_host *);
 	int (*resume)(struct mmc_host *);
+	int (*runtime_suspend)(struct mmc_host *);
+	int (*runtime_resume)(struct mmc_host *);
 	int (*power_save)(struct mmc_host *);
 	int (*power_restore)(struct mmc_host *);
 	int (*alive)(struct mmc_host *);
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index aa0719a..0e6e8c1 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -988,6 +988,24 @@ static int mmc_sdio_resume(struct mmc_host *host)
 	return err;
 }
 
+static int mmc_sdio_runtime_suspend(struct mmc_host *host)
+{
+	/*
+	 * Once sdio clients has entirely switched to runtime pm we wrap
+	 * the call to power_save here.
+	 */
+	return mmc_power_save_host(host);
+}
+
+static int mmc_sdio_runtime_resume(struct mmc_host *host)
+{
+	/*
+	 * Once sdio clients has entirely switched to runtime pm we wrap
+	 * the call to power_restore here.
+	 */
+	return mmc_power_restore_host(host);
+}
+
 static int mmc_sdio_power_restore(struct mmc_host *host)
 {
 	int ret;
@@ -1054,6 +1072,8 @@ static const struct mmc_bus_ops mmc_sdio_ops = {
 	.detect = mmc_sdio_detect,
 	.suspend = mmc_sdio_suspend,
 	.resume = mmc_sdio_resume,
+	.runtime_suspend = mmc_sdio_runtime_suspend,
+	.runtime_resume = mmc_sdio_runtime_resume,
 	.power_restore = mmc_sdio_power_restore,
 	.alive = mmc_sdio_alive,
 };
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V2 2/2] mmc: block: Enable runtime pm for mmc blkdevice
  2013-04-04 16:41 [PATCH V2 0/2] mmc: Use runtime pm for blkdevice Ulf Hansson
  2013-04-04 16:41 ` [PATCH V2 1/2] mmc: core: Add bus_ops for runtime pm callbacks Ulf Hansson
@ 2013-04-04 16:41 ` Ulf Hansson
  2013-04-05 20:10   ` merez
  1 sibling, 1 reply; 12+ messages in thread
From: Ulf Hansson @ 2013-04-04 16:41 UTC (permalink / raw)
  To: linux-mmc, Chris Ball; +Cc: Ulf Hansson

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>
Acked-by: Maya Erez <merez@codeaurora.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Kevin Liu <kliu5@marvell.com>
---
 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 e12a03c..536331a 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) {
@@ -1933,7 +1942,7 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
 
 out:
 	if ((!req && !(mq->flags & MMC_QUEUE_NEW_REQUEST)) ||
-	     (req && (req->cmd_flags & MMC_REQ_SPECIAL_MASK)))
+	     (req && (req->cmd_flags & MMC_REQ_SPECIAL_MASK))) {
 		/*
 		 * Release host when there are no more requests
 		 * and after special request(discard, flush) is done.
@@ -1941,6 +1950,10 @@ out:
 		 * the 'mmc_blk_issue_rq' with 'mqrq_prev->req'.
 		 */
 		mmc_release_host(card->host);
+		pm_runtime_mark_last_busy(&card->dev);
+		pm_runtime_put_autosuspend(&card->dev);
+	}
+
 	return ret;
 }
 
@@ -2337,6 +2350,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:
@@ -2350,9 +2369,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);
 }
@@ -2364,6 +2386,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 +2410,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;
 }
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 1/2] mmc: core: Add bus_ops for runtime pm callbacks
  2013-04-04 16:41 ` [PATCH V2 1/2] mmc: core: Add bus_ops for runtime pm callbacks Ulf Hansson
@ 2013-04-05 20:10   ` merez
  0 siblings, 0 replies; 12+ messages in thread
From: merez @ 2013-04-05 20:10 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: linux-mmc, Chris Ball, Ulf Hansson


> From: Ulf Hansson <ulf.hansson@linaro.org>
>
> SDIO is the only protocol that uses runtime pm for the card device
> right now. To provide the option for sd and mmc to use runtime pm as
> well the bus_ops callback are extended with two new functions. One for
> runtime_suspend and one for runtime_resume.
>
> This patch will also implement the callbacks for SDIO to make sure
> existing functionallity is maintained.
functionality
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Acked-by: Maya Erez <merez@codeaurora.org>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Kevin Liu <kliu5@marvell.com>
> ---
>  drivers/mmc/core/bus.c  |   14 ++++++++++++--
>  drivers/mmc/core/core.h |    2 ++
>  drivers/mmc/core/sdio.c |   20 ++++++++++++++++++++
>  3 files changed, 34 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> index e219c97..d9e8c2b 100644
> --- a/drivers/mmc/core/bus.c
> +++ b/drivers/mmc/core/bus.c
> @@ -151,15 +151,25 @@ static int mmc_bus_resume(struct device *dev)
>  static int mmc_runtime_suspend(struct device *dev)
>  {
>  	struct mmc_card *card = mmc_dev_to_card(dev);
> +	struct mmc_host *host = card->host;
> +	int ret = 0;
> +
> +	if (host->bus_ops->runtime_suspend)
> +		ret = host->bus_ops->runtime_suspend(host);
>
> -	return mmc_power_save_host(card->host);
> +	return ret;
>  }
>
>  static int mmc_runtime_resume(struct device *dev)
>  {
>  	struct mmc_card *card = mmc_dev_to_card(dev);
> +	struct mmc_host *host = card->host;
> +	int ret = 0;
> +
> +	if (host->bus_ops->runtime_resume)
> +		ret = host->bus_ops->runtime_resume(host);
>
> -	return mmc_power_restore_host(card->host);
> +	return ret;
>  }
>
>  static int mmc_runtime_idle(struct device *dev)
> diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
> index b9f18a2..9445519 100644
> --- a/drivers/mmc/core/core.h
> +++ b/drivers/mmc/core/core.h
> @@ -22,6 +22,8 @@ struct mmc_bus_ops {
>  	void (*detect)(struct mmc_host *);
>  	int (*suspend)(struct mmc_host *);
>  	int (*resume)(struct mmc_host *);
> +	int (*runtime_suspend)(struct mmc_host *);
> +	int (*runtime_resume)(struct mmc_host *);
>  	int (*power_save)(struct mmc_host *);
>  	int (*power_restore)(struct mmc_host *);
>  	int (*alive)(struct mmc_host *);
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index aa0719a..0e6e8c1 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -988,6 +988,24 @@ static int mmc_sdio_resume(struct mmc_host *host)
>  	return err;
>  }
>
> +static int mmc_sdio_runtime_suspend(struct mmc_host *host)
> +{
> +	/*
> +	 * Once sdio clients has entirely switched to runtime pm we wrap
> +	 * the call to power_save here.
> +	 */
> +	return mmc_power_save_host(host);
> +}
> +
> +static int mmc_sdio_runtime_resume(struct mmc_host *host)
> +{
> +	/*
> +	 * Once sdio clients has entirely switched to runtime pm we wrap
> +	 * the call to power_restore here.
> +	 */
> +	return mmc_power_restore_host(host);
> +}
> +
>  static int mmc_sdio_power_restore(struct mmc_host *host)
>  {
>  	int ret;
> @@ -1054,6 +1072,8 @@ static const struct mmc_bus_ops mmc_sdio_ops = {
>  	.detect = mmc_sdio_detect,
>  	.suspend = mmc_sdio_suspend,
>  	.resume = mmc_sdio_resume,
> +	.runtime_suspend = mmc_sdio_runtime_suspend,
> +	.runtime_resume = mmc_sdio_runtime_resume,
>  	.power_restore = mmc_sdio_power_restore,
>  	.alive = mmc_sdio_alive,
>  };
> --
> 1.7.10
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Maya Erez
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 2/2] mmc: block: Enable runtime pm for mmc blkdevice
  2013-04-04 16:41 ` [PATCH V2 2/2] mmc: block: Enable runtime pm for mmc blkdevice Ulf Hansson
@ 2013-04-05 20:10   ` merez
  0 siblings, 0 replies; 12+ messages in thread
From: merez @ 2013-04-05 20:10 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: linux-mmc, Chris Ball, Ulf Hansson


> 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
powered
> 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>
> Acked-by: Maya Erez <merez@codeaurora.org>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Kevin Liu <kliu5@marvell.com>
> ---
>  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 e12a03c..536331a 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) {
> @@ -1933,7 +1942,7 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq,
> struct request *req)
>
>  out:
>  	if ((!req && !(mq->flags & MMC_QUEUE_NEW_REQUEST)) ||
> -	     (req && (req->cmd_flags & MMC_REQ_SPECIAL_MASK)))
> +	     (req && (req->cmd_flags & MMC_REQ_SPECIAL_MASK))) {
>  		/*
>  		 * Release host when there are no more requests
>  		 * and after special request(discard, flush) is done.
> @@ -1941,6 +1950,10 @@ out:
>  		 * the 'mmc_blk_issue_rq' with 'mqrq_prev->req'.
>  		 */
>  		mmc_release_host(card->host);
> +		pm_runtime_mark_last_busy(&card->dev);
> +		pm_runtime_put_autosuspend(&card->dev);
> +	}
> +
>  	return ret;
>  }
>
> @@ -2337,6 +2350,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:
> @@ -2350,9 +2369,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);
>  }
> @@ -2364,6 +2386,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 +2410,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;
>  }
> --
> 1.7.10
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Maya Erez
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 2/2] mmc: block: Enable runtime pm for mmc blkdevice
  2013-04-11 12:28         ` Adrian Hunter
@ 2013-04-11 14:17           ` Ulf Hansson
  0 siblings, 0 replies; 12+ messages in thread
From: Ulf Hansson @ 2013-04-11 14:17 UTC (permalink / raw)
  To: Adrian Hunter; +Cc: Ulf Hansson, linux-mmc, Chris Ball

On 11 April 2013 14:28, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 11/04/13 13:14, Ulf Hansson wrote:
>> On 11 April 2013 11:58, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> On 11/04/13 12:40, Ulf Hansson wrote:
>>>> On 11 April 2013 10:31, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>>> On 08/04/13 14:44, 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 want 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 powered 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>
>>>>>> Acked-by: Maya Erez <merez@codeaurora.org>
>>>>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>>>>>> Acked-by: Kevin Liu <kliu5@marvell.com>
>>>>>> ---
>>>>>>  drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
>>>>>>  1 file changed, 26 insertions(+), 2 deletions(-)
>>>>>>
>>>>>
>>>>> There are debugfs uses of the card also (e.g.mmc_dbg_card_status_get)
>>>>> that will need get/put added.
>>>>
>>>> In the end it all depends on what kind of operations you decide to do
>>>> in the runtime_supend|resume callbacks.
>>>> Since the callbacks is not yet implemented for sd and mmc, this is not
>>>> required as of now.
>>>>
>>>> Nevertheless a most valid point that needs to be considered, while
>>>> implementing the callbacks. Thanks for pointing this out.
>>>>
>>>>>
>>>>> There might be others.  Please check.
>>>>
>>>> mmc_rescan needs to be considered at card removal and at resume. But,
>>>> again this does not need to be handled as of now.
>>>
>>> I disagree.  If you are adding runtime PM for SD/MMC cards, it must not be
>>> possible to access the card without going through runtime PM first.  That
>>> includes *all* code paths. We should not leave holes for others to fall in.
>>>
>>
>> This patchset shall not be considered as full blown common solution
>> for runtime pm for mmc/sd/sdio. It is an initial step that we can
>> start build upon.
>
> That does not justify leaving holes.
>
>> I took the approach of only adding, pm_runtime_get|put from the mmc
>> block layer, thus it will also _not_ affect the SDIO pm_runtime
>> implementation, which is already being used today. Adding what you
>> propose will affect SDIO as well, I think it is better to handle this
>> in the next steps instead.
>
> I disagree.  SDIO is easily avoided by coding for card->type.

Yes, it can be done, but on the other hand, I believe the runtime
support for SDIO does need additional improvement.

For example, the debugfs, which you pointed out, is as of today not
handled for sdio.

My, patch is not a fixup patch, it is adding a new feature to be able
to do additional power save. I don't want to mix a new feature with
fixups.

>
> Fix debugfs handling.

Will do it in a separate patch on top of this patch set.

>
> Don't enable runtime pm for removable cards. Document why i.e. rescan of
> removable cards needs special handling depending on the power-saving
> strategy e.g. if the power is off there is a risk of confusing a new card
> with an old card.

Removable cards can be power saved. We have been discussing the
"aggressive mmc_suspend_host" option here on the mmc list; certainly
it makes sense for removable cards as well.

Regarding documentation, I will add some information in header file
for the runtime_supend|resume callbacks.

>
> That covers two omissions that we know about.  Check for others -
> essentially check all mmc_claim_host() / __mmc_claim_host() /
> mmc_try_claim_host() calls.

Will handle this is a separate patch.

>
> SD-Combo cards also look like a problem, so don't enable runtime pm for them
> either.

Unfortunate I don't have an SD-combo card, so can't test these properly.

Anyway, there are problems with SD-combo cards before my patchset
around runtime PM. Using runtime pm for sdio (MMC_CAP_POWER_OFF_CARD),
will at runtime_suspend cut the power to the card. A block request for
the SD-combo card after this point will thus not be served
successfully.

This patchset will enable runtime pm for combo cards as you state.
That should actually fix the above problem, since the block layer will
do pm_runtime_get and thus make sure the card is properly powered when
needed. On the other hand, if MMC_CAP_POWER_OFF_CARD is not set and
thus we could expect the sdio client not doing runtime pm for the card
device, it will mean the exact opposite. Once the block layer decides
to power down the card due to request inactivity, the SDIO client will
suddenly not be able to access the SDIO interface.

The the new bug this patchset invents is probably worse than what it
fix. :-). I will adapt to you suggestion somehow.

>
> Document what works (i.e. non-removable SD and MMC cards) and what doesn't
> (removable and SD-Combo cards) and why.
>
>

Kind regards
Ulf Hansson

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 2/2] mmc: block: Enable runtime pm for mmc blkdevice
  2013-04-11 10:14       ` Ulf Hansson
@ 2013-04-11 12:28         ` Adrian Hunter
  2013-04-11 14:17           ` Ulf Hansson
  0 siblings, 1 reply; 12+ messages in thread
From: Adrian Hunter @ 2013-04-11 12:28 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: Ulf Hansson, linux-mmc, Chris Ball

On 11/04/13 13:14, Ulf Hansson wrote:
> On 11 April 2013 11:58, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 11/04/13 12:40, Ulf Hansson wrote:
>>> On 11 April 2013 10:31, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>>> On 08/04/13 14:44, 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 want 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 powered 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>
>>>>> Acked-by: Maya Erez <merez@codeaurora.org>
>>>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>>>>> Acked-by: Kevin Liu <kliu5@marvell.com>
>>>>> ---
>>>>>  drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
>>>>>  1 file changed, 26 insertions(+), 2 deletions(-)
>>>>>
>>>>
>>>> There are debugfs uses of the card also (e.g.mmc_dbg_card_status_get)
>>>> that will need get/put added.
>>>
>>> In the end it all depends on what kind of operations you decide to do
>>> in the runtime_supend|resume callbacks.
>>> Since the callbacks is not yet implemented for sd and mmc, this is not
>>> required as of now.
>>>
>>> Nevertheless a most valid point that needs to be considered, while
>>> implementing the callbacks. Thanks for pointing this out.
>>>
>>>>
>>>> There might be others.  Please check.
>>>
>>> mmc_rescan needs to be considered at card removal and at resume. But,
>>> again this does not need to be handled as of now.
>>
>> I disagree.  If you are adding runtime PM for SD/MMC cards, it must not be
>> possible to access the card without going through runtime PM first.  That
>> includes *all* code paths. We should not leave holes for others to fall in.
>>
> 
> This patchset shall not be considered as full blown common solution
> for runtime pm for mmc/sd/sdio. It is an initial step that we can
> start build upon.

That does not justify leaving holes.

> I took the approach of only adding, pm_runtime_get|put from the mmc
> block layer, thus it will also _not_ affect the SDIO pm_runtime
> implementation, which is already being used today. Adding what you
> propose will affect SDIO as well, I think it is better to handle this
> in the next steps instead.

I disagree.  SDIO is easily avoided by coding for card->type.

Fix debugfs handling.

Don't enable runtime pm for removable cards.  Document why i.e. rescan of
removable cards needs special handling depending on the power-saving
strategy e.g. if the power is off there is a risk of confusing a new card
with an old card.

That covers two omissions that we know about.  Check for others -
essentially check all mmc_claim_host() / __mmc_claim_host() /
mmc_try_claim_host() calls.

SD-Combo cards also look like a problem, so don't enable runtime pm for them
either.

Document what works (i.e. non-removable SD and MMC cards) and what doesn't
(removable and SD-Combo cards) and why.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 2/2] mmc: block: Enable runtime pm for mmc blkdevice
  2013-04-11  9:58     ` Adrian Hunter
@ 2013-04-11 10:14       ` Ulf Hansson
  2013-04-11 12:28         ` Adrian Hunter
  0 siblings, 1 reply; 12+ messages in thread
From: Ulf Hansson @ 2013-04-11 10:14 UTC (permalink / raw)
  To: Adrian Hunter; +Cc: Ulf Hansson, linux-mmc, Chris Ball

On 11 April 2013 11:58, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 11/04/13 12:40, Ulf Hansson wrote:
>> On 11 April 2013 10:31, Adrian Hunter <adrian.hunter@intel.com> wrote:
>>> On 08/04/13 14:44, 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 want 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 powered 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>
>>>> Acked-by: Maya Erez <merez@codeaurora.org>
>>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>>>> Acked-by: Kevin Liu <kliu5@marvell.com>
>>>> ---
>>>>  drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
>>>>  1 file changed, 26 insertions(+), 2 deletions(-)
>>>>
>>>
>>> There are debugfs uses of the card also (e.g.mmc_dbg_card_status_get)
>>> that will need get/put added.
>>
>> In the end it all depends on what kind of operations you decide to do
>> in the runtime_supend|resume callbacks.
>> Since the callbacks is not yet implemented for sd and mmc, this is not
>> required as of now.
>>
>> Nevertheless a most valid point that needs to be considered, while
>> implementing the callbacks. Thanks for pointing this out.
>>
>>>
>>> There might be others.  Please check.
>>
>> mmc_rescan needs to be considered at card removal and at resume. But,
>> again this does not need to be handled as of now.
>
> I disagree.  If you are adding runtime PM for SD/MMC cards, it must not be
> possible to access the card without going through runtime PM first.  That
> includes *all* code paths. We should not leave holes for others to fall in.
>

This patchset shall not be considered as full blown common solution
for runtime pm for mmc/sd/sdio. It is an initial step that we can
start build upon.

I took the approach of only adding, pm_runtime_get|put from the mmc
block layer, thus it will also _not_ affect the SDIO pm_runtime
implementation, which is already being used today. Adding what you
propose will affect SDIO as well, I think it is better to handle this
in the next steps instead.

Kind regards
Ulf Hansson

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 2/2] mmc: block: Enable runtime pm for mmc blkdevice
  2013-04-11  9:40   ` Ulf Hansson
@ 2013-04-11  9:58     ` Adrian Hunter
  2013-04-11 10:14       ` Ulf Hansson
  0 siblings, 1 reply; 12+ messages in thread
From: Adrian Hunter @ 2013-04-11  9:58 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: Ulf Hansson, linux-mmc, Chris Ball

On 11/04/13 12:40, Ulf Hansson wrote:
> On 11 April 2013 10:31, Adrian Hunter <adrian.hunter@intel.com> wrote:
>> On 08/04/13 14:44, 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 want 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 powered 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>
>>> Acked-by: Maya Erez <merez@codeaurora.org>
>>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>>> Acked-by: Kevin Liu <kliu5@marvell.com>
>>> ---
>>>  drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
>>>  1 file changed, 26 insertions(+), 2 deletions(-)
>>>
>>
>> There are debugfs uses of the card also (e.g.mmc_dbg_card_status_get)
>> that will need get/put added.
> 
> In the end it all depends on what kind of operations you decide to do
> in the runtime_supend|resume callbacks.
> Since the callbacks is not yet implemented for sd and mmc, this is not
> required as of now.
> 
> Nevertheless a most valid point that needs to be considered, while
> implementing the callbacks. Thanks for pointing this out.
> 
>>
>> There might be others.  Please check.
> 
> mmc_rescan needs to be considered at card removal and at resume. But,
> again this does not need to be handled as of now.

I disagree.  If you are adding runtime PM for SD/MMC cards, it must not be
possible to access the card without going through runtime PM first.  That
includes *all* code paths.  We should not leave holes for others to fall in.

> 
>>
>> It might be worth adding helpers e.g. mmc_claim_card()/mmc_release_card
>> so that it is easy to see the places that the host is claimed but runtime pm
>> is not used.
> 
> I am not sure we gain more visibility adding helpers at this initial
> step. We already have three scenarios for get/claim.
> 1. mmc_claim_host and pm_runtime_get is done in conjuction.
> 2. only mmc_claim_host.
> 3. only pm_runtime_get.
> 
> For put, we have a similar situation, and we don't even use the same
> runtime put API for all cases.
> 
> I see your point, it could very well be wanted to add these helpers if
> we see that the callbacks force the pm_runtime API to be used from
> several more places.
> 
>>
>> void mmc_claim_card(card)
>> {
>>         pm_runtime_get_sync(&card->dev);
>>         mmc_claim_host(card->host);
>> }
>>
>> void mmc_release_card(card)
>> {
>>         mmc_release_host(card->host);
>>         pm_runtime_mark_last_busy(&card->dev);
>>         pm_runtime_put_autosuspend(&card->dev);
>> }
>>
>>
>>
> 
> Kind regards
> Ulf Hansson
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 2/2] mmc: block: Enable runtime pm for mmc blkdevice
  2013-04-11  8:31 ` Adrian Hunter
@ 2013-04-11  9:40   ` Ulf Hansson
  2013-04-11  9:58     ` Adrian Hunter
  0 siblings, 1 reply; 12+ messages in thread
From: Ulf Hansson @ 2013-04-11  9:40 UTC (permalink / raw)
  To: Adrian Hunter; +Cc: Ulf Hansson, linux-mmc, Chris Ball

On 11 April 2013 10:31, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 08/04/13 14:44, 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 want 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 powered 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>
>> Acked-by: Maya Erez <merez@codeaurora.org>
>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>> Acked-by: Kevin Liu <kliu5@marvell.com>
>> ---
>>  drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
>>  1 file changed, 26 insertions(+), 2 deletions(-)
>>
>
> There are debugfs uses of the card also (e.g.mmc_dbg_card_status_get)
> that will need get/put added.

In the end it all depends on what kind of operations you decide to do
in the runtime_supend|resume callbacks.
Since the callbacks is not yet implemented for sd and mmc, this is not
required as of now.

Nevertheless a most valid point that needs to be considered, while
implementing the callbacks. Thanks for pointing this out.

>
> There might be others.  Please check.

mmc_rescan needs to be considered at card removal and at resume. But,
again this does not need to be handled as of now.

>
> It might be worth adding helpers e.g. mmc_claim_card()/mmc_release_card
> so that it is easy to see the places that the host is claimed but runtime pm
> is not used.

I am not sure we gain more visibility adding helpers at this initial
step. We already have three scenarios for get/claim.
1. mmc_claim_host and pm_runtime_get is done in conjuction.
2. only mmc_claim_host.
3. only pm_runtime_get.

For put, we have a similar situation, and we don't even use the same
runtime put API for all cases.

I see your point, it could very well be wanted to add these helpers if
we see that the callbacks force the pm_runtime API to be used from
several more places.

>
> void mmc_claim_card(card)
> {
>         pm_runtime_get_sync(&card->dev);
>         mmc_claim_host(card->host);
> }
>
> void mmc_release_card(card)
> {
>         mmc_release_host(card->host);
>         pm_runtime_mark_last_busy(&card->dev);
>         pm_runtime_put_autosuspend(&card->dev);
> }
>
>
>

Kind regards
Ulf Hansson

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 2/2] mmc: block: Enable runtime pm for mmc blkdevice
  2013-04-08 11:44 Ulf Hansson
@ 2013-04-11  8:31 ` Adrian Hunter
  2013-04-11  9:40   ` Ulf Hansson
  0 siblings, 1 reply; 12+ messages in thread
From: Adrian Hunter @ 2013-04-11  8:31 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: linux-mmc, Chris Ball, Ulf Hansson

On 08/04/13 14:44, 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 want 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 powered 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>
> Acked-by: Maya Erez <merez@codeaurora.org>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Kevin Liu <kliu5@marvell.com>
> ---
>  drivers/mmc/card/block.c |   28 ++++++++++++++++++++++++++--
>  1 file changed, 26 insertions(+), 2 deletions(-)
> 

There are debugfs uses of the card also (e.g.mmc_dbg_card_status_get)
that will need get/put added.

There might be others.  Please check.

It might be worth adding helpers e.g. mmc_claim_card()/mmc_release_card
so that it is easy to see the places that the host is claimed but runtime pm
is not used.

void mmc_claim_card(card)
{
	pm_runtime_get_sync(&card->dev);
	mmc_claim_host(card->host);
}

void mmc_release_card(card)
{
	mmc_release_host(card->host);
	pm_runtime_mark_last_busy(&card->dev);
	pm_runtime_put_autosuspend(&card->dev);
}




^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 2/2] mmc: block: Enable runtime pm for mmc blkdevice
@ 2013-04-08 11:44 Ulf Hansson
  2013-04-11  8:31 ` Adrian Hunter
  0 siblings, 1 reply; 12+ messages in thread
From: Ulf Hansson @ 2013-04-08 11:44 UTC (permalink / raw)
  To: linux-mmc, Chris Ball; +Cc: Ulf Hansson

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 want 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 powered 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>
Acked-by: Maya Erez <merez@codeaurora.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Kevin Liu <kliu5@marvell.com>
---
 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 e12a03c..536331a 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) {
@@ -1933,7 +1942,7 @@ static int mmc_blk_issue_rq(struct mmc_queue *mq, struct request *req)
 
 out:
 	if ((!req && !(mq->flags & MMC_QUEUE_NEW_REQUEST)) ||
-	     (req && (req->cmd_flags & MMC_REQ_SPECIAL_MASK)))
+	     (req && (req->cmd_flags & MMC_REQ_SPECIAL_MASK))) {
 		/*
 		 * Release host when there are no more requests
 		 * and after special request(discard, flush) is done.
@@ -1941,6 +1950,10 @@ out:
 		 * the 'mmc_blk_issue_rq' with 'mqrq_prev->req'.
 		 */
 		mmc_release_host(card->host);
+		pm_runtime_mark_last_busy(&card->dev);
+		pm_runtime_put_autosuspend(&card->dev);
+	}
+
 	return ret;
 }
 
@@ -2337,6 +2350,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:
@@ -2350,9 +2369,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);
 }
@@ -2364,6 +2386,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 +2410,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;
 }
-- 
1.7.10


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2013-04-11 14:17 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-04-04 16:41 [PATCH V2 0/2] mmc: Use runtime pm for blkdevice Ulf Hansson
2013-04-04 16:41 ` [PATCH V2 1/2] mmc: core: Add bus_ops for runtime pm callbacks Ulf Hansson
2013-04-05 20:10   ` merez
2013-04-04 16:41 ` [PATCH V2 2/2] mmc: block: Enable runtime pm for mmc blkdevice Ulf Hansson
2013-04-05 20:10   ` merez
2013-04-08 11:44 Ulf Hansson
2013-04-11  8:31 ` Adrian Hunter
2013-04-11  9:40   ` Ulf Hansson
2013-04-11  9:58     ` Adrian Hunter
2013-04-11 10:14       ` Ulf Hansson
2013-04-11 12:28         ` Adrian Hunter
2013-04-11 14:17           ` Ulf Hansson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.