All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv4 0/3] OMAP: HSMMC: cleanup and runtime pm
@ 2011-07-01 16:39 Balaji T K
  2011-07-01 16:39 ` [PATCHv4 1/3] MMC: OMAP: HSMMC: Remove lazy_disable Balaji T K
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Balaji T K @ 2011-07-01 16:39 UTC (permalink / raw)
  To: linux-omap, linux-mmc, cjb
  Cc: tony, madhu.cr, khilman, b-cousson, paul, kishore.kadiyala, Balaji T K

Removing the custom state machine - lazy disable framework in omap_hsmmc
to make way for runtime pm to handle host controller
power states.
This allows mmc_host_enable/mmc_host_disable to be replaced by
runtime get_sync and put_sync at host controller driver.

Enable runtime PM in omap_hsmmc

Rebased to MMC tree : mmc-next branch
Tested on OMAP4430SDP, OMAP3430SDP, OMAP2430SDP

MMC runtime patch has dependency on
[PATCH 0/6] OMAP2+: hwmod framework fixes [1]
for MMC1/MMC2 clock to get ungated after idle in OMAP4.

Without [1] patches, MMC1/MMC2 fails to get detected on OMAP4.

[1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg51457.html

Balaji T K (3):
  MMC: OMAP: HSMMC: Remove lazy_disable
  MMC: OMAP: HSMMC: add runtime pm support
  MMC: OMAP: HSMMC: Remove unused iclk

 drivers/mmc/host/omap_hsmmc.c |  365 +++++++----------------------------------
 1 files changed, 57 insertions(+), 308 deletions(-)


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

* [PATCHv4 1/3] MMC: OMAP: HSMMC: Remove lazy_disable
  2011-07-01 16:39 [PATCHv4 0/3] OMAP: HSMMC: cleanup and runtime pm Balaji T K
@ 2011-07-01 16:39 ` Balaji T K
  2011-07-01 16:39 ` [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support Balaji T K
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Balaji T K @ 2011-07-01 16:39 UTC (permalink / raw)
  To: linux-omap, linux-mmc, cjb
  Cc: tony, madhu.cr, khilman, b-cousson, paul, kishore.kadiyala, Balaji T K

lazy_disable framework in OMAP HSMMC manages multiple low power states
and Card is powered off after inactivity time of 8 seconds.
Based on previous discussion on the list, card power (regulator)
handling (when to power OFF/ON) should ideally be handled by core layer.
Remove usage of lazy disable to allow core layer _only_ to handle card power.
With the removal of lazy disable framework, MMC regulators
are left ON until MMC_POWER_OFF via set_ios.

Signed-off-by: Balaji T K <balajitk@ti.com>
---
 drivers/mmc/host/omap_hsmmc.c |  246 +----------------------------------------
 1 files changed, 2 insertions(+), 244 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index cd317af..819ff08 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -120,11 +120,6 @@
 #define OMAP_MMC_MASTER_CLOCK	96000000
 #define DRIVER_NAME		"omap_hsmmc"
 
-/* Timeouts for entering power saving states on inactivity, msec */
-#define OMAP_MMC_DISABLED_TIMEOUT	100
-#define OMAP_MMC_SLEEP_TIMEOUT		1000
-#define OMAP_MMC_OFF_TIMEOUT		8000
-
 /*
  * One controller can have multiple slots, like on some omap boards using
  * omap.c controller driver. Luckily this is not currently done on any known
@@ -1628,8 +1623,6 @@ static void omap_hsmmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 	if (host->power_mode == MMC_POWER_OFF)
 		mmc_host_disable(host->mmc);
-	else
-		mmc_host_lazy_disable(host->mmc);
 }
 
 static int omap_hsmmc_get_cd(struct mmc_host *mmc)
@@ -1685,220 +1678,6 @@ static void omap_hsmmc_conf_bus_power(struct omap_hsmmc_host *host)
 	set_sd_bus_power(host);
 }
 
-/*
- * Dynamic power saving handling, FSM:
- *   ENABLED -> DISABLED -> CARDSLEEP / REGSLEEP -> OFF
- *     ^___________|          |                      |
- *     |______________________|______________________|
- *
- * ENABLED:   mmc host is fully functional
- * DISABLED:  fclk is off
- * CARDSLEEP: fclk is off, card is asleep, voltage regulator is asleep
- * REGSLEEP:  fclk is off, voltage regulator is asleep
- * OFF:       fclk is off, voltage regulator is off
- *
- * Transition handlers return the timeout for the next state transition
- * or negative error.
- */
-
-enum {ENABLED = 0, DISABLED, CARDSLEEP, REGSLEEP, OFF};
-
-/* Handler for [ENABLED -> DISABLED] transition */
-static int omap_hsmmc_enabled_to_disabled(struct omap_hsmmc_host *host)
-{
-	omap_hsmmc_context_save(host);
-	clk_disable(host->fclk);
-	host->dpm_state = DISABLED;
-
-	dev_dbg(mmc_dev(host->mmc), "ENABLED -> DISABLED\n");
-
-	if (host->power_mode == MMC_POWER_OFF)
-		return 0;
-
-	return OMAP_MMC_SLEEP_TIMEOUT;
-}
-
-/* Handler for [DISABLED -> REGSLEEP / CARDSLEEP] transition */
-static int omap_hsmmc_disabled_to_sleep(struct omap_hsmmc_host *host)
-{
-	int err, new_state;
-
-	if (!mmc_try_claim_host(host->mmc))
-		return 0;
-
-	clk_enable(host->fclk);
-	omap_hsmmc_context_restore(host);
-	if (mmc_card_can_sleep(host->mmc)) {
-		err = mmc_card_sleep(host->mmc);
-		if (err < 0) {
-			clk_disable(host->fclk);
-			mmc_release_host(host->mmc);
-			return err;
-		}
-		new_state = CARDSLEEP;
-	} else {
-		new_state = REGSLEEP;
-	}
-	if (mmc_slot(host).set_sleep)
-		mmc_slot(host).set_sleep(host->dev, host->slot_id, 1, 0,
-					 new_state == CARDSLEEP);
-	/* FIXME: turn off bus power and perhaps interrupts too */
-	clk_disable(host->fclk);
-	host->dpm_state = new_state;
-
-	mmc_release_host(host->mmc);
-
-	dev_dbg(mmc_dev(host->mmc), "DISABLED -> %s\n",
-		host->dpm_state == CARDSLEEP ? "CARDSLEEP" : "REGSLEEP");
-
-	if (mmc_slot(host).no_off)
-		return 0;
-
-	if ((host->mmc->caps & MMC_CAP_NONREMOVABLE) ||
-	    mmc_slot(host).card_detect ||
-	    (mmc_slot(host).get_cover_state &&
-	     mmc_slot(host).get_cover_state(host->dev, host->slot_id)))
-		return OMAP_MMC_OFF_TIMEOUT;
-
-	return 0;
-}
-
-/* Handler for [REGSLEEP / CARDSLEEP -> OFF] transition */
-static int omap_hsmmc_sleep_to_off(struct omap_hsmmc_host *host)
-{
-	if (!mmc_try_claim_host(host->mmc))
-		return 0;
-
-	if (mmc_slot(host).no_off)
-		return 0;
-
-	if (!((host->mmc->caps & MMC_CAP_NONREMOVABLE) ||
-	      mmc_slot(host).card_detect ||
-	      (mmc_slot(host).get_cover_state &&
-	       mmc_slot(host).get_cover_state(host->dev, host->slot_id)))) {
-		mmc_release_host(host->mmc);
-		return 0;
-	}
-
-	mmc_slot(host).set_power(host->dev, host->slot_id, 0, 0);
-	host->vdd = 0;
-	host->power_mode = MMC_POWER_OFF;
-
-	dev_dbg(mmc_dev(host->mmc), "%s -> OFF\n",
-		host->dpm_state == CARDSLEEP ? "CARDSLEEP" : "REGSLEEP");
-
-	host->dpm_state = OFF;
-
-	mmc_release_host(host->mmc);
-
-	return 0;
-}
-
-/* Handler for [DISABLED -> ENABLED] transition */
-static int omap_hsmmc_disabled_to_enabled(struct omap_hsmmc_host *host)
-{
-	int err;
-
-	err = clk_enable(host->fclk);
-	if (err < 0)
-		return err;
-
-	omap_hsmmc_context_restore(host);
-	host->dpm_state = ENABLED;
-
-	dev_dbg(mmc_dev(host->mmc), "DISABLED -> ENABLED\n");
-
-	return 0;
-}
-
-/* Handler for [SLEEP -> ENABLED] transition */
-static int omap_hsmmc_sleep_to_enabled(struct omap_hsmmc_host *host)
-{
-	if (!mmc_try_claim_host(host->mmc))
-		return 0;
-
-	clk_enable(host->fclk);
-	omap_hsmmc_context_restore(host);
-	if (mmc_slot(host).set_sleep)
-		mmc_slot(host).set_sleep(host->dev, host->slot_id, 0,
-			 host->vdd, host->dpm_state == CARDSLEEP);
-	if (mmc_card_can_sleep(host->mmc))
-		mmc_card_awake(host->mmc);
-
-	dev_dbg(mmc_dev(host->mmc), "%s -> ENABLED\n",
-		host->dpm_state == CARDSLEEP ? "CARDSLEEP" : "REGSLEEP");
-
-	host->dpm_state = ENABLED;
-
-	mmc_release_host(host->mmc);
-
-	return 0;
-}
-
-/* Handler for [OFF -> ENABLED] transition */
-static int omap_hsmmc_off_to_enabled(struct omap_hsmmc_host *host)
-{
-	clk_enable(host->fclk);
-
-	omap_hsmmc_context_restore(host);
-	omap_hsmmc_conf_bus_power(host);
-	mmc_power_restore_host(host->mmc);
-
-	host->dpm_state = ENABLED;
-
-	dev_dbg(mmc_dev(host->mmc), "OFF -> ENABLED\n");
-
-	return 0;
-}
-
-/*
- * Bring MMC host to ENABLED from any other PM state.
- */
-static int omap_hsmmc_enable(struct mmc_host *mmc)
-{
-	struct omap_hsmmc_host *host = mmc_priv(mmc);
-
-	switch (host->dpm_state) {
-	case DISABLED:
-		return omap_hsmmc_disabled_to_enabled(host);
-	case CARDSLEEP:
-	case REGSLEEP:
-		return omap_hsmmc_sleep_to_enabled(host);
-	case OFF:
-		return omap_hsmmc_off_to_enabled(host);
-	default:
-		dev_dbg(mmc_dev(host->mmc), "UNKNOWN state\n");
-		return -EINVAL;
-	}
-}
-
-/*
- * Bring MMC host in PM state (one level deeper).
- */
-static int omap_hsmmc_disable(struct mmc_host *mmc, int lazy)
-{
-	struct omap_hsmmc_host *host = mmc_priv(mmc);
-
-	switch (host->dpm_state) {
-	case ENABLED: {
-		int delay;
-
-		delay = omap_hsmmc_enabled_to_disabled(host);
-		if (lazy || delay < 0)
-			return delay;
-		return 0;
-	}
-	case DISABLED:
-		return omap_hsmmc_disabled_to_sleep(host);
-	case CARDSLEEP:
-	case REGSLEEP:
-		return omap_hsmmc_sleep_to_off(host);
-	default:
-		dev_dbg(mmc_dev(host->mmc), "UNKNOWN state\n");
-		return -EINVAL;
-	}
-}
-
 static int omap_hsmmc_enable_fclk(struct mmc_host *mmc)
 {
 	struct omap_hsmmc_host *host = mmc_priv(mmc);
@@ -1933,17 +1712,6 @@ static const struct mmc_host_ops omap_hsmmc_ops = {
 	/* NYET -- enable_sdio_irq */
 };
 
-static const struct mmc_host_ops omap_hsmmc_ps_ops = {
-	.enable = omap_hsmmc_enable,
-	.disable = omap_hsmmc_disable,
-	.request = omap_hsmmc_request,
-	.set_ios = omap_hsmmc_set_ios,
-	.get_cd = omap_hsmmc_get_cd,
-	.get_ro = omap_hsmmc_get_ro,
-	.init_card = omap_hsmmc_init_card,
-	/* NYET -- enable_sdio_irq */
-};
-
 #ifdef CONFIG_DEBUG_FS
 
 static int omap_hsmmc_regs_show(struct seq_file *s, void *data)
@@ -1965,7 +1733,7 @@ static int omap_hsmmc_regs_show(struct seq_file *s, void *data)
 			host->dpm_state, mmc->nesting_cnt,
 			host->context_loss, context_loss);
 
-	if (host->suspended || host->dpm_state == OFF) {
+	if (host->suspended) {
 		seq_printf(s, "host suspended, can't read registers\n");
 		return 0;
 	}
@@ -2078,10 +1846,7 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, host);
 	INIT_WORK(&host->mmc_carddetect_work, omap_hsmmc_detect);
 
-	if (mmc_slot(host).power_saving)
-		mmc->ops	= &omap_hsmmc_ps_ops;
-	else
-		mmc->ops	= &omap_hsmmc_ops;
+	mmc->ops	= &omap_hsmmc_ops;
 
 	/*
 	 * If regulator_disable can only put vcc_aux to sleep then there is
@@ -2112,9 +1877,6 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
 	omap_hsmmc_context_save(host);
 
 	mmc->caps |= MMC_CAP_DISABLE;
-	mmc_set_disable_delay(mmc, OMAP_MMC_DISABLED_TIMEOUT);
-	/* we start off in DISABLED state */
-	host->dpm_state = DISABLED;
 
 	if (clk_enable(host->iclk) != 0) {
 		clk_put(host->iclk);
@@ -2237,8 +1999,6 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
 
 	omap_hsmmc_disable_irq(host);
 
-	mmc_host_lazy_disable(host->mmc);
-
 	omap_hsmmc_protect_card(host);
 
 	mmc_add_host(mmc);
@@ -2417,8 +2177,6 @@ static int omap_hsmmc_resume(struct device *dev)
 		ret = mmc_resume_host(host->mmc);
 		if (ret == 0)
 			host->suspended = 0;
-
-		mmc_host_lazy_disable(host->mmc);
 	}
 
 	return ret;
-- 
1.7.0.4


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

* [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support
  2011-07-01 16:39 [PATCHv4 0/3] OMAP: HSMMC: cleanup and runtime pm Balaji T K
  2011-07-01 16:39 ` [PATCHv4 1/3] MMC: OMAP: HSMMC: Remove lazy_disable Balaji T K
@ 2011-07-01 16:39 ` Balaji T K
  2011-07-08 18:24   ` Kevin Hilman
  2011-07-01 16:39 ` [PATCHv4 3/3] MMC: OMAP: HSMMC: Remove unused iclk Balaji T K
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Balaji T K @ 2011-07-01 16:39 UTC (permalink / raw)
  To: linux-omap, linux-mmc, cjb
  Cc: tony, madhu.cr, khilman, b-cousson, paul, kishore.kadiyala, Balaji T K

add runtime pm support to HSMMC host controller
Use runtime pm API to enable/disable HSMMC clock
Use runtime autosuspend APIs to enable auto suspend delay

Based on OMAP HSMMC runtime implementation by Kevin Hilman, Kishore Kadiyala

Signed-off-by: Balaji T K <balajitk@ti.com>
---
changes since v3
remove "host:" prefix for dev_dbg in omap_hsmmc_runtime_resume
retain omap_hsmmc_enable/disable_fclk function names to avoid
possible merge conflicts with [1]
[1] http://www.spinics.net/lists/linux-mmc/msg08836.html

changes since v2
Change autosuspend delay to 100ms.

changes since v1
Removed pm_runtime_allow, pm_runtime_forbid, pm_suspend_ignore_children calls.

 drivers/mmc/host/omap_hsmmc.c |  111 +++++++++++++++++++++--------------------
 1 files changed, 56 insertions(+), 55 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 819ff08..3d01e3f 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -33,6 +33,7 @@
 #include <linux/semaphore.h>
 #include <linux/gpio.h>
 #include <linux/regulator/consumer.h>
+#include <linux/pm_runtime.h>
 #include <plat/dma.h>
 #include <mach/hardware.h>
 #include <plat/board.h>
@@ -116,6 +117,7 @@
 #define OMAP_MMC4_DEVID		3
 #define OMAP_MMC5_DEVID		4
 
+#define MMC_AUTOSUSPEND_DELAY	100
 #define MMC_TIMEOUT_MS		20
 #define OMAP_MMC_MASTER_CLOCK	96000000
 #define DRIVER_NAME		"omap_hsmmc"
@@ -1147,8 +1149,7 @@ static int omap_hsmmc_switch_opcond(struct omap_hsmmc_host *host, int vdd)
 	int ret;
 
 	/* Disable the clocks */
-	clk_disable(host->fclk);
-	clk_disable(host->iclk);
+	pm_runtime_put_sync(host->dev);
 	if (host->got_dbclk)
 		clk_disable(host->dbclk);
 
@@ -1159,8 +1160,7 @@ static int omap_hsmmc_switch_opcond(struct omap_hsmmc_host *host, int vdd)
 	if (!ret)
 		ret = mmc_slot(host).set_power(host->dev, host->slot_id, 1,
 					       vdd);
-	clk_enable(host->iclk);
-	clk_enable(host->fclk);
+	pm_runtime_get_sync(host->dev);
 	if (host->got_dbclk)
 		clk_enable(host->dbclk);
 
@@ -1526,7 +1526,7 @@ static void omap_hsmmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	u32 con;
 	int do_send_init_stream = 0;
 
-	mmc_host_enable(host->mmc);
+	pm_runtime_get_sync(host->dev);
 
 	if (ios->power_mode != host->power_mode) {
 		switch (ios->power_mode) {
@@ -1621,8 +1621,7 @@ static void omap_hsmmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	else
 		OMAP_HSMMC_WRITE(host->base, CON, con & ~OD);
 
-	if (host->power_mode == MMC_POWER_OFF)
-		mmc_host_disable(host->mmc);
+	pm_runtime_put_autosuspend(host->dev);
 }
 
 static int omap_hsmmc_get_cd(struct mmc_host *mmc)
@@ -1681,13 +1680,9 @@ static void omap_hsmmc_conf_bus_power(struct omap_hsmmc_host *host)
 static int omap_hsmmc_enable_fclk(struct mmc_host *mmc)
 {
 	struct omap_hsmmc_host *host = mmc_priv(mmc);
-	int err;
 
-	err = clk_enable(host->fclk);
-	if (err)
-		return err;
-	dev_dbg(mmc_dev(host->mmc), "mmc_fclk: enabled\n");
-	omap_hsmmc_context_restore(host);
+	pm_runtime_get_sync(host->dev);
+
 	return 0;
 }
 
@@ -1695,9 +1690,9 @@ static int omap_hsmmc_disable_fclk(struct mmc_host *mmc, int lazy)
 {
 	struct omap_hsmmc_host *host = mmc_priv(mmc);
 
-	omap_hsmmc_context_save(host);
-	clk_disable(host->fclk);
-	dev_dbg(mmc_dev(host->mmc), "mmc_fclk: disabled\n");
+	pm_runtime_mark_last_busy(host->dev);
+	pm_runtime_put_autosuspend(host->dev);
+
 	return 0;
 }
 
@@ -1738,10 +1733,7 @@ static int omap_hsmmc_regs_show(struct seq_file *s, void *data)
 		return 0;
 	}
 
-	if (clk_enable(host->fclk) != 0) {
-		seq_printf(s, "can't read the regs\n");
-		return 0;
-	}
+	pm_runtime_get_sync(host->dev);
 
 	seq_printf(s, "SYSCONFIG:\t0x%08x\n",
 			OMAP_HSMMC_READ(host->base, SYSCONFIG));
@@ -1758,7 +1750,8 @@ static int omap_hsmmc_regs_show(struct seq_file *s, void *data)
 	seq_printf(s, "CAPA:\t\t0x%08x\n",
 			OMAP_HSMMC_READ(host->base, CAPA));
 
-	clk_disable(host->fclk);
+	pm_runtime_mark_last_busy(host->dev);
+	pm_runtime_put_autosuspend(host->dev);
 
 	return 0;
 }
@@ -1878,18 +1871,10 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
 
 	mmc->caps |= MMC_CAP_DISABLE;
 
-	if (clk_enable(host->iclk) != 0) {
-		clk_put(host->iclk);
-		clk_put(host->fclk);
-		goto err1;
-	}
-
-	if (mmc_host_enable(host->mmc) != 0) {
-		clk_disable(host->iclk);
-		clk_put(host->iclk);
-		clk_put(host->fclk);
-		goto err1;
-	}
+	pm_runtime_enable(host->dev);
+	pm_runtime_get_sync(host->dev);
+	pm_runtime_set_autosuspend_delay(host->dev, MMC_AUTOSUSPEND_DELAY);
+	pm_runtime_use_autosuspend(host->dev);
 
 	if (cpu_is_omap2430()) {
 		host->dbclk = clk_get(&pdev->dev, "mmchsdb_fck");
@@ -2016,6 +2001,8 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
 	}
 
 	omap_hsmmc_debugfs(mmc);
+	pm_runtime_mark_last_busy(host->dev);
+	pm_runtime_put_autosuspend(host->dev);
 
 	return 0;
 
@@ -2031,8 +2018,8 @@ err_reg:
 err_irq_cd_init:
 	free_irq(host->irq, host);
 err_irq:
-	mmc_host_disable(host->mmc);
-	clk_disable(host->iclk);
+	pm_runtime_mark_last_busy(host->dev);
+	pm_runtime_put_autosuspend(host->dev);
 	clk_put(host->fclk);
 	clk_put(host->iclk);
 	if (host->got_dbclk) {
@@ -2056,7 +2043,7 @@ static int omap_hsmmc_remove(struct platform_device *pdev)
 	struct resource *res;
 
 	if (host) {
-		mmc_host_enable(host->mmc);
+		pm_runtime_get_sync(host->dev);
 		mmc_remove_host(host->mmc);
 		if (host->use_reg)
 			omap_hsmmc_reg_put(host);
@@ -2067,8 +2054,8 @@ static int omap_hsmmc_remove(struct platform_device *pdev)
 			free_irq(mmc_slot(host).card_detect_irq, host);
 		flush_work_sync(&host->mmc_carddetect_work);
 
-		mmc_host_disable(host->mmc);
-		clk_disable(host->iclk);
+		pm_runtime_put_sync(host->dev);
+		pm_runtime_disable(host->dev);
 		clk_put(host->fclk);
 		clk_put(host->iclk);
 		if (host->got_dbclk) {
@@ -2100,6 +2087,7 @@ static int omap_hsmmc_suspend(struct device *dev)
 		return 0;
 
 	if (host) {
+		pm_runtime_get_sync(host->dev);
 		host->suspended = 1;
 		if (host->pdata->suspend) {
 			ret = host->pdata->suspend(&pdev->dev,
@@ -2114,13 +2102,11 @@ static int omap_hsmmc_suspend(struct device *dev)
 		}
 		cancel_work_sync(&host->mmc_carddetect_work);
 		ret = mmc_suspend_host(host->mmc);
-		mmc_host_enable(host->mmc);
+
 		if (ret == 0) {
 			omap_hsmmc_disable_irq(host);
 			OMAP_HSMMC_WRITE(host->base, HCTL,
 				OMAP_HSMMC_READ(host->base, HCTL) & ~SDBP);
-			mmc_host_disable(host->mmc);
-			clk_disable(host->iclk);
 			if (host->got_dbclk)
 				clk_disable(host->dbclk);
 		} else {
@@ -2132,9 +2118,8 @@ static int omap_hsmmc_suspend(struct device *dev)
 					dev_dbg(mmc_dev(host->mmc),
 						"Unmask interrupt failed\n");
 			}
-			mmc_host_disable(host->mmc);
 		}
-
+		pm_runtime_put_sync(host->dev);
 	}
 	return ret;
 }
@@ -2150,14 +2135,7 @@ static int omap_hsmmc_resume(struct device *dev)
 		return 0;
 
 	if (host) {
-		ret = clk_enable(host->iclk);
-		if (ret)
-			goto clk_en_err;
-
-		if (mmc_host_enable(host->mmc) != 0) {
-			clk_disable(host->iclk);
-			goto clk_en_err;
-		}
+		pm_runtime_get_sync(host->dev);
 
 		if (host->got_dbclk)
 			clk_enable(host->dbclk);
@@ -2177,14 +2155,13 @@ static int omap_hsmmc_resume(struct device *dev)
 		ret = mmc_resume_host(host->mmc);
 		if (ret == 0)
 			host->suspended = 0;
+
+		pm_runtime_mark_last_busy(host->dev);
+		pm_runtime_put_autosuspend(host->dev);
 	}
 
 	return ret;
 
-clk_en_err:
-	dev_dbg(mmc_dev(host->mmc),
-		"Failed to enable MMC clocks during resume\n");
-	return ret;
 }
 
 #else
@@ -2192,9 +2169,33 @@ clk_en_err:
 #define omap_hsmmc_resume		NULL
 #endif
 
+static int omap_hsmmc_runtime_suspend(struct device *dev)
+{
+	struct omap_hsmmc_host *host;
+
+	host = platform_get_drvdata(to_platform_device(dev));
+	omap_hsmmc_context_save(host);
+	dev_dbg(mmc_dev(host->mmc), "disabled\n");
+
+	return 0;
+}
+
+static int omap_hsmmc_runtime_resume(struct device *dev)
+{
+	struct omap_hsmmc_host *host;
+
+	host = platform_get_drvdata(to_platform_device(dev));
+	omap_hsmmc_context_restore(host);
+	dev_dbg(mmc_dev(host->mmc), "enabled\n");
+
+	return 0;
+}
+
 static struct dev_pm_ops omap_hsmmc_dev_pm_ops = {
 	.suspend	= omap_hsmmc_suspend,
 	.resume		= omap_hsmmc_resume,
+	.runtime_suspend = omap_hsmmc_runtime_suspend,
+	.runtime_resume = omap_hsmmc_runtime_resume,
 };
 
 static struct platform_driver omap_hsmmc_driver = {
-- 
1.7.0.4


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

* [PATCHv4 3/3] MMC: OMAP: HSMMC: Remove unused iclk
  2011-07-01 16:39 [PATCHv4 0/3] OMAP: HSMMC: cleanup and runtime pm Balaji T K
  2011-07-01 16:39 ` [PATCHv4 1/3] MMC: OMAP: HSMMC: Remove lazy_disable Balaji T K
  2011-07-01 16:39 ` [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support Balaji T K
@ 2011-07-01 16:39 ` Balaji T K
       [not found] ` <8762nlzy1d.fsf@ti.com>
  2011-07-09 22:30 ` Chris Ball
  4 siblings, 0 replies; 13+ messages in thread
From: Balaji T K @ 2011-07-01 16:39 UTC (permalink / raw)
  To: linux-omap, linux-mmc, cjb
  Cc: tony, madhu.cr, khilman, b-cousson, paul, kishore.kadiyala, Balaji T K

After runtime conversion to handle clk,
iclk node is not used
However fclk node is still used to get clock rate.

Signed-off-by: Balaji T K <balajitk@ti.com>
---
 drivers/mmc/host/omap_hsmmc.c |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 3d01e3f..aae4105 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -145,7 +145,6 @@ struct omap_hsmmc_host {
 	struct	mmc_command	*cmd;
 	struct	mmc_data	*data;
 	struct	clk		*fclk;
-	struct	clk		*iclk;
 	struct	clk		*dbclk;
 	/*
 	 * vcc == configured supply
@@ -1853,17 +1852,10 @@ static int __init omap_hsmmc_probe(struct platform_device *pdev)
 
 	spin_lock_init(&host->irq_lock);
 
-	host->iclk = clk_get(&pdev->dev, "ick");
-	if (IS_ERR(host->iclk)) {
-		ret = PTR_ERR(host->iclk);
-		host->iclk = NULL;
-		goto err1;
-	}
 	host->fclk = clk_get(&pdev->dev, "fck");
 	if (IS_ERR(host->fclk)) {
 		ret = PTR_ERR(host->fclk);
 		host->fclk = NULL;
-		clk_put(host->iclk);
 		goto err1;
 	}
 
@@ -2021,7 +2013,6 @@ err_irq:
 	pm_runtime_mark_last_busy(host->dev);
 	pm_runtime_put_autosuspend(host->dev);
 	clk_put(host->fclk);
-	clk_put(host->iclk);
 	if (host->got_dbclk) {
 		clk_disable(host->dbclk);
 		clk_put(host->dbclk);
@@ -2057,7 +2048,6 @@ static int omap_hsmmc_remove(struct platform_device *pdev)
 		pm_runtime_put_sync(host->dev);
 		pm_runtime_disable(host->dev);
 		clk_put(host->fclk);
-		clk_put(host->iclk);
 		if (host->got_dbclk) {
 			clk_disable(host->dbclk);
 			clk_put(host->dbclk);
-- 
1.7.0.4


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

* Re: [PATCHv4 0/3] OMAP: HSMMC: cleanup and runtime pm
       [not found]   ` <CANrkHUb-i4cQmGzrDjcooZPRvyQLSd0+kqLMA04hGzgVjCT+=A@mail.gmail.com>
@ 2011-07-04 18:05     ` S, Venkatraman
  2011-07-05 17:43       ` Kevin Hilman
  0 siblings, 1 reply; 13+ messages in thread
From: S, Venkatraman @ 2011-07-04 18:05 UTC (permalink / raw)
  To: T Krishnamoorthy, Balaji, Chris Ball
  Cc: Madhusudhan, Kevin Hilman, linux-omap, linux-mmc Mailing List,
	Benoit Cousson, Kadiyala, Kishore

> From: Kevin Hilman <khilman@ti.com>
> Date: Fri, Jul 1, 2011 at 11:24 PM
> Subject: Re: [PATCHv4 0/3] OMAP: HSMMC: cleanup and runtime pm
> To: cjb@laptop.org
> Cc: Balaji T K <balajitk@ti.com>, linux-omap@vger.kernel.org,
> linux-mmc@vger.kernel.org, tony@atomide.com, madhu.cr@ti.com,
> b-cousson@ti.com, paul@pwsan.com, kishore.kadiyala@ti.com
>
>
> Chris,
>
> Balaji T K <balajitk@ti.com> writes:
>
>> Removing the custom state machine - lazy disable framework in
>> omap_hsmmc to make way for runtime pm to handle host controller power
>> states.
>>
>> This allows mmc_host_enable/mmc_host_disable to be replaced
>> by runtime get_sync and put_sync at host controller driver.
>>
>> Enable runtime PM in omap_hsmmc
>>
>> Rebased to MMC tree : mmc-next branch
>> Tested on OMAP4430SDP, OMAP3430SDP, OMAP2430SDP
>
> Reviewed-by: Kevin Hilman <khilman@ti.com>
> Tested-by: Kevin Hilman <khilman@ti.com>
>
> Could you queue this series for the v3.1 merge window?  Or, with your
> ack, we are happy to take this through the OMAP tree along with the
> other dependecies mentioned below.
>
> I've also tested this on OMAP4430/Blaze, OMAP3630/Zoom3 and
> OMAP3430/n900.
>
> We have OMAP PM core code changes queued for v3.1 which require the MMC
> driver to correctly use runtime PM or we can get hangs if MMC is used
> during boot.
>
> Kevin

Kevin,
  Another series from Per Forlin [1] is also modifying the same file,
and might result in merge conflict if this series is queued under OMAP
and the other is queued by Chris.

Chris,
  If you intend to queue [1] into mmc-next, Balaji / myself can repost
this series on
top of it or you'd like to practice some git merge ? Let me know if
there is anything that I can do to help.

[1] https://lkml.org/lkml/2011/7/1/303

>
>> MMC runtime patch has dependency on
>> [PATCH 0/6] OMAP2+: hwmod framework fixes [1]
>> for MMC1/MMC2 clock to get ungated after idle in OMAP4.
>>
>> Without [1] patches, MMC1/MMC2 fails to get detected on OMAP4.
>>
>> [1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg51457.html
>>
>> Balaji T K (3):
>>   MMC: OMAP: HSMMC: Remove lazy_disable
>>   MMC: OMAP: HSMMC: add runtime pm support
>>   MMC: OMAP: HSMMC: Remove unused iclk
>>
>>  drivers/mmc/host/omap_hsmmc.c |  365 +++++++----------------------------------
>>  1 files changed, 57 insertions(+), 308 deletions(-)
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCHv4 0/3] OMAP: HSMMC: cleanup and runtime pm
  2011-07-04 18:05     ` [PATCHv4 0/3] OMAP: HSMMC: cleanup and runtime pm S, Venkatraman
@ 2011-07-05 17:43       ` Kevin Hilman
  0 siblings, 0 replies; 13+ messages in thread
From: Kevin Hilman @ 2011-07-05 17:43 UTC (permalink / raw)
  To: S, Venkatraman
  Cc: T Krishnamoorthy, Balaji, Chris Ball, Madhusudhan, linux-omap,
	linux-mmc Mailing List, Benoit Cousson, Kadiyala, Kishore

"S, Venkatraman" <svenkatr@ti.com> writes:

>> From: Kevin Hilman <khilman@ti.com>
>> Date: Fri, Jul 1, 2011 at 11:24 PM
>> Subject: Re: [PATCHv4 0/3] OMAP: HSMMC: cleanup and runtime pm
>> To: cjb@laptop.org
>> Cc: Balaji T K <balajitk@ti.com>, linux-omap@vger.kernel.org,
>> linux-mmc@vger.kernel.org, tony@atomide.com, madhu.cr@ti.com,
>> b-cousson@ti.com, paul@pwsan.com, kishore.kadiyala@ti.com
>>
>>
>> Chris,
>>
>> Balaji T K <balajitk@ti.com> writes:
>>
>>> Removing the custom state machine - lazy disable framework in
>>> omap_hsmmc to make way for runtime pm to handle host controller power
>>> states.
>>>
>>> This allows mmc_host_enable/mmc_host_disable to be replaced
>>> by runtime get_sync and put_sync at host controller driver.
>>>
>>> Enable runtime PM in omap_hsmmc
>>>
>>> Rebased to MMC tree : mmc-next branch
>>> Tested on OMAP4430SDP, OMAP3430SDP, OMAP2430SDP
>>
>> Reviewed-by: Kevin Hilman <khilman@ti.com>
>> Tested-by: Kevin Hilman <khilman@ti.com>
>>
>> Could you queue this series for the v3.1 merge window?  Or, with your
>> ack, we are happy to take this through the OMAP tree along with the
>> other dependecies mentioned below.
>>
>> I've also tested this on OMAP4430/Blaze, OMAP3630/Zoom3 and
>> OMAP3430/n900.
>>
>> We have OMAP PM core code changes queued for v3.1 which require the MMC
>> driver to correctly use runtime PM or we can get hangs if MMC is used
>> during boot.
>>
>> Kevin
>
> Kevin,
>   Another series from Per Forlin [1] is also modifying the same file,
> and might result in merge conflict if this series is queued under OMAP
> and the other is queued by Chris.

OK, then Chris should take this series through his tree.

Thanks,

Kevin

> Chris,
>   If you intend to queue [1] into mmc-next, Balaji / myself can repost
> this series on
> top of it or you'd like to practice some git merge ? Let me know if
> there is anything that I can do to help.
>
> [1] https://lkml.org/lkml/2011/7/1/303
>
>>
>>> MMC runtime patch has dependency on
>>> [PATCH 0/6] OMAP2+: hwmod framework fixes [1]
>>> for MMC1/MMC2 clock to get ungated after idle in OMAP4.
>>>
>>> Without [1] patches, MMC1/MMC2 fails to get detected on OMAP4.
>>>
>>> [1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg51457.html
>>>
>>> Balaji T K (3):
>>>   MMC: OMAP: HSMMC: Remove lazy_disable
>>>   MMC: OMAP: HSMMC: add runtime pm support
>>>   MMC: OMAP: HSMMC: Remove unused iclk
>>>
>>>  drivers/mmc/host/omap_hsmmc.c |  365 +++++++----------------------------------
>>>  1 files changed, 57 insertions(+), 308 deletions(-)
>>

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

* Re: [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support
  2011-07-01 16:39 ` [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support Balaji T K
@ 2011-07-08 18:24   ` Kevin Hilman
  2011-07-13  9:09     ` Dong, Chuanxiao
  0 siblings, 1 reply; 13+ messages in thread
From: Kevin Hilman @ 2011-07-08 18:24 UTC (permalink / raw)
  To: Balaji T K
  Cc: linux-omap, linux-mmc, cjb, tony, madhu.cr, b-cousson, paul,
	kishore.kadiyala

Balaji T K <balajitk@ti.com> writes:

> add runtime pm support to HSMMC host controller
> Use runtime pm API to enable/disable HSMMC clock
> Use runtime autosuspend APIs to enable auto suspend delay
>
> Based on OMAP HSMMC runtime implementation by Kevin Hilman, Kishore Kadiyala
>
> Signed-off-by: Balaji T K <balajitk@ti.com>

It's not relevant for this merge window, but I'm exploring some future
changes to our PM core code and have a question about how MMC works for
system suspend.

Basially, the question is: can the driver be reworked such that a system
suspend does not need to runtime resume the device?  For most devices,
we kind of expect that if the device is runtime suspended, a system
suspend will have nothing extra to do, but this driver runtime resumes
the device during system suspend in order to do "stuff", which I
admitedly don't fully undestand.

Ideally, the "stuff" needed for runtime suspend and system suspend could
be made to be common such that a system suspend of a runtime suspended
device would be a noop.

Is this possible?

Kevin

> @@ -2100,6 +2087,7 @@ static int omap_hsmmc_suspend(struct device *dev)
>  		return 0;
>  
>  	if (host) {
> +		pm_runtime_get_sync(host->dev);
>  		host->suspended = 1;
>  		if (host->pdata->suspend) {
>  			ret = host->pdata->suspend(&pdev->dev,
> @@ -2114,13 +2102,11 @@ static int omap_hsmmc_suspend(struct device *dev)
>  		}
>  		cancel_work_sync(&host->mmc_carddetect_work);
>  		ret = mmc_suspend_host(host->mmc);
> -		mmc_host_enable(host->mmc);
> +
>  		if (ret == 0) {
>  			omap_hsmmc_disable_irq(host);
>  			OMAP_HSMMC_WRITE(host->base, HCTL,
>  				OMAP_HSMMC_READ(host->base, HCTL) & ~SDBP);
> -			mmc_host_disable(host->mmc);
> -			clk_disable(host->iclk);
>  			if (host->got_dbclk)
>  				clk_disable(host->dbclk);
>  		} else {
> @@ -2132,9 +2118,8 @@ static int omap_hsmmc_suspend(struct device *dev)
>  					dev_dbg(mmc_dev(host->mmc),
>  						"Unmask interrupt failed\n");
>  			}
> -			mmc_host_disable(host->mmc);
>  		}
> -
> +		pm_runtime_put_sync(host->dev);
>  	}
>  	return ret;
>  }

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

* Re: [PATCHv4 0/3] OMAP: HSMMC: cleanup and runtime pm
  2011-07-01 16:39 [PATCHv4 0/3] OMAP: HSMMC: cleanup and runtime pm Balaji T K
                   ` (3 preceding siblings ...)
       [not found] ` <8762nlzy1d.fsf@ti.com>
@ 2011-07-09 22:30 ` Chris Ball
  2011-07-09 22:33   ` Paul Walmsley
  4 siblings, 1 reply; 13+ messages in thread
From: Chris Ball @ 2011-07-09 22:30 UTC (permalink / raw)
  To: Balaji T K
  Cc: linux-omap, linux-mmc, tony, madhu.cr, khilman, b-cousson, paul,
	kishore.kadiyala

Hi Balaji,

On Fri, Jul 01 2011, Balaji T K wrote:
> Balaji T K (3):
>   MMC: OMAP: HSMMC: Remove lazy_disable
>   MMC: OMAP: HSMMC: add runtime pm support
>   MMC: OMAP: HSMMC: Remove unused iclk

I've merged these three patches to mmc-next for 3.1 now.

> MMC runtime patch has dependency on
> [PATCH 0/6] OMAP2+: hwmod framework fixes [1]
> for MMC1/MMC2 clock to get ungated after idle in OMAP4.
>
> Without [1] patches, MMC1/MMC2 fails to get detected on OMAP4.
>
> [1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg51457.html

I'm assuming that these patches are going to make their way to Linus via
someone else's tree.

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

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

* Re: [PATCHv4 0/3] OMAP: HSMMC: cleanup and runtime pm
  2011-07-09 22:30 ` Chris Ball
@ 2011-07-09 22:33   ` Paul Walmsley
  0 siblings, 0 replies; 13+ messages in thread
From: Paul Walmsley @ 2011-07-09 22:33 UTC (permalink / raw)
  To: Chris Ball
  Cc: Balaji T K, linux-omap, linux-mmc, tony, madhu.cr, khilman,
	b-cousson, kishore.kadiyala

Hi Chris,

On Sat, 9 Jul 2011, Chris Ball wrote:

> On Fri, Jul 01 2011, Balaji T K wrote:
> > Balaji T K (3):
> >   MMC: OMAP: HSMMC: Remove lazy_disable
> >   MMC: OMAP: HSMMC: add runtime pm support
> >   MMC: OMAP: HSMMC: Remove unused iclk
> 
> I've merged these three patches to mmc-next for 3.1 now.

Thanks.

> > MMC runtime patch has dependency on
> > [PATCH 0/6] OMAP2+: hwmod framework fixes [1]
> > for MMC1/MMC2 clock to get ungated after idle in OMAP4.
> >
> > Without [1] patches, MMC1/MMC2 fails to get detected on OMAP4.
> >
> > [1] http://www.mail-archive.com/linux-omap@vger.kernel.org/msg51457.html
> 
> I'm assuming that these patches are going to make their way to Linus via
> someone else's tree.

Yes - we will try to merge them through the new ARM upstream path.


- Paul

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

* RE: [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support
  2011-07-08 18:24   ` Kevin Hilman
@ 2011-07-13  9:09     ` Dong, Chuanxiao
  2011-07-13 14:59       ` Kevin Hilman
  0 siblings, 1 reply; 13+ messages in thread
From: Dong, Chuanxiao @ 2011-07-13  9:09 UTC (permalink / raw)
  To: Kevin Hilman, Balaji T K
  Cc: linux-omap, linux-mmc, cjb, tony, madhu.cr, b-cousson, paul,
	kishore.kadiyala



> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org
> [mailto:linux-mmc-owner@vger.kernel.org] On Behalf Of Kevin Hilman
> Sent: Saturday, July 09, 2011 2:24 AM
> To: Balaji T K
> Cc: linux-omap@vger.kernel.org; linux-mmc@vger.kernel.org; cjb@laptop.org;
> tony@atomide.com; madhu.cr@ti.com; b-cousson@ti.com; paul@pwsan.com;
> kishore.kadiyala@ti.com
> Subject: Re: [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support
> 
> Balaji T K <balajitk@ti.com> writes:
> 
> > add runtime pm support to HSMMC host controller
> > Use runtime pm API to enable/disable HSMMC clock
> > Use runtime autosuspend APIs to enable auto suspend delay
> >
> > Based on OMAP HSMMC runtime implementation by Kevin Hilman, Kishore
> Kadiyala
> >
> > Signed-off-by: Balaji T K <balajitk@ti.com>
> 
> It's not relevant for this merge window, but I'm exploring some future
> changes to our PM core code and have a question about how MMC works for
> system suspend.
> 
> Basially, the question is: can the driver be reworked such that a system
> suspend does not need to runtime resume the device?  For most devices,
> we kind of expect that if the device is runtime suspended, a system
> suspend will have nothing extra to do, but this driver runtime resumes
> the device during system suspend in order to do "stuff", which I
> admitedly don't fully undestand.
> 
> Ideally, the "stuff" needed for runtime suspend and system suspend could
> be made to be common such that a system suspend of a runtime suspended
> device would be a noop.
> 
> Is this possible?
> 
> Kevin

During system suspended patch, a callback named .prepare will be first done before .suspend is called, and .complete callback will be called after .resume is called. These two callbacks are in pair. If driver can implement the .prepare and hold the usage count in this callback, then runtime pm suspend/resume will not happen during device suspending. So there will be no need to add pm_runtime_get* and pm_runtime_put* in .suspend/.resume.
BTW, if .prepare has hold the usage count, then .complete callback need to release the usage count and put device in runtime suspended mode.

Thanks
Chuanxiao

> 
> > @@ -2100,6 +2087,7 @@ static int omap_hsmmc_suspend(struct device *dev)
> >  		return 0;
> >
> >  	if (host) {
> > +		pm_runtime_get_sync(host->dev);
> >  		host->suspended = 1;
> >  		if (host->pdata->suspend) {
> >  			ret = host->pdata->suspend(&pdev->dev,
> > @@ -2114,13 +2102,11 @@ static int omap_hsmmc_suspend(struct device
> *dev)
> >  		}
> >  		cancel_work_sync(&host->mmc_carddetect_work);
> >  		ret = mmc_suspend_host(host->mmc);
> > -		mmc_host_enable(host->mmc);
> > +
> >  		if (ret == 0) {
> >  			omap_hsmmc_disable_irq(host);
> >  			OMAP_HSMMC_WRITE(host->base, HCTL,
> >  				OMAP_HSMMC_READ(host->base, HCTL) & ~SDBP);
> > -			mmc_host_disable(host->mmc);
> > -			clk_disable(host->iclk);
> >  			if (host->got_dbclk)
> >  				clk_disable(host->dbclk);
> >  		} else {
> > @@ -2132,9 +2118,8 @@ static int omap_hsmmc_suspend(struct device *dev)
> >  					dev_dbg(mmc_dev(host->mmc),
> >  						"Unmask interrupt failed\n");
> >  			}
> > -			mmc_host_disable(host->mmc);
> >  		}
> > -
> > +		pm_runtime_put_sync(host->dev);
> >  	}
> >  	return ret;
> >  }
> --
> 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

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

* Re: [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support
  2011-07-13  9:09     ` Dong, Chuanxiao
@ 2011-07-13 14:59       ` Kevin Hilman
  2011-07-13 15:34         ` S, Venkatraman
  0 siblings, 1 reply; 13+ messages in thread
From: Kevin Hilman @ 2011-07-13 14:59 UTC (permalink / raw)
  To: Dong, Chuanxiao
  Cc: Balaji T K, linux-omap, linux-mmc, cjb, tony, madhu.cr,
	b-cousson, paul, kishore.kadiyala

"Dong, Chuanxiao" <chuanxiao.dong@intel.com> writes:

[...]

>> 
>> Basially, the question is: can the driver be reworked such that a system
>> suspend does not need to runtime resume the device?  For most devices,
>> we kind of expect that if the device is runtime suspended, a system
>> suspend will have nothing extra to do, but this driver runtime resumes
>> the device during system suspend in order to do "stuff", which I
>> admitedly don't fully undestand.
>> 
>> Ideally, the "stuff" needed for runtime suspend and system suspend could
>> be made to be common such that a system suspend of a runtime suspended
>> device would be a noop.
>> 
>> Is this possible?
>> 
>> Kevin
>
> During system suspended patch, a callback named .prepare will be first
> done before .suspend is called, and .complete callback will be called
> after .resume is called. These two callbacks are in pair. If driver
> can implement the .prepare and hold the usage count in this callback,
> then runtime pm suspend/resume will not happen during device
> suspending. So there will be no need to add pm_runtime_get* and
> pm_runtime_put* in .suspend/.resume.

That doesn't avoid the problem, since the device is still runtime
resumed and then re-suspended during system suspend.

My basic question is this: why does this device need to be runtime
resumed during system suspend?  Why can't it just stay runtime
suspended?

Kevin

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

* Re: [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support
  2011-07-13 14:59       ` Kevin Hilman
@ 2011-07-13 15:34         ` S, Venkatraman
  2011-07-13 15:56           ` Kevin Hilman
  0 siblings, 1 reply; 13+ messages in thread
From: S, Venkatraman @ 2011-07-13 15:34 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Dong, Chuanxiao, Balaji T K, linux-omap, linux-mmc, cjb, tony,
	madhu.cr, b-cousson, paul, kishore.kadiyala

On Wed, Jul 13, 2011 at 8:29 PM, Kevin Hilman <khilman@ti.com> wrote:
> "Dong, Chuanxiao" <chuanxiao.dong@intel.com> writes:
>
> [...]
>
>>>
>>> Basially, the question is: can the driver be reworked such that a system
>>> suspend does not need to runtime resume the device?  For most devices,
>>> we kind of expect that if the device is runtime suspended, a system
>>> suspend will have nothing extra to do, but this driver runtime resumes
>>> the device during system suspend in order to do "stuff", which I
>>> admitedly don't fully undestand.
>>>
>>> Ideally, the "stuff" needed for runtime suspend and system suspend could
>>> be made to be common such that a system suspend of a runtime suspended
>>> device would be a noop.
>>>
>>> Is this possible?
>>>
>>> Kevin
>>
>> During system suspended patch, a callback named .prepare will be first
>> done before .suspend is called, and .complete callback will be called
>> after .resume is called. These two callbacks are in pair. If driver
>> can implement the .prepare and hold the usage count in this callback,
>> then runtime pm suspend/resume will not happen during device
>> suspending. So there will be no need to add pm_runtime_get* and
>> pm_runtime_put* in .suspend/.resume.
>
> That doesn't avoid the problem, since the device is still runtime
> resumed and then re-suspended during system suspend.
>
> My basic question is this: why does this device need to be runtime
> resumed during system suspend?  Why can't it just stay runtime
> suspended?
>

From my understanding, the runtime suspend is usually implemented to not
lose the card 'context', i.e. transactions can continue after a
runtime suspend /
resume cycle.

For system suspend, the MMC core sends a sleep command (which, in itself,
is a transaction) to the card to go to sleep state, and for all
practical purposes,
the card is treated as 'removed'. When the system resumes, the card is rescanned
and re-initialized.

Hence, for system suspend, the MMC controller needs to be enabled to actually
send the command which puts the card to sleep (and hence the resume).

Best regards,
Venkat.

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

* Re: [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support
  2011-07-13 15:34         ` S, Venkatraman
@ 2011-07-13 15:56           ` Kevin Hilman
  0 siblings, 0 replies; 13+ messages in thread
From: Kevin Hilman @ 2011-07-13 15:56 UTC (permalink / raw)
  To: S, Venkatraman
  Cc: Dong, Chuanxiao, Balaji T K, linux-omap, linux-mmc, cjb, tony,
	madhu.cr, b-cousson, paul, kishore.kadiyala

"S, Venkatraman" <svenkatr@ti.com> writes:

> On Wed, Jul 13, 2011 at 8:29 PM, Kevin Hilman <khilman@ti.com> wrote:

[...]

>>
>> My basic question is this: why does this device need to be runtime
>> resumed during system suspend?  Why can't it just stay runtime
>> suspended?
>>
>
> From my understanding, the runtime suspend is usually implemented to not
> lose the card 'context', i.e. transactions can continue after a
> runtime suspend /
> resume cycle.
>
> For system suspend, the MMC core sends a sleep command (which, in
> itself, is a transaction) to the card to go to sleep state, and for
> all practical purposes, the card is treated as 'removed'. When the
> system resumes, the card is rescanned and re-initialized.
>
> Hence, for system suspend, the MMC controller needs to be enabled to
> actually send the command which puts the card to sleep (and hence the
> resume).

Great, this is the detail I was looking for since my MMC knowledge is
quite limited.  

So, in theory, this same sleep command could be sent during runtime
suspend as well, so that a system suspend would not have to runtime
resume, correct?  But I suppose it would result in a much higher latency
runtime resumes.  It might be worth experimenting with doing this,
possibly in combination with longer auto-suspend delay times.

Thanks for the explanation,

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2011-07-13 15:56 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-01 16:39 [PATCHv4 0/3] OMAP: HSMMC: cleanup and runtime pm Balaji T K
2011-07-01 16:39 ` [PATCHv4 1/3] MMC: OMAP: HSMMC: Remove lazy_disable Balaji T K
2011-07-01 16:39 ` [PATCHv4 2/3] MMC: OMAP: HSMMC: add runtime pm support Balaji T K
2011-07-08 18:24   ` Kevin Hilman
2011-07-13  9:09     ` Dong, Chuanxiao
2011-07-13 14:59       ` Kevin Hilman
2011-07-13 15:34         ` S, Venkatraman
2011-07-13 15:56           ` Kevin Hilman
2011-07-01 16:39 ` [PATCHv4 3/3] MMC: OMAP: HSMMC: Remove unused iclk Balaji T K
     [not found] ` <8762nlzy1d.fsf@ti.com>
     [not found]   ` <CANrkHUb-i4cQmGzrDjcooZPRvyQLSd0+kqLMA04hGzgVjCT+=A@mail.gmail.com>
2011-07-04 18:05     ` [PATCHv4 0/3] OMAP: HSMMC: cleanup and runtime pm S, Venkatraman
2011-07-05 17:43       ` Kevin Hilman
2011-07-09 22:30 ` Chris Ball
2011-07-09 22:33   ` Paul Walmsley

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.