[PATCH v2] MMC: tmio: fix unbalanced power-on calls with clock-gating enabled

[PATCH v2] MMC: tmio: fix unbalanced power-on calls with clock-gating enabled

[Guennadi Liakhovetski]

With MMC clock gating enabled the MMC core currently calls MMC host driver's
.set_ios() method with .power_mode = MMC_POWER_ON and the clock value set
either to 0 or to the target rate. The tmio MMC driver then wrongly
translates the latter calls to card slot power-on requests, even when the
slot already was on. This patch fixes the driver to avoid needlessly
incrementing power-supplying regulator's use count.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---

v2: Also make runtime PM handling safer and more resistant to possible 
MMC core changes. V1 worked fine too, but theoretically if the core 
decided to issue a sequence like

	.set_ios(ios->power_mode = MMC_POWER_ON, ios->clock != 0);
	.set_ios(ios->power_mode = MMC_POWER_ON, ios->clock = 0);
	.set_ios(ios->power_mode = MMC_POWER_OFF, ios->clock = 0);

we would end up calling pm_runtime_put() twice in a row, which is wrong. 
V2 only toggles the controller runtime PM status only when entering or 
leaving the TMIO_MMC_ON_RUN state.

 drivers/mmc/host/tmio_mmc.h     |   20 ++++++++++++++++++--
 drivers/mmc/host/tmio_mmc_pio.c |   27 +++++++++++++++++----------
 2 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index d857f5c..3cc589a 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -40,6 +40,22 @@
 
 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;
@@ -48,8 +64,8 @@ struct tmio_mmc_host {
 	struct mmc_data         *data;
 	struct mmc_host         *mmc;
 
-	/* Controller power state */
-	bool			power;
+	/* Controller and card power state */
+	enum tmio_mmc_power	power;
 
 	/* Callbacks for clock / power control */
 	void (*set_pwr)(struct platform_device *host, int state);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f508ecb..1d5ef64 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -859,32 +859,39 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	 * is kept positive, so no suspending actually takes place.
 	 */
 	if (ios->power_mode = MMC_POWER_ON && ios->clock) {
-		if (!host->power) {
+		if (host->power != TMIO_MMC_ON_RUN) {
 			tmio_mmc_clk_update(mmc);
 			pm_runtime_get_sync(dev);
 		}
 		tmio_mmc_set_clock(host, ios->clock);
-		if (!host->power) {
+		if (host->power = TMIO_MMC_OFF_STOP)
 			/* power up SD card and the bus */
 			tmio_mmc_power_on(host, ios->vdd);
-			host->power = true;
-		}
+		host->power = TMIO_MMC_ON_RUN;
 		/* start bus clock */
 		tmio_mmc_clk_start(host);
 	} else if (ios->power_mode != MMC_POWER_UP) {
-		if (host->power) {
-			struct tmio_mmc_data *pdata = host->pdata;
-			if (ios->power_mode = MMC_POWER_OFF)
+		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);
-			host->power = false;
 			pm_runtime_put(dev);
 			if (pdata->clk_disable)
 				pdata->clk_disable(host->pdev);
 		}
 	}
 
-	if (host->power) {
+	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);
@@ -1025,7 +1032,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 				  mmc->caps & MMC_CAP_NONREMOVABLE ||
 				  mmc->slot.cd_irq >= 0);
 
-	_host->power = false;
+	_host->power = TMIO_MMC_OFF_STOP;
 	pm_runtime_enable(&pdev->dev);
 	ret = pm_runtime_resume(&pdev->dev);
 	if (ret < 0)
-- 
1.7.2.5

[PATCH v2] MMC: tmio: fix unbalanced power-on calls with clock-gating enabled

[Guennadi Liakhovetski]

With MMC clock gating enabled the MMC core currently calls MMC host driver's
.set_ios() method with .power_mode == MMC_POWER_ON and the clock value set
either to 0 or to the target rate. The tmio MMC driver then wrongly
translates the latter calls to card slot power-on requests, even when the
slot already was on. This patch fixes the driver to avoid needlessly
incrementing power-supplying regulator's use count.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
---

v2: Also make runtime PM handling safer and more resistant to possible 
MMC core changes. V1 worked fine too, but theoretically if the core 
decided to issue a sequence like

	.set_ios(ios->power_mode == MMC_POWER_ON, ios->clock != 0);
	.set_ios(ios->power_mode == MMC_POWER_ON, ios->clock == 0);
	.set_ios(ios->power_mode == MMC_POWER_OFF, ios->clock == 0);

we would end up calling pm_runtime_put() twice in a row, which is wrong. 
V2 only toggles the controller runtime PM status only when entering or 
leaving the TMIO_MMC_ON_RUN state.

 drivers/mmc/host/tmio_mmc.h     |   20 ++++++++++++++++++--
 drivers/mmc/host/tmio_mmc_pio.c |   27 +++++++++++++++++----------
 2 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index d857f5c..3cc589a 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -40,6 +40,22 @@
 
 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;
@@ -48,8 +64,8 @@ struct tmio_mmc_host {
 	struct mmc_data         *data;
 	struct mmc_host         *mmc;
 
-	/* Controller power state */
-	bool			power;
+	/* Controller and card power state */
+	enum tmio_mmc_power	power;
 
 	/* Callbacks for clock / power control */
 	void (*set_pwr)(struct platform_device *host, int state);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index f508ecb..1d5ef64 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -859,32 +859,39 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	 * is kept positive, so no suspending actually takes place.
 	 */
 	if (ios->power_mode == MMC_POWER_ON && ios->clock) {
-		if (!host->power) {
+		if (host->power != TMIO_MMC_ON_RUN) {
 			tmio_mmc_clk_update(mmc);
 			pm_runtime_get_sync(dev);
 		}
 		tmio_mmc_set_clock(host, ios->clock);
-		if (!host->power) {
+		if (host->power == TMIO_MMC_OFF_STOP)
 			/* power up SD card and the bus */
 			tmio_mmc_power_on(host, ios->vdd);
-			host->power = true;
-		}
+		host->power = TMIO_MMC_ON_RUN;
 		/* start bus clock */
 		tmio_mmc_clk_start(host);
 	} else if (ios->power_mode != MMC_POWER_UP) {
-		if (host->power) {
-			struct tmio_mmc_data *pdata = host->pdata;
-			if (ios->power_mode == MMC_POWER_OFF)
+		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);
-			host->power = false;
 			pm_runtime_put(dev);
 			if (pdata->clk_disable)
 				pdata->clk_disable(host->pdev);
 		}
 	}
 
-	if (host->power) {
+	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);
@@ -1025,7 +1032,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 				  mmc->caps & MMC_CAP_NONREMOVABLE ||
 				  mmc->slot.cd_irq >= 0);
 
-	_host->power = false;
+	_host->power = TMIO_MMC_OFF_STOP;
 	pm_runtime_enable(&pdev->dev);
 	ret = pm_runtime_resume(&pdev->dev);
 	if (ret < 0)
-- 
1.7.2.5

Re: [PATCH v2] MMC: tmio: fix unbalanced power-on calls with clock-gating enabled

[Laurent Pinchart]

Hi Guennadi,

Thanks for the patch.

On Tuesday 23 April 2013 09:29:44 Guennadi Liakhovetski wrote:
> With MMC clock gating enabled the MMC core currently calls MMC host driver's
> .set_ios() method with .power_mode = MMC_POWER_ON and the clock value set
> either to 0 or to the target rate. The tmio MMC driver then wrongly
> translates the latter calls to card slot power-on requests, even when the
> slot already was on. This patch fixes the driver to avoid needlessly
> incrementing power-supplying regulator's use count.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

With this patch the number of users of the vqmmc regulator stays at 1 
(regardless of whether the card is inserted or now).

> ---
> 
> v2: Also make runtime PM handling safer and more resistant to possible
> MMC core changes. V1 worked fine too, but theoretically if the core
> decided to issue a sequence like
> 
> 	.set_ios(ios->power_mode = MMC_POWER_ON, ios->clock != 0);
> 	.set_ios(ios->power_mode = MMC_POWER_ON, ios->clock = 0);
> 	.set_ios(ios->power_mode = MMC_POWER_OFF, ios->clock = 0);
> 
> we would end up calling pm_runtime_put() twice in a row, which is wrong.
> V2 only toggles the controller runtime PM status only when entering or
> leaving the TMIO_MMC_ON_RUN state.
> 
>  drivers/mmc/host/tmio_mmc.h     |   20 ++++++++++++++++++--
>  drivers/mmc/host/tmio_mmc_pio.c |   27 +++++++++++++++++----------
>  2 files changed, 35 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index d857f5c..3cc589a 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -40,6 +40,22 @@
> 
>  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;
> @@ -48,8 +64,8 @@ struct tmio_mmc_host {
>  	struct mmc_data         *data;
>  	struct mmc_host         *mmc;
> 
> -	/* Controller power state */
> -	bool			power;
> +	/* Controller and card power state */
> +	enum tmio_mmc_power	power;
> 
>  	/* Callbacks for clock / power control */
>  	void (*set_pwr)(struct platform_device *host, int state);
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c
> b/drivers/mmc/host/tmio_mmc_pio.c index f508ecb..1d5ef64 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -859,32 +859,39 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc,
> struct mmc_ios *ios) * is kept positive, so no suspending actually takes
> place.
>  	 */
>  	if (ios->power_mode = MMC_POWER_ON && ios->clock) {
> -		if (!host->power) {
> +		if (host->power != TMIO_MMC_ON_RUN) {
>  			tmio_mmc_clk_update(mmc);
>  			pm_runtime_get_sync(dev);
>  		}
>  		tmio_mmc_set_clock(host, ios->clock);
> -		if (!host->power) {
> +		if (host->power = TMIO_MMC_OFF_STOP)
>  			/* power up SD card and the bus */
>  			tmio_mmc_power_on(host, ios->vdd);
> -			host->power = true;
> -		}
> +		host->power = TMIO_MMC_ON_RUN;
>  		/* start bus clock */
>  		tmio_mmc_clk_start(host);
>  	} else if (ios->power_mode != MMC_POWER_UP) {
> -		if (host->power) {
> -			struct tmio_mmc_data *pdata = host->pdata;
> -			if (ios->power_mode = MMC_POWER_OFF)
> +		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);
> -			host->power = false;
>  			pm_runtime_put(dev);
>  			if (pdata->clk_disable)
>  				pdata->clk_disable(host->pdev);
>  		}
>  	}
> 
> -	if (host->power) {
> +	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);
> @@ -1025,7 +1032,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
>  				  mmc->caps & MMC_CAP_NONREMOVABLE ||
>  				  mmc->slot.cd_irq >= 0);
> 
> -	_host->power = false;
> +	_host->power = TMIO_MMC_OFF_STOP;
>  	pm_runtime_enable(&pdev->dev);
>  	ret = pm_runtime_resume(&pdev->dev);
>  	if (ret < 0)
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] MMC: tmio: fix unbalanced power-on calls with clock-gating enabled

[Laurent Pinchart]

Hi Guennadi,

Thanks for the patch.

On Tuesday 23 April 2013 09:29:44 Guennadi Liakhovetski wrote:
> With MMC clock gating enabled the MMC core currently calls MMC host driver's
> .set_ios() method with .power_mode == MMC_POWER_ON and the clock value set
> either to 0 or to the target rate. The tmio MMC driver then wrongly
> translates the latter calls to card slot power-on requests, even when the
> slot already was on. This patch fixes the driver to avoid needlessly
> incrementing power-supplying regulator's use count.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

With this patch the number of users of the vqmmc regulator stays at 1 
(regardless of whether the card is inserted or now).

> ---
> 
> v2: Also make runtime PM handling safer and more resistant to possible
> MMC core changes. V1 worked fine too, but theoretically if the core
> decided to issue a sequence like
> 
> 	.set_ios(ios->power_mode == MMC_POWER_ON, ios->clock != 0);
> 	.set_ios(ios->power_mode == MMC_POWER_ON, ios->clock == 0);
> 	.set_ios(ios->power_mode == MMC_POWER_OFF, ios->clock == 0);
> 
> we would end up calling pm_runtime_put() twice in a row, which is wrong.
> V2 only toggles the controller runtime PM status only when entering or
> leaving the TMIO_MMC_ON_RUN state.
> 
>  drivers/mmc/host/tmio_mmc.h     |   20 ++++++++++++++++++--
>  drivers/mmc/host/tmio_mmc_pio.c |   27 +++++++++++++++++----------
>  2 files changed, 35 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index d857f5c..3cc589a 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -40,6 +40,22 @@
> 
>  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;
> @@ -48,8 +64,8 @@ struct tmio_mmc_host {
>  	struct mmc_data         *data;
>  	struct mmc_host         *mmc;
> 
> -	/* Controller power state */
> -	bool			power;
> +	/* Controller and card power state */
> +	enum tmio_mmc_power	power;
> 
>  	/* Callbacks for clock / power control */
>  	void (*set_pwr)(struct platform_device *host, int state);
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c
> b/drivers/mmc/host/tmio_mmc_pio.c index f508ecb..1d5ef64 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -859,32 +859,39 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc,
> struct mmc_ios *ios) * is kept positive, so no suspending actually takes
> place.
>  	 */
>  	if (ios->power_mode == MMC_POWER_ON && ios->clock) {
> -		if (!host->power) {
> +		if (host->power != TMIO_MMC_ON_RUN) {
>  			tmio_mmc_clk_update(mmc);
>  			pm_runtime_get_sync(dev);
>  		}
>  		tmio_mmc_set_clock(host, ios->clock);
> -		if (!host->power) {
> +		if (host->power == TMIO_MMC_OFF_STOP)
>  			/* power up SD card and the bus */
>  			tmio_mmc_power_on(host, ios->vdd);
> -			host->power = true;
> -		}
> +		host->power = TMIO_MMC_ON_RUN;
>  		/* start bus clock */
>  		tmio_mmc_clk_start(host);
>  	} else if (ios->power_mode != MMC_POWER_UP) {
> -		if (host->power) {
> -			struct tmio_mmc_data *pdata = host->pdata;
> -			if (ios->power_mode == MMC_POWER_OFF)
> +		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);
> -			host->power = false;
>  			pm_runtime_put(dev);
>  			if (pdata->clk_disable)
>  				pdata->clk_disable(host->pdev);
>  		}
>  	}
> 
> -	if (host->power) {
> +	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);
> @@ -1025,7 +1032,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
>  				  mmc->caps & MMC_CAP_NONREMOVABLE ||
>  				  mmc->slot.cd_irq >= 0);
> 
> -	_host->power = false;
> +	_host->power = TMIO_MMC_OFF_STOP;
>  	pm_runtime_enable(&pdev->dev);
>  	ret = pm_runtime_resume(&pdev->dev);
>  	if (ret < 0)
-- 
Regards,

Laurent Pinchart

Re: [PATCH v2] MMC: tmio: fix unbalanced power-on calls with clock-gating enabled

[Guennadi Liakhovetski]

Hi Chris

On Tue, 23 Apr 2013, Guennadi Liakhovetski wrote:

> With MMC clock gating enabled the MMC core currently calls MMC host driver's
> .set_ios() method with .power_mode = MMC_POWER_ON and the clock value set
> either to 0 or to the target rate. The tmio MMC driver then wrongly
> translates the latter calls to card slot power-on requests, even when the
> slot already was on. This patch fixes the driver to avoid needlessly
> incrementing power-supplying regulator's use count.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

Can we get this one into 3.10?

Thanks
Guennadi

> ---
> 
> v2: Also make runtime PM handling safer and more resistant to possible 
> MMC core changes. V1 worked fine too, but theoretically if the core 
> decided to issue a sequence like
> 
> 	.set_ios(ios->power_mode = MMC_POWER_ON, ios->clock != 0);
> 	.set_ios(ios->power_mode = MMC_POWER_ON, ios->clock = 0);
> 	.set_ios(ios->power_mode = MMC_POWER_OFF, ios->clock = 0);
> 
> we would end up calling pm_runtime_put() twice in a row, which is wrong. 
> V2 only toggles the controller runtime PM status only when entering or 
> leaving the TMIO_MMC_ON_RUN state.
> 
>  drivers/mmc/host/tmio_mmc.h     |   20 ++++++++++++++++++--
>  drivers/mmc/host/tmio_mmc_pio.c |   27 +++++++++++++++++----------
>  2 files changed, 35 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index d857f5c..3cc589a 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -40,6 +40,22 @@
>  
>  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;
> @@ -48,8 +64,8 @@ struct tmio_mmc_host {
>  	struct mmc_data         *data;
>  	struct mmc_host         *mmc;
>  
> -	/* Controller power state */
> -	bool			power;
> +	/* Controller and card power state */
> +	enum tmio_mmc_power	power;
>  
>  	/* Callbacks for clock / power control */
>  	void (*set_pwr)(struct platform_device *host, int state);
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index f508ecb..1d5ef64 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -859,32 +859,39 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  	 * is kept positive, so no suspending actually takes place.
>  	 */
>  	if (ios->power_mode = MMC_POWER_ON && ios->clock) {
> -		if (!host->power) {
> +		if (host->power != TMIO_MMC_ON_RUN) {
>  			tmio_mmc_clk_update(mmc);
>  			pm_runtime_get_sync(dev);
>  		}
>  		tmio_mmc_set_clock(host, ios->clock);
> -		if (!host->power) {
> +		if (host->power = TMIO_MMC_OFF_STOP)
>  			/* power up SD card and the bus */
>  			tmio_mmc_power_on(host, ios->vdd);
> -			host->power = true;
> -		}
> +		host->power = TMIO_MMC_ON_RUN;
>  		/* start bus clock */
>  		tmio_mmc_clk_start(host);
>  	} else if (ios->power_mode != MMC_POWER_UP) {
> -		if (host->power) {
> -			struct tmio_mmc_data *pdata = host->pdata;
> -			if (ios->power_mode = MMC_POWER_OFF)
> +		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);
> -			host->power = false;
>  			pm_runtime_put(dev);
>  			if (pdata->clk_disable)
>  				pdata->clk_disable(host->pdev);
>  		}
>  	}
>  
> -	if (host->power) {
> +	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);
> @@ -1025,7 +1032,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
>  				  mmc->caps & MMC_CAP_NONREMOVABLE ||
>  				  mmc->slot.cd_irq >= 0);
>  
> -	_host->power = false;
> +	_host->power = TMIO_MMC_OFF_STOP;
>  	pm_runtime_enable(&pdev->dev);
>  	ret = pm_runtime_resume(&pdev->dev);
>  	if (ret < 0)
> -- 
> 1.7.2.5
> 
> 

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

Re: [PATCH v2] MMC: tmio: fix unbalanced power-on calls with clock-gating enabled

[Guennadi Liakhovetski]

Hi Chris

On Tue, 23 Apr 2013, Guennadi Liakhovetski wrote:

> With MMC clock gating enabled the MMC core currently calls MMC host driver's
> .set_ios() method with .power_mode == MMC_POWER_ON and the clock value set
> either to 0 or to the target rate. The tmio MMC driver then wrongly
> translates the latter calls to card slot power-on requests, even when the
> slot already was on. This patch fixes the driver to avoid needlessly
> incrementing power-supplying regulator's use count.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

Can we get this one into 3.10?

Thanks
Guennadi

> ---
> 
> v2: Also make runtime PM handling safer and more resistant to possible 
> MMC core changes. V1 worked fine too, but theoretically if the core 
> decided to issue a sequence like
> 
> 	.set_ios(ios->power_mode == MMC_POWER_ON, ios->clock != 0);
> 	.set_ios(ios->power_mode == MMC_POWER_ON, ios->clock == 0);
> 	.set_ios(ios->power_mode == MMC_POWER_OFF, ios->clock == 0);
> 
> we would end up calling pm_runtime_put() twice in a row, which is wrong. 
> V2 only toggles the controller runtime PM status only when entering or 
> leaving the TMIO_MMC_ON_RUN state.
> 
>  drivers/mmc/host/tmio_mmc.h     |   20 ++++++++++++++++++--
>  drivers/mmc/host/tmio_mmc_pio.c |   27 +++++++++++++++++----------
>  2 files changed, 35 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index d857f5c..3cc589a 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -40,6 +40,22 @@
>  
>  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;
> @@ -48,8 +64,8 @@ struct tmio_mmc_host {
>  	struct mmc_data         *data;
>  	struct mmc_host         *mmc;
>  
> -	/* Controller power state */
> -	bool			power;
> +	/* Controller and card power state */
> +	enum tmio_mmc_power	power;
>  
>  	/* Callbacks for clock / power control */
>  	void (*set_pwr)(struct platform_device *host, int state);
> diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
> index f508ecb..1d5ef64 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -859,32 +859,39 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  	 * is kept positive, so no suspending actually takes place.
>  	 */
>  	if (ios->power_mode == MMC_POWER_ON && ios->clock) {
> -		if (!host->power) {
> +		if (host->power != TMIO_MMC_ON_RUN) {
>  			tmio_mmc_clk_update(mmc);
>  			pm_runtime_get_sync(dev);
>  		}
>  		tmio_mmc_set_clock(host, ios->clock);
> -		if (!host->power) {
> +		if (host->power == TMIO_MMC_OFF_STOP)
>  			/* power up SD card and the bus */
>  			tmio_mmc_power_on(host, ios->vdd);
> -			host->power = true;
> -		}
> +		host->power = TMIO_MMC_ON_RUN;
>  		/* start bus clock */
>  		tmio_mmc_clk_start(host);
>  	} else if (ios->power_mode != MMC_POWER_UP) {
> -		if (host->power) {
> -			struct tmio_mmc_data *pdata = host->pdata;
> -			if (ios->power_mode == MMC_POWER_OFF)
> +		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);
> -			host->power = false;
>  			pm_runtime_put(dev);
>  			if (pdata->clk_disable)
>  				pdata->clk_disable(host->pdev);
>  		}
>  	}
>  
> -	if (host->power) {
> +	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);
> @@ -1025,7 +1032,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
>  				  mmc->caps & MMC_CAP_NONREMOVABLE ||
>  				  mmc->slot.cd_irq >= 0);
>  
> -	_host->power = false;
> +	_host->power = TMIO_MMC_OFF_STOP;
>  	pm_runtime_enable(&pdev->dev);
>  	ret = pm_runtime_resume(&pdev->dev);
>  	if (ret < 0)
> -- 
> 1.7.2.5
> 
> 

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

Re: [PATCH v2] MMC: tmio: fix unbalanced power-on calls with clock-gating enabled

[Chris Ball]

Hi,

On Tue, Apr 23 2013, Guennadi Liakhovetski wrote:
> With MMC clock gating enabled the MMC core currently calls MMC host driver's
> .set_ios() method with .power_mode = MMC_POWER_ON and the clock value set
> either to 0 or to the target rate. The tmio MMC driver then wrongly
> translates the latter calls to card slot power-on requests, even when the
> slot already was on. This patch fixes the driver to avoid needlessly
> incrementing power-supplying regulator's use count.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

This doesn't apply to mmc-next -- please could you rebase/resend?

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

Re: [PATCH v2] MMC: tmio: fix unbalanced power-on calls with clock-gating enabled

[Chris Ball]

Hi,

On Tue, Apr 23 2013, Guennadi Liakhovetski wrote:
> With MMC clock gating enabled the MMC core currently calls MMC host driver's
> .set_ios() method with .power_mode == MMC_POWER_ON and the clock value set
> either to 0 or to the target rate. The tmio MMC driver then wrongly
> translates the latter calls to card slot power-on requests, even when the
> slot already was on. This patch fixes the driver to avoid needlessly
> incrementing power-supplying regulator's use count.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

This doesn't apply to mmc-next -- please could you rebase/resend?

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

[PATCH v3 1/2] mmc: tmio: fix unbalanced power-on calls with clock-gating enabled

[Guennadi Liakhovetski]

With MMC clock gating enabled the MMC core currently calls MMC host driver's
.set_ios() method with .power_mode = MMC_POWER_ON and the clock value set
either to 0 or to the target rate. The tmio MMC driver then wrongly
translates the latter calls to card slot power-on requests, even when the
slot already was on. This patch fixes the driver to avoid needlessly
incrementing power-supplying regulator's use count.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---

v3: rebased on top of current mmc-next / 3.10-rc

 drivers/mmc/host/tmio_mmc.h     |   20 ++++++++++++++++++--
 drivers/mmc/host/tmio_mmc_pio.c |   27 +++++++++++++++++----------
 2 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 759d8f4..86fd21e 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -40,6 +40,22 @@
 
 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;
@@ -48,8 +64,8 @@ struct tmio_mmc_host {
 	struct mmc_data         *data;
 	struct mmc_host         *mmc;
 
-	/* Controller power state */
-	bool			power;
+	/* Controller and card power state */
+	enum tmio_mmc_power	power;
 
 	/* Callbacks for clock / power control */
 	void (*set_pwr)(struct platform_device *host, int state);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 435cc4d..67d9642 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -859,7 +859,7 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	 * is kept positive, so no suspending actually takes place.
 	 */
 	if (ios->power_mode = MMC_POWER_ON && ios->clock) {
-		if (!host->power) {
+		if (host->power != TMIO_MMC_ON_RUN) {
 			tmio_mmc_clk_update(mmc);
 			pm_runtime_get_sync(dev);
 			if (host->resuming) {
@@ -868,27 +868,34 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			}
 		}
 		tmio_mmc_set_clock(host, ios->clock);
-		if (!host->power) {
+		if (host->power = TMIO_MMC_OFF_STOP)
 			/* power up SD card and the bus */
 			tmio_mmc_power_on(host, ios->vdd);
-			host->power = true;
-		}
+		host->power = TMIO_MMC_ON_RUN;
 		/* start bus clock */
 		tmio_mmc_clk_start(host);
 	} else if (ios->power_mode != MMC_POWER_UP) {
-		if (host->power) {
-			struct tmio_mmc_data *pdata = host->pdata;
-			if (ios->power_mode = MMC_POWER_OFF)
+		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);
-			host->power = false;
 			pm_runtime_put(dev);
 			if (pdata->clk_disable)
 				pdata->clk_disable(host->pdev);
 		}
 	}
 
-	if (host->power) {
+	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);
@@ -1029,7 +1036,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 				  mmc->caps & MMC_CAP_NONREMOVABLE ||
 				  mmc->slot.cd_irq >= 0);
 
-	_host->power = false;
+	_host->power = TMIO_MMC_OFF_STOP;
 	pm_runtime_enable(&pdev->dev);
 	ret = pm_runtime_resume(&pdev->dev);
 	if (ret < 0)
-- 
1.7.2.5

[PATCH v3 1/2] mmc: tmio: fix unbalanced power-on calls with clock-gating enabled

[Guennadi Liakhovetski]

With MMC clock gating enabled the MMC core currently calls MMC host driver's
.set_ios() method with .power_mode == MMC_POWER_ON and the clock value set
either to 0 or to the target rate. The tmio MMC driver then wrongly
translates the latter calls to card slot power-on requests, even when the
slot already was on. This patch fixes the driver to avoid needlessly
incrementing power-supplying regulator's use count.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---

v3: rebased on top of current mmc-next / 3.10-rc

 drivers/mmc/host/tmio_mmc.h     |   20 ++++++++++++++++++--
 drivers/mmc/host/tmio_mmc_pio.c |   27 +++++++++++++++++----------
 2 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index 759d8f4..86fd21e 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -40,6 +40,22 @@
 
 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;
@@ -48,8 +64,8 @@ struct tmio_mmc_host {
 	struct mmc_data         *data;
 	struct mmc_host         *mmc;
 
-	/* Controller power state */
-	bool			power;
+	/* Controller and card power state */
+	enum tmio_mmc_power	power;
 
 	/* Callbacks for clock / power control */
 	void (*set_pwr)(struct platform_device *host, int state);
diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 435cc4d..67d9642 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -859,7 +859,7 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 	 * is kept positive, so no suspending actually takes place.
 	 */
 	if (ios->power_mode == MMC_POWER_ON && ios->clock) {
-		if (!host->power) {
+		if (host->power != TMIO_MMC_ON_RUN) {
 			tmio_mmc_clk_update(mmc);
 			pm_runtime_get_sync(dev);
 			if (host->resuming) {
@@ -868,27 +868,34 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			}
 		}
 		tmio_mmc_set_clock(host, ios->clock);
-		if (!host->power) {
+		if (host->power == TMIO_MMC_OFF_STOP)
 			/* power up SD card and the bus */
 			tmio_mmc_power_on(host, ios->vdd);
-			host->power = true;
-		}
+		host->power = TMIO_MMC_ON_RUN;
 		/* start bus clock */
 		tmio_mmc_clk_start(host);
 	} else if (ios->power_mode != MMC_POWER_UP) {
-		if (host->power) {
-			struct tmio_mmc_data *pdata = host->pdata;
-			if (ios->power_mode == MMC_POWER_OFF)
+		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);
-			host->power = false;
 			pm_runtime_put(dev);
 			if (pdata->clk_disable)
 				pdata->clk_disable(host->pdev);
 		}
 	}
 
-	if (host->power) {
+	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);
@@ -1029,7 +1036,7 @@ int tmio_mmc_host_probe(struct tmio_mmc_host **host,
 				  mmc->caps & MMC_CAP_NONREMOVABLE ||
 				  mmc->slot.cd_irq >= 0);
 
-	_host->power = false;
+	_host->power = TMIO_MMC_OFF_STOP;
 	pm_runtime_enable(&pdev->dev);
 	ret = pm_runtime_resume(&pdev->dev);
 	if (ret < 0)
-- 
1.7.2.5

[PATCH v3 2/2] mmc: tmio: reset the controller after power-up

[Guennadi Liakhovetski]

This fixes two reported problems:
1. after a system resume the controller isn't functioning until a command
   runs on a timeout and a controller reset is performed.
2. if a card is ejected during a running write operation, its re-insertion
   isn't detected.

Reported-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
Reported-by: Nguyen Hong Ky <nh-ky@jinso.co.jp>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
Tested-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
Tested-by: Nguyen Hong Ky <nh-ky@jinso.co.jp>
---

v3: rebased on top of current mmc-next / 3.10-rc

 drivers/mmc/host/tmio_mmc_pio.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 67d9642..f294708 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -867,6 +867,8 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 				host->resuming = false;
 			}
 		}
+		if (host->power = TMIO_MMC_OFF_STOP)
+			tmio_mmc_reset(host);
 		tmio_mmc_set_clock(host, ios->clock);
 		if (host->power = TMIO_MMC_OFF_STOP)
 			/* power up SD card and the bus */
@@ -1186,7 +1188,6 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
 	struct mmc_host *mmc = dev_get_drvdata(dev);
 	struct tmio_mmc_host *host = mmc_priv(mmc);
 
-	tmio_mmc_reset(host);
 	tmio_mmc_enable_dma(host, true);
 
 	return 0;
-- 
1.7.2.5

[PATCH v3 2/2] mmc: tmio: reset the controller after power-up

[Guennadi Liakhovetski]

This fixes two reported problems:
1. after a system resume the controller isn't functioning until a command
   runs on a timeout and a controller reset is performed.
2. if a card is ejected during a running write operation, its re-insertion
   isn't detected.

Reported-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
Reported-by: Nguyen Hong Ky <nh-ky@jinso.co.jp>
Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
Tested-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
Tested-by: Nguyen Hong Ky <nh-ky@jinso.co.jp>
---

v3: rebased on top of current mmc-next / 3.10-rc

 drivers/mmc/host/tmio_mmc_pio.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_pio.c b/drivers/mmc/host/tmio_mmc_pio.c
index 67d9642..f294708 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -867,6 +867,8 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 				host->resuming = false;
 			}
 		}
+		if (host->power == TMIO_MMC_OFF_STOP)
+			tmio_mmc_reset(host);
 		tmio_mmc_set_clock(host, ios->clock);
 		if (host->power == TMIO_MMC_OFF_STOP)
 			/* power up SD card and the bus */
@@ -1186,7 +1188,6 @@ int tmio_mmc_host_runtime_resume(struct device *dev)
 	struct mmc_host *mmc = dev_get_drvdata(dev);
 	struct tmio_mmc_host *host = mmc_priv(mmc);
 
-	tmio_mmc_reset(host);
 	tmio_mmc_enable_dma(host, true);
 
 	return 0;
-- 
1.7.2.5

Re: [PATCH v3 1/2] mmc: tmio: fix unbalanced power-on calls with clock-gating enabled

[Chris Ball]

Hi,

On Thu, Jun 06 2013, Guennadi Liakhovetski wrote:
> With MMC clock gating enabled the MMC core currently calls MMC host driver's
> .set_ios() method with .power_mode = MMC_POWER_ON and the clock value set
> either to 0 or to the target rate. The tmio MMC driver then wrongly
> translates the latter calls to card slot power-on requests, even when the
> slot already was on. This patch fixes the driver to avoid needlessly
> incrementing power-supplying regulator's use count.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>
> v3: rebased on top of current mmc-next / 3.10-rc

Thanks, pushed to mmc-next for 3.10.

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

Re: [PATCH v3 1/2] mmc: tmio: fix unbalanced power-on calls with clock-gating enabled

[Chris Ball]

Hi,

On Thu, Jun 06 2013, Guennadi Liakhovetski wrote:
> With MMC clock gating enabled the MMC core currently calls MMC host driver's
> .set_ios() method with .power_mode == MMC_POWER_ON and the clock value set
> either to 0 or to the target rate. The tmio MMC driver then wrongly
> translates the latter calls to card slot power-on requests, even when the
> slot already was on. This patch fixes the driver to avoid needlessly
> incrementing power-supplying regulator's use count.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> Tested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>
> v3: rebased on top of current mmc-next / 3.10-rc

Thanks, pushed to mmc-next for 3.10.

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

Re: [PATCH v3 2/2] mmc: tmio: reset the controller after power-up

[Chris Ball]

Hi,

On Thu, Jun 06 2013, Guennadi Liakhovetski wrote:
> This fixes two reported problems:
> 1. after a system resume the controller isn't functioning until a command
>    runs on a timeout and a controller reset is performed.
> 2. if a card is ejected during a running write operation, its re-insertion
>    isn't detected.
>
> Reported-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
> Reported-by: Nguyen Hong Ky <nh-ky@jinso.co.jp>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> Tested-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
> Tested-by: Nguyen Hong Ky <nh-ky@jinso.co.jp>
> ---
>
> v3: rebased on top of current mmc-next / 3.10-rc

Thanks, pushed to mmc-next for 3.10.

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

Re: [PATCH v3 2/2] mmc: tmio: reset the controller after power-up

[Chris Ball]

Hi,

On Thu, Jun 06 2013, Guennadi Liakhovetski wrote:
> This fixes two reported problems:
> 1. after a system resume the controller isn't functioning until a command
>    runs on a timeout and a controller reset is performed.
> 2. if a card is ejected during a running write operation, its re-insertion
>    isn't detected.
>
> Reported-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
> Reported-by: Nguyen Hong Ky <nh-ky@jinso.co.jp>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> Tested-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
> Tested-by: Nguyen Hong Ky <nh-ky@jinso.co.jp>
> ---
>
> v3: rebased on top of current mmc-next / 3.10-rc

Thanks, pushed to mmc-next for 3.10.

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