mmc: sdhci-msm: Update system suspend/resume callbacks of sdhci-msm platform driver.
Subsystems (3)
PATCH v2 1/1 mmc: sdhci-msm: Update system suspend/resume callbacks of sdhci-msm platform driver.
The existing suspend/resume callbacks of sdhci-msm driver are just gating/un-gating the clocks. During suspend cycle more can be done like disabling controller, interrupts and card detection.
So updating the system pm callbacks for performing these extra actions besides controlling the clocks.
Changes since V1:
Addressed review comments
--- drivers/mmc/host/sdhci-msm.c | 50 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-)
drivers/mmc/host/sdhci-msm.c +48 -2
@@ -2159,9 +2159,55 @@ 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; + +if (host->mmc->caps2 &MMC_CAP2_CQE ) { + ret =cqhci_suspend (host->mmc); +if (ret) +return ret; + } + + ret =sdhci_suspend_host (host); +if (ret) +return ret; +/* Disable pwr-irq since SDHC would be inactive */ +disable_irq (msm_host->pwr_irq); + +return pm_runtime_force_suspend (dev); +} + +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; + + ret =pm_runtime_force_resume (dev); +if (ret) +return ret; + +/* Re-enable pwr irq before SDHC gets reset */ +enable_irq (msm_host->pwr_irq); + + ret =sdhci_resume_host (host); +if (ret <0 ) +return ret; + +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 )
Signature · 2 lines
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Quoting Shaik Sajida Bhanu (2020-02-07 05:20:50)
Quoted 11 lines
The existing suspend/resume callbacks of sdhci-msm driver are just gating/un-gating the clocks. During suspend cycle more can be done like disabling controller, interrupts and card detection.
So updating the system pm callbacks for performing these extra actions besides controlling the clocks.
Signed-off-by Shaik Sajida Bhanu <[email protected]>Changes since V1:
Addressed review comments
Please don't write this. Instead, describe what's actually different so the reader doesn't have to go figure out what the review comments were.
Quoted 20 lines
--- drivers/mmc/host/sdhci-msm.c | 50 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-)drivers/mmc/host/sdhci-msm.c +8
-0@@ -2159,9 +2159,55 @@ 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; + +if (host->mmc->caps2 &MMC_CAP2_CQE ) {
drivers/mmc/host/sdhci-msm.c static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev) · line 2179
Quoted 10 lines
drivers/mmc/host/sdhci-msm.c +10
-0+ ret =cqhci_suspend (host->mmc); +if (ret) +return ret; + } + + ret =sdhci_suspend_host (host); +if (ret) +return ret; +/* Disable pwr-irq since SDHC would be inactive */ +disable_irq (msm_host->pwr_irq);
Why do we need to do this? If it's inactive then the irq won't be raised by the inactive hardware. Given that we're going to suspend the device, the irq won't matter unless it's marked for wakeup. Please remove this irq enable/disable logic, or explain why it's really needed.
Quoted 4 lines
drivers/mmc/host/sdhci-msm.c +4
-0+ +return pm_runtime_force_suspend (dev); +} +
Quoted 8 lines · Stephen Boyd
Quoting Shaik Sajida Bhanu (2020-02-07 05:20:50)
Quoted 7 lines
The existing suspend/resume callbacks of sdhci-msm driver are just gating/un-gating the clocks. During suspend cycle more can be done like disabling controller, interrupts and card detection.
So updating the system pm callbacks for performing these extra actions besides controlling the clocks.
Quoted 8 lines
Quoted 4 lines
Signed-off-by Shaik Sajida Bhanu <[email protected]>Changes since V1:
Addressed review commentsPlease don't write this. Instead, describe what's actually different so the reader doesn't have to go figure out what the review comments were.
sure
Quoted 30 lines
Quoted 30 lines
--- drivers/mmc/host/sdhci-msm.c | 50 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-)drivers/mmc/host/sdhci-msm.c +15
-0@@ -2159,9 +2159,55 @@ 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; + +if (host->mmc->caps2 &MMC_CAP2_CQE ) { + ret =cqhci_suspend (host->mmc); +if (ret) +return ret; + } + + ret =sdhci_suspend_host (host); +if (ret)
Quoted 9 lines
drivers/mmc/host/sdhci-msm.c static __maybe_unused int sdhci_msm_runtime_resume(struct device *dev) · line 2179
Quoted 3 lines
drivers/mmc/host/sdhci-msm.c +3
-0+return ret; +/* Disable pwr-irq since SDHC would be inactive */ +disable_irq (msm_host->pwr_irq);Why do we need to do this? If it's inactive then the irq won't be raised by the inactive hardware. Given that we're going to suspend the device, the irq won't matter unless it's marked for wakeup. Please remove this irq enable/disable logic, or explain why it's really needed.
You are right. This is not needed.
We have checked more on this and interrupt are getting disabled in suspend_device_irqs(). Will remove this.
Quoted 4 lines
Quoted 4 lines
drivers/mmc/host/sdhci-msm.c +4
-0+ +return pm_runtime_force_suspend (dev); +} +