Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1474437 > unrolled thread

[PATCH v3] mmc: sdhci-msm: Add pm_runtime and system PM support

Back to article view | Back to linux.kernel

Contents

  [PATCH v3] mmc: sdhci-msm: Add pm_runtime and system PM support Pramod Gurav <pramod.gurav@linaro.org> - 2016-09-01 16:30 +0200
    Re: [PATCH v3] mmc: sdhci-msm: Add pm_runtime and system PM support Adrian Hunter <adrian.hunter@intel.com> - 2016-09-08 10:10 +0200
      Re: [PATCH v3] mmc: sdhci-msm: Add pm_runtime and system PM support Georgi Djakov <georgi.djakov@linaro.org> - 2016-09-09 12:20 +0200
    Re: [PATCH v3] mmc: sdhci-msm: Add pm_runtime and system PM support "Tummala, Sahitya" <stummala@codeaurora.org> - 2016-09-09 12:10 +0200

#1474437 — [PATCH v3] mmc: sdhci-msm: Add pm_runtime and system PM support

Provides runtime PM callbacks to enable and disable clock resources
when idle. Also support system PM callbacks to be called during system
suspend and resume.

Signed-off-by: Pramod Gurav <pramod.gurav@linaro.org>
---
Changes in v3:
- Added CONFIG_PM around runtime pm function.
- Replaced msm suspend/resume with generic function directly
- Use SET_SYSTEM_SLEEP_PM_OPS instead of late version

Changes in v2:
- Moved pm_rutime enabling before adding host
- Handled pm_rutime in remove
- Changed runtime handling with reference from sdhci-of-at91.c

 drivers/mmc/host/sdhci-msm.c | 71 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 70 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 8ef44a2a..0ef4f29 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -18,6 +18,7 @@
 #include <linux/of_device.h>
 #include <linux/delay.h>
 #include <linux/mmc/mmc.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 
 #include "sdhci-pltfm.h"
@@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
 		goto clk_disable;
 	}
 
+	pm_runtime_get_noresume(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
+	pm_runtime_use_autosuspend(&pdev->dev);
+
 	ret = sdhci_add_host(host);
 	if (ret)
-		goto clk_disable;
+		goto pm_runtime_disable;
+
+	platform_set_drvdata(pdev, host);
+
+	pm_runtime_put_autosuspend(&pdev->dev);
 
 	return 0;
 
+pm_runtime_disable:
+	pm_runtime_disable(&pdev->dev);
+	pm_runtime_set_suspended(&pdev->dev);
+	pm_runtime_put_noidle(&pdev->dev);
 clk_disable:
 	clk_disable_unprepare(msm_host->clk);
 pclk_disable:
@@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
 		    0xffffffff);
 
 	sdhci_remove_host(host, dead);
+
+	pm_runtime_get_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+	pm_runtime_put_noidle(&pdev->dev);
+
 	clk_disable_unprepare(msm_host->clk);
 	clk_disable_unprepare(msm_host->pclk);
 	if (!IS_ERR(msm_host->bus_clk))
@@ -693,12 +713,61 @@ static int sdhci_msm_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int sdhci_msm_runtime_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;
+
+	ret = sdhci_runtime_suspend_host(host);
+	if (ret)
+		return ret;
+
+	clk_disable_unprepare(msm_host->clk);
+	clk_disable_unprepare(msm_host->pclk);
+
+	return 0;
+}
+
+static int sdhci_msm_runtime_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 = clk_prepare_enable(msm_host->clk);
+	if (ret) {
+		dev_err(dev, "clk_enable failed: %d\n", ret);
+		return ret;
+	}
+	ret = clk_prepare_enable(msm_host->pclk);
+	if (ret) {
+		dev_err(dev, "clk_enable failed: %d\n", ret);
+		clk_disable_unprepare(msm_host->clk);
+		return ret;
+	}
+
+	return sdhci_runtime_resume_host(host);
+}
+#endif
+
+static const struct dev_pm_ops sdhci_msm_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+					pm_runtime_force_resume)
+	SET_RUNTIME_PM_OPS(sdhci_msm_runtime_suspend, sdhci_msm_runtime_resume,
+				NULL)
+};
+
 static struct platform_driver sdhci_msm_driver = {
 	.probe = sdhci_msm_probe,
 	.remove = sdhci_msm_remove,
 	.driver = {
 		   .name = "sdhci_msm",
 		   .of_match_table = sdhci_msm_dt_match,
+		   .pm = &sdhci_msm_pm_ops,
 	},
 };
 
-- 
2.9.3

[toc] | [next] | [standalone]

#1478890

On 01/09/16 17:23, Pramod Gurav wrote:
> Provides runtime PM callbacks to enable and disable clock resources
> when idle. Also support system PM callbacks to be called during system
> suspend and resume.
> 
> Signed-off-by: Pramod Gurav <pramod.gurav@linaro.org>

Can we get some Tested/Reviewed/Acked-by from people using this driver?

> ---
> Changes in v3:
> - Added CONFIG_PM around runtime pm function.
> - Replaced msm suspend/resume with generic function directly
> - Use SET_SYSTEM_SLEEP_PM_OPS instead of late version
> 
> Changes in v2:
> - Moved pm_rutime enabling before adding host
> - Handled pm_rutime in remove
> - Changed runtime handling with reference from sdhci-of-at91.c
> 
>  drivers/mmc/host/sdhci-msm.c | 71 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 8ef44a2a..0ef4f29 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -18,6 +18,7 @@
>  #include <linux/of_device.h>
>  #include <linux/delay.h>
>  #include <linux/mmc/mmc.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/slab.h>
>  
>  #include "sdhci-pltfm.h"
> @@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>  		goto clk_disable;
>  	}
>  
> +	pm_runtime_get_noresume(&pdev->dev);
> +	pm_runtime_set_active(&pdev->dev);
> +	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
> +	pm_runtime_use_autosuspend(&pdev->dev);
> +
>  	ret = sdhci_add_host(host);
>  	if (ret)
> -		goto clk_disable;
> +		goto pm_runtime_disable;
> +
> +	platform_set_drvdata(pdev, host);
> +
> +	pm_runtime_put_autosuspend(&pdev->dev);
>  
>  	return 0;
>  
> +pm_runtime_disable:
> +	pm_runtime_disable(&pdev->dev);
> +	pm_runtime_set_suspended(&pdev->dev);
> +	pm_runtime_put_noidle(&pdev->dev);
>  clk_disable:
>  	clk_disable_unprepare(msm_host->clk);
>  pclk_disable:
> @@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
>  		    0xffffffff);
>  
>  	sdhci_remove_host(host, dead);
> +
> +	pm_runtime_get_sync(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +	pm_runtime_put_noidle(&pdev->dev);
> +
>  	clk_disable_unprepare(msm_host->clk);
>  	clk_disable_unprepare(msm_host->pclk);
>  	if (!IS_ERR(msm_host->bus_clk))
> @@ -693,12 +713,61 @@ static int sdhci_msm_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_PM
> +static int sdhci_msm_runtime_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;
> +
> +	ret = sdhci_runtime_suspend_host(host);
> +	if (ret)
> +		return ret;
> +
> +	clk_disable_unprepare(msm_host->clk);
> +	clk_disable_unprepare(msm_host->pclk);
> +
> +	return 0;
> +}
> +
> +static int sdhci_msm_runtime_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 = clk_prepare_enable(msm_host->clk);
> +	if (ret) {
> +		dev_err(dev, "clk_enable failed: %d\n", ret);
> +		return ret;
> +	}
> +	ret = clk_prepare_enable(msm_host->pclk);
> +	if (ret) {
> +		dev_err(dev, "clk_enable failed: %d\n", ret);
> +		clk_disable_unprepare(msm_host->clk);
> +		return ret;
> +	}
> +
> +	return sdhci_runtime_resume_host(host);
> +}
> +#endif
> +
> +static const struct dev_pm_ops sdhci_msm_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +					pm_runtime_force_resume)
> +	SET_RUNTIME_PM_OPS(sdhci_msm_runtime_suspend, sdhci_msm_runtime_resume,
> +				NULL)
> +};
> +
>  static struct platform_driver sdhci_msm_driver = {
>  	.probe = sdhci_msm_probe,
>  	.remove = sdhci_msm_remove,
>  	.driver = {
>  		   .name = "sdhci_msm",
>  		   .of_match_table = sdhci_msm_dt_match,
> +		   .pm = &sdhci_msm_pm_ops,
>  	},
>  };
>  
> 

[toc] | [prev] | [next] | [standalone]

#1479830

On 09/08/2016 11:02 AM, Adrian Hunter wrote:
> On 01/09/16 17:23, Pramod Gurav wrote:
>> Provides runtime PM callbacks to enable and disable clock resources
>> when idle. Also support system PM callbacks to be called during system
>> suspend and resume.
>>
>> Signed-off-by: Pramod Gurav <pramod.gurav@linaro.org>
>
> Can we get some Tested/Reviewed/Acked-by from people using this driver?
>

Hi Pramod,
Thanks for the patch. Unfortunately, my db410c board fails to
boot when i apply it.

[    1.778433] mmc0: new HS200 MMC card at address 0001
[    1.783115] mmcblk0: mmc0:0001 DS2008 7.28 GiB
[    1.783337] mmcblk0boot0: mmc0:0001 DS2008 partition 1 4.00 MiB
[    1.787025] mmcblk0boot1: mmc0:0001 DS2008 partition 2 4.00 MiB
[    1.792893] mmcblk0rpmb: mmc0:0001 DS2008 partition 3 4.00 MiB
[    1.802603]  mmcblk0: p1 p2 p3 p4 p5 p6 p7 p8 p9 p10
[    2.693631] blk_update_request: I/O error, dev mmcblk0, sector 462880
[    2.710381] blk_update_request: I/O error, dev mmcblk0, sector 462880
[    2.710443] Buffer I/O error on dev mmcblk0p10, logical block 0, 
async page read
[    2.724827] blk_update_request: I/O error, dev mmcblk0, sector 462881
[    2.724853] Buffer I/O error on dev mmcblk0p10, logical block 1, 
async page read
...

More I/O errors are following and it is unable to mount the rootfs from
the eMMC. When i retried booting, got also the following:

[    2.877149] mmcblk0: error -110 sending status command, retrying
[    2.879408] mmcblk0: error -110 sending status command, retrying
[    2.884436] mmcblk0: error -110 sending status command, aborting
[    2.896826] mmc0: cache flush error -110

BR,
Georgi

[toc] | [prev] | [next] | [standalone]

#1479824

Hi Pramod,

On 9/1/2016 7:53 PM, Pramod Gurav wrote:
> Provides runtime PM callbacks to enable and disable clock resources
> when idle. Also support system PM callbacks to be called during system
> suspend and resume.
>
> Signed-off-by: Pramod Gurav <pramod.gurav@linaro.org>
> ---
> Changes in v3:
> - Added CONFIG_PM around runtime pm function.
> - Replaced msm suspend/resume with generic function directly
> - Use SET_SYSTEM_SLEEP_PM_OPS instead of late version
>
> Changes in v2:
> - Moved pm_rutime enabling before adding host
> - Handled pm_rutime in remove
> - Changed runtime handling with reference from sdhci-of-at91.c
>
>   drivers/mmc/host/sdhci-msm.c | 71 +++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
> index 8ef44a2a..0ef4f29 100644
> --- a/drivers/mmc/host/sdhci-msm.c
> +++ b/drivers/mmc/host/sdhci-msm.c
> @@ -18,6 +18,7 @@
>   #include <linux/of_device.h>
>   #include <linux/delay.h>
>   #include <linux/mmc/mmc.h>
> +#include <linux/pm_runtime.h>
>   #include <linux/slab.h>
>   
>   #include "sdhci-pltfm.h"
> @@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
>   		goto clk_disable;
>   	}
>   
> +	pm_runtime_get_noresume(&pdev->dev);
> +	pm_runtime_set_active(&pdev->dev);
> +	pm_runtime_enable(&pdev->dev);
> +	pm_runtime_set_autosuspend_delay(&pdev->dev, 50);
> +	pm_runtime_use_autosuspend(&pdev->dev);
> +
>   	ret = sdhci_add_host(host);
>   	if (ret)
> -		goto clk_disable;
> +		goto pm_runtime_disable;
> +
> +	platform_set_drvdata(pdev, host);
> +
> +	pm_runtime_put_autosuspend(&pdev->dev);
>   
>   	return 0;
>   
> +pm_runtime_disable:
> +	pm_runtime_disable(&pdev->dev);
> +	pm_runtime_set_suspended(&pdev->dev);
> +	pm_runtime_put_noidle(&pdev->dev);
>   clk_disable:
>   	clk_disable_unprepare(msm_host->clk);
>   pclk_disable:
> @@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
>   		    0xffffffff);
>   
>   	sdhci_remove_host(host, dead);
> +
> +	pm_runtime_get_sync(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +	pm_runtime_put_noidle(&pdev->dev);
> +
>   	clk_disable_unprepare(msm_host->clk);
>   	clk_disable_unprepare(msm_host->pclk);
>   	if (!IS_ERR(msm_host->bus_clk))
> @@ -693,12 +713,61 @@ static int sdhci_msm_remove(struct platform_device *pdev)
>   	return 0;
>   }
>   
> +#ifdef CONFIG_PM
> +static int sdhci_msm_runtime_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;
> +
> +	ret = sdhci_runtime_suspend_host(host);
> +	if (ret)
> +		return ret;
> +
> +	clk_disable_unprepare(msm_host->clk);
> +	clk_disable_unprepare(msm_host->pclk);
> +
> +	return 0;
> +}
> +
> +static int sdhci_msm_runtime_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 = clk_prepare_enable(msm_host->clk);
> +	if (ret) {
> +		dev_err(dev, "clk_enable failed: %d\n", ret);
A minor comment - Both error prints related to clock enable are same. 
Better to print the clock name as well to know which clock enable got 
failed.
> +		return ret;
> +	}
> +	ret = clk_prepare_enable(msm_host->pclk);
> +	if (ret) {
> +		dev_err(dev, "clk_enable failed: %d\n", ret);
> +		clk_disable_unprepare(msm_host->clk);
> +		return ret;
> +	}
> +
> +	return sdhci_runtime_resume_host(host);
> +}
> +#endif
> +
> +static const struct dev_pm_ops sdhci_msm_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
> +					pm_runtime_force_resume)
> +	SET_RUNTIME_PM_OPS(sdhci_msm_runtime_suspend, sdhci_msm_runtime_resume,
> +				NULL)
> +};
> +
>   static struct platform_driver sdhci_msm_driver = {
>   	.probe = sdhci_msm_probe,
>   	.remove = sdhci_msm_remove,
>   	.driver = {
>   		   .name = "sdhci_msm",
>   		   .of_match_table = sdhci_msm_dt_match,
> +		   .pm = &sdhci_msm_pm_ops,
>   	},
>   };
>   

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web