| Message ID | Pine.LNX.4.64.1304221025300.23906@axis700.grange (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 --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);
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(-)