[3/3] mmc: tmio: Fix reset operation

Commit Message

Niklas Söderlund June 6, 2018, 11:22 p.m. UTC

From: Masaharu Hayakawa <[e-mail]>

SD / MMC did not operate properly when suspend transition failed.
Because the SCC was not reset at resume, issue of the command failed.
Changed tmio_mmc_reset() to tmio_mmc_hw_reset() in order to add reset
of SCC to tmio_mmc_host_runtime_resume().

On runtime power management resume, the host clock needs to be
enabled before calling tmio_mmc_reset. If the mmc device has a power
domain entry, the host clock is enabled via genpd_runtime_resume,
running before tmio_mmc_host_runtime_resume. If the mmc device has
no power domain entry, however, genpd_runtime_resume is not called.
This patch changes tmio_mmc_host_runtime_resume to enable the host
clock before calling tmio_mmc_reset.

Signed-off-by: Masaharu Hayakawa <[e-mail]>
[Niklas: small fixes when rebase to mmc/next]
Signed-off-by: Niklas Söderlund <[e-mail]>
---
 drivers/mmc/host/tmio_mmc_core.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comments

Wolfram Sang June 9, 2018, 6:38 p.m. UTC | #1

On Thu, Jun 07, 2018 at 01:22:52AM +0200, Niklas Söderlund wrote:
> From: Masaharu Hayakawa <[e-mail]>
> 
> SD / MMC did not operate properly when suspend transition failed.
> Because the SCC was not reset at resume, issue of the command failed.
> Changed tmio_mmc_reset() to tmio_mmc_hw_reset() in order to add reset
> of SCC to tmio_mmc_host_runtime_resume().

So, my gut feeling is that this change, in general, seems OK. However,
the naming (tmio_mmc_reset and tmio_mmc_hw_reset) becomes confusing and
unneeded. tmio_mmc_reset now only gets called in tmio_mmc_hw_reset, so
we can merge the two functions into one to reduce the confusion.

Because we add stuff to the function which gets pointed to by
mmc_ops->hw_reset, it makes sense to me to double check when the MMC
core calls this pointer and if our additions still make sense there.
I'd think so, but better safe than sorry.

> On runtime power management resume, the host clock needs to be
> enabled before calling tmio_mmc_reset. If the mmc device has a power
> domain entry, the host clock is enabled via genpd_runtime_resume,
> running before tmio_mmc_host_runtime_resume. If the mmc device has
> no power domain entry, however, genpd_runtime_resume is not called.
> This patch changes tmio_mmc_host_runtime_resume to enable the host
> clock before calling tmio_mmc_reset.

I think this makes sense but should be a seperate patch. Or maybe even
something slightly different? Because we have a patch in the BSP [1] for
which I ever wondered in what case it was needed. But seeing the above,
it might make sense to add the clock enablement to the reset routine?
Niklas, do you have time to check that?

Thanks,

   Wolfram

[1] 940904fe4ea8 ("mmc: tmio: Add SDCLK enable after reset")

Patch

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 9f5793407552a23a..da10b26d8f2d80e7 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -247,6 +247,10 @@  static void tmio_mmc_hw_reset(struct mmc_host *mmc)
 {
 	struct tmio_mmc_host *host = mmc_priv(mmc);
 
+	tmio_mmc_reset(host);
+
+	tmio_mmc_abort_dma(host);
+
 	if (host->hw_reset)
 		host->hw_reset(host);
 }
@@ -289,7 +293,7 @@  static void tmio_mmc_reset_work(struct work_struct *work)
 
 	spin_unlock_irqrestore(&host->lock, flags);
 
-	tmio_mmc_reset(host);
+	tmio_mmc_hw_reset(host->mmc);
 
 	/* Ready for new calls */
 	host->mrq = NULL;
@@ -1271,7 +1275,7 @@  int tmio_mmc_host_probe(struct tmio_mmc_host *_host)
 		_host->sdio_irq_mask = TMIO_SDIO_MASK_ALL;
 
 	tmio_mmc_clk_stop(_host);
-	tmio_mmc_reset(_host);
+	tmio_mmc_hw_reset(mmc);
 
 	sd_ctrl_write32_as_16_and_16(_host, CTL_IRQ_MASK, TMIO_MASK_INIT);
 	_host->sdcard_irq_mask = sd_ctrl_read16_and_16_as_32(_host, CTL_IRQ_MASK);
@@ -1372,8 +1376,8 @@  int tmio_mmc_host_runtime_resume(struct device *dev)
 {
 	struct tmio_mmc_host *host = dev_get_drvdata(dev);
 
-	tmio_mmc_reset(host);
 	tmio_mmc_clk_enable(host);
+	tmio_mmc_hw_reset(host->mmc);
 
 	if (host->clk_cache)
 		tmio_mmc_set_clock(host, host->clk_cache);