diff mbox

mmc: tmio: postpone controller reset during resume

Message ID Pine.LNX.4.64.1304221025300.23906@axis700.grange (mailing list archive)
State New, archived
Headers show

Commit Message

Guennadi Liakhovetski April 22, 2013, 8:29 a.m. UTC
When resuming, the tmio_mmc_host_resume() function is run when the
controller might still be powered down. Issuing a reset command to it at
that time has no effect. This patch postpones resetting the controller
until the first powering-up .set_ios() call.

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

There is some risk associated with this patch: it removes unconditional 
(attempt of) controller reset from the tmio_mmc_host_resume() function. 
AFAICS, this shouldn't have any negative effects. There _should_ always be 
a powering-up .set_ios() call after a resume. However, if anyone sees any 
(potential) problems with it, please, let me know.

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

Comments

Guennadi Liakhovetski June 6, 2013, 7:30 a.m. UTC | #1
Hi Chris

On Mon, 22 Apr 2013, Guennadi Liakhovetski wrote:

> When resuming, the tmio_mmc_host_resume() function is run when the
> controller might still be powered down. Issuing a reset command to it at
> that time has no effect. This patch postpones resetting the controller
> until the first powering-up .set_ios() call.
> 
> Reported-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>

Any reason why this hasn't been applied yet? Please, push for 3.10.

Thanks
Guennadi

> ---
> 
> There is some risk associated with this patch: it removes unconditional 
> (attempt of) controller reset from the tmio_mmc_host_resume() function. 
> AFAICS, this shouldn't have any negative effects. There _should_ always be 
> a powering-up .set_ios() call after a resume. However, if anyone sees any 
> (potential) problems with it, please, let me know.
> 
>  drivers/mmc/host/tmio_mmc.h     |    1 +
>  drivers/mmc/host/tmio_mmc_pio.c |    6 +++++-
>  2 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index d857f5c..759d8f4 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -85,6 +85,7 @@ struct tmio_mmc_host {
>  	unsigned long		last_req_ts;
>  	struct mutex		ios_lock;	/* protect set_ios() context */
>  	bool			native_hotplug;
> +	bool			resuming;
>  };
>  
>  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 f508ecb..435cc4d 100644
> --- a/drivers/mmc/host/tmio_mmc_pio.c
> +++ b/drivers/mmc/host/tmio_mmc_pio.c
> @@ -862,6 +862,10 @@ static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>  		if (!host->power) {
>  			tmio_mmc_clk_update(mmc);
>  			pm_runtime_get_sync(dev);
> +			if (host->resuming) {
> +				tmio_mmc_reset(host);
> +				host->resuming = false;
> +			}
>  		}
>  		tmio_mmc_set_clock(host, ios->clock);
>  		if (!host->power) {
> @@ -1154,10 +1158,10 @@ int tmio_mmc_host_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);
>  
>  	/* The MMC core will perform the complete set up */
> +	host->resuming = true;
>  	return mmc_resume_host(mmc);
>  }
>  EXPORT_SYMBOL(tmio_mmc_host_resume);
> -- 
> 1.7.2.5
> 
> 

---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
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
Chris Ball June 6, 2013, 11:41 a.m. UTC | #2
Hi,

On Thu, Jun 06 2013, Guennadi Liakhovetski wrote:
> Hi Chris
>
> On Mon, 22 Apr 2013, Guennadi Liakhovetski wrote:
>
>> When resuming, the tmio_mmc_host_resume() function is run when the
>> controller might still be powered down. Issuing a reset command to it at
>> that time has no effect. This patch postpones resetting the controller
>> until the first powering-up .set_ios() call.
>> 
>> Reported-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
>> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
>
> Any reason why this hasn't been applied yet? Please, push for 3.10.

Pushed to mmc-next for 3.10, thanks.

- Chris.
diff mbox

Patch

diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index d857f5c..759d8f4 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -85,6 +85,7 @@  struct tmio_mmc_host {
 	unsigned long		last_req_ts;
 	struct mutex		ios_lock;	/* protect set_ios() context */
 	bool			native_hotplug;
+	bool			resuming;
 };
 
 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 f508ecb..435cc4d 100644
--- a/drivers/mmc/host/tmio_mmc_pio.c
+++ b/drivers/mmc/host/tmio_mmc_pio.c
@@ -862,6 +862,10 @@  static void tmio_mmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		if (!host->power) {
 			tmio_mmc_clk_update(mmc);
 			pm_runtime_get_sync(dev);
+			if (host->resuming) {
+				tmio_mmc_reset(host);
+				host->resuming = false;
+			}
 		}
 		tmio_mmc_set_clock(host, ios->clock);
 		if (!host->power) {
@@ -1154,10 +1158,10 @@  int tmio_mmc_host_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);
 
 	/* The MMC core will perform the complete set up */
+	host->resuming = true;
 	return mmc_resume_host(mmc);
 }
 EXPORT_SYMBOL(tmio_mmc_host_resume);