[PATCH 0/8] mmc: tmio: Convert to use runtime PM at request idle

[PATCH 0/8] mmc: tmio: Convert to use runtime PM at request idle

[Ulf Hansson]

In a step in removing CONFIG_MMC_CLKGATE, host drivers can implement same
functionality through runtime PM. This patchset is converting the tmio
accordingly.

On the way, it was reasonable to include some minor cleanups to simplify code.
The first three patch has beend sent through an another patchset recently, and
acked but since it touches related code to this patchset, I decided to fold it
in here as well.

An important note, this patchset has only been compile tested. I would
appreciate if any that has a hw can help out in testing.

Ulf Hansson (8):
  mmc: sh_mobile_sdhi: Use modern PM macros to define pm callbacks
  mmc: tmio_mmc: Convert from legacy to modern PM ops
  mmc: tmio: Adapt to proper PM configs for exported functions
  mmc: tmio: Keep host active while SDIO irq is enabled
  mmc: tmio: Keep host active while serving requests
  mmc: tmio: Extract bus_width modifications to a function
  mmc: tmio: Restructure .set_ios and adapt probe sequence to it
  mmc: tmio: Handle clock gating from runtime PM functions

 drivers/mmc/host/sh_mobile_sdhi.c |    8 +-
 drivers/mmc/host/tmio_mmc.c       |   30 +++---
 drivers/mmc/host/tmio_mmc.h       |   31 ++-----
 drivers/mmc/host/tmio_mmc_pio.c   |  185 ++++++++++++++++++++-----------------
 4 files changed, 125 insertions(+), 129 deletions(-)

-- 
1.7.9.5

[PATCH 1/8] mmc: sh_mobile_sdhi: Use modern PM macros to define pm callbacks

[Ulf Hansson]

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/sh_mobile_sdhi.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/sh_mobile_sdhi.c b/drivers/mmc/host/sh_mobile_sdhi.c
index 87ed3fb..2227a9f 100644
--- a/drivers/mmc/host/sh_mobile_sdhi.c
+++ b/drivers/mmc/host/sh_mobile_sdhi.c
@@ -297,10 +297,10 @@ static int sh_mobile_sdhi_remove(struct platform_device *pdev)
 }
 
 static const struct dev_pm_ops tmio_mmc_dev_pm_ops = {
-	.suspend = tmio_mmc_host_suspend,
-	.resume = tmio_mmc_host_resume,
-	.runtime_suspend = tmio_mmc_host_runtime_suspend,
-	.runtime_resume = tmio_mmc_host_runtime_resume,
+	SET_SYSTEM_SLEEP_PM_OPS(tmio_mmc_host_suspend, tmio_mmc_host_resume)
+	SET_RUNTIME_PM_OPS(tmio_mmc_host_runtime_suspend,
+			tmio_mmc_host_runtime_resume,
+			NULL)
 };
 
 static struct platform_driver sh_mobile_sdhi_driver = {
-- 
1.7.9.5

[PATCH 2/8] mmc: tmio_mmc: Convert from legacy to modern PM ops

[Ulf Hansson]

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/tmio_mmc.c |   30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c
index 8860d4d..42da904 100644
--- a/drivers/mmc/host/tmio_mmc.c
+++ b/drivers/mmc/host/tmio_mmc.c
@@ -23,38 +23,37 @@
 
 #include "tmio_mmc.h"
 
-#ifdef CONFIG_PM
-static int tmio_mmc_suspend(struct platform_device *dev, pm_message_t state)
+#ifdef CONFIG_PM_SLEEP
+static int tmio_mmc_suspend(struct device *dev)
 {
-	const struct mfd_cell *cell = mfd_get_cell(dev);
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct mfd_cell *cell = mfd_get_cell(pdev);
 	int ret;
 
-	ret = tmio_mmc_host_suspend(&dev->dev);
+	ret = tmio_mmc_host_suspend(dev);
 
 	/* Tell MFD core it can disable us now.*/
 	if (!ret && cell->disable)
-		cell->disable(dev);
+		cell->disable(pdev);
 
 	return ret;
 }
 
-static int tmio_mmc_resume(struct platform_device *dev)
+static int tmio_mmc_resume(struct device *dev)
 {
-	const struct mfd_cell *cell = mfd_get_cell(dev);
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct mfd_cell *cell = mfd_get_cell(pdev);
 	int ret = 0;
 
 	/* Tell the MFD core we are ready to be enabled */
 	if (cell->resume)
-		ret = cell->resume(dev);
+		ret = cell->resume(pdev);
 
 	if (!ret)
-		ret = tmio_mmc_host_resume(&dev->dev);
+		ret = tmio_mmc_host_resume(dev);
 
 	return ret;
 }
-#else
-#define tmio_mmc_suspend NULL
-#define tmio_mmc_resume NULL
 #endif
 
 static int tmio_mmc_probe(struct platform_device *pdev)
@@ -125,15 +124,18 @@ static int tmio_mmc_remove(struct platform_device *pdev)
 
 /* ------------------- device registration ----------------------- */
 
+static const struct dev_pm_ops tmio_mmc_dev_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(tmio_mmc_suspend, tmio_mmc_resume)
+};
+
 static struct platform_driver tmio_mmc_driver = {
 	.driver = {
 		.name = "tmio-mmc",
 		.owner = THIS_MODULE,
+		.pm = &tmio_mmc_dev_pm_ops,
 	},
 	.probe = tmio_mmc_probe,
 	.remove = tmio_mmc_remove,
-	.suspend = tmio_mmc_suspend,
-	.resume = tmio_mmc_resume,
 };
 
 module_platform_driver(tmio_mmc_driver);
-- 
1.7.9.5

[PATCH 3/8] mmc: tmio: Adapt to proper PM configs for exported functions

[Ulf Hansson]

Since the users of the exported PM functions are now using the modern
PM ops macros, we can convert to the proper corresponding PM configs.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
 drivers/mmc/host/tmio_mmc.h     |    7 +++----
 drivers/mmc/host/tmio_mmc_pio.c |    7 ++++---
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 86fd21e..6c5b45a 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -163,16 +163,15 @@ static inline void tmio_mmc_abort_dma(struct tmio_mmc_host *host)
 }
 #endif
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
 int tmio_mmc_host_suspend(struct device *dev);
 int tmio_mmc_host_resume(struct device *dev);
-#else
-#define tmio_mmc_host_suspend NULL
-#define tmio_mmc_host_resume NULL
 #endif
 
+#ifdef CONFIG_PM_RUNTIME
 int tmio_mmc_host_runtime_suspend(struct device *dev);
 int tmio_mmc_host_runtime_resume(struct device *dev);
+#endif
 
 static inline u16 sd_ctrl_read16(struct tmio_mmc_host *host, int addr)
 {
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f3b2d8c..472e803 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -1140,7 +1140,7 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host)
 }
 EXPORT_SYMBOL(tmio_mmc_host_remove);
 
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
 int tmio_mmc_host_suspend(struct device *dev)
 {
 	struct mmc_host *mmc = dev_get_drvdata(dev);
@@ -1163,9 +1163,9 @@ int tmio_mmc_host_resume(struct device *dev)
 	return 0;
 }
 EXPORT_SYMBOL(tmio_mmc_host_resume);
+#endif
 
-#endif	/* CONFIG_PM */
-
+#ifdef CONFIG_PM_RUNTIME
 int tmio_mmc_host_runtime_suspend(struct device *dev)
 {
 	return 0;
@@ -1182,5 +1182,6 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
 	return 0;
 }
 EXPORT_SYMBOL(tmio_mmc_host_runtime_resume);
+#endif
 
 MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

[PATCH 4/8] mmc: tmio: Keep host active while SDIO irq is enabled

[Ulf Hansson]

The host must be kept active to be able to serve with SDIO irqs. We
prevent it from being put into in-active while the SDIO irq is enabled
by simply adding balanced calls to pm_runtime_get|put.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/tmio_mmc.h     |    1 +
 drivers/mmc/host/tmio_mmc_pio.c |   19 +++++++++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 6c5b45a..c2c9546 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -102,6 +102,7 @@ struct tmio_mmc_host {
 	struct mutex		ios_lock;	/* protect set_ios() context */
 	bool			native_hotplug;
 	bool			resuming;
+	bool			sdio_irq_enabled;
 };
 
 int tmio_mmc_host_probe(struct tmio_mmc_host **host,
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 472e803..377157e 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -129,15 +129,22 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
 {
 	struct tmio_mmc_host *host = mmc_priv(mmc);
 
-	if (enable) {
+	if (enable && !host->sdio_irq_enabled) {
+		/* Keep device active while SDIO irq is enabled */
+		pm_runtime_get_sync(mmc_dev(mmc));
+		host->sdio_irq_enabled = true;
+
 		host->sdio_irq_mask = TMIO_SDIO_MASK_ALL &
 					~TMIO_SDIO_STAT_IOIRQ;
 		sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001);
 		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
-	} else {
+	} else if (!enable && host->sdio_irq_enabled) {
 		host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
 		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
 		sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000);
+
+		host->sdio_irq_enabled = false;
+		pm_runtime_put(mmc_dev(mmc));
 	}
 }
 
@@ -1072,8 +1079,12 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 
 	_host->sdcard_irq_mask &= ~irq_mask;
 
-	if (pdata->flags & TMIO_MMC_SDIO_IRQ)
-		tmio_mmc_enable_sdio_irq(mmc, 0);
+	_host->sdio_irq_enabled = false;
+	if (pdata->flags & TMIO_MMC_SDIO_IRQ) {
+		_host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
+		sd_ctrl_write16(_host, CTL_SDIO_IRQ_MASK, _host->sdio_irq_mask);
+		sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x0000);
+	}
 
 	spin_lock_init(&_host->lock);
 	mutex_init(&_host->ios_lock);
-- 
1.7.9.5

[PATCH 5/8] mmc: tmio: Keep host active while serving requests

[Ulf Hansson]

Use runtime PM to keep the host active during I/O operations and other
requests which requires the tmio hardware to be powered.

Additionally make use of the runtime PM autosuspend feature with a
default timeout of 50 ms.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/tmio_mmc_pio.c |   55 +++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 23 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 377157e..33a5e26 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -144,7 +144,8 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
 		sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000);
 
 		host->sdio_irq_enabled = false;
-		pm_runtime_put(mmc_dev(mmc));
+		pm_runtime_mark_last_busy(mmc_dev(mmc));
+		pm_runtime_put_autosuspend(mmc_dev(mmc));
 	}
 }
 
@@ -258,6 +259,9 @@ static void tmio_mmc_reset_work(struct work_struct *work)
 
 	tmio_mmc_abort_dma(host);
 	mmc_request_done(host->mmc, mrq);
+
+	pm_runtime_mark_last_busy(mmc_dev(host->mmc));
+	pm_runtime_put_autosuspend(mmc_dev(host->mmc));
 }
 
 /* called with host->lock held, interrupts disabled */
@@ -287,6 +291,9 @@ static void tmio_mmc_finish_request(struct tmio_mmc_host *host)
 		tmio_mmc_abort_dma(host);
 
 	mmc_request_done(host->mmc, mrq);
+
+	pm_runtime_mark_last_busy(mmc_dev(host->mmc));
+	pm_runtime_put_autosuspend(mmc_dev(host->mmc));
 }
 
 static void tmio_mmc_done_work(struct work_struct *work)
@@ -741,6 +748,8 @@ static void tmio_mmc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 
 	spin_unlock_irqrestore(&host->lock, flags);
 
+	pm_runtime_get_sync(mmc_dev(mmc));
+
 	if (mrq->data) {
 		ret = tmio_mmc_start_data(host, mrq->data);
 		if (ret)
@@ -759,6 +768,9 @@ fail:
 	host->mrq = NULL;
 	mrq->cmd->error = ret;
 	mmc_request_done(mmc, mrq);
+
+	pm_runtime_mark_last_busy(mmc_dev(mmc));
+	pm_runtime_put_autosuspend(mmc_dev(mmc));
 }
 
 static int tmio_mmc_clk_update(struct mmc_host *mmc)
@@ -837,6 +849,8 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	struct device *dev = &host->pdev->dev;
 	unsigned long flags;
 
+	pm_runtime_get_sync(mmc_dev(mmc));
+
 	mutex_lock(&host->ios_lock);
 
 	spin_lock_irqsave(&host->lock, flags);
@@ -872,7 +886,6 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	if (ios->power_mode == MMC_POWER_ON && ios->clock) {
 		if (host->power != TMIO_MMC_ON_RUN) {
 			tmio_mmc_clk_update(mmc);
-			pm_runtime_get_sync(dev);
 			if (host->resuming) {
 				tmio_mmc_reset(host);
 				host->resuming = false;
@@ -902,7 +915,6 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 		if (old_power == TMIO_MMC_ON_RUN) {
 			tmio_mmc_clk_stop(host);
-			pm_runtime_put(dev);
 			if (pdata->clk_disable)
 				pdata->clk_disable(host->pdev);
 		}
@@ -929,6 +941,9 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	host->mrq = NULL;
 
 	mutex_unlock(&host->ios_lock);
+
+	pm_runtime_mark_last_busy(mmc_dev(mmc));
+	pm_runtime_put_autosuspend(mmc_dev(mmc));
 }
 
 static int tmio_mmc_get_ro(struct mmc_host *mmc)
@@ -939,8 +954,13 @@ static int tmio_mmc_get_ro(struct mmc_host *mmc)
 	if (ret >= 0)
 		return ret;
 
-	return !((pdata->flags & TMIO_MMC_WRPROTECT_DISABLE) ||
-		 (sd_ctrl_read32(host, CTL_STATUS) & TMIO_STAT_WRPROTECT));
+	pm_runtime_get_sync(mmc_dev(mmc));
+	ret = !((pdata->flags & TMIO_MMC_WRPROTECT_DISABLE) ||
+		(sd_ctrl_read32(host, CTL_STATUS) & TMIO_STAT_WRPROTECT));
+	pm_runtime_mark_last_busy(mmc_dev(mmc));
+	pm_runtime_put_autosuspend(mmc_dev(mmc));
+
+	return ret;
 }
 
 static const struct mmc_host_ops tmio_mmc_ops = {
@@ -1038,27 +1058,14 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 				  mmc->slot.cd_irq >= 0);
 
 	_host->power = TMIO_MMC_OFF_STOP;
-	pm_runtime_enable(&pdev->dev);
-	ret = pm_runtime_resume(&pdev->dev);
-	if (ret < 0)
-		goto pm_disable;
-
 	if (tmio_mmc_clk_update(mmc) < 0) {
 		mmc->f_max = pdata->hclk;
 		mmc->f_min = mmc->f_max / 512;
 	}
 
 	/*
-	 * There are 4 different scenarios for the card detection:
-	 *  1) an external gpio irq handles the cd (best for power savings)
-	 *  2) internal sdhi irq handles the cd
-	 *  3) a worker thread polls the sdhi - indicated by MMC_CAP_NEEDS_POLL
-	 *  4) the medium is non-removable - indicated by MMC_CAP_NONREMOVABLE
-	 *
-	 *  While we increment the runtime PM counter for all scenarios when
-	 *  the mmc core activates us by calling an appropriate set_ios(), we
-	 *  must additionally ensure that in case 2) the tmio mmc hardware stays
-	 *  powered on during runtime for the card detection to work.
+	 * While using internal tmio hardware logic for card detection, we need
+	 * to ensure it stays powered for it to work.
 	 */
 	if (_host->native_hotplug)
 		pm_runtime_get_noresume(&pdev->dev);
@@ -1096,6 +1103,11 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	/* See if we also get DMA */
 	tmio_mmc_request_dma(_host, pdata);
 
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
+	pm_runtime_use_autosuspend(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
 	ret = mmc_add_host(mmc);
 	if (pdata->clk_disable)
 		pdata->clk_disable(pdev);
@@ -1118,9 +1130,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 
 	return 0;
 
-pm_disable:
-	pm_runtime_disable(&pdev->dev);
-	iounmap(_host->ctl);
 host_free:
 	mmc_free_host(mmc);
 
-- 
1.7.9.5

[PATCH 6/8] mmc: tmio: Extract bus_width modifications to a function

[Ulf Hansson]

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/tmio_mmc_pio.c |   25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 33a5e26..fefe86c 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -837,6 +837,19 @@ static void tmio_mmc_power_off(struct tmio_mmc_host *host)
 		host->set_pwr(host->pdev, 0);
 }
 
+static void tmio_mmc_set_bus_width(struct tmio_mmc_host *host,
+				unsigned char bus_width)
+{
+	switch (bus_width) {
+	case MMC_BUS_WIDTH_1:
+		sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, 0x80e0);
+		break;
+	case MMC_BUS_WIDTH_4:
+		sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, 0x00e0);
+		break;
+	}
+}
+
 /* Set MMC clock / power.
  * Note: This controller uses a simple divider scheme therefore it cannot
  * run a MMC card at full speed (20MHz). The max clock is 24MHz on SD, but as
@@ -920,16 +933,8 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		}
 	}
 
-	if (host->power != TMIO_MMC_OFF_STOP) {
-		switch (ios->bus_width) {
-		case MMC_BUS_WIDTH_1:
-			sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, 0x80e0);
-		break;
-		case MMC_BUS_WIDTH_4:
-			sd_ctrl_write16(host, CTL_SD_MEM_CARD_OPT, 0x00e0);
-		break;
-		}
-	}
+	if (host->power != TMIO_MMC_OFF_STOP)
+		tmio_mmc_set_bus_width(host, ios->bus_width);
 
 	/* Let things settle. delay taken from winCE driver */
 	udelay(140);
-- 
1.7.9.5

[PATCH 7/8] mmc: tmio: Restructure .set_ios and adapt probe sequence to it

[Ulf Hansson]

An internal power state machine has earlier been used to keep probe and
.set_ios in sync. Moreover it was used to handle the specific scenario
of using MMC_CLKGATE. A dependency to MMC_CLKGATE existed to handle
runtime PM properly, which we moves away from here.

By removing the state machine and instead make .set_ios rely on the
information provided through the function's in-parameters, the code
becomes significantly simplier. Additonally as a part of this rework we
prepares for making the runtime callbacks responsible of clock gating.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/tmio_mmc.h     |   20 -------------
 drivers/mmc/host/tmio_mmc_pio.c |   61 +++++++++------------------------------
 2 files changed, 14 insertions(+), 67 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index c2c9546..58d3c05 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -40,22 +40,6 @@
 
 struct tmio_mmc_data;
 
-/*
- * We differentiate between the following 3 power states:
- * 1. card slot powered off, controller stopped. This is used, when either there
- *    is no card in the slot, or the card really has to be powered down.
- * 2. card slot powered on, controller stopped. This is used, when a card is in
- *    the slot, but no activity is currently taking place. This is a power-
- *    saving mode with card-state preserved. This state can be entered, e.g.
- *    when MMC clock-gating is used.
- * 3. card slot powered on, controller running. This is the actual active state.
- */
-enum tmio_mmc_power {
-	TMIO_MMC_OFF_STOP,	/* card power off, controller stopped */
-	TMIO_MMC_ON_STOP,	/* card power on, controller stopped */
-	TMIO_MMC_ON_RUN,	/* card power on, controller running */
-};
-
 struct tmio_mmc_host {
 	void __iomem *ctl;
 	unsigned long bus_shift;
@@ -64,9 +48,6 @@ struct tmio_mmc_host {
 	struct mmc_data         *data;
 	struct mmc_host         *mmc;
 
-	/* Controller and card power state */
-	enum tmio_mmc_power	power;
-
 	/* Callbacks for clock / power control */
 	void (*set_pwr)(struct platform_device *host, int state);
 	void (*set_clk_div)(struct platform_device *host, int state);
@@ -101,7 +82,6 @@ struct tmio_mmc_host {
 	unsigned long		last_req_ts;
 	struct mutex		ios_lock;	/* protect set_ios() context */
 	bool			native_hotplug;
-	bool			resuming;
 	bool			sdio_irq_enabled;
 };
 
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index fefe86c..cbb96a8 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -890,51 +890,23 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 	spin_unlock_irqrestore(&host->lock, flags);
 
-	/*
-	 * host->power toggles between false and true in both cases - either
-	 * or not the controller can be runtime-suspended during inactivity.
-	 * But if the controller has to be kept on, the runtime-pm usage_count
-	 * is kept positive, so no suspending actually takes place.
-	 */
-	if (ios->power_mode == MMC_POWER_ON && ios->clock) {
-		if (host->power != TMIO_MMC_ON_RUN) {
-			tmio_mmc_clk_update(mmc);
-			if (host->resuming) {
-				tmio_mmc_reset(host);
-				host->resuming = false;
-			}
-		}
-		if (host->power == TMIO_MMC_OFF_STOP)
-			tmio_mmc_reset(host);
+	switch (ios->power_mode) {
+	case MMC_POWER_OFF:
+		tmio_mmc_power_off(host);
+		tmio_mmc_clk_stop(host);
+		break;
+	case MMC_POWER_UP:
 		tmio_mmc_set_clock(host, ios->clock);
-		if (host->power == TMIO_MMC_OFF_STOP)
-			/* power up SD card and the bus */
-			tmio_mmc_power_on(host, ios->vdd);
-		host->power = TMIO_MMC_ON_RUN;
-		/* start bus clock */
+		tmio_mmc_power_on(host, ios->vdd);
 		tmio_mmc_clk_start(host);
-	} else if (ios->power_mode != MMC_POWER_UP) {
-		struct tmio_mmc_data *pdata = host->pdata;
-		unsigned int old_power = host->power;
-
-		if (old_power != TMIO_MMC_OFF_STOP) {
-			if (ios->power_mode == MMC_POWER_OFF) {
-				tmio_mmc_power_off(host);
-				host->power = TMIO_MMC_OFF_STOP;
-			} else {
-				host->power = TMIO_MMC_ON_STOP;
-			}
-		}
-
-		if (old_power == TMIO_MMC_ON_RUN) {
-			tmio_mmc_clk_stop(host);
-			if (pdata->clk_disable)
-				pdata->clk_disable(host->pdev);
-		}
-	}
-
-	if (host->power != TMIO_MMC_OFF_STOP)
 		tmio_mmc_set_bus_width(host, ios->bus_width);
+		break;
+	case MMC_POWER_ON:
+		tmio_mmc_set_clock(host, ios->clock);
+		tmio_mmc_clk_start(host);
+		tmio_mmc_set_bus_width(host, ios->bus_width);
+		break;
+	}
 
 	/* Let things settle. delay taken from winCE driver */
 	udelay(140);
@@ -1062,7 +1034,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 				  mmc->caps & MMC_CAP_NONREMOVABLE ||
 				  mmc->slot.cd_irq >= 0);
 
-	_host->power = TMIO_MMC_OFF_STOP;
 	if (tmio_mmc_clk_update(mmc) < 0) {
 		mmc->f_max = pdata->hclk;
 		mmc->f_min = mmc->f_max / 512;
@@ -1114,8 +1085,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 	pm_runtime_enable(&pdev->dev);
 
 	ret = mmc_add_host(mmc);
-	if (pdata->clk_disable)
-		pdata->clk_disable(pdev);
 	if (ret < 0) {
 		tmio_mmc_host_remove(_host);
 		return ret;
@@ -1183,8 +1152,6 @@ int tmio_mmc_host_resume(struct device *dev)
 
 	tmio_mmc_enable_dma(host, true);
 
-	/* The MMC core will perform the complete set up */
-	host->resuming = true;
 	return 0;
 }
 EXPORT_SYMBOL(tmio_mmc_host_resume);
-- 
1.7.9.5

[PATCH 8/8] mmc: tmio: Handle clock gating from runtime PM functions

[Ulf Hansson]

Add clock gating control as a part of the exported runtime PM
functions to provide tmio variants to use this in a common way.

Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/host/tmio_mmc.h     |    3 ++-
 drivers/mmc/host/tmio_mmc_pio.c |   28 ++++++++++++++++++++++++----
 2 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 58d3c05..3b41568 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -74,9 +74,10 @@ struct tmio_mmc_host {
 	struct delayed_work	delayed_reset_work;
 	struct work_struct	done;
 
-	/* Cache IRQ mask */
+	/* Cache */
 	u32			sdcard_irq_mask;
 	u32			sdio_irq_mask;
+	unsigned int		clk_cache;
 
 	spinlock_t		lock;		/* protect host private data */
 	unsigned long		last_req_ts;
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index cbb96a8..bb18892 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -149,7 +149,8 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
 	}
 }
 
-static void tmio_mmc_set_clock(struct tmio_mmc_host *host, int new_clock)
+static void tmio_mmc_set_clock(struct tmio_mmc_host *host,
+				unsigned int new_clock)
 {
 	u32 clk = 0, clock;
 
@@ -773,9 +774,9 @@ fail:
 	pm_runtime_put_autosuspend(mmc_dev(mmc));
 }
 
-static int tmio_mmc_clk_update(struct mmc_host *mmc)
+static int tmio_mmc_clk_update(struct tmio_mmc_host *host)
 {
-	struct tmio_mmc_host *host = mmc_priv(mmc);
+	struct mmc_host *mmc = host->mmc;
 	struct tmio_mmc_data *pdata = host->pdata;
 	int ret;
 
@@ -917,6 +918,8 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			ios->clock, ios->power_mode);
 	host->mrq = NULL;
 
+	host->clk_cache = ios->clock;
+
 	mutex_unlock(&host->ios_lock);
 
 	pm_runtime_mark_last_busy(mmc_dev(mmc));
@@ -1034,7 +1037,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 				  mmc->caps & MMC_CAP_NONREMOVABLE ||
 				  mmc->slot.cd_irq >= 0);
 
-	if (tmio_mmc_clk_update(mmc) < 0) {
+	if (tmio_mmc_clk_update(_host) < 0) {
 		mmc->f_max = pdata->hclk;
 		mmc->f_min = mmc->f_max / 512;
 	}
@@ -1160,6 +1163,15 @@ EXPORT_SYMBOL(tmio_mmc_host_resume);
 #ifdef CONFIG_PM_RUNTIME
 int tmio_mmc_host_runtime_suspend(struct device *dev)
 {
+	struct mmc_host *mmc = dev_get_drvdata(dev);
+	struct tmio_mmc_host *host = mmc_priv(mmc);
+
+	if (host->clk_cache)
+		tmio_mmc_clk_stop(host);
+
+	if (host->pdata->clk_disable)
+		host->pdata->clk_disable(host->pdev);
+
 	return 0;
 }
 EXPORT_SYMBOL(tmio_mmc_host_runtime_suspend);
@@ -1171,6 +1183,14 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
 
 	tmio_mmc_enable_dma(host, true);
 
+	tmio_mmc_reset(host);
+	tmio_mmc_clk_update(host);
+
+	if (host->clk_cache) {
+		tmio_mmc_set_clock(host, host->clk_cache);
+		tmio_mmc_clk_start(host);
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL(tmio_mmc_host_runtime_resume);
-- 
1.7.9.5

Re: [PATCH 4/8] mmc: tmio: Keep host active while SDIO irq is enabled

[Guennadi Liakhovetski]

Hi Ulf,

On Fri, 8 Nov 2013, Ulf Hansson wrote:

> The host must be kept active to be able to serve with SDIO irqs. We
> prevent it from being put into in-active while the SDIO irq is enabled
> by simply adding balanced calls to pm_runtime_get|put.
> 
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>  drivers/mmc/host/tmio_mmc.h     |    1 +
>  drivers/mmc/host/tmio_mmc_pio.c |   19 +++++++++++++++----
>  2 files changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index 6c5b45a..c2c9546 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -102,6 +102,7 @@ struct tmio_mmc_host {
>  	struct mutex		ios_lock;	/* protect set_ios() context */
>  	bool			native_hotplug;
>  	bool			resuming;
> +	bool			sdio_irq_enabled;
>  };
>  
>  int tmio_mmc_host_probe(struct tmio_mmc_host **host,
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index 472e803..377157e 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -129,15 +129,22 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
>  {
>  	struct tmio_mmc_host *host = mmc_priv(mmc);
>  
> -	if (enable) {
> +	if (enable && !host->sdio_irq_enabled) {
> +		/* Keep device active while SDIO irq is enabled */
> +		pm_runtime_get_sync(mmc_dev(mmc));
> +		host->sdio_irq_enabled = true;
> +
>  		host->sdio_irq_mask = TMIO_SDIO_MASK_ALL &
>  					~TMIO_SDIO_STAT_IOIRQ;
>  		sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001);
>  		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
> -	} else {
> +	} else if (!enable && host->sdio_irq_enabled) {
>  		host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
>  		sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
>  		sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000);
> +
> +		host->sdio_irq_enabled = false;
> +		pm_runtime_put(mmc_dev(mmc));
>  	}
>  }
>  
> @@ -1072,8 +1079,12 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
>  
>  	_host->sdcard_irq_mask &= ~irq_mask;
>  
> -	if (pdata->flags & TMIO_MMC_SDIO_IRQ)
> -		tmio_mmc_enable_sdio_irq(mmc, 0);
> +	_host->sdio_irq_enabled = false;

This by itself is unneeded as the data is allocated per kzalloc(). It can 
be argued, that such explicit assignments make code better readable, but 
we also don't initialise other boolean variables in struct tmio_mmc_host 
during probe(): force_pio and resuming, so, to stay consistent it could be 
better to preserve that pattern.

> +	if (pdata->flags & TMIO_MMC_SDIO_IRQ) {
> +		_host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
> +		sd_ctrl_write16(_host, CTL_SDIO_IRQ_MASK, _host->sdio_irq_mask);
> +		sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x0000);
> +	}

I don't think I like this open-coding a lot. I think, in 
tmio_mmc_enable_sdio_irq() above it would be better to only balance calls 
to pm_runtime_enable/disable() by checking and setting the 
.sdio_irq_enabled flag. Then we wouldn't have to modify this location.

Thanks
Guennadi

>  
>  	spin_lock_init(&_host->lock);
>  	mutex_init(&_host->ios_lock);
> -- 
> 1.7.9.5
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

Re: [PATCH 4/8] mmc: tmio: Keep host active while SDIO irq is enabled

[Ulf Hansson]

On 8 November 2013 07:56, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote:
> Hi Ulf,
>
> On Fri, 8 Nov 2013, Ulf Hansson wrote:
>
>> The host must be kept active to be able to serve with SDIO irqs. We
>> prevent it from being put into in-active while the SDIO irq is enabled
>> by simply adding balanced calls to pm_runtime_get|put.
>>
>> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>  drivers/mmc/host/tmio_mmc.h     |    1 +
>>  drivers/mmc/host/tmio_mmc_pio.c |   19 +++++++++++++++----
>>  2 files changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
>> index 6c5b45a..c2c9546 100644
>> --- a/drivers/mmc/host/tmio_mmc.h
>> +++ b/drivers/mmc/host/tmio_mmc.h
>> @@ -102,6 +102,7 @@ struct tmio_mmc_host {
>>       struct mutex            ios_lock;       /* protect set_ios() context */
>>       bool                    native_hotplug;
>>       bool                    resuming;
>> +     bool                    sdio_irq_enabled;
>>  };
>>
>>  int tmio_mmc_host_probe(struct tmio_mmc_host **host,
>> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
>> index 472e803..377157e 100644
>> --- a/drivers/mmc/host/tmio_mmc_pio.c
>> +++ b/drivers/mmc/host/tmio_mmc_pio.c
>> @@ -129,15 +129,22 @@ static void tmio_mmc_enable_sdio_irq(struct mmc_host *mmc, int enable)
>>  {
>>       struct tmio_mmc_host *host = mmc_priv(mmc);
>>
>> -     if (enable) {
>> +     if (enable && !host->sdio_irq_enabled) {
>> +             /* Keep device active while SDIO irq is enabled */
>> +             pm_runtime_get_sync(mmc_dev(mmc));
>> +             host->sdio_irq_enabled = true;
>> +
>>               host->sdio_irq_mask = TMIO_SDIO_MASK_ALL &
>>                                       ~TMIO_SDIO_STAT_IOIRQ;
>>               sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0001);
>>               sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
>> -     } else {
>> +     } else if (!enable && host->sdio_irq_enabled) {
>>               host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
>>               sd_ctrl_write16(host, CTL_SDIO_IRQ_MASK, host->sdio_irq_mask);
>>               sd_ctrl_write16(host, CTL_TRANSACTION_CTL, 0x0000);
>> +
>> +             host->sdio_irq_enabled = false;
>> +             pm_runtime_put(mmc_dev(mmc));
>>       }
>>  }
>>
>> @@ -1072,8 +1079,12 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
>>
>>       _host->sdcard_irq_mask &= ~irq_mask;
>>
>> -     if (pdata->flags & TMIO_MMC_SDIO_IRQ)
>> -             tmio_mmc_enable_sdio_irq(mmc, 0);
>> +     _host->sdio_irq_enabled = false;
>
> This by itself is unneeded as the data is allocated per kzalloc(). It can
> be argued, that such explicit assignments make code better readable, but
> we also don't initialise other boolean variables in struct tmio_mmc_host
> during probe(): force_pio and resuming, so, to stay consistent it could be
> better to preserve that pattern.

Sure, makes sense!

>
>> +     if (pdata->flags & TMIO_MMC_SDIO_IRQ) {
>> +             _host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
>> +             sd_ctrl_write16(_host, CTL_SDIO_IRQ_MASK, _host->sdio_irq_mask);
>> +             sd_ctrl_write16(_host, CTL_TRANSACTION_CTL, 0x0000);
>> +     }
>
> I don't think I like this open-coding a lot. I think, in
> tmio_mmc_enable_sdio_irq() above it would be better to only balance calls
> to pm_runtime_enable/disable() by checking and setting the
> .sdio_irq_enabled flag. Then we wouldn't have to modify this location.

I can rework like you proposed, but do note that since calls to
tmio_mmc_enable_sdio_irq are not to be considered balanced, we might
end up in re-writing the CTL_SDIO_IRQ_MASK and CTL_TRANSACTION_CTL
with the same values they already have. Is this really wanted? That
was the reason to why I moved out the initial pieces to probe.

Kind regards
Uffe

>
> Thanks
> Guennadi
>
>>
>>       spin_lock_init(&_host->lock);
>>       mutex_init(&_host->ios_lock);
>> --
>> 1.7.9.5
>>
>
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/