On 2020-01-22 22:27, Stephen Boyd wrote:
Quoting Shaik Sajida Bhanu (2020-01-21 06:30:22)
Add system suspend/resume callbacks to sdhci-msm platform driver.
Yes, but why? There are already suspend/resume callbacks so this is
replacing them too.
Sure. Will update the commit text.
The present suspend/resume callbacks are only disabling the clocks.
Since eMMC/SDcard would be powered off during system suspend, we can do
little more like
resetting controller, disabling interrupts. So updating the existing
callbacks.
Signed-off-by: Shaik Sajida Bhanu <sbhanu@xxxxxxxxxxxxxx>
---
drivers/mmc/host/sdhci-msm.c | 47
++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 45 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/sdhci-msm.c
b/drivers/mmc/host/sdhci-msm.c
index 71f29ba..4984857 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -2028,9 +2028,52 @@ static __maybe_unused int
sdhci_msm_runtime_resume(struct device *dev)
return 0;
}
+static int sdhci_msm_suspend(struct device *dev)
+{
+ struct sdhci_host *host = dev_get_drvdata(dev);
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_msm_host *msm_host =
sdhci_pltfm_priv(pltfm_host);
+ int ret = 0;
+
+ if (host->mmc->caps2 & MMC_CAP2_CQE) {
It would be nice if this if check was rolled into cqhci_suspend so that
all the callers wouldn't have to check it.
+ ret = cqhci_suspend(host->mmc);
+ if (ret)
+ return ret;
+ }
+
+ disable_irq(msm_host->pwr_irq);
Why is the irq disabled? Please add a comment.
Will add a comment.
During system suspend all SDHC interrupts can be disabled since device
would be
in power down state.
+ ret = sdhci_suspend_host(host);
+ if (ret)
+ return ret;
+
+ return sdhci_msm_runtime_suspend(dev);
pm_runtime_force_suspend() does different things than just call the
runtime suspend function for the driver. For example, it disables
runtime PM on the device. Can you explain in the commit text how this
is
a correct conversion?
Will invoke pm_runtime_force_suspend() here instead of
sdhci_msm_runtime_suspend()
+}
+
+static int sdhci_msm_resume(struct device *dev)
+{
+ struct sdhci_host *host = dev_get_drvdata(dev);
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_msm_host *msm_host =
sdhci_pltfm_priv(pltfm_host);
+ int ret = 0;
+
+ ret = sdhci_msm_runtime_resume(dev);
+ if (ret)
+ return ret;
+
+ ret = sdhci_resume_host(host);
+ if (ret < 0)
+ return ret;
+ enable_irq(msm_host->pwr_irq);
Same question here about irq. Deserves a comment.
+
+ if (host->mmc->caps2 & MMC_CAP2_CQE)
+ ret = cqhci_resume(host->mmc);
+
+ return ret;
+}
+
static const struct dev_pm_ops sdhci_msm_pm_ops = {
- SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
- pm_runtime_force_resume)
+ SET_SYSTEM_SLEEP_PM_OPS(sdhci_msm_suspend,
+ sdhci_msm_resume)
SET_RUNTIME_PM_OPS(sdhci_msm_runtime_suspend,
sdhci_msm_runtime_resume,
NULL)
