Re: [PATCH V1] mmc: sdhci-msm: Add system suspend/resume callbacks

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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)



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux