On 3/5/2020 2:29 PM, Adrian Hunter wrote:
On 4/03/20 6:50 pm, Veerabhadrarao Badiganti wrote:
On 3/4/2020 7:40 PM, Adrian Hunter wrote:
On 4/03/20 3:10 pm, Veerabhadrarao Badiganti wrote:
Hi Adrian
On 3/4/2020 5:58 PM, Adrian Hunter wrote:
On 4/03/20 1:54 pm, Veerabhadrarao Badiganti wrote:
When SDHC gets reset (E.g. in suspend path), CQE also gets reset
and goes to disable state. But s/w state still points it as CQE
is in enabled state. Since s/w and h/w states goes out of sync,
it results in s/w request timeout for subsequent CQE requests.
To synchronize CQE s/w and h/w state during SDHC reset,
explicitly disable CQE after reset.
Shouldn't you be calling cqhci_suspend() / cqhci_resume() in the suspend
and
resume paths?
This issue is seen during mmc runtime suspend. I can add it
sdhci_msm_runtime_suspend
but sdhci_msm runtime delay is aggressive, its 50ms. It may get invoked very
frequently.
So Im of the opinion that disabling CQE very often from platform runtime
suspend is overkill.
It doesn't look like sdhci-msm calls any sdhci.c pm ops, so how does SDHC
get reset?
With MMC_CAP_AGGRESSIVE_PM flag enabled, it getting called from
mmc_runtime_suspend()
Below is the call stack()
sdhci_reset
sdhci_do_reset
sdhci_init
sdhci_set_ios
mmc_set_initial_state
mmc_power_off
_mmc_suspend
mmc_runtime_suspend
OK, cqhci_suspend does the right thing, but it is not an
appropriate function for this. I suggest introducing
cqhci_deactivate() as below.
From: Adrian Hunter <adrian.hunter@xxxxxxxxx>
Date: Thu, 5 Mar 2020 10:42:09 +0200
Subject: [PATCH] mmc: cqhci: Add cqhci_deactivate()
Host controllers can reset CQHCI either directly or as a consequence of
host controller reset. Add cqhci_deactivate() which puts the CQHCI
driver into a state that is consistent with that.
Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx>
---
drivers/mmc/host/cqhci.c | 6 +++---
drivers/mmc/host/cqhci.h | 5 ++++-
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
index e2ea2c4b6b94..d8d024a1682b 100644
--- a/drivers/mmc/host/cqhci.c
+++ b/drivers/mmc/host/cqhci.c
@@ -298,16 +298,16 @@ static void __cqhci_disable(struct cqhci_host *cq_host)
cq_host->activated = false;
}
-int cqhci_suspend(struct mmc_host *mmc)
+int cqhci_deactivate(struct mmc_host *mmc)
{
struct cqhci_host *cq_host = mmc->cqe_private;
- if (cq_host->enabled)
+ if (cq_host->enabled && cq_host->activated)
__cqhci_disable(cq_host);
return 0;
}
-EXPORT_SYMBOL(cqhci_suspend);
+EXPORT_SYMBOL(cqhci_deactivate);
int cqhci_resume(struct mmc_host *mmc)
{
diff --git a/drivers/mmc/host/cqhci.h b/drivers/mmc/host/cqhci.h
index def76e9b5cac..8648846a0213 100644
--- a/drivers/mmc/host/cqhci.h
+++ b/drivers/mmc/host/cqhci.h
@@ -230,7 +230,10 @@ irqreturn_t cqhci_irq(struct mmc_host *mmc, u32 intmask, int cmd_error,
int data_error);
int cqhci_init(struct cqhci_host *cq_host, struct mmc_host *mmc, bool dma64);
struct cqhci_host *cqhci_pltfm_init(struct platform_device *pdev);
-int cqhci_suspend(struct mmc_host *mmc);
+static inline int cqhci_suspend(struct mmc_host *mmc)
+{
+ return cqhci_deactivate(mmc);
+}
int cqhci_resume(struct mmc_host *mmc);
#endif
Thanks Adrian for the suggestion. Will post this change and my updated fix.
Thanks
Veera
