[PATCH] spi: qup: Fix runtime and system PM callbacks.

[Pramod Gurav]

The SPI clocks were being turned on every suspend/resume cycle.
This was increamenting the prepare/enable count after every resume.
Fix the same.

Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
---
Tested on db410c

 drivers/spi/spi-qup.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index 1bfa889..a9731e8 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -956,8 +956,10 @@ static int spi_qup_pm_resume_runtime(struct device *device)
return ret;
 
ret = clk_prepare_enable(controller->cclk);
-   if (ret)
+   if (ret) {
+   clk_disable_unprepare(controller->iclk);
return ret;
+   }
 
/* Disable clocks auto gaiting */
config = readl_relaxed(controller->base + QUP_CONFIG);
@@ -983,8 +985,7 @@ static int spi_qup_suspend(struct device *device)
return ret;
 
if (!pm_runtime_suspended(device)) {
-   clk_disable_unprepare(controller->cclk);
-   clk_disable_unprepare(controller->iclk);
+   pm_runtime_put(device);
}
return 0;
 }
@@ -995,18 +996,17 @@ static int spi_qup_resume(struct device *device)
struct spi_qup *controller = spi_master_get_devdata(master);
int ret;
 
-   ret = clk_prepare_enable(controller->iclk);
-   if (ret)
-   return ret;
-
-   ret = clk_prepare_enable(controller->cclk);
-   if (ret)
+   ret = pm_runtime_get_sync(device);
+   if (ret < 0) {
+   dev_err(device, "pm runtime failed in resume\n");
return ret;
+   }
 
ret = spi_qup_set_state(controller, QUP_STATE_RESET);
if (ret)
return ret;
 
+   pm_runtime_put(device);
return spi_master_resume(master);
 }
 #endif /* CONFIG_PM_SLEEP */
-- 
2.10.2

[PATCH] spi: qup: Fix runtime and system PM callbacks.

[Pramod Gurav]

The SPI clocks were being turned on every suspend/resume cycle.
This was increamenting the prepare/enable count after every resume.
Fix the same.

Signed-off-by: Pramod Gurav 
---
Tested on db410c

 drivers/spi/spi-qup.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index 1bfa889..a9731e8 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -956,8 +956,10 @@ static int spi_qup_pm_resume_runtime(struct device *device)
return ret;
 
ret = clk_prepare_enable(controller->cclk);
-   if (ret)
+   if (ret) {
+   clk_disable_unprepare(controller->iclk);
return ret;
+   }
 
/* Disable clocks auto gaiting */
config = readl_relaxed(controller->base + QUP_CONFIG);
@@ -983,8 +985,7 @@ static int spi_qup_suspend(struct device *device)
return ret;
 
if (!pm_runtime_suspended(device)) {
-   clk_disable_unprepare(controller->cclk);
-   clk_disable_unprepare(controller->iclk);
+   pm_runtime_put(device);
}
return 0;
 }
@@ -995,18 +996,17 @@ static int spi_qup_resume(struct device *device)
struct spi_qup *controller = spi_master_get_devdata(master);
int ret;
 
-   ret = clk_prepare_enable(controller->iclk);
-   if (ret)
-   return ret;
-
-   ret = clk_prepare_enable(controller->cclk);
-   if (ret)
+   ret = pm_runtime_get_sync(device);
+   if (ret < 0) {
+   dev_err(device, "pm runtime failed in resume\n");
return ret;
+   }
 
ret = spi_qup_set_state(controller, QUP_STATE_RESET);
if (ret)
return ret;
 
+   pm_runtime_put(device);
return spi_master_resume(master);
 }
 #endif /* CONFIG_PM_SLEEP */
-- 
2.10.2

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

[Pramod Gurav]

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.

Reviewed-by: Ritesh Harjani <rite...@codeaurora.org>
Reviewed-by: Georgi Djakov <georgi.dja...@linaro.org>
Tested-by: Ritesh Harjani <rite...@codeaurora.org>
Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
---
Tested on DB410C.

Changes in v5:
- Added pm_runtime_mark_last_busy in probe before calling autosuspend
- included clock names in error logs
- Used micro instead of constant for autosuspend delay
- Removed platform_set_drvdata in probe as sdhci_pltfm_init does it.
- Aligned PM ops structure with the parenthesis

Changes in v4:
- Remove calls to sdhci_runtime_resume_host/sdhci_runtime_suspend_host
  from runtime callbacks as sdhc msm controller is capable of restoring
  it's register values after clocks are disabled and re-enabled.

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 | 68 +++-
 1 file changed, 67 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 8ef44a2a..795f16f 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "sdhci-pltfm.h"
@@ -68,6 +69,7 @@
 #define CMUX_SHIFT_PHASE_SHIFT 24
 #define CMUX_SHIFT_PHASE_MASK  (7 << CMUX_SHIFT_PHASE_SHIFT)
 
+#define MSM_MMC_AUTOSUSPEND_DELAY_MS   50
 struct sdhci_msm_host {
struct platform_device *pdev;
void __iomem *core_mem; /* MSM SDCC mapped address */
@@ -658,12 +660,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
goto clk_disable;
}
 
+   pm_runtime_get_noresume(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+   pm_runtime_set_autosuspend_delay(>dev,
+MSM_MMC_AUTOSUSPEND_DELAY_MS);
+   pm_runtime_use_autosuspend(>dev);
+
ret = sdhci_add_host(host);
if (ret)
-   goto clk_disable;
+   goto pm_runtime_disable;
+
+   pm_runtime_mark_last_busy(>dev);
+   pm_runtime_put_autosuspend(>dev);
 
return 0;
 
+pm_runtime_disable:
+   pm_runtime_disable(>dev);
+   pm_runtime_set_suspended(>dev);
+   pm_runtime_put_noidle(>dev);
 clk_disable:
clk_disable_unprepare(msm_host->clk);
 pclk_disable:
@@ -685,6 +701,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
0x);
 
sdhci_remove_host(host, dead);
+
+   pm_runtime_get_sync(>dev);
+   pm_runtime_disable(>dev);
+   pm_runtime_put_noidle(>dev);
+
clk_disable_unprepare(msm_host->clk);
clk_disable_unprepare(msm_host->pclk);
if (!IS_ERR(msm_host->bus_clk))
@@ -693,12 +714,57 @@ 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);
+
+   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 for core_clk: %d\n", ret);
+   return ret;
+   }
+   ret = clk_prepare_enable(msm_host->pclk);
+   if (ret) {
+   dev_err(dev, "clk_enable failed for iface_clk: %d\n", ret);
+   clk_disable_unprepare(msm_host->clk);
+   return ret;
+   }
+
+   return 0;
+}
+#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 = {
   

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

[Pramod Gurav]

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.

Reviewed-by: Ritesh Harjani 
Reviewed-by: Georgi Djakov 
Tested-by: Ritesh Harjani 
Signed-off-by: Pramod Gurav 
---
Tested on DB410C.

Changes in v5:
- Added pm_runtime_mark_last_busy in probe before calling autosuspend
- included clock names in error logs
- Used micro instead of constant for autosuspend delay
- Removed platform_set_drvdata in probe as sdhci_pltfm_init does it.
- Aligned PM ops structure with the parenthesis

Changes in v4:
- Remove calls to sdhci_runtime_resume_host/sdhci_runtime_suspend_host
  from runtime callbacks as sdhc msm controller is capable of restoring
  it's register values after clocks are disabled and re-enabled.

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 | 68 +++-
 1 file changed, 67 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 8ef44a2a..795f16f 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "sdhci-pltfm.h"
@@ -68,6 +69,7 @@
 #define CMUX_SHIFT_PHASE_SHIFT 24
 #define CMUX_SHIFT_PHASE_MASK  (7 << CMUX_SHIFT_PHASE_SHIFT)
 
+#define MSM_MMC_AUTOSUSPEND_DELAY_MS   50
 struct sdhci_msm_host {
struct platform_device *pdev;
void __iomem *core_mem; /* MSM SDCC mapped address */
@@ -658,12 +660,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
goto clk_disable;
}
 
+   pm_runtime_get_noresume(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+   pm_runtime_set_autosuspend_delay(>dev,
+MSM_MMC_AUTOSUSPEND_DELAY_MS);
+   pm_runtime_use_autosuspend(>dev);
+
ret = sdhci_add_host(host);
if (ret)
-   goto clk_disable;
+   goto pm_runtime_disable;
+
+   pm_runtime_mark_last_busy(>dev);
+   pm_runtime_put_autosuspend(>dev);
 
return 0;
 
+pm_runtime_disable:
+   pm_runtime_disable(>dev);
+   pm_runtime_set_suspended(>dev);
+   pm_runtime_put_noidle(>dev);
 clk_disable:
clk_disable_unprepare(msm_host->clk);
 pclk_disable:
@@ -685,6 +701,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
0x);
 
sdhci_remove_host(host, dead);
+
+   pm_runtime_get_sync(>dev);
+   pm_runtime_disable(>dev);
+   pm_runtime_put_noidle(>dev);
+
clk_disable_unprepare(msm_host->clk);
clk_disable_unprepare(msm_host->pclk);
if (!IS_ERR(msm_host->bus_clk))
@@ -693,12 +714,57 @@ 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);
+
+   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 for core_clk: %d\n", ret);
+   return ret;
+   }
+   ret = clk_prepare_enable(msm_host->pclk);
+   if (ret) {
+   dev_err(dev, "clk_enable failed for iface_clk: %d\n", ret);
+   clk_disable_unprepare(msm_host->clk);
+   return ret;
+   }
+
+   return 0;
+}
+#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 = _msm_pm_ops,
},
 };
 
-- 
2.9.3

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

[Pramod Gurav]

Hi Ritesh,

On 20 October 2016 at 20:20, Ritesh Harjani <rite...@codeaurora.org> wrote:
> Hi Pramod,
>
> Thanks for this patch. Few minor comments.
>
> I have tested your patch on db410c and 8996 based internal platform and it
> works fine.
Thanks for the review and testing.
>
>
>
> On 10/18/2016 3:46 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.gu...@linaro.org>
>> ---
>>
>> Tested on DB410C.
>>
>> Changes in v4:
>> - Remove calls to sdhci_runtime_resume_host/sdhci_runtime_suspend_host
>>   from runtime callbacks as sdhc msm controller is capable of restoring
>>   it's register values after clocks are disabled and re-enabled.
>>
>> 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 | 66
>> +++-
>>  1 file changed, 65 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 8ef44a2a..33ec809 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -18,6 +18,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>
>>  #include "sdhci-pltfm.h"
>> @@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device
>> *pdev)
>> goto clk_disable;
>> }
>>
>> +   pm_runtime_get_noresume(>dev);
>> +   pm_runtime_set_active(>dev);
>> +   pm_runtime_enable(>dev);
>> +   pm_runtime_set_autosuspend_delay(>dev, 50);
>
> Use a macro MSM_MMC_AUTOSUSPEND_DELAY_MS instead of using 50 directly here.

Will add.
>
>> +   pm_runtime_use_autosuspend(>dev);
>> +
>> ret = sdhci_add_host(host);
>> if (ret)
>> -   goto clk_disable;
>> +   goto pm_runtime_disable;
>> +
>> +   platform_set_drvdata(pdev, host);
>
> No need to set platform_set_drvdata here. sdhci_pltfm_init will do it
> anyways.
Good to know this. Will do away with this.
>
>> +
>
> pm_runtime_mark_last_busy(>dev) can be added here.
Yeah. Agree.
>
>> +   pm_runtime_put_autosuspend(>dev);
>>
>> return 0;
>>
>> +pm_runtime_disable:
>> +   pm_runtime_disable(>dev);
>> +   pm_runtime_set_suspended(>dev);
>> +   pm_runtime_put_noidle(>dev);
>>  clk_disable:
>> clk_disable_unprepare(msm_host->clk);
>>  pclk_disable:
>> @@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device
>> *pdev)
>> 0x);
>>
>> sdhci_remove_host(host, dead);
>> +
>> +   pm_runtime_get_sync(>dev);
>> +   pm_runtime_disable(>dev);
>> +   pm_runtime_put_noidle(>dev);
>> +
>> clk_disable_unprepare(msm_host->clk);
>> clk_disable_unprepare(msm_host->pclk);
>> if (!IS_ERR(msm_host->bus_clk))
>> @@ -693,12 +713,56 @@ 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);
>> +
>> +   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_en

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

[Pramod Gurav]

Hi Ritesh,

On 20 October 2016 at 20:20, Ritesh Harjani  wrote:
> Hi Pramod,
>
> Thanks for this patch. Few minor comments.
>
> I have tested your patch on db410c and 8996 based internal platform and it
> works fine.
Thanks for the review and testing.
>
>
>
> On 10/18/2016 3:46 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 
>> ---
>>
>> Tested on DB410C.
>>
>> Changes in v4:
>> - Remove calls to sdhci_runtime_resume_host/sdhci_runtime_suspend_host
>>   from runtime callbacks as sdhc msm controller is capable of restoring
>>   it's register values after clocks are disabled and re-enabled.
>>
>> 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 | 66
>> +++-
>>  1 file changed, 65 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
>> index 8ef44a2a..33ec809 100644
>> --- a/drivers/mmc/host/sdhci-msm.c
>> +++ b/drivers/mmc/host/sdhci-msm.c
>> @@ -18,6 +18,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>
>>  #include "sdhci-pltfm.h"
>> @@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device
>> *pdev)
>> goto clk_disable;
>> }
>>
>> +   pm_runtime_get_noresume(>dev);
>> +   pm_runtime_set_active(>dev);
>> +   pm_runtime_enable(>dev);
>> +   pm_runtime_set_autosuspend_delay(>dev, 50);
>
> Use a macro MSM_MMC_AUTOSUSPEND_DELAY_MS instead of using 50 directly here.

Will add.
>
>> +   pm_runtime_use_autosuspend(>dev);
>> +
>> ret = sdhci_add_host(host);
>> if (ret)
>> -   goto clk_disable;
>> +   goto pm_runtime_disable;
>> +
>> +   platform_set_drvdata(pdev, host);
>
> No need to set platform_set_drvdata here. sdhci_pltfm_init will do it
> anyways.
Good to know this. Will do away with this.
>
>> +
>
> pm_runtime_mark_last_busy(>dev) can be added here.
Yeah. Agree.
>
>> +   pm_runtime_put_autosuspend(>dev);
>>
>> return 0;
>>
>> +pm_runtime_disable:
>> +   pm_runtime_disable(>dev);
>> +   pm_runtime_set_suspended(>dev);
>> +   pm_runtime_put_noidle(>dev);
>>  clk_disable:
>> clk_disable_unprepare(msm_host->clk);
>>  pclk_disable:
>> @@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device
>> *pdev)
>> 0x);
>>
>> sdhci_remove_host(host, dead);
>> +
>> +   pm_runtime_get_sync(>dev);
>> +   pm_runtime_disable(>dev);
>> +   pm_runtime_put_noidle(>dev);
>> +
>> clk_disable_unprepare(msm_host->clk);
>> clk_disable_unprepare(msm_host->pclk);
>> if (!IS_ERR(msm_host->bus_clk))
>> @@ -693,12 +713,56 @@ 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);
>> +
>> +   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);
>
> dev_err(dev,

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

[Pramod Gurav]

Thanks Georgi for the review.

On 20 October 2016 at 18:38, Georgi Djakov <georgi.dja...@linaro.org> wrote:
> Hi Pramod,
>
> Thanks for the patch!
>
> On 10/18/2016 01:16 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.gu...@linaro.org>
>> ---
>>
>> Tested on DB410C.
>>
>
> [..]
>
>> +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);
>
>
> Nit: Maybe mention in the prints which clock failed - core or peripheral.

Agree. Will add in v5.

>
>> +   clk_disable_unprepare(msm_host->clk);
>> +   return ret;
>> +   }
>> +
>> +   return 0;
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops sdhci_msm_pm_ops = {
>> +   SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>> +   pm_runtime_force_resume)
>
>
> Nit: Please align with the parenthesis.
>
>> +   SET_RUNTIME_PM_OPS(sdhci_msm_runtime_suspend,
>> sdhci_msm_runtime_resume,
>> +   NULL)
>
>
> Ditto.
Yes. Will take care of this as well.
>
> Reviewed-by: Georgi Djakov <georgi.dja...@linaro.org>

Thanks again. :)
>
> BR,
> Georgi

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

[Pramod Gurav]

Thanks Georgi for the review.

On 20 October 2016 at 18:38, Georgi Djakov  wrote:
> Hi Pramod,
>
> Thanks for the patch!
>
> On 10/18/2016 01:16 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 
>> ---
>>
>> Tested on DB410C.
>>
>
> [..]
>
>> +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);
>
>
> Nit: Maybe mention in the prints which clock failed - core or peripheral.

Agree. Will add in v5.

>
>> +   clk_disable_unprepare(msm_host->clk);
>> +   return ret;
>> +   }
>> +
>> +   return 0;
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops sdhci_msm_pm_ops = {
>> +   SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
>> +   pm_runtime_force_resume)
>
>
> Nit: Please align with the parenthesis.
>
>> +   SET_RUNTIME_PM_OPS(sdhci_msm_runtime_suspend,
>> sdhci_msm_runtime_resume,
>> +   NULL)
>
>
> Ditto.
Yes. Will take care of this as well.
>
> Reviewed-by: Georgi Djakov 

Thanks again. :)
>
> BR,
> Georgi

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

[Pramod Gurav]

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.gu...@linaro.org>
---

Tested on DB410C.

Changes in v4:
- Remove calls to sdhci_runtime_resume_host/sdhci_runtime_suspend_host
  from runtime callbacks as sdhc msm controller is capable of restoring
  it's register values after clocks are disabled and re-enabled.

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 | 66 +++-
 1 file changed, 65 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 8ef44a2a..33ec809 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "sdhci-pltfm.h"
@@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
goto clk_disable;
}
 
+   pm_runtime_get_noresume(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+   pm_runtime_set_autosuspend_delay(>dev, 50);
+   pm_runtime_use_autosuspend(>dev);
+
ret = sdhci_add_host(host);
if (ret)
-   goto clk_disable;
+   goto pm_runtime_disable;
+
+   platform_set_drvdata(pdev, host);
+
+   pm_runtime_put_autosuspend(>dev);
 
return 0;
 
+pm_runtime_disable:
+   pm_runtime_disable(>dev);
+   pm_runtime_set_suspended(>dev);
+   pm_runtime_put_noidle(>dev);
 clk_disable:
clk_disable_unprepare(msm_host->clk);
 pclk_disable:
@@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
0x);
 
sdhci_remove_host(host, dead);
+
+   pm_runtime_get_sync(>dev);
+   pm_runtime_disable(>dev);
+   pm_runtime_put_noidle(>dev);
+
clk_disable_unprepare(msm_host->clk);
clk_disable_unprepare(msm_host->pclk);
if (!IS_ERR(msm_host->bus_clk))
@@ -693,12 +713,56 @@ 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);
+
+   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 0;
+}
+#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 = _msm_pm_ops,
},
 };
 
-- 
2.9.3

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

[Pramod Gurav]

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 
---

Tested on DB410C.

Changes in v4:
- Remove calls to sdhci_runtime_resume_host/sdhci_runtime_suspend_host
  from runtime callbacks as sdhc msm controller is capable of restoring
  it's register values after clocks are disabled and re-enabled.

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 | 66 +++-
 1 file changed, 65 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 8ef44a2a..33ec809 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "sdhci-pltfm.h"
@@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
goto clk_disable;
}
 
+   pm_runtime_get_noresume(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+   pm_runtime_set_autosuspend_delay(>dev, 50);
+   pm_runtime_use_autosuspend(>dev);
+
ret = sdhci_add_host(host);
if (ret)
-   goto clk_disable;
+   goto pm_runtime_disable;
+
+   platform_set_drvdata(pdev, host);
+
+   pm_runtime_put_autosuspend(>dev);
 
return 0;
 
+pm_runtime_disable:
+   pm_runtime_disable(>dev);
+   pm_runtime_set_suspended(>dev);
+   pm_runtime_put_noidle(>dev);
 clk_disable:
clk_disable_unprepare(msm_host->clk);
 pclk_disable:
@@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
0x);
 
sdhci_remove_host(host, dead);
+
+   pm_runtime_get_sync(>dev);
+   pm_runtime_disable(>dev);
+   pm_runtime_put_noidle(>dev);
+
clk_disable_unprepare(msm_host->clk);
clk_disable_unprepare(msm_host->pclk);
if (!IS_ERR(msm_host->bus_clk))
@@ -693,12 +713,56 @@ 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);
+
+   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 0;
+}
+#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 = _msm_pm_ops,
},
 };
 
-- 
2.9.3

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

[Pramod Gurav]

Hi Ritesh,

Thanks for the inputs.

On 22 September 2016 at 20:02, Ritesh Harjani  wrote:
> Hi Pramod,



>> Thanks Ulf for the comments. Will check this and see if there is
>> something of this sort we have to do to achieve auto tuning.
>> Adding Ritesh who has been posting some SDHCI MSM patches recently in
>> case he knows about this.
>
>
> Internally, we don't use this Auto re-tuning and rely on explicit re-tune by
> host driver.
>
> Question though -
> 1. why do we need to call sdhci_runtime_resume/suspend from
> sdhci_msm_runtime_suspend/resume?
> From what I see is, sdhci_runtime_susend/resume will do reset and re-program
> of host->pwr and host->clk because of which a retune will be required for
> the next command after runtime resume.

Honestly I took reference from existing SDHCI HC driver which
implement the runtime PM and each one uses this function.

>
> We can *only* disable and enable the clocks in
> sdhci_msm_runtime_suspend/resume?
> Thoughts? With this, I suppose you would not see any issue.
This should work as I did not have this funtion call in my V1 but
later included when I referred the other sdhci drivers.

>
>
> Though for this issue, since internally also auto retuning is never used, we
> can have this mode disabled. I can once again check with HW team to get more
> details about this mode for MSM controller.
This seems a read only register. And I could not find any other
reference of this mode in any of the docs.
>
>>
>> Regards,
>> Pramod
>>
>

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

[Pramod Gurav]

Hi Ritesh,

Thanks for the inputs.

On 22 September 2016 at 20:02, Ritesh Harjani  wrote:
> Hi Pramod,



>> Thanks Ulf for the comments. Will check this and see if there is
>> something of this sort we have to do to achieve auto tuning.
>> Adding Ritesh who has been posting some SDHCI MSM patches recently in
>> case he knows about this.
>
>
> Internally, we don't use this Auto re-tuning and rely on explicit re-tune by
> host driver.
>
> Question though -
> 1. why do we need to call sdhci_runtime_resume/suspend from
> sdhci_msm_runtime_suspend/resume?
> From what I see is, sdhci_runtime_susend/resume will do reset and re-program
> of host->pwr and host->clk because of which a retune will be required for
> the next command after runtime resume.

Honestly I took reference from existing SDHCI HC driver which
implement the runtime PM and each one uses this function.

>
> We can *only* disable and enable the clocks in
> sdhci_msm_runtime_suspend/resume?
> Thoughts? With this, I suppose you would not see any issue.
This should work as I did not have this funtion call in my V1 but
later included when I referred the other sdhci drivers.

>
>
> Though for this issue, since internally also auto retuning is never used, we
> can have this mode disabled. I can once again check with HW team to get more
> details about this mode for MSM controller.
This seems a read only register. And I could not find any other
reference of this mode in any of the docs.
>
>>
>> Regards,
>> Pramod
>>
>

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

[Pramod Gurav]

On 15 September 2016 at 15:49, Ulf Hansson <ulf.hans...@linaro.org> wrote:
> On 15 September 2016 at 09:59, Pramod Gurav <pramod.gu...@linaro.org> wrote:
>> On 9 September 2016 at 15:48, Georgi Djakov <georgi.dja...@linaro.org> wrote:
>>> 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.gu...@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.
>>>
>>
>> Thanks Georgi for testing the patch. Its my wrong I did not update my
>> kernel and continued fixing comments on old kernel.
>> After spending some time I came to know that below change is causing the 
>> issue:
>>
>> Author: Dong Aisheng <aisheng.d...@nxp.com>
>> Date:   Tue Jul 12 15:46:17 2016 +0800
>>
>> mmc: sdhci: add standard hw auto retuning support
>>
>> If HW supports SDHCI_TUNING_MODE_3 which is auto retuning, we won't
>> retune during runtime suspend and resume, instead we use Re-tuning
>> Request signaled via SDHCI_INT_RETUNE interrupt to do retuning and
>> hw auto retuning during data transfer to guarantee the signal sample
>> window correction.
>>
>> This can avoid a mass of repeatedly retuning during small file system
>> data access and improve the performance.
>>
>> Specially these lines that was added to suspend path:
>>
>> +   if (host->tuning_mode != SDHCI_TUNING_MODE_3)
>> +   mmc_retune_needed(host->mmc);
>>
>> During sdhci setup in msm driver, the host returns the values to set
>> sdhci auto tuning as supported.
>> Hence host->tuning_mode is set to SDHCI_TUNING_MODE_3 during setup.
>> But some how the auto tuning is not happening.
>> Just to verify my case, I removed the 'if' part in above code and got
>> the FS mounted.
>>
>> Is there anything else needed in msm sdhci driver so that the auto
>> tuning is taken care of?
>
> I am not familiar with any other than sdhci-esdhc-imx which supports
> the SDHCI_TUNING_MODE_3. I may be wrong though.
>
> In the sdhci-esdhc-imx case, enabling of auto tuning seems to be done
> in esdhc_post_tuning(), where a vendor specific register
> (ESDHC_MIX_CTRL) is being written to. Perhaps something similar in
> your case?
>
Thanks Ulf for the comments. Will check this and see if there is
something of this sort we have to do to achieve auto tuning.
Adding Ritesh who has been posting some SDHCI MSM patches recently in
case he knows about this.

Regards,
Pramod

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

[Pramod Gurav]

On 15 September 2016 at 15:49, Ulf Hansson  wrote:
> On 15 September 2016 at 09:59, Pramod Gurav  wrote:
>> On 9 September 2016 at 15:48, Georgi Djakov  wrote:
>>> 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 
>>>>
>>>>
>>>> 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.
>>>
>>
>> Thanks Georgi for testing the patch. Its my wrong I did not update my
>> kernel and continued fixing comments on old kernel.
>> After spending some time I came to know that below change is causing the 
>> issue:
>>
>> Author: Dong Aisheng 
>> Date:   Tue Jul 12 15:46:17 2016 +0800
>>
>> mmc: sdhci: add standard hw auto retuning support
>>
>> If HW supports SDHCI_TUNING_MODE_3 which is auto retuning, we won't
>> retune during runtime suspend and resume, instead we use Re-tuning
>> Request signaled via SDHCI_INT_RETUNE interrupt to do retuning and
>> hw auto retuning during data transfer to guarantee the signal sample
>> window correction.
>>
>> This can avoid a mass of repeatedly retuning during small file system
>> data access and improve the performance.
>>
>> Specially these lines that was added to suspend path:
>>
>> +   if (host->tuning_mode != SDHCI_TUNING_MODE_3)
>> +   mmc_retune_needed(host->mmc);
>>
>> During sdhci setup in msm driver, the host returns the values to set
>> sdhci auto tuning as supported.
>> Hence host->tuning_mode is set to SDHCI_TUNING_MODE_3 during setup.
>> But some how the auto tuning is not happening.
>> Just to verify my case, I removed the 'if' part in above code and got
>> the FS mounted.
>>
>> Is there anything else needed in msm sdhci driver so that the auto
>> tuning is taken care of?
>
> I am not familiar with any other than sdhci-esdhc-imx which supports
> the SDHCI_TUNING_MODE_3. I may be wrong though.
>
> In the sdhci-esdhc-imx case, enabling of auto tuning seems to be done
> in esdhc_post_tuning(), where a vendor specific register
> (ESDHC_MIX_CTRL) is being written to. Perhaps something similar in
> your case?
>
Thanks Ulf for the comments. Will check this and see if there is
something of this sort we have to do to achieve auto tuning.
Adding Ritesh who has been posting some SDHCI MSM patches recently in
case he knows about this.

Regards,
Pramod

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

[Pramod Gurav]

On 9 September 2016 at 15:48, Georgi Djakov <georgi.dja...@linaro.org> wrote:
> 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.gu...@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.
>

Thanks Georgi for testing the patch. Its my wrong I did not update my
kernel and continued fixing comments on old kernel.
After spending some time I came to know that below change is causing the issue:

Author: Dong Aisheng <aisheng.d...@nxp.com>
Date:   Tue Jul 12 15:46:17 2016 +0800

mmc: sdhci: add standard hw auto retuning support

If HW supports SDHCI_TUNING_MODE_3 which is auto retuning, we won't
retune during runtime suspend and resume, instead we use Re-tuning
Request signaled via SDHCI_INT_RETUNE interrupt to do retuning and
hw auto retuning during data transfer to guarantee the signal sample
window correction.

This can avoid a mass of repeatedly retuning during small file system
data access and improve the performance.

Specially these lines that was added to suspend path:

+   if (host->tuning_mode != SDHCI_TUNING_MODE_3)
+   mmc_retune_needed(host->mmc);

During sdhci setup in msm driver, the host returns the values to set
sdhci auto tuning as supported.
Hence host->tuning_mode is set to SDHCI_TUNING_MODE_3 during setup.
But some how the auto tuning is not happening.
Just to verify my case, I removed the 'if' part in above code and got
the FS mounted.

Is there anything else needed in msm sdhci driver so that the auto
tuning is taken care of?

> [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

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

[Pramod Gurav]

On 9 September 2016 at 15:48, Georgi Djakov  wrote:
> 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 
>>
>>
>> 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.
>

Thanks Georgi for testing the patch. Its my wrong I did not update my
kernel and continued fixing comments on old kernel.
After spending some time I came to know that below change is causing the issue:

Author: Dong Aisheng 
Date:   Tue Jul 12 15:46:17 2016 +0800

mmc: sdhci: add standard hw auto retuning support

If HW supports SDHCI_TUNING_MODE_3 which is auto retuning, we won't
retune during runtime suspend and resume, instead we use Re-tuning
Request signaled via SDHCI_INT_RETUNE interrupt to do retuning and
hw auto retuning during data transfer to guarantee the signal sample
window correction.

This can avoid a mass of repeatedly retuning during small file system
data access and improve the performance.

Specially these lines that was added to suspend path:

+   if (host->tuning_mode != SDHCI_TUNING_MODE_3)
+   mmc_retune_needed(host->mmc);

During sdhci setup in msm driver, the host returns the values to set
sdhci auto tuning as supported.
Hence host->tuning_mode is set to SDHCI_TUNING_MODE_3 during setup.
But some how the auto tuning is not happening.
Just to verify my case, I removed the 'if' part in above code and got
the FS mounted.

Is there anything else needed in msm sdhci driver so that the auto
tuning is taken care of?

> [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

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

[Pramod Gurav]

On 9 September 2016 at 15:30, Tummala, Sahitya  wrote:
> Hi Pramod,

>> +   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.
>

Thanks Sahitya for comments. Will take care of this in next version.
>> +   return ret;
>> +   }

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

[Pramod Gurav]

On 9 September 2016 at 15:30, Tummala, Sahitya  wrote:
> Hi Pramod,

>> +   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.
>

Thanks Sahitya for comments. Will take care of this in next version.
>> +   return ret;
>> +   }

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

[Pramod Gurav]

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.gu...@linaro.org>
---
Changes in v1:
- Added CONFIG_PM around runtime pm function.
- Replaced msm suspend/resume with generic function directly

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..881c564 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "sdhci-pltfm.h"
@@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
goto clk_disable;
}
 
+   pm_runtime_get_noresume(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+   pm_runtime_set_autosuspend_delay(>dev, 50);
+   pm_runtime_use_autosuspend(>dev);
+
ret = sdhci_add_host(host);
if (ret)
-   goto clk_disable;
+   goto pm_runtime_disable;
+
+   platform_set_drvdata(pdev, host);
+
+   pm_runtime_put_autosuspend(>dev);
 
return 0;
 
+pm_runtime_disable:
+   pm_runtime_disable(>dev);
+   pm_runtime_set_suspended(>dev);
+   pm_runtime_put_noidle(>dev);
 clk_disable:
clk_disable_unprepare(msm_host->clk);
 pclk_disable:
@@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
0x);
 
sdhci_remove_host(host, dead);
+
+   pm_runtime_get_sync(>dev);
+   pm_runtime_disable(>dev);
+   pm_runtime_put_noidle(>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_LATE_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 = _msm_pm_ops,
},
 };
 
-- 
2.9.3

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

[Pramod Gurav]

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 
---
Changes in v1:
- Added CONFIG_PM around runtime pm function.
- Replaced msm suspend/resume with generic function directly

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..881c564 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "sdhci-pltfm.h"
@@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
goto clk_disable;
}
 
+   pm_runtime_get_noresume(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+   pm_runtime_set_autosuspend_delay(>dev, 50);
+   pm_runtime_use_autosuspend(>dev);
+
ret = sdhci_add_host(host);
if (ret)
-   goto clk_disable;
+   goto pm_runtime_disable;
+
+   platform_set_drvdata(pdev, host);
+
+   pm_runtime_put_autosuspend(>dev);
 
return 0;
 
+pm_runtime_disable:
+   pm_runtime_disable(>dev);
+   pm_runtime_set_suspended(>dev);
+   pm_runtime_put_noidle(>dev);
 clk_disable:
clk_disable_unprepare(msm_host->clk);
 pclk_disable:
@@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
0x);
 
sdhci_remove_host(host, dead);
+
+   pm_runtime_get_sync(>dev);
+   pm_runtime_disable(>dev);
+   pm_runtime_put_noidle(>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_LATE_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 = _msm_pm_ops,
},
 };
 
-- 
2.9.3

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

[Pramod Gurav]

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.gu...@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 
 #include 
 #include 
+#include 
 #include 
 
 #include "sdhci-pltfm.h"
@@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
goto clk_disable;
}
 
+   pm_runtime_get_noresume(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+   pm_runtime_set_autosuspend_delay(>dev, 50);
+   pm_runtime_use_autosuspend(>dev);
+
ret = sdhci_add_host(host);
if (ret)
-   goto clk_disable;
+   goto pm_runtime_disable;
+
+   platform_set_drvdata(pdev, host);
+
+   pm_runtime_put_autosuspend(>dev);
 
return 0;
 
+pm_runtime_disable:
+   pm_runtime_disable(>dev);
+   pm_runtime_set_suspended(>dev);
+   pm_runtime_put_noidle(>dev);
 clk_disable:
clk_disable_unprepare(msm_host->clk);
 pclk_disable:
@@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
0x);
 
sdhci_remove_host(host, dead);
+
+   pm_runtime_get_sync(>dev);
+   pm_runtime_disable(>dev);
+   pm_runtime_put_noidle(>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 = _msm_pm_ops,
},
 };
 
-- 
2.9.3

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

[Pramod Gurav]

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 
---
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 
 #include 
 #include 
+#include 
 #include 
 
 #include "sdhci-pltfm.h"
@@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
goto clk_disable;
}
 
+   pm_runtime_get_noresume(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+   pm_runtime_set_autosuspend_delay(>dev, 50);
+   pm_runtime_use_autosuspend(>dev);
+
ret = sdhci_add_host(host);
if (ret)
-   goto clk_disable;
+   goto pm_runtime_disable;
+
+   platform_set_drvdata(pdev, host);
+
+   pm_runtime_put_autosuspend(>dev);
 
return 0;
 
+pm_runtime_disable:
+   pm_runtime_disable(>dev);
+   pm_runtime_set_suspended(>dev);
+   pm_runtime_put_noidle(>dev);
 clk_disable:
clk_disable_unprepare(msm_host->clk);
 pclk_disable:
@@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
0x);
 
sdhci_remove_host(host, dead);
+
+   pm_runtime_get_sync(>dev);
+   pm_runtime_disable(>dev);
+   pm_runtime_put_noidle(>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 = _msm_pm_ops,
},
 };
 
-- 
2.9.3

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

[Pramod Gurav]

On 1 September 2016 at 19:38, Ulf Hansson  wrote:
> [...]
>
>>> +
>>> +static const struct dev_pm_ops sdhci_msm_pm_ops = {
>>> +   SET_LATE_SYSTEM_SLEEP_PM_OPS(sdhci_msm_suspend, sdhci_msm_resume)
>
> One more thing. Why do you need the late versions of the system PM callbacks?
>
> I think it's fine to use the regular:
> SET_SYSTEM_SLEEP_PM_OPS(sdhci_msm_suspend, sdhci_msm_resume)
>
Should have waited for this review as well. :)
Anyway I forgot to add v3 tag in my new patch.
Will resend v3 with this change as well.

> [...]
>
> Kind regards
> Uffe

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

[Pramod Gurav]

On 1 September 2016 at 19:38, Ulf Hansson  wrote:
> [...]
>
>>> +
>>> +static const struct dev_pm_ops sdhci_msm_pm_ops = {
>>> +   SET_LATE_SYSTEM_SLEEP_PM_OPS(sdhci_msm_suspend, sdhci_msm_resume)
>
> One more thing. Why do you need the late versions of the system PM callbacks?
>
> I think it's fine to use the regular:
> SET_SYSTEM_SLEEP_PM_OPS(sdhci_msm_suspend, sdhci_msm_resume)
>
Should have waited for this review as well. :)
Anyway I forgot to add v3 tag in my new patch.
Will resend v3 with this change as well.

> [...]
>
> Kind regards
> Uffe

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

[Pramod Gurav]

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.gu...@linaro.org>
---
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 | 82 +++-
 1 file changed, 81 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 8ef44a2a..8273a71 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "sdhci-pltfm.h"
@@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
goto clk_disable;
}
 
+   pm_runtime_get_noresume(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+   pm_runtime_set_autosuspend_delay(>dev, 50);
+   pm_runtime_use_autosuspend(>dev);
+
ret = sdhci_add_host(host);
if (ret)
-   goto clk_disable;
+   goto pm_runtime_disable;
+
+   platform_set_drvdata(pdev, host);
+
+   pm_runtime_put_autosuspend(>dev);
 
return 0;
 
+pm_runtime_disable:
+   pm_runtime_disable(>dev);
+   pm_runtime_set_suspended(>dev);
+   pm_runtime_put_noidle(>dev);
 clk_disable:
clk_disable_unprepare(msm_host->clk);
 pclk_disable:
@@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
0x);
 
sdhci_remove_host(host, dead);
+
+   pm_runtime_get_sync(>dev);
+   pm_runtime_disable(>dev);
+   pm_runtime_put_noidle(>dev);
+
clk_disable_unprepare(msm_host->clk);
clk_disable_unprepare(msm_host->pclk);
if (!IS_ERR(msm_host->bus_clk))
@@ -693,12 +713,72 @@ static int sdhci_msm_remove(struct platform_device *pdev)
return 0;
 }
 
+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);
+}
+
+static int sdhci_msm_suspend(struct device *dev)
+{
+   pm_runtime_force_suspend(dev);
+
+   return 0;
+}
+
+static int sdhci_msm_resume(struct device *dev)
+{
+   pm_runtime_force_resume(dev);
+
+   return 0;
+}
+
+static const struct dev_pm_ops sdhci_msm_pm_ops = {
+   SET_LATE_SYSTEM_SLEEP_PM_OPS(sdhci_msm_suspend, sdhci_msm_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 = _msm_pm_ops,
},
 };
 
-- 
2.9.3

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

[Pramod Gurav]

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 
---
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 | 82 +++-
 1 file changed, 81 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 8ef44a2a..8273a71 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "sdhci-pltfm.h"
@@ -658,12 +659,26 @@ static int sdhci_msm_probe(struct platform_device *pdev)
goto clk_disable;
}
 
+   pm_runtime_get_noresume(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+   pm_runtime_set_autosuspend_delay(>dev, 50);
+   pm_runtime_use_autosuspend(>dev);
+
ret = sdhci_add_host(host);
if (ret)
-   goto clk_disable;
+   goto pm_runtime_disable;
+
+   platform_set_drvdata(pdev, host);
+
+   pm_runtime_put_autosuspend(>dev);
 
return 0;
 
+pm_runtime_disable:
+   pm_runtime_disable(>dev);
+   pm_runtime_set_suspended(>dev);
+   pm_runtime_put_noidle(>dev);
 clk_disable:
clk_disable_unprepare(msm_host->clk);
 pclk_disable:
@@ -685,6 +700,11 @@ static int sdhci_msm_remove(struct platform_device *pdev)
0x);
 
sdhci_remove_host(host, dead);
+
+   pm_runtime_get_sync(>dev);
+   pm_runtime_disable(>dev);
+   pm_runtime_put_noidle(>dev);
+
clk_disable_unprepare(msm_host->clk);
clk_disable_unprepare(msm_host->pclk);
if (!IS_ERR(msm_host->bus_clk))
@@ -693,12 +713,72 @@ static int sdhci_msm_remove(struct platform_device *pdev)
return 0;
 }
 
+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);
+}
+
+static int sdhci_msm_suspend(struct device *dev)
+{
+   pm_runtime_force_suspend(dev);
+
+   return 0;
+}
+
+static int sdhci_msm_resume(struct device *dev)
+{
+   pm_runtime_force_resume(dev);
+
+   return 0;
+}
+
+static const struct dev_pm_ops sdhci_msm_pm_ops = {
+   SET_LATE_SYSTEM_SLEEP_PM_OPS(sdhci_msm_suspend, sdhci_msm_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 = _msm_pm_ops,
},
 };
 
-- 
2.9.3

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

[Pramod Gurav]

Thanks Ulf for the review.

On 29 August 2016 at 19:50, Ulf Hansson <ulf.hans...@linaro.org> wrote:
> On 16 June 2016 at 14:35, Pramod Gurav <pramod.gu...@linaro.org> wrote:



>> +   platform_set_drvdata(pdev, msm_host);
>> +
>> +   pm_runtime_set_active(>dev);
>> +   pm_runtime_enable(>dev);
>
> I think you need to move these a bit earlier, before calling sdhci_add_host().
>
> Maybe it's just easier if you look at the sdhci-of-at91.c driver,
> which behaves nicely around runtime PM deployment. You can probably
> use the very similar code, except the ->runtime_suspend|resume()
> callbacks.
>
> And don't forget to deploy runtime PM support in the ->remove()
> callback as well, again sdhci-of-at91 is a good reference.
>

Will take a look at the said driver and do necessary changes and repost.
Thanks again.

Regards,
Pramod

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

[Pramod Gurav]

Thanks Ulf for the review.

On 29 August 2016 at 19:50, Ulf Hansson  wrote:
> On 16 June 2016 at 14:35, Pramod Gurav  wrote:



>> +   platform_set_drvdata(pdev, msm_host);
>> +
>> +   pm_runtime_set_active(>dev);
>> +   pm_runtime_enable(>dev);
>
> I think you need to move these a bit earlier, before calling sdhci_add_host().
>
> Maybe it's just easier if you look at the sdhci-of-at91.c driver,
> which behaves nicely around runtime PM deployment. You can probably
> use the very similar code, except the ->runtime_suspend|resume()
> callbacks.
>
> And don't forget to deploy runtime PM support in the ->remove()
> callback as well, again sdhci-of-at91 is a good reference.
>

Will take a look at the said driver and do necessary changes and repost.
Thanks again.

Regards,
Pramod

Re: [PATCH] tty: serial: msm: Add runtime PM and system sleep support

[Pramod Gurav]

Hi Stephen,

Thanks for having a look.

On 26 August 2016 at 04:20, Stephen Boyd <sb...@codeaurora.org> wrote:
> On 06/17, Pramod Gurav wrote:
>> @@ -1220,12 +1293,26 @@ static void msm_power(struct uart_port *port, 
>> unsigned int state,
>>
>>   switch (state) {
>>   case 0:
>> - clk_prepare_enable(msm_port->clk);
>> - clk_prepare_enable(msm_port->pclk);
>> + /*
>> +  * UART clk must be kept enabled to
>> +  * avoid losing received character
>> +  */
>
> Don't we have a wakeup irq? Two wire interfaces probably don't
> work though (like the debug uart).
I am not aware of wakeup irq for UART.

>
>> + if (clk_prepare_enable(msm_port->clk))
>> + return;
>> + if (clk_prepare(msm_port->pclk)) {
>> + clk_disable_unprepare(msm_port->clk);
>> + return;
>> + }
>> + if (pm_runtime_get_sync(port->dev) < 0) {
>> + clk_unprepare(msm_port->pclk);
>> + clk_disable_unprepare(msm_port->clk);
>
> I guess that's why we gate the interface clk and not the core clk
> during runtime PM? core clk goes off and then device is basically
> suspended unless it can wakeup with an irq.
Yes. With core clock disabled we cant get RX working as we dont have
any wakeup mechanism after which we could carry out RX.

>
>> + return;
>> + }
>>   break;
>>   case 3:
>> + pm_runtime_put(port->dev);
>> + clk_unprepare(msm_port->pclk);
>>   clk_disable_unprepare(msm_port->clk);
>> - clk_disable_unprepare(msm_port->pclk);
>>   break;
>>   default:
>>   pr_err("msm_serial: Unknown PM state %d\n", state);
>> @@ -1465,7 +1552,11 @@ static void msm_console_write(struct console *co, 
>> const char *s,
>>   port = msm_get_port_from_line(co->index);
>>   msm_port = UART_TO_MSM(port);
>>
>> + if (pm_runtime_get_sync(port->dev) < 0)
>> + return;
>>   __msm_console_write(port, s, count, msm_port->is_uartdm);
>> + pm_runtime_mark_last_busy(port->dev);
>> + pm_runtime_put_autosuspend(port->dev);
>
> Hmm ok, perhaps we should differentiate runtime PM for devices
> that use the console and ones that are being used for other
> things? I would guess that console can only turn off the
> interface clk while idle, but the non-console devices could turn
> off everything at runtime and rely on some out of band signaling
> to wakeup when something comes over the rx wire?
I will see if there is any way to wakeup the UART like you are saying.

>
>>  }
>>
>>  static int __init msm_console_setup(struct console *co, char *options)
>> @@ -1484,7 +1575,7 @@ static int __init msm_console_setup(struct console 
>> *co, char *options)
>>   if (unlikely(!port->membase))
>>   return -ENXIO;
>>
>> - msm_init_clock(port);
>> + msm_serial_set_mnd_regs(port);
>>
>>   if (options)
>>   uart_parse_options(options, , , , );
>
> Doesn't uart_set_options() go and touch hardware registers during
> termios settings? The clks are no longer enabled here though so I
> hope this isn't relying on the fact that the clks are enabled in
> the bootloader?
The clocks are enabled in serial_core with call to uart_change_pm()
just before console_setup and hence this should be okay.

>
>> @@ -1627,6 +1718,12 @@ static int msm_serial_probe(struct platform_device 
>> *pdev)
>>
>>   platform_set_drvdata(pdev, port);
>>
>> + pm_runtime_use_autosuspend(>dev);
>> + pm_runtime_set_autosuspend_delay(>dev, 500);
>> + pm_runtime_irq_safe(>dev);
>
> So this means irqs are always disabled while runtime PM
> callbacks are run
Because we are accessing UART registers in IRQ handler.
>
>> + pm_runtime_enable(>dev);
>> + pm_runtime_set_suspended(>dev);
>> +
>>   return uart_add_one_port(_uart_driver, port);
>>  }
>>
>> @@ -1645,12 +1743,67 @@ static const struct of_device_id msm_match_table[] = 
>> {
>>   {}
>>  };
>>
>> +#ifdef CONFIG_PM
>> +static int msm_serial_runtime_suspend(struct device *dev)
>> +{
>> + struct uart_port *port = dev_get_drvdata(dev);
>> + struct msm_port *msm_port = UART_TO_MSM(port);
>> +
>> + if (msm_port->is_uartdm)
>> + clk_disable(msm_port->pclk);
>
> ... so we can't unprepare clks here. That's unfortunate because
> clks that are ancestors of these clks will be kept prepared and
> that could lead to things like PLLs being kept enabled, etc.
Yes. clk_prepare/unprepare may sleep and we want to avoid that in runtime PM.
This is all we can do in runtime PM. suspend will achieve full
resource release though.

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

Re: [PATCH] tty: serial: msm: Add runtime PM and system sleep support

[Pramod Gurav]

Hi Stephen,

Thanks for having a look.

On 26 August 2016 at 04:20, Stephen Boyd  wrote:
> On 06/17, Pramod Gurav wrote:
>> @@ -1220,12 +1293,26 @@ static void msm_power(struct uart_port *port, 
>> unsigned int state,
>>
>>   switch (state) {
>>   case 0:
>> - clk_prepare_enable(msm_port->clk);
>> - clk_prepare_enable(msm_port->pclk);
>> + /*
>> +  * UART clk must be kept enabled to
>> +  * avoid losing received character
>> +  */
>
> Don't we have a wakeup irq? Two wire interfaces probably don't
> work though (like the debug uart).
I am not aware of wakeup irq for UART.

>
>> + if (clk_prepare_enable(msm_port->clk))
>> + return;
>> + if (clk_prepare(msm_port->pclk)) {
>> + clk_disable_unprepare(msm_port->clk);
>> + return;
>> + }
>> + if (pm_runtime_get_sync(port->dev) < 0) {
>> + clk_unprepare(msm_port->pclk);
>> + clk_disable_unprepare(msm_port->clk);
>
> I guess that's why we gate the interface clk and not the core clk
> during runtime PM? core clk goes off and then device is basically
> suspended unless it can wakeup with an irq.
Yes. With core clock disabled we cant get RX working as we dont have
any wakeup mechanism after which we could carry out RX.

>
>> + return;
>> + }
>>   break;
>>   case 3:
>> + pm_runtime_put(port->dev);
>> + clk_unprepare(msm_port->pclk);
>>   clk_disable_unprepare(msm_port->clk);
>> - clk_disable_unprepare(msm_port->pclk);
>>   break;
>>   default:
>>   pr_err("msm_serial: Unknown PM state %d\n", state);
>> @@ -1465,7 +1552,11 @@ static void msm_console_write(struct console *co, 
>> const char *s,
>>   port = msm_get_port_from_line(co->index);
>>   msm_port = UART_TO_MSM(port);
>>
>> + if (pm_runtime_get_sync(port->dev) < 0)
>> + return;
>>   __msm_console_write(port, s, count, msm_port->is_uartdm);
>> + pm_runtime_mark_last_busy(port->dev);
>> + pm_runtime_put_autosuspend(port->dev);
>
> Hmm ok, perhaps we should differentiate runtime PM for devices
> that use the console and ones that are being used for other
> things? I would guess that console can only turn off the
> interface clk while idle, but the non-console devices could turn
> off everything at runtime and rely on some out of band signaling
> to wakeup when something comes over the rx wire?
I will see if there is any way to wakeup the UART like you are saying.

>
>>  }
>>
>>  static int __init msm_console_setup(struct console *co, char *options)
>> @@ -1484,7 +1575,7 @@ static int __init msm_console_setup(struct console 
>> *co, char *options)
>>   if (unlikely(!port->membase))
>>   return -ENXIO;
>>
>> - msm_init_clock(port);
>> + msm_serial_set_mnd_regs(port);
>>
>>   if (options)
>>   uart_parse_options(options, , , , );
>
> Doesn't uart_set_options() go and touch hardware registers during
> termios settings? The clks are no longer enabled here though so I
> hope this isn't relying on the fact that the clks are enabled in
> the bootloader?
The clocks are enabled in serial_core with call to uart_change_pm()
just before console_setup and hence this should be okay.

>
>> @@ -1627,6 +1718,12 @@ static int msm_serial_probe(struct platform_device 
>> *pdev)
>>
>>   platform_set_drvdata(pdev, port);
>>
>> + pm_runtime_use_autosuspend(>dev);
>> + pm_runtime_set_autosuspend_delay(>dev, 500);
>> + pm_runtime_irq_safe(>dev);
>
> So this means irqs are always disabled while runtime PM
> callbacks are run
Because we are accessing UART registers in IRQ handler.
>
>> + pm_runtime_enable(>dev);
>> + pm_runtime_set_suspended(>dev);
>> +
>>   return uart_add_one_port(_uart_driver, port);
>>  }
>>
>> @@ -1645,12 +1743,67 @@ static const struct of_device_id msm_match_table[] = 
>> {
>>   {}
>>  };
>>
>> +#ifdef CONFIG_PM
>> +static int msm_serial_runtime_suspend(struct device *dev)
>> +{
>> + struct uart_port *port = dev_get_drvdata(dev);
>> + struct msm_port *msm_port = UART_TO_MSM(port);
>> +
>> + if (msm_port->is_uartdm)
>> + clk_disable(msm_port->pclk);
>
> ... so we can't unprepare clks here. That's unfortunate because
> clks that are ancestors of these clks will be kept prepared and
> that could lead to things like PLLs being kept enabled, etc.
Yes. clk_prepare/unprepare may sleep and we want to avoid that in runtime PM.
This is all we can do in runtime PM. suspend will achieve full
resource release though.

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

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

[Pramod Gurav]

Hi Ulf,


On 16 June 2016 at 18:05, Pramod Gurav <pramod.gu...@linaro.org> 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.gu...@linaro.org>

Any comments on this patch?

> ---
>  drivers/mmc/host/sdhci-msm.c | 57 
> 
>  1 file changed, 57 insertions(+)
>

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

[Pramod Gurav]

Hi Ulf,


On 16 June 2016 at 18:05, 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 

Any comments on this patch?

> ---
>  drivers/mmc/host/sdhci-msm.c | 57 
> 
>  1 file changed, 57 insertions(+)
>

Re: [PATCH] tty: serial: msm: Add runtime PM and system sleep support

[Pramod Gurav]

Hi,

On 17 June 2016 at 15:46, Pramod Gurav <pramod.gu...@linaro.org> wrote:
> Add runtime pm and suspend/resume callback support to serial msm
> driver so that clock resources are managed runtime to save power.
>

Any comments on this patch?

> Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
> ---
>  drivers/tty/serial/msm_serial.c | 183 
> 
>  1 file changed, 168 insertions(+), 15 deletions(-)
>

>

Re: [PATCH] tty: serial: msm: Add runtime PM and system sleep support

[Pramod Gurav]

Hi,

On 17 June 2016 at 15:46, Pramod Gurav  wrote:
> Add runtime pm and suspend/resume callback support to serial msm
> driver so that clock resources are managed runtime to save power.
>

Any comments on this patch?

> Signed-off-by: Pramod Gurav 
> ---
>  drivers/tty/serial/msm_serial.c | 183 
> 
>  1 file changed, 168 insertions(+), 15 deletions(-)
>

>

Re: [PATCH] tty: serial: msm: Add runtime PM and system sleep support

[Pramod Gurav]

On 25 August 2016 at 10:05, Andy Gross <andy.gr...@linaro.org> wrote:
> On 17 June 2016 at 05:16, Pramod Gurav <pramod.gu...@linaro.org> wrote:
>



>> +   if (msm_port->is_uartdm) {
>> +   ret = clk_enable(msm_port->pclk);
>
> Ditto here.

Thanks Andy, will include these two changes in v2.
>
>> +   if (ret)

Re: [PATCH] tty: serial: msm: Add runtime PM and system sleep support

[Pramod Gurav]

On 25 August 2016 at 10:05, Andy Gross  wrote:
> On 17 June 2016 at 05:16, Pramod Gurav  wrote:
>



>> +   if (msm_port->is_uartdm) {
>> +   ret = clk_enable(msm_port->pclk);
>
> Ditto here.

Thanks Andy, will include these two changes in v2.
>
>> +   if (ret)

[PATCH v4] dmaengine: qcom-bam-dma: Add pm_runtime support

[Pramod Gurav]

Adds pm_runtime support for BAM DMA so that clock is enabled only
when there is a transaction going on to help save power.

Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
---
Changes in v3:
- pm_runtime_get_sync returns < 0 on error hence chech for return < 0
- Move pm_runtime_get_sync/put in bam_start_dma
- Tested large size file than tested with previous patch

Changes in v2:
- Handled return values of pm_runtime_get_sync to return error
- Rework remove function

 drivers/dma/qcom/bam_dma.c | 110 -
 1 file changed, 109 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 969b481..4754891 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -48,6 +48,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "../dmaengine.h"
 #include "../virt-dma.h"
@@ -58,6 +59,8 @@ struct bam_desc_hw {
__le16 flags;
 };
 
+#define BAM_DMA_AUTOSUSPEND_DELAY 100
+
 #define DESC_FLAG_INT BIT(15)
 #define DESC_FLAG_EOT BIT(14)
 #define DESC_FLAG_EOB BIT(13)
@@ -527,12 +530,17 @@ static void bam_free_chan(struct dma_chan *chan)
struct bam_device *bdev = bchan->bdev;
u32 val;
unsigned long flags;
+   int ret;
+
+   ret = pm_runtime_get_sync(bdev->dev);
+   if (ret < 0)
+   return;
 
vchan_free_chan_resources(to_virt_chan(chan));
 
if (bchan->curr_txd) {
dev_err(bchan->bdev->dev, "Cannot free busy channel\n");
-   return;
+   goto err;
}
 
spin_lock_irqsave(>vc.lock, flags);
@@ -550,6 +558,10 @@ static void bam_free_chan(struct dma_chan *chan)
 
/* disable irq */
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_IRQ_EN));
+
+err:
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 }
 
 /**
@@ -696,11 +708,18 @@ static int bam_pause(struct dma_chan *chan)
struct bam_chan *bchan = to_bam_chan(chan);
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
+   int ret;
+
+   ret = pm_runtime_get_sync(bdev->dev);
+   if (ret < 0)
+   return ret;
 
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(1, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 1;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -715,11 +734,18 @@ static int bam_resume(struct dma_chan *chan)
struct bam_chan *bchan = to_bam_chan(chan);
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
+   int ret;
+
+   ret = pm_runtime_get_sync(bdev->dev);
+   if (ret < 0)
+   return ret;
 
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 0;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -795,6 +821,7 @@ static irqreturn_t bam_dma_irq(int irq, void *data)
 {
struct bam_device *bdev = data;
u32 clr_mask = 0, srcs = 0;
+   int ret;
 
srcs |= process_channel_irqs(bdev);
 
@@ -802,6 +829,10 @@ static irqreturn_t bam_dma_irq(int irq, void *data)
if (srcs & P_IRQ)
tasklet_schedule(>task);
 
+   ret = pm_runtime_get_sync(bdev->dev);
+   if (ret < 0)
+   return ret;
+
if (srcs & BAM_IRQ) {
clr_mask = readl_relaxed(bam_addr(bdev, 0, BAM_IRQ_STTS));
 
@@ -814,6 +845,9 @@ static irqreturn_t bam_dma_irq(int irq, void *data)
writel_relaxed(clr_mask, bam_addr(bdev, 0, BAM_IRQ_CLR));
}
 
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
+
return IRQ_HANDLED;
 }
 
@@ -893,6 +927,7 @@ static void bam_start_dma(struct bam_chan *bchan)
struct bam_desc_hw *desc;
struct bam_desc_hw *fifo = PTR_ALIGN(bchan->fifo_virt,
sizeof(struct bam_desc_hw));
+   int ret;
 
lockdep_assert_held(>vc.lock);
 
@@ -904,6 +939,10 @@ static void bam_start_dma(struct bam_chan *bchan)
async_desc = container_of(vd, struct bam_async_desc, vd);
bchan->curr_txd = async_desc;
 
+   ret = pm_runtime_get_sync(bdev->dev);
+   if (ret < 0)
+   return;
+
/* on first use, initialize the channel hardware */
if (!bchan->initialized)
bam_chan_init_hw(bchan, async_desc->dir);
@@ -946,6 +985,9 @@ static void bam_start_dma(struct bam_chan *bchan)
wmb();
writel_relaxed(bchan->tail * sizeo

[PATCH v4] dmaengine: qcom-bam-dma: Add pm_runtime support

[Pramod Gurav]

Adds pm_runtime support for BAM DMA so that clock is enabled only
when there is a transaction going on to help save power.

Signed-off-by: Pramod Gurav 
---
Changes in v3:
- pm_runtime_get_sync returns < 0 on error hence chech for return < 0
- Move pm_runtime_get_sync/put in bam_start_dma
- Tested large size file than tested with previous patch

Changes in v2:
- Handled return values of pm_runtime_get_sync to return error
- Rework remove function

 drivers/dma/qcom/bam_dma.c | 110 -
 1 file changed, 109 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 969b481..4754891 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -48,6 +48,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "../dmaengine.h"
 #include "../virt-dma.h"
@@ -58,6 +59,8 @@ struct bam_desc_hw {
__le16 flags;
 };
 
+#define BAM_DMA_AUTOSUSPEND_DELAY 100
+
 #define DESC_FLAG_INT BIT(15)
 #define DESC_FLAG_EOT BIT(14)
 #define DESC_FLAG_EOB BIT(13)
@@ -527,12 +530,17 @@ static void bam_free_chan(struct dma_chan *chan)
struct bam_device *bdev = bchan->bdev;
u32 val;
unsigned long flags;
+   int ret;
+
+   ret = pm_runtime_get_sync(bdev->dev);
+   if (ret < 0)
+   return;
 
vchan_free_chan_resources(to_virt_chan(chan));
 
if (bchan->curr_txd) {
dev_err(bchan->bdev->dev, "Cannot free busy channel\n");
-   return;
+   goto err;
}
 
spin_lock_irqsave(>vc.lock, flags);
@@ -550,6 +558,10 @@ static void bam_free_chan(struct dma_chan *chan)
 
/* disable irq */
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_IRQ_EN));
+
+err:
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 }
 
 /**
@@ -696,11 +708,18 @@ static int bam_pause(struct dma_chan *chan)
struct bam_chan *bchan = to_bam_chan(chan);
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
+   int ret;
+
+   ret = pm_runtime_get_sync(bdev->dev);
+   if (ret < 0)
+   return ret;
 
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(1, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 1;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -715,11 +734,18 @@ static int bam_resume(struct dma_chan *chan)
struct bam_chan *bchan = to_bam_chan(chan);
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
+   int ret;
+
+   ret = pm_runtime_get_sync(bdev->dev);
+   if (ret < 0)
+   return ret;
 
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 0;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -795,6 +821,7 @@ static irqreturn_t bam_dma_irq(int irq, void *data)
 {
struct bam_device *bdev = data;
u32 clr_mask = 0, srcs = 0;
+   int ret;
 
srcs |= process_channel_irqs(bdev);
 
@@ -802,6 +829,10 @@ static irqreturn_t bam_dma_irq(int irq, void *data)
if (srcs & P_IRQ)
tasklet_schedule(>task);
 
+   ret = pm_runtime_get_sync(bdev->dev);
+   if (ret < 0)
+   return ret;
+
if (srcs & BAM_IRQ) {
clr_mask = readl_relaxed(bam_addr(bdev, 0, BAM_IRQ_STTS));
 
@@ -814,6 +845,9 @@ static irqreturn_t bam_dma_irq(int irq, void *data)
writel_relaxed(clr_mask, bam_addr(bdev, 0, BAM_IRQ_CLR));
}
 
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
+
return IRQ_HANDLED;
 }
 
@@ -893,6 +927,7 @@ static void bam_start_dma(struct bam_chan *bchan)
struct bam_desc_hw *desc;
struct bam_desc_hw *fifo = PTR_ALIGN(bchan->fifo_virt,
sizeof(struct bam_desc_hw));
+   int ret;
 
lockdep_assert_held(>vc.lock);
 
@@ -904,6 +939,10 @@ static void bam_start_dma(struct bam_chan *bchan)
async_desc = container_of(vd, struct bam_async_desc, vd);
bchan->curr_txd = async_desc;
 
+   ret = pm_runtime_get_sync(bdev->dev);
+   if (ret < 0)
+   return;
+
/* on first use, initialize the channel hardware */
if (!bchan->initialized)
bam_chan_init_hw(bchan, async_desc->dir);
@@ -946,6 +985,9 @@ static void bam_start_dma(struct bam_chan *bchan)
wmb();
writel_relaxed(bchan->tail * sizeof(struct bam_des

[PATCH] tty: serial: msm: Add runtime PM and system sleep support

[Pramod Gurav]

Add runtime pm and suspend/resume callback support to serial msm
driver so that clock resources are managed runtime to save power.

Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
---
 drivers/tty/serial/msm_serial.c | 183 
 1 file changed, 168 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index b7d80bd..6b5776a 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -234,8 +235,12 @@ static void msm_stop_tx(struct uart_port *port)
 {
struct msm_port *msm_port = UART_TO_MSM(port);
 
+   if (pm_runtime_get_sync(port->dev) < 0)
+   return;
msm_port->imr &= ~UART_IMR_TXLEV;
msm_write(port, msm_port->imr, UART_IMR);
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
 }
 
 static void msm_start_tx(struct uart_port *port)
@@ -247,8 +252,12 @@ static void msm_start_tx(struct uart_port *port)
if (dma->count)
return;
 
+   if (pm_runtime_get_sync(port->dev) < 0)
+   return;
msm_port->imr |= UART_IMR_TXLEV;
msm_write(port, msm_port->imr, UART_IMR);
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
 }
 
 static void msm_reset_dm_count(struct uart_port *port, int count)
@@ -270,6 +279,8 @@ static void msm_complete_tx_dma(void *args)
unsigned int count;
u32 val;
 
+   if (pm_runtime_get_sync(port->dev) < 0)
+   return;
spin_lock_irqsave(>lock, flags);
 
/* Already stopped */
@@ -306,6 +317,8 @@ static void msm_complete_tx_dma(void *args)
msm_handle_tx(port);
 done:
spin_unlock_irqrestore(>lock, flags);
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
 }
 
 static int msm_handle_tx_dma(struct msm_port *msm_port, unsigned int count)
@@ -378,6 +391,8 @@ static void msm_complete_rx_dma(void *args)
unsigned long flags;
u32 val;
 
+   if (pm_runtime_get_sync(port->dev) < 0)
+   return;
spin_lock_irqsave(>lock, flags);
 
/* Already stopped */
@@ -433,6 +448,8 @@ done:
 
if (count)
tty_flip_buffer_push(tport);
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
 }
 
 static void msm_start_rx_dma(struct msm_port *msm_port)
@@ -507,19 +524,28 @@ static void msm_stop_rx(struct uart_port *port)
struct msm_port *msm_port = UART_TO_MSM(port);
struct msm_dma *dma = _port->rx_dma;
 
+   if (pm_runtime_get_sync(port->dev) < 0)
+   return;
msm_port->imr &= ~(UART_IMR_RXLEV | UART_IMR_RXSTALE);
msm_write(port, msm_port->imr, UART_IMR);
 
if (dma->chan)
msm_stop_dma(port, dma);
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
 }
 
 static void msm_enable_ms(struct uart_port *port)
 {
struct msm_port *msm_port = UART_TO_MSM(port);
 
+   if (pm_runtime_get_sync(port->dev) < 0)
+   return;
+
msm_port->imr |= UART_IMR_DELTA_CTS;
msm_write(port, msm_port->imr, UART_IMR);
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
 }
 
 static void msm_handle_rx_dm(struct uart_port *port, unsigned int misr)
@@ -766,6 +792,8 @@ static irqreturn_t msm_uart_irq(int irq, void *dev_id)
unsigned int misr;
u32 val;
 
+   if (pm_runtime_get_sync(port->dev) < 0)
+   return IRQ_NONE;
spin_lock_irqsave(>lock, flags);
misr = msm_read(port, UART_MISR);
msm_write(port, 0, UART_IMR); /* disable interrupt */
@@ -799,13 +827,25 @@ static irqreturn_t msm_uart_irq(int irq, void *dev_id)
 
msm_write(port, msm_port->imr, UART_IMR); /* restore interrupt */
spin_unlock_irqrestore(>lock, flags);
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
 
return IRQ_HANDLED;
 }
 
 static unsigned int msm_tx_empty(struct uart_port *port)
 {
-   return (msm_read(port, UART_SR) & UART_SR_TX_EMPTY) ? TIOCSER_TEMT : 0;
+   int ret;
+
+   ret = pm_runtime_get_sync(port->dev);
+   if (ret < 0)
+   return ret;
+
+   ret = msm_read(port, UART_SR) & UART_SR_TX_EMPTY ? TIOCSER_TEMT : 0;
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
+
+   return ret;
 }
 
 static unsigned int msm_get_mctrl(struct uart_port *port)
@@ -834,6 +874,8 @@ static void msm_set_mctrl(struct uart_port *port, unsigned 
int mctrl)
 {
  

[PATCH] tty: serial: msm: Add runtime PM and system sleep support

[Pramod Gurav]

Add runtime pm and suspend/resume callback support to serial msm
driver so that clock resources are managed runtime to save power.

Signed-off-by: Pramod Gurav 
---
 drivers/tty/serial/msm_serial.c | 183 
 1 file changed, 168 insertions(+), 15 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index b7d80bd..6b5776a 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -36,6 +36,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -234,8 +235,12 @@ static void msm_stop_tx(struct uart_port *port)
 {
struct msm_port *msm_port = UART_TO_MSM(port);
 
+   if (pm_runtime_get_sync(port->dev) < 0)
+   return;
msm_port->imr &= ~UART_IMR_TXLEV;
msm_write(port, msm_port->imr, UART_IMR);
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
 }
 
 static void msm_start_tx(struct uart_port *port)
@@ -247,8 +252,12 @@ static void msm_start_tx(struct uart_port *port)
if (dma->count)
return;
 
+   if (pm_runtime_get_sync(port->dev) < 0)
+   return;
msm_port->imr |= UART_IMR_TXLEV;
msm_write(port, msm_port->imr, UART_IMR);
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
 }
 
 static void msm_reset_dm_count(struct uart_port *port, int count)
@@ -270,6 +279,8 @@ static void msm_complete_tx_dma(void *args)
unsigned int count;
u32 val;
 
+   if (pm_runtime_get_sync(port->dev) < 0)
+   return;
spin_lock_irqsave(>lock, flags);
 
/* Already stopped */
@@ -306,6 +317,8 @@ static void msm_complete_tx_dma(void *args)
msm_handle_tx(port);
 done:
spin_unlock_irqrestore(>lock, flags);
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
 }
 
 static int msm_handle_tx_dma(struct msm_port *msm_port, unsigned int count)
@@ -378,6 +391,8 @@ static void msm_complete_rx_dma(void *args)
unsigned long flags;
u32 val;
 
+   if (pm_runtime_get_sync(port->dev) < 0)
+   return;
spin_lock_irqsave(>lock, flags);
 
/* Already stopped */
@@ -433,6 +448,8 @@ done:
 
if (count)
tty_flip_buffer_push(tport);
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
 }
 
 static void msm_start_rx_dma(struct msm_port *msm_port)
@@ -507,19 +524,28 @@ static void msm_stop_rx(struct uart_port *port)
struct msm_port *msm_port = UART_TO_MSM(port);
struct msm_dma *dma = _port->rx_dma;
 
+   if (pm_runtime_get_sync(port->dev) < 0)
+   return;
msm_port->imr &= ~(UART_IMR_RXLEV | UART_IMR_RXSTALE);
msm_write(port, msm_port->imr, UART_IMR);
 
if (dma->chan)
msm_stop_dma(port, dma);
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
 }
 
 static void msm_enable_ms(struct uart_port *port)
 {
struct msm_port *msm_port = UART_TO_MSM(port);
 
+   if (pm_runtime_get_sync(port->dev) < 0)
+   return;
+
msm_port->imr |= UART_IMR_DELTA_CTS;
msm_write(port, msm_port->imr, UART_IMR);
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
 }
 
 static void msm_handle_rx_dm(struct uart_port *port, unsigned int misr)
@@ -766,6 +792,8 @@ static irqreturn_t msm_uart_irq(int irq, void *dev_id)
unsigned int misr;
u32 val;
 
+   if (pm_runtime_get_sync(port->dev) < 0)
+   return IRQ_NONE;
spin_lock_irqsave(>lock, flags);
misr = msm_read(port, UART_MISR);
msm_write(port, 0, UART_IMR); /* disable interrupt */
@@ -799,13 +827,25 @@ static irqreturn_t msm_uart_irq(int irq, void *dev_id)
 
msm_write(port, msm_port->imr, UART_IMR); /* restore interrupt */
spin_unlock_irqrestore(>lock, flags);
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
 
return IRQ_HANDLED;
 }
 
 static unsigned int msm_tx_empty(struct uart_port *port)
 {
-   return (msm_read(port, UART_SR) & UART_SR_TX_EMPTY) ? TIOCSER_TEMT : 0;
+   int ret;
+
+   ret = pm_runtime_get_sync(port->dev);
+   if (ret < 0)
+   return ret;
+
+   ret = msm_read(port, UART_SR) & UART_SR_TX_EMPTY ? TIOCSER_TEMT : 0;
+   pm_runtime_mark_last_busy(port->dev);
+   pm_runtime_put_autosuspend(port->dev);
+
+   return ret;
 }
 
 static unsigned int msm_get_mctrl(struct uart_port *port)
@@ -834,6 +874,8 @@ static void msm_set_mctrl(struct uart_port *port, unsigned 
int mctrl)
 {
unsigned int mr;
 
+   

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

[Pramod Gurav]

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.gu...@linaro.org>
---
 drivers/mmc/host/sdhci-msm.c | 57 
 1 file changed, 57 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 0653fe7..f4394c8 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "sdhci-pltfm.h"
@@ -549,6 +550,11 @@ static int sdhci_msm_probe(struct platform_device *pdev)
if (ret)
goto clk_disable;
 
+   platform_set_drvdata(pdev, msm_host);
+
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+
return 0;
 
 clk_disable:
@@ -580,12 +586,63 @@ static int sdhci_msm_remove(struct platform_device *pdev)
return 0;
 }
 
+static int sdhci_msm_runtime_suspend(struct device *dev)
+{
+   struct sdhci_msm_host *msm_host = dev_get_drvdata(dev);
+
+   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_msm_host *msm_host = dev_get_drvdata(dev);
+   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 0;
+}
+
+static int sdhci_msm_suspend(struct device *dev)
+{
+   pm_runtime_force_suspend(dev);
+
+   return 0;
+}
+
+static int sdhci_msm_resume(struct device *dev)
+{
+   pm_runtime_force_resume(dev);
+
+   return 0;
+}
+
+static const struct dev_pm_ops sdhci_msm_pm_ops = {
+   SET_LATE_SYSTEM_SLEEP_PM_OPS(sdhci_msm_suspend, sdhci_msm_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 = _msm_pm_ops,
},
 };
 
-- 
1.8.2.1

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

[Pramod Gurav]

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 
---
 drivers/mmc/host/sdhci-msm.c | 57 
 1 file changed, 57 insertions(+)

diff --git a/drivers/mmc/host/sdhci-msm.c b/drivers/mmc/host/sdhci-msm.c
index 0653fe7..f4394c8 100644
--- a/drivers/mmc/host/sdhci-msm.c
+++ b/drivers/mmc/host/sdhci-msm.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "sdhci-pltfm.h"
@@ -549,6 +550,11 @@ static int sdhci_msm_probe(struct platform_device *pdev)
if (ret)
goto clk_disable;
 
+   platform_set_drvdata(pdev, msm_host);
+
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+
return 0;
 
 clk_disable:
@@ -580,12 +586,63 @@ static int sdhci_msm_remove(struct platform_device *pdev)
return 0;
 }
 
+static int sdhci_msm_runtime_suspend(struct device *dev)
+{
+   struct sdhci_msm_host *msm_host = dev_get_drvdata(dev);
+
+   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_msm_host *msm_host = dev_get_drvdata(dev);
+   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 0;
+}
+
+static int sdhci_msm_suspend(struct device *dev)
+{
+   pm_runtime_force_suspend(dev);
+
+   return 0;
+}
+
+static int sdhci_msm_resume(struct device *dev)
+{
+   pm_runtime_force_resume(dev);
+
+   return 0;
+}
+
+static const struct dev_pm_ops sdhci_msm_pm_ops = {
+   SET_LATE_SYSTEM_SLEEP_PM_OPS(sdhci_msm_suspend, sdhci_msm_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 = _msm_pm_ops,
},
 };
 
-- 
1.8.2.1

Re: [RFC PATCH] tty: serial: msm_serial: Don't reset uart on set_termios

[Pramod Gurav]

On 14 June 2016 at 00:32, Bjorn Andersson <bjorn.anders...@linaro.org> wrote:
> Upon opening the tty, uart_open() ends up calling msm_set_baud_rate()
> which resets the uart block. If this happens as we're coming out of
> msm_console_write() a full fifo worth of console output will be
> discarded.
>
> Cc: Stephen Boyd <sb...@codeaurora.org>
> Signed-off-by: Bjorn Andersson <bjorn.anders...@linaro.org>
> ---
>
> As reported here:
> https://bugs.96boards.org/show_bug.cgi?id=378
>
>  drivers/tty/serial/msm_serial.c | 18 --
>  1 file changed, 18 deletions(-)

Thanks for the patch.

I no longer see these corruptions with this patch. This is what I used
to see on my DB410C with debain FS:
http://paste.ubuntu.com/17319106/

Tested-by: Pramod Gurav <pramod.gu...@linaro.org>

Regards,
Pramod

Re: [RFC PATCH] tty: serial: msm_serial: Don't reset uart on set_termios

[Pramod Gurav]

On 14 June 2016 at 00:32, Bjorn Andersson  wrote:
> Upon opening the tty, uart_open() ends up calling msm_set_baud_rate()
> which resets the uart block. If this happens as we're coming out of
> msm_console_write() a full fifo worth of console output will be
> discarded.
>
> Cc: Stephen Boyd 
> Signed-off-by: Bjorn Andersson 
> ---
>
> As reported here:
> https://bugs.96boards.org/show_bug.cgi?id=378
>
>  drivers/tty/serial/msm_serial.c | 18 --
>  1 file changed, 18 deletions(-)

Thanks for the patch.

I no longer see these corruptions with this patch. This is what I used
to see on my DB410C with debain FS:
http://paste.ubuntu.com/17319106/

Tested-by: Pramod Gurav 

Regards,
Pramod

Re: [PATCH] serial_core: Change UART PM state to OFF on failure

[Pramod Gurav]

Hi Peter,

On 8 June 2016 at 22:17, Peter Hurley <pe...@hurleysoftware.com> wrote:
> Hi Pramod,
>
> On 05/06/2016 02:46 AM, Pramod Gurav wrote:
>> uart_change_pm is used to turn on the UART controller resources and
>> change UART's PM status. On failure to allocate pages the controller
>> be left in ON state. This will change the state to OFF on failure.
>>
>> Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
>> ---
>>  drivers/tty/serial/serial_core.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/serial_core.c 
>> b/drivers/tty/serial/serial_core.c
>> index 62fe368..58af2e9 100644
>> --- a/drivers/tty/serial/serial_core.c
>> +++ b/drivers/tty/serial/serial_core.c
>> @@ -156,9 +156,10 @@ static int uart_port_startup(struct tty_struct *tty, 
>> struct uart_state *state,
>>   if (!state->xmit.buf) {
>>   /* This is protected by the per port mutex */
>>   page = get_zeroed_page(GFP_KERNEL);
>> - if (!page)
>> + if (!page) {
>
> if (!uart_console(uport))
>
> Otherwise, you'll be powering off the console.
>

Agree. Should take care console is not disabled.
> Just out of curiosity, did you actually hit this error?
No, I did not. I thought we should not be leaving port power enabled
in error case.

Thanks for review and comments.

Regards,
Pramod

Re: [PATCH] serial_core: Change UART PM state to OFF on failure

[Pramod Gurav]

Hi Peter,

On 8 June 2016 at 22:17, Peter Hurley  wrote:
> Hi Pramod,
>
> On 05/06/2016 02:46 AM, Pramod Gurav wrote:
>> uart_change_pm is used to turn on the UART controller resources and
>> change UART's PM status. On failure to allocate pages the controller
>> be left in ON state. This will change the state to OFF on failure.
>>
>> Signed-off-by: Pramod Gurav 
>> ---
>>  drivers/tty/serial/serial_core.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/serial_core.c 
>> b/drivers/tty/serial/serial_core.c
>> index 62fe368..58af2e9 100644
>> --- a/drivers/tty/serial/serial_core.c
>> +++ b/drivers/tty/serial/serial_core.c
>> @@ -156,9 +156,10 @@ static int uart_port_startup(struct tty_struct *tty, 
>> struct uart_state *state,
>>   if (!state->xmit.buf) {
>>   /* This is protected by the per port mutex */
>>   page = get_zeroed_page(GFP_KERNEL);
>> - if (!page)
>> + if (!page) {
>
> if (!uart_console(uport))
>
> Otherwise, you'll be powering off the console.
>

Agree. Should take care console is not disabled.
> Just out of curiosity, did you actually hit this error?
No, I did not. I thought we should not be leaving port power enabled
in error case.

Thanks for review and comments.

Regards,
Pramod

Re: [PATCH] serial_core: Change UART PM state to OFF on failure

[Pramod Gurav]

On 6 May 2016 at 15:16, Pramod Gurav <pramod.gu...@linaro.org> wrote:
> uart_change_pm is used to turn on the UART controller resources and
> change UART's PM status. On failure to allocate pages the controller
> be left in ON state. This will change the state to OFF on failure.
>
> Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
> ---
>  drivers/tty/serial/serial_core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_core.c 
> b/drivers/tty/serial/serial_core.c
> index 62fe368..58af2e9 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -156,9 +156,10 @@ static int uart_port_startup(struct tty_struct *tty, 
> struct uart_state *state,
> if (!state->xmit.buf) {
> /* This is protected by the per port mutex */
> page = get_zeroed_page(GFP_KERNEL);
> -   if (!page)
> +   if (!page) {
> +   uart_change_pm(state, UART_PM_STATE_OFF);
> return -ENOMEM;
> -
> +   }
> state->xmit.buf = (unsigned char *) page;
> uart_circ_clear(>xmit);
> }

Greg,
Any comments on this change?

> --
> 1.8.2.1
>

Re: [PATCH] serial_core: Change UART PM state to OFF on failure

[Pramod Gurav]

On 6 May 2016 at 15:16, Pramod Gurav  wrote:
> uart_change_pm is used to turn on the UART controller resources and
> change UART's PM status. On failure to allocate pages the controller
> be left in ON state. This will change the state to OFF on failure.
>
> Signed-off-by: Pramod Gurav 
> ---
>  drivers/tty/serial/serial_core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_core.c 
> b/drivers/tty/serial/serial_core.c
> index 62fe368..58af2e9 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -156,9 +156,10 @@ static int uart_port_startup(struct tty_struct *tty, 
> struct uart_state *state,
> if (!state->xmit.buf) {
> /* This is protected by the per port mutex */
> page = get_zeroed_page(GFP_KERNEL);
> -   if (!page)
> +   if (!page) {
> +   uart_change_pm(state, UART_PM_STATE_OFF);
> return -ENOMEM;
> -
> +   }
> state->xmit.buf = (unsigned char *) page;
> uart_circ_clear(>xmit);
> }

Greg,
Any comments on this change?

> --
> 1.8.2.1
>

Re: [PATCH] usb: echi-hcd: Add ehci_setup check before echi_shutdown

[Pramod Gurav]

On 19 May 2016 at 15:42, Srinivas Kandagatla
<srinivas.kandaga...@linaro.org> wrote:



> Fixes 4bb3cad7125b ("usb: host: ehci-msm: Register usb shutdown function")
> Signed-off-by: Srinivas Kandagatla <srinivas.kandaga...@linaro.org>

Was seeing this crash while doing a reboot on db410c which is fixed
with this patch:

Tested-by: Pramod Gurav <pramod.gu...@linaro.org>

Re: [PATCH] usb: echi-hcd: Add ehci_setup check before echi_shutdown

[Pramod Gurav]

On 19 May 2016 at 15:42, Srinivas Kandagatla
 wrote:



> Fixes 4bb3cad7125b ("usb: host: ehci-msm: Register usb shutdown function")
> Signed-off-by: Srinivas Kandagatla 

Was seeing this crash while doing a reboot on db410c which is fixed
with this patch:

Tested-by: Pramod Gurav 

Re: [PATCH] usb: host: ehci-msm: Conditionally call ehci suspend/resume

[Pramod Gurav]

On 21 May 2016 at 03:05, Andy Gross <andy.gr...@linaro.org> wrote:
> This patch fixes a suspend/resume issue where the driver is blindly
> calling ehci_suspend/resume functions when the ehci hasn't been setup.
> This results in a crash during suspend/resume operations.
>
> Signed-off-by: Andy Gross <andy.gr...@linaro.org>

Fixes below crash while doing a system suspend:

root@linaro-alip:~# echo freeze > /sys/power/state
[   22.348960] PM: Syncing filesystems ... done.
[   22.387778] Freezing user space processes ... (elapsed 0.001 seconds) done.
[   22.393614] Freezing remaining freezable tasks ... (elapsed 0.001
seconds) done.
[   22.512736] mmc0: Reset 0x1 never completed.
[   22.514296] Unable to handle kernel NULL pointer dereference at
virtual address 04e8
[   22.516068] pgd = 80003817d000
[   22.524161] [04e8] *pgd=b817e003,
*pud=b817f003, *pmd=
[   22.535414] Internal error: Oops: 9606 [#1] PREEMPT SMP
[   22.535680] Modules linked in:
[   22.544183] CPU: 3 PID: 1499 Comm: bash Not tainted 4.6.0+ #65
[   22.544275] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
[   22.550006] task: 800039abd780 ti: 80003928c000 task.ti:
80003928c000
[   22.556870] PC is at ehci_suspend+0x34/0xe4
[   22.564240] LR is at ehci_msm_pm_suspend+0x2c/0x34
[   22.568232] pc : [] lr : []
pstate: a0000145

Tested-by: Pramod Gurav <pramod.gu...@linaro.org>

> ---
>  drivers/usb/host/ehci-msm.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
Regards,
Pramod

Re: [PATCH] usb: host: ehci-msm: Conditionally call ehci suspend/resume

[Pramod Gurav]

On 21 May 2016 at 03:05, Andy Gross  wrote:
> This patch fixes a suspend/resume issue where the driver is blindly
> calling ehci_suspend/resume functions when the ehci hasn't been setup.
> This results in a crash during suspend/resume operations.
>
> Signed-off-by: Andy Gross 

Fixes below crash while doing a system suspend:

root@linaro-alip:~# echo freeze > /sys/power/state
[   22.348960] PM: Syncing filesystems ... done.
[   22.387778] Freezing user space processes ... (elapsed 0.001 seconds) done.
[   22.393614] Freezing remaining freezable tasks ... (elapsed 0.001
seconds) done.
[   22.512736] mmc0: Reset 0x1 never completed.
[   22.514296] Unable to handle kernel NULL pointer dereference at
virtual address 04e8
[   22.516068] pgd = 80003817d000
[   22.524161] [04e8] *pgd=b817e003,
*pud=b817f003, *pmd=
[   22.535414] Internal error: Oops: 9606 [#1] PREEMPT SMP
[   22.535680] Modules linked in:
[   22.544183] CPU: 3 PID: 1499 Comm: bash Not tainted 4.6.0+ #65
[   22.544275] Hardware name: Qualcomm Technologies, Inc. APQ 8016 SBC (DT)
[   22.550006] task: 800039abd780 ti: 80003928c000 task.ti:
80003928c000
[   22.556870] PC is at ehci_suspend+0x34/0xe4
[   22.564240] LR is at ehci_msm_pm_suspend+0x2c/0x34
[   22.568232] pc : [] lr : []
pstate: a0000145

Tested-by: Pramod Gurav 

> ---
>  drivers/usb/host/ehci-msm.c | 14 --
>  1 file changed, 12 insertions(+), 2 deletions(-)
>
Regards,
Pramod

Re: [PATCH] tty: serial: msm: Remove duplicate handling of clocks

[Pramod Gurav]

On 11 May 2016 at 14:30, Pramod Gurav <pramod.gu...@linaro.org> wrote:
> msm_serial driver provides a .pm callback to the serial core to enable
> and disable clock resource in suspend/resume path. This function is
> also called before msm_startup. msm_startup also enables the clocks which
> is not needed. Hence remove the duplcate clock operation from msm_startup
> and msm_shutdown. Same is done in console setup to get rid of duplicate
> clock operation.
>
> Tested on DB410C console.
>
> Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
> ---
>  drivers/tty/serial/msm_serial.c | 15 ++-
>  1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
> index dcde955..73c3217 100644
> --- a/drivers/tty/serial/msm_serial.c
> +++ b/drivers/tty/serial/msm_serial.c
> @@ -959,15 +959,6 @@ static int msm_set_baud_rate(struct uart_port *port, 
> unsigned int baud,
> return baud;
>  }
>
> -static void msm_init_clock(struct uart_port *port)
> -{
> -   struct msm_port *msm_port = UART_TO_MSM(port);
> -
> -   clk_prepare_enable(msm_port->clk);
> -   clk_prepare_enable(msm_port->pclk);
> -   msm_serial_set_mnd_regs(port);
> -}
> -
>  static int msm_startup(struct uart_port *port)
>  {
> struct msm_port *msm_port = UART_TO_MSM(port);
> @@ -982,7 +973,7 @@ static int msm_startup(struct uart_port *port)
> if (unlikely(ret))
> return ret;
>
> -   msm_init_clock(port);
> +   msm_serial_set_mnd_regs(port);

Further testing with another UART port made me realize that serial
port does not work after disabling msm_port->clk clock. The RX data
part will be affected.
Confirmed from manual that this clock should never be turned of to
avoid loss of incoming data. Sorry for the noise.

>
> if (likely(port->fifosize > 12))
> rfr_level = port->fifosize - 12;
> @@ -1021,8 +1012,6 @@ static void msm_shutdown(struct uart_port *port)
> if (msm_port->is_uartdm)
> msm_release_dma(msm_port);
>
> -   clk_disable_unprepare(msm_port->clk);
> -
> free_irq(port->irq, port);
>  }
>
> @@ -1451,7 +1440,7 @@ static int __init msm_console_setup(struct console *co, 
> char *options)
> if (unlikely(!port->membase))
> return -ENXIO;
>
> -   msm_init_clock(port);
> +   msm_serial_set_mnd_regs(port);
>
> if (options)
> uart_parse_options(options, , , , );
> --
> 1.8.2.1
>

Re: [PATCH] tty: serial: msm: Remove duplicate handling of clocks

[Pramod Gurav]

On 11 May 2016 at 14:30, Pramod Gurav  wrote:
> msm_serial driver provides a .pm callback to the serial core to enable
> and disable clock resource in suspend/resume path. This function is
> also called before msm_startup. msm_startup also enables the clocks which
> is not needed. Hence remove the duplcate clock operation from msm_startup
> and msm_shutdown. Same is done in console setup to get rid of duplicate
> clock operation.
>
> Tested on DB410C console.
>
> Signed-off-by: Pramod Gurav 
> ---
>  drivers/tty/serial/msm_serial.c | 15 ++-
>  1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
> index dcde955..73c3217 100644
> --- a/drivers/tty/serial/msm_serial.c
> +++ b/drivers/tty/serial/msm_serial.c
> @@ -959,15 +959,6 @@ static int msm_set_baud_rate(struct uart_port *port, 
> unsigned int baud,
> return baud;
>  }
>
> -static void msm_init_clock(struct uart_port *port)
> -{
> -   struct msm_port *msm_port = UART_TO_MSM(port);
> -
> -   clk_prepare_enable(msm_port->clk);
> -   clk_prepare_enable(msm_port->pclk);
> -   msm_serial_set_mnd_regs(port);
> -}
> -
>  static int msm_startup(struct uart_port *port)
>  {
> struct msm_port *msm_port = UART_TO_MSM(port);
> @@ -982,7 +973,7 @@ static int msm_startup(struct uart_port *port)
> if (unlikely(ret))
> return ret;
>
> -   msm_init_clock(port);
> +   msm_serial_set_mnd_regs(port);

Further testing with another UART port made me realize that serial
port does not work after disabling msm_port->clk clock. The RX data
part will be affected.
Confirmed from manual that this clock should never be turned of to
avoid loss of incoming data. Sorry for the noise.

>
> if (likely(port->fifosize > 12))
> rfr_level = port->fifosize - 12;
> @@ -1021,8 +1012,6 @@ static void msm_shutdown(struct uart_port *port)
> if (msm_port->is_uartdm)
> msm_release_dma(msm_port);
>
> -   clk_disable_unprepare(msm_port->clk);
> -
> free_irq(port->irq, port);
>  }
>
> @@ -1451,7 +1440,7 @@ static int __init msm_console_setup(struct console *co, 
> char *options)
> if (unlikely(!port->membase))
> return -ENXIO;
>
> -   msm_init_clock(port);
> +   msm_serial_set_mnd_regs(port);
>
> if (options)
> uart_parse_options(options, , , , );
> --
> 1.8.2.1
>

Re: [PATCH] tty: serial: msm: Remove duplicate handling of clocks

[Pramod Gurav]

On 12 May 2016 at 05:48, Stephen Boyd <sb...@codeaurora.org> wrote:
> On 05/11, Pramod Gurav wrote:
>> msm_serial driver provides a .pm callback to the serial core to enable
>> and disable clock resource in suspend/resume path. This function is
>> also called before msm_startup. msm_startup also enables the clocks which
>> is not needed. Hence remove the duplcate clock operation from msm_startup
>> and msm_shutdown. Same is done in console setup to get rid of duplicate
>> clock operation.
>
> I had to check and I see that for the console case we call the
> .pm callback and don't turn it off until suspend/resume paths
> (would be nice to add suspend/resume to this driver too). I
> guess that's what you meant by this last sentence?
>
Yes the clocks would be kept ON if its console.
I am working to add suspend/resume and runtime pm as it will not
happen by default through core.
>>
>> Tested on DB410C console.
>>
>> Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
>
> Reviewed-by: Stephen Boyd <sb...@codeaurora.org>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

Re: [PATCH] tty: serial: msm: Remove duplicate handling of clocks

[Pramod Gurav]

On 12 May 2016 at 05:48, Stephen Boyd  wrote:
> On 05/11, Pramod Gurav wrote:
>> msm_serial driver provides a .pm callback to the serial core to enable
>> and disable clock resource in suspend/resume path. This function is
>> also called before msm_startup. msm_startup also enables the clocks which
>> is not needed. Hence remove the duplcate clock operation from msm_startup
>> and msm_shutdown. Same is done in console setup to get rid of duplicate
>> clock operation.
>
> I had to check and I see that for the console case we call the
> .pm callback and don't turn it off until suspend/resume paths
> (would be nice to add suspend/resume to this driver too). I
> guess that's what you meant by this last sentence?
>
Yes the clocks would be kept ON if its console.
I am working to add suspend/resume and runtime pm as it will not
happen by default through core.
>>
>> Tested on DB410C console.
>>
>> Signed-off-by: Pramod Gurav 
>
> Reviewed-by: Stephen Boyd 
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

[PATCH] tty: serial: msm: Remove duplicate handling of clocks

[Pramod Gurav]

msm_serial driver provides a .pm callback to the serial core to enable
and disable clock resource in suspend/resume path. This function is
also called before msm_startup. msm_startup also enables the clocks which
is not needed. Hence remove the duplcate clock operation from msm_startup
and msm_shutdown. Same is done in console setup to get rid of duplicate
clock operation.

Tested on DB410C console.

Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
---
 drivers/tty/serial/msm_serial.c | 15 ++-
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index dcde955..73c3217 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -959,15 +959,6 @@ static int msm_set_baud_rate(struct uart_port *port, 
unsigned int baud,
return baud;
 }
 
-static void msm_init_clock(struct uart_port *port)
-{
-   struct msm_port *msm_port = UART_TO_MSM(port);
-
-   clk_prepare_enable(msm_port->clk);
-   clk_prepare_enable(msm_port->pclk);
-   msm_serial_set_mnd_regs(port);
-}
-
 static int msm_startup(struct uart_port *port)
 {
struct msm_port *msm_port = UART_TO_MSM(port);
@@ -982,7 +973,7 @@ static int msm_startup(struct uart_port *port)
if (unlikely(ret))
return ret;
 
-   msm_init_clock(port);
+   msm_serial_set_mnd_regs(port);
 
if (likely(port->fifosize > 12))
rfr_level = port->fifosize - 12;
@@ -1021,8 +1012,6 @@ static void msm_shutdown(struct uart_port *port)
if (msm_port->is_uartdm)
msm_release_dma(msm_port);
 
-   clk_disable_unprepare(msm_port->clk);
-
free_irq(port->irq, port);
 }
 
@@ -1451,7 +1440,7 @@ static int __init msm_console_setup(struct console *co, 
char *options)
if (unlikely(!port->membase))
return -ENXIO;
 
-   msm_init_clock(port);
+   msm_serial_set_mnd_regs(port);
 
if (options)
uart_parse_options(options, , , , );
-- 
1.8.2.1

[PATCH] tty: serial: msm: Remove duplicate handling of clocks

[Pramod Gurav]

msm_serial driver provides a .pm callback to the serial core to enable
and disable clock resource in suspend/resume path. This function is
also called before msm_startup. msm_startup also enables the clocks which
is not needed. Hence remove the duplcate clock operation from msm_startup
and msm_shutdown. Same is done in console setup to get rid of duplicate
clock operation.

Tested on DB410C console.

Signed-off-by: Pramod Gurav 
---
 drivers/tty/serial/msm_serial.c | 15 ++-
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index dcde955..73c3217 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -959,15 +959,6 @@ static int msm_set_baud_rate(struct uart_port *port, 
unsigned int baud,
return baud;
 }
 
-static void msm_init_clock(struct uart_port *port)
-{
-   struct msm_port *msm_port = UART_TO_MSM(port);
-
-   clk_prepare_enable(msm_port->clk);
-   clk_prepare_enable(msm_port->pclk);
-   msm_serial_set_mnd_regs(port);
-}
-
 static int msm_startup(struct uart_port *port)
 {
struct msm_port *msm_port = UART_TO_MSM(port);
@@ -982,7 +973,7 @@ static int msm_startup(struct uart_port *port)
if (unlikely(ret))
return ret;
 
-   msm_init_clock(port);
+   msm_serial_set_mnd_regs(port);
 
if (likely(port->fifosize > 12))
rfr_level = port->fifosize - 12;
@@ -1021,8 +1012,6 @@ static void msm_shutdown(struct uart_port *port)
if (msm_port->is_uartdm)
msm_release_dma(msm_port);
 
-   clk_disable_unprepare(msm_port->clk);
-
free_irq(port->irq, port);
 }
 
@@ -1451,7 +1440,7 @@ static int __init msm_console_setup(struct console *co, 
char *options)
if (unlikely(!port->membase))
return -ENXIO;
 
-   msm_init_clock(port);
+   msm_serial_set_mnd_regs(port);
 
if (options)
uart_parse_options(options, , , , );
-- 
1.8.2.1

[PATCH] serial_core: Change UART PM state to OFF on failure

[Pramod Gurav]

uart_change_pm is used to turn on the UART controller resources and
change UART's PM status. On failure to allocate pages the controller
be left in ON state. This will change the state to OFF on failure.

Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
---
 drivers/tty/serial/serial_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 62fe368..58af2e9 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -156,9 +156,10 @@ static int uart_port_startup(struct tty_struct *tty, 
struct uart_state *state,
if (!state->xmit.buf) {
/* This is protected by the per port mutex */
page = get_zeroed_page(GFP_KERNEL);
-   if (!page)
+   if (!page) {
+   uart_change_pm(state, UART_PM_STATE_OFF);
return -ENOMEM;
-
+   }
state->xmit.buf = (unsigned char *) page;
uart_circ_clear(>xmit);
}
-- 
1.8.2.1

[PATCH] serial_core: Change UART PM state to OFF on failure

[Pramod Gurav]

uart_change_pm is used to turn on the UART controller resources and
change UART's PM status. On failure to allocate pages the controller
be left in ON state. This will change the state to OFF on failure.

Signed-off-by: Pramod Gurav 
---
 drivers/tty/serial/serial_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 62fe368..58af2e9 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -156,9 +156,10 @@ static int uart_port_startup(struct tty_struct *tty, 
struct uart_state *state,
if (!state->xmit.buf) {
/* This is protected by the per port mutex */
page = get_zeroed_page(GFP_KERNEL);
-   if (!page)
+   if (!page) {
+   uart_change_pm(state, UART_PM_STATE_OFF);
return -ENOMEM;
-
+   }
state->xmit.buf = (unsigned char *) page;
uart_circ_clear(>xmit);
}
-- 
1.8.2.1

[PATCH v3 2/2] MAINTAINERS: Update the files to include the Qualcomm DMA folder

[Pramod Gurav]

Recently all qcom dma drivers were moved a separate directory. Update
the files to include the same

Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d826f1b..2327f92 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1407,6 +1407,7 @@ L:linux-arm-...@vger.kernel.org
 L: linux-...@vger.kernel.org
 S: Maintained
 F: arch/arm/mach-qcom/
+F: drivers/dma/qcom/
 F: drivers/soc/qcom/
 F: drivers/tty/serial/msm_serial.h
 F: drivers/tty/serial/msm_serial.c
-- 
1.8.2.1

[PATCH v3 2/2] MAINTAINERS: Update the files to include the Qualcomm DMA folder

[Pramod Gurav]

Recently all qcom dma drivers were moved a separate directory. Update
the files to include the same

Signed-off-by: Pramod Gurav 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d826f1b..2327f92 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1407,6 +1407,7 @@ L:linux-arm-...@vger.kernel.org
 L: linux-...@vger.kernel.org
 S: Maintained
 F: arch/arm/mach-qcom/
+F: drivers/dma/qcom/
 F: drivers/soc/qcom/
 F: drivers/tty/serial/msm_serial.h
 F: drivers/tty/serial/msm_serial.c
-- 
1.8.2.1

[PATCH v3 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support

[Pramod Gurav]

Adds pm_runtime support for BAM DMA so that clock
is enabled only when there is a transaction going on to help
save power.

Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
---
Changes in v2:
- Handled return values of pm_runtime_get_sync to return error
- Rework remove function

 drivers/dma/qcom/bam_dma.c | 98 +-
 1 file changed, 97 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 5b427c4..da64511 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -48,6 +48,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "../dmaengine.h"
 #include "../virt-dma.h"
@@ -58,6 +59,8 @@ struct bam_desc_hw {
u16 flags;
 };
 
+#define BAM_DMA_AUTOSUSPEND_DELAY 100
+
 #define DESC_FLAG_INT BIT(15)
 #define DESC_FLAG_EOT BIT(14)
 #define DESC_FLAG_EOB BIT(13)
@@ -528,11 +531,14 @@ static void bam_free_chan(struct dma_chan *chan)
u32 val;
unsigned long flags;
 
+   if (pm_runtime_get_sync(bdev->dev))
+   return;
+
vchan_free_chan_resources(to_virt_chan(chan));
 
if (bchan->curr_txd) {
dev_err(bchan->bdev->dev, "Cannot free busy channel\n");
-   return;
+   goto err;
}
 
spin_lock_irqsave(>vc.lock, flags);
@@ -550,6 +556,10 @@ static void bam_free_chan(struct dma_chan *chan)
 
/* disable irq */
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_IRQ_EN));
+
+err:
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 }
 
 /**
@@ -695,11 +705,18 @@ static int bam_pause(struct dma_chan *chan)
struct bam_chan *bchan = to_bam_chan(chan);
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
+   int ret;
+
+   ret = pm_runtime_get_sync(bdev->dev);
+   if (ret)
+   return ret;
 
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(1, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 1;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -714,11 +731,17 @@ static int bam_resume(struct dma_chan *chan)
struct bam_chan *bchan = to_bam_chan(chan);
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
+   int ret;
 
+   ret = pm_runtime_get_sync(bdev->dev);
+   if (ret)
+   return ret;
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 0;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -967,6 +990,9 @@ static void dma_tasklet(unsigned long data)
bam_start_dma(bchan);
spin_unlock_irqrestore(>vc.lock, flags);
}
+
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 }
 
 /**
@@ -978,8 +1004,13 @@ static void dma_tasklet(unsigned long data)
 static void bam_issue_pending(struct dma_chan *chan)
 {
struct bam_chan *bchan = to_bam_chan(chan);
+   struct bam_device *bdev = bchan->bdev;
unsigned long flags;
 
+   if (pm_runtime_status_suspended(bdev->dev))
+   if (pm_runtime_get_sync(bdev->dev))
+   return;
+
spin_lock_irqsave(>vc.lock, flags);
 
/* if work pending and idle, start a transaction */
@@ -1210,6 +1241,13 @@ static int bam_dma_probe(struct platform_device *pdev)
if (ret)
goto err_unregister_dma;
 
+   pm_runtime_irq_safe(>dev);
+   pm_runtime_set_autosuspend_delay(>dev, BAM_DMA_AUTOSUSPEND_DELAY);
+   pm_runtime_use_autosuspend(>dev);
+   pm_runtime_mark_last_busy(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+
return 0;
 
 err_unregister_dma:
@@ -1230,6 +1268,8 @@ static int bam_dma_remove(struct platform_device *pdev)
struct bam_device *bdev = platform_get_drvdata(pdev);
u32 i;
 
+   pm_runtime_force_suspend(>dev);
+
of_dma_controller_free(pdev->dev.of_node);
dma_async_device_unregister(>common);
 
@@ -1257,11 +1297,67 @@ static int bam_dma_remove(struct platform_device *pdev)
return 0;
 }
 
+static int bam_dma_runtime_suspend(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+
+   clk_disable(bdev->bamclk);
+
+   return 0;
+}
+
+static int bam_dma_runtime_resume(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+   int ret;
+
+   ret = clk_enable(bdev->bamclk);
+   if (ret <

[PATCH v3 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support

[Pramod Gurav]

Adds pm_runtime support for BAM DMA so that clock
is enabled only when there is a transaction going on to help
save power.

Signed-off-by: Pramod Gurav 
---
Changes in v2:
- Handled return values of pm_runtime_get_sync to return error
- Rework remove function

 drivers/dma/qcom/bam_dma.c | 98 +-
 1 file changed, 97 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 5b427c4..da64511 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -48,6 +48,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "../dmaengine.h"
 #include "../virt-dma.h"
@@ -58,6 +59,8 @@ struct bam_desc_hw {
u16 flags;
 };
 
+#define BAM_DMA_AUTOSUSPEND_DELAY 100
+
 #define DESC_FLAG_INT BIT(15)
 #define DESC_FLAG_EOT BIT(14)
 #define DESC_FLAG_EOB BIT(13)
@@ -528,11 +531,14 @@ static void bam_free_chan(struct dma_chan *chan)
u32 val;
unsigned long flags;
 
+   if (pm_runtime_get_sync(bdev->dev))
+   return;
+
vchan_free_chan_resources(to_virt_chan(chan));
 
if (bchan->curr_txd) {
dev_err(bchan->bdev->dev, "Cannot free busy channel\n");
-   return;
+   goto err;
}
 
spin_lock_irqsave(>vc.lock, flags);
@@ -550,6 +556,10 @@ static void bam_free_chan(struct dma_chan *chan)
 
/* disable irq */
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_IRQ_EN));
+
+err:
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 }
 
 /**
@@ -695,11 +705,18 @@ static int bam_pause(struct dma_chan *chan)
struct bam_chan *bchan = to_bam_chan(chan);
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
+   int ret;
+
+   ret = pm_runtime_get_sync(bdev->dev);
+   if (ret)
+   return ret;
 
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(1, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 1;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -714,11 +731,17 @@ static int bam_resume(struct dma_chan *chan)
struct bam_chan *bchan = to_bam_chan(chan);
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
+   int ret;
 
+   ret = pm_runtime_get_sync(bdev->dev);
+   if (ret)
+   return ret;
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 0;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -967,6 +990,9 @@ static void dma_tasklet(unsigned long data)
bam_start_dma(bchan);
spin_unlock_irqrestore(>vc.lock, flags);
}
+
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 }
 
 /**
@@ -978,8 +1004,13 @@ static void dma_tasklet(unsigned long data)
 static void bam_issue_pending(struct dma_chan *chan)
 {
struct bam_chan *bchan = to_bam_chan(chan);
+   struct bam_device *bdev = bchan->bdev;
unsigned long flags;
 
+   if (pm_runtime_status_suspended(bdev->dev))
+   if (pm_runtime_get_sync(bdev->dev))
+   return;
+
spin_lock_irqsave(>vc.lock, flags);
 
/* if work pending and idle, start a transaction */
@@ -1210,6 +1241,13 @@ static int bam_dma_probe(struct platform_device *pdev)
if (ret)
goto err_unregister_dma;
 
+   pm_runtime_irq_safe(>dev);
+   pm_runtime_set_autosuspend_delay(>dev, BAM_DMA_AUTOSUSPEND_DELAY);
+   pm_runtime_use_autosuspend(>dev);
+   pm_runtime_mark_last_busy(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+
return 0;
 
 err_unregister_dma:
@@ -1230,6 +1268,8 @@ static int bam_dma_remove(struct platform_device *pdev)
struct bam_device *bdev = platform_get_drvdata(pdev);
u32 i;
 
+   pm_runtime_force_suspend(>dev);
+
of_dma_controller_free(pdev->dev.of_node);
dma_async_device_unregister(>common);
 
@@ -1257,11 +1297,67 @@ static int bam_dma_remove(struct platform_device *pdev)
return 0;
 }
 
+static int bam_dma_runtime_suspend(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+
+   clk_disable(bdev->bamclk);
+
+   return 0;
+}
+
+static int bam_dma_runtime_resume(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+   int ret;
+
+   ret = clk_enable(bdev->bamclk);
+   if (ret < 0) {
+   dev_err(dev, 

Re: [PATCH v2 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support

[Pramod Gurav]

Hi Manish,

Thanks for review.

On 3 May 2016 at 15:21, Manish Badarkhe  wrote:
> Hi Pramod
>
>> @@ -715,10 +724,13 @@ static int bam_resume(struct dma_chan *chan)
>> struct bam_device *bdev = bchan->bdev;
>> unsigned long flag;
>>
>> +   pm_runtime_get_sync(bdev->dev);
>> spin_lock_irqsave(>vc.lock, flag);
>> writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
>> bchan->paused = 0;
>> spin_unlock_irqrestore(>vc.lock, flag);
>> +   pm_runtime_mark_last_busy(bdev->dev);
>> +   pm_runtime_put_autosuspend(bdev->dev);
>>
>> return 0;
>>  }
>
> Why this function simply return 'success' without any error capture?
>
I should check return of pm_runtime_get_sync.

>> @@ -1252,16 +1278,76 @@ static int bam_dma_remove(struct platform_device 
>> *pdev)
>>
>> tasklet_kill(>task);
>>
>> +   pm_runtime_get_sync(>dev);
>> clk_disable_unprepare(bdev->bamclk);
>> +   pm_runtime_disable(>dev);
>> +   pm_runtime_put_noidle(>dev);
>> +   pm_runtime_set_suspended(>dev);
>> +
>> +   return 0;
>> +}
>
> Why this function simply return 'success' without any error capture?
Need to handle it better.

>
>> +static int bam_dma_runtime_suspend(struct device *dev)
>> +{
>> +   struct bam_device *bdev = dev_get_drvdata(dev);
>> +
>> +   clk_disable(bdev->bamclk);
>> +
>> +   return 0;
>> +}
>
> Why this function simply return 'success' without any error capture?
If probe has succeeded means bdev->bamclk is not NULL and error hence
clk_disable wont return anything.

>
>
>> +#ifdef CONFIG_PM_SLEEP
>> +static int bam_dma_suspend(struct device *dev)
>> +{
>> +   struct bam_device *bdev = dev_get_drvdata(dev);
>> +
>> +   pm_runtime_force_suspend(dev);
>> +
>> +   clk_unprepare(bdev->bamclk);
>>
>> return 0;
>>  }
>
> Why this function simply return 'success' without any error capture?
Same logic applies here as well, as pm_runtime_force_suspend and
clk_unprepare may not return error in our case once probe is through.

>
> Regards
> Manish Badarkhe

Re: [PATCH v2 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support

[Pramod Gurav]

Hi Manish,

Thanks for review.

On 3 May 2016 at 15:21, Manish Badarkhe  wrote:
> Hi Pramod
>
>> @@ -715,10 +724,13 @@ static int bam_resume(struct dma_chan *chan)
>> struct bam_device *bdev = bchan->bdev;
>> unsigned long flag;
>>
>> +   pm_runtime_get_sync(bdev->dev);
>> spin_lock_irqsave(>vc.lock, flag);
>> writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
>> bchan->paused = 0;
>> spin_unlock_irqrestore(>vc.lock, flag);
>> +   pm_runtime_mark_last_busy(bdev->dev);
>> +   pm_runtime_put_autosuspend(bdev->dev);
>>
>> return 0;
>>  }
>
> Why this function simply return 'success' without any error capture?
>
I should check return of pm_runtime_get_sync.

>> @@ -1252,16 +1278,76 @@ static int bam_dma_remove(struct platform_device 
>> *pdev)
>>
>> tasklet_kill(>task);
>>
>> +   pm_runtime_get_sync(>dev);
>> clk_disable_unprepare(bdev->bamclk);
>> +   pm_runtime_disable(>dev);
>> +   pm_runtime_put_noidle(>dev);
>> +   pm_runtime_set_suspended(>dev);
>> +
>> +   return 0;
>> +}
>
> Why this function simply return 'success' without any error capture?
Need to handle it better.

>
>> +static int bam_dma_runtime_suspend(struct device *dev)
>> +{
>> +   struct bam_device *bdev = dev_get_drvdata(dev);
>> +
>> +   clk_disable(bdev->bamclk);
>> +
>> +   return 0;
>> +}
>
> Why this function simply return 'success' without any error capture?
If probe has succeeded means bdev->bamclk is not NULL and error hence
clk_disable wont return anything.

>
>
>> +#ifdef CONFIG_PM_SLEEP
>> +static int bam_dma_suspend(struct device *dev)
>> +{
>> +   struct bam_device *bdev = dev_get_drvdata(dev);
>> +
>> +   pm_runtime_force_suspend(dev);
>> +
>> +   clk_unprepare(bdev->bamclk);
>>
>> return 0;
>>  }
>
> Why this function simply return 'success' without any error capture?
Same logic applies here as well, as pm_runtime_force_suspend and
clk_unprepare may not return error in our case once probe is through.

>
> Regards
> Manish Badarkhe

[PATCH v2 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support

[Pramod Gurav]

Adds pm_runtime support for BAM DMA so that clock
is enabled only when there is a transaction going on to help
save power.

Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
---
Changes since v1:
- Removed unnecessary extra line additions and remavals

 drivers/dma/qcom/bam_dma.c | 86 ++
 1 file changed, 86 insertions(+)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 5b427c4..f2a8b17 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -48,6 +48,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "../dmaengine.h"
 #include "../virt-dma.h"
@@ -58,6 +59,8 @@ struct bam_desc_hw {
u16 flags;
 };
 
+#define BAM_DMA_AUTOSUSPEND_DELAY 100
+
 #define DESC_FLAG_INT BIT(15)
 #define DESC_FLAG_EOT BIT(14)
 #define DESC_FLAG_EOB BIT(13)
@@ -535,6 +538,7 @@ static void bam_free_chan(struct dma_chan *chan)
return;
}
 
+   pm_runtime_get_sync(bdev->dev);
spin_lock_irqsave(>vc.lock, flags);
bam_reset_channel(bchan);
spin_unlock_irqrestore(>vc.lock, flags);
@@ -550,6 +554,8 @@ static void bam_free_chan(struct dma_chan *chan)
 
/* disable irq */
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_IRQ_EN));
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 }
 
 /**
@@ -696,10 +702,13 @@ static int bam_pause(struct dma_chan *chan)
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
 
+   pm_runtime_get_sync(bdev->dev);
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(1, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 1;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -715,10 +724,13 @@ static int bam_resume(struct dma_chan *chan)
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
 
+   pm_runtime_get_sync(bdev->dev);
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 0;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -967,6 +979,9 @@ static void dma_tasklet(unsigned long data)
bam_start_dma(bchan);
spin_unlock_irqrestore(>vc.lock, flags);
}
+
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 }
 
 /**
@@ -978,8 +993,12 @@ static void dma_tasklet(unsigned long data)
 static void bam_issue_pending(struct dma_chan *chan)
 {
struct bam_chan *bchan = to_bam_chan(chan);
+   struct bam_device *bdev = bchan->bdev;
unsigned long flags;
 
+   if (pm_runtime_status_suspended(bdev->dev))
+   pm_runtime_get_sync(bdev->dev);
+
spin_lock_irqsave(>vc.lock, flags);
 
/* if work pending and idle, start a transaction */
@@ -1210,6 +1229,13 @@ static int bam_dma_probe(struct platform_device *pdev)
if (ret)
goto err_unregister_dma;
 
+   pm_runtime_irq_safe(>dev);
+   pm_runtime_set_autosuspend_delay(>dev, BAM_DMA_AUTOSUSPEND_DELAY);
+   pm_runtime_use_autosuspend(>dev);
+   pm_runtime_mark_last_busy(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+
return 0;
 
 err_unregister_dma:
@@ -1252,16 +1278,76 @@ static int bam_dma_remove(struct platform_device *pdev)
 
tasklet_kill(>task);
 
+   pm_runtime_get_sync(>dev);
clk_disable_unprepare(bdev->bamclk);
+   pm_runtime_disable(>dev);
+   pm_runtime_put_noidle(>dev);
+   pm_runtime_set_suspended(>dev);
+
+   return 0;
+}
+
+static int bam_dma_runtime_suspend(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+
+   clk_disable(bdev->bamclk);
+
+   return 0;
+}
+
+static int bam_dma_runtime_resume(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+   int ret;
+
+   ret = clk_enable(bdev->bamclk);
+   if (ret < 0) {
+   dev_err(dev, "clk_enable failed: %d\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+#ifdef CONFIG_PM_SLEEP
+static int bam_dma_suspend(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+
+   pm_runtime_force_suspend(dev);
+
+   clk_unprepare(bdev->bamclk);
 
return 0;
 }
 
+static int bam_dma_resume(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+   int ret;
+
+   ret = clk_prepare(bdev->bamclk);
+   if (ret)
+   return ret;
+

[PATCH v2 2/2] MAINTAINERS: Update the files to include the Qualcomm DMA folder

[Pramod Gurav]

Recently all qcom dma drivers were moved a separate directory. Update
the files to include the same

Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d826f1b..2327f92 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1407,6 +1407,7 @@ L:linux-arm-...@vger.kernel.org
 L: linux-...@vger.kernel.org
 S: Maintained
 F: arch/arm/mach-qcom/
+F: drivers/dma/qcom/
 F: drivers/soc/qcom/
 F: drivers/tty/serial/msm_serial.h
 F: drivers/tty/serial/msm_serial.c
-- 
1.8.2.1

[PATCH v2 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support

[Pramod Gurav]

Adds pm_runtime support for BAM DMA so that clock
is enabled only when there is a transaction going on to help
save power.

Signed-off-by: Pramod Gurav 
---
Changes since v1:
- Removed unnecessary extra line additions and remavals

 drivers/dma/qcom/bam_dma.c | 86 ++
 1 file changed, 86 insertions(+)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 5b427c4..f2a8b17 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -48,6 +48,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "../dmaengine.h"
 #include "../virt-dma.h"
@@ -58,6 +59,8 @@ struct bam_desc_hw {
u16 flags;
 };
 
+#define BAM_DMA_AUTOSUSPEND_DELAY 100
+
 #define DESC_FLAG_INT BIT(15)
 #define DESC_FLAG_EOT BIT(14)
 #define DESC_FLAG_EOB BIT(13)
@@ -535,6 +538,7 @@ static void bam_free_chan(struct dma_chan *chan)
return;
}
 
+   pm_runtime_get_sync(bdev->dev);
spin_lock_irqsave(>vc.lock, flags);
bam_reset_channel(bchan);
spin_unlock_irqrestore(>vc.lock, flags);
@@ -550,6 +554,8 @@ static void bam_free_chan(struct dma_chan *chan)
 
/* disable irq */
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_IRQ_EN));
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 }
 
 /**
@@ -696,10 +702,13 @@ static int bam_pause(struct dma_chan *chan)
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
 
+   pm_runtime_get_sync(bdev->dev);
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(1, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 1;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -715,10 +724,13 @@ static int bam_resume(struct dma_chan *chan)
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
 
+   pm_runtime_get_sync(bdev->dev);
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 0;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -967,6 +979,9 @@ static void dma_tasklet(unsigned long data)
bam_start_dma(bchan);
spin_unlock_irqrestore(>vc.lock, flags);
}
+
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 }
 
 /**
@@ -978,8 +993,12 @@ static void dma_tasklet(unsigned long data)
 static void bam_issue_pending(struct dma_chan *chan)
 {
struct bam_chan *bchan = to_bam_chan(chan);
+   struct bam_device *bdev = bchan->bdev;
unsigned long flags;
 
+   if (pm_runtime_status_suspended(bdev->dev))
+   pm_runtime_get_sync(bdev->dev);
+
spin_lock_irqsave(>vc.lock, flags);
 
/* if work pending and idle, start a transaction */
@@ -1210,6 +1229,13 @@ static int bam_dma_probe(struct platform_device *pdev)
if (ret)
goto err_unregister_dma;
 
+   pm_runtime_irq_safe(>dev);
+   pm_runtime_set_autosuspend_delay(>dev, BAM_DMA_AUTOSUSPEND_DELAY);
+   pm_runtime_use_autosuspend(>dev);
+   pm_runtime_mark_last_busy(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+
return 0;
 
 err_unregister_dma:
@@ -1252,16 +1278,76 @@ static int bam_dma_remove(struct platform_device *pdev)
 
tasklet_kill(>task);
 
+   pm_runtime_get_sync(>dev);
clk_disable_unprepare(bdev->bamclk);
+   pm_runtime_disable(>dev);
+   pm_runtime_put_noidle(>dev);
+   pm_runtime_set_suspended(>dev);
+
+   return 0;
+}
+
+static int bam_dma_runtime_suspend(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+
+   clk_disable(bdev->bamclk);
+
+   return 0;
+}
+
+static int bam_dma_runtime_resume(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+   int ret;
+
+   ret = clk_enable(bdev->bamclk);
+   if (ret < 0) {
+   dev_err(dev, "clk_enable failed: %d\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+#ifdef CONFIG_PM_SLEEP
+static int bam_dma_suspend(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+
+   pm_runtime_force_suspend(dev);
+
+   clk_unprepare(bdev->bamclk);
 
return 0;
 }
 
+static int bam_dma_resume(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+   int ret;
+
+   ret = clk_prepare(bdev->bamclk);
+   if (ret)
+   return ret;
+
+   pm_runtime_for

[PATCH v2 2/2] MAINTAINERS: Update the files to include the Qualcomm DMA folder

[Pramod Gurav]

Recently all qcom dma drivers were moved a separate directory. Update
the files to include the same

Signed-off-by: Pramod Gurav 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d826f1b..2327f92 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1407,6 +1407,7 @@ L:linux-arm-...@vger.kernel.org
 L: linux-...@vger.kernel.org
 S: Maintained
 F: arch/arm/mach-qcom/
+F: drivers/dma/qcom/
 F: drivers/soc/qcom/
 F: drivers/tty/serial/msm_serial.h
 F: drivers/tty/serial/msm_serial.c
-- 
1.8.2.1

Re: [PATCH 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support

[Pramod Gurav]

Hi Kedar,
Thanks for having a look and comments.

On 2 May 2016 at 17:58, Appana Durga Kedareswara Rao
<appana.durga@xilinx.com> wrote:
>
> Hi,
>
> > -Original Message-
> > From: dmaengine-ow...@vger.kernel.org [mailto:dmaengine-
> > ow...@vger.kernel.org] On Behalf Of Pramod Gurav
> > Sent: Monday, May 02, 2016 5:48 PM
> > To: vinod.k...@intel.com; andy.gr...@linaro.org; ulf.hans...@linaro.org;
> > r...@rjwysocki.net; linux-arm-...@vger.kernel.org
> > Cc: linux...@vger.kernel.org; linux-kernel@vger.kernel.org;
> > dmaeng...@vger.kernel.org; stanimir.varba...@linaro.org;
> > ok...@codeaurora.org; Pramod Gurav <pramod.gu...@linaro.org>
> > Subject: [PATCH 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support
> >
> > Adds pm_runtime support for BAM DMA so that clock
> > is enabled only when there is a transaction going on to help
> > save power.
> >
> > Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
> > ---
> >  drivers/dma/qcom/bam_dma.c | 88
> > +-
> >  1 file changed, 87 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> > index 5b427c4..577f323 100644
> > --- a/drivers/dma/qcom/bam_dma.c
> > +++ b/drivers/dma/qcom/bam_dma.c
> > @@ -48,6 +48,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #include "../dmaengine.h"
> >  #include "../virt-dma.h"
> > @@ -58,6 +59,8 @@ struct bam_desc_hw {
> >   u16 flags;
> >  };
> >
> > +#define BAM_DMA_AUTOSUSPEND_DELAY 100
> > +
> >  #define DESC_FLAG_INT BIT(15)
> >  #define DESC_FLAG_EOT BIT(14)
> >  #define DESC_FLAG_EOB BIT(13)
> > @@ -535,6 +538,7 @@ static void bam_free_chan(struct dma_chan *chan)
> >   return;
> >   }
> >
> > + pm_runtime_get_sync(bdev->dev);
> >   spin_lock_irqsave(>vc.lock, flags);
> >   bam_reset_channel(bchan);
> >   spin_unlock_irqrestore(>vc.lock, flags);
> > @@ -550,6 +554,8 @@ static void bam_free_chan(struct dma_chan *chan)
> >
> >   /* disable irq */
> >   writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_IRQ_EN));
> > + pm_runtime_mark_last_busy(bdev->dev);
> > + pm_runtime_put_autosuspend(bdev->dev);
> >  }
> >
> >  /**
> > @@ -696,10 +702,13 @@ static int bam_pause(struct dma_chan *chan)
> >   struct bam_device *bdev = bchan->bdev;
> >   unsigned long flag;
> >
> > + pm_runtime_get_sync(bdev->dev);
> >   spin_lock_irqsave(>vc.lock, flag);
> >   writel_relaxed(1, bam_addr(bdev, bchan->id, BAM_P_HALT));
> >   bchan->paused = 1;
> >   spin_unlock_irqrestore(>vc.lock, flag);
> > + pm_runtime_mark_last_busy(bdev->dev);
> > + pm_runtime_put_autosuspend(bdev->dev);
> >
> >   return 0;
> >  }
> > @@ -715,10 +724,13 @@ static int bam_resume(struct dma_chan *chan)
> >   struct bam_device *bdev = bchan->bdev;
> >   unsigned long flag;
> >
> > + pm_runtime_get_sync(bdev->dev);
> >   spin_lock_irqsave(>vc.lock, flag);
> >   writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
> >   bchan->paused = 0;
> >   spin_unlock_irqrestore(>vc.lock, flag);
> > + pm_runtime_mark_last_busy(bdev->dev);
> > + pm_runtime_put_autosuspend(bdev->dev);
> >
> >   return 0;
> >  }
> > @@ -943,6 +955,7 @@ static void bam_start_dma(struct bam_chan *bchan)
> >   wmb();
> >   writel_relaxed(bchan->tail * sizeof(struct bam_desc_hw),
> >   bam_addr(bdev, bchan->id, BAM_P_EVNT_REG));
> > +
>
> Unrelated change...
>
> >  }
> >
> >  /**
> > @@ -967,6 +980,9 @@ static void dma_tasklet(unsigned long data)
> >   bam_start_dma(bchan);
> >   spin_unlock_irqrestore(>vc.lock, flags);
> >   }
> > +
> > + pm_runtime_mark_last_busy(bdev->dev);
> > + pm_runtime_put_autosuspend(bdev->dev);
> >  }
> >
> >  /**
> > @@ -978,8 +994,12 @@ static void dma_tasklet(unsigned long data)
> >  static void bam_issue_pending(struct dma_chan *chan)
> >  {
> >   struct bam_chan *bchan = to_bam_chan(chan);
> > + struct bam_device *bdev = bchan->bdev;
> >   unsigned long flags;
> >
> > + if (pm_runtime_status_suspended(bdev->dev))
>

Re: [PATCH 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support

[Pramod Gurav]

Hi Kedar,
Thanks for having a look and comments.

On 2 May 2016 at 17:58, Appana Durga Kedareswara Rao
 wrote:
>
> Hi,
>
> > -Original Message-
> > From: dmaengine-ow...@vger.kernel.org [mailto:dmaengine-
> > ow...@vger.kernel.org] On Behalf Of Pramod Gurav
> > Sent: Monday, May 02, 2016 5:48 PM
> > To: vinod.k...@intel.com; andy.gr...@linaro.org; ulf.hans...@linaro.org;
> > r...@rjwysocki.net; linux-arm-...@vger.kernel.org
> > Cc: linux...@vger.kernel.org; linux-kernel@vger.kernel.org;
> > dmaeng...@vger.kernel.org; stanimir.varba...@linaro.org;
> > ok...@codeaurora.org; Pramod Gurav 
> > Subject: [PATCH 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support
> >
> > Adds pm_runtime support for BAM DMA so that clock
> > is enabled only when there is a transaction going on to help
> > save power.
> >
> > Signed-off-by: Pramod Gurav 
> > ---
> >  drivers/dma/qcom/bam_dma.c | 88
> > +-
> >  1 file changed, 87 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
> > index 5b427c4..577f323 100644
> > --- a/drivers/dma/qcom/bam_dma.c
> > +++ b/drivers/dma/qcom/bam_dma.c
> > @@ -48,6 +48,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> >  #include "../dmaengine.h"
> >  #include "../virt-dma.h"
> > @@ -58,6 +59,8 @@ struct bam_desc_hw {
> >   u16 flags;
> >  };
> >
> > +#define BAM_DMA_AUTOSUSPEND_DELAY 100
> > +
> >  #define DESC_FLAG_INT BIT(15)
> >  #define DESC_FLAG_EOT BIT(14)
> >  #define DESC_FLAG_EOB BIT(13)
> > @@ -535,6 +538,7 @@ static void bam_free_chan(struct dma_chan *chan)
> >   return;
> >   }
> >
> > + pm_runtime_get_sync(bdev->dev);
> >   spin_lock_irqsave(>vc.lock, flags);
> >   bam_reset_channel(bchan);
> >   spin_unlock_irqrestore(>vc.lock, flags);
> > @@ -550,6 +554,8 @@ static void bam_free_chan(struct dma_chan *chan)
> >
> >   /* disable irq */
> >   writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_IRQ_EN));
> > + pm_runtime_mark_last_busy(bdev->dev);
> > + pm_runtime_put_autosuspend(bdev->dev);
> >  }
> >
> >  /**
> > @@ -696,10 +702,13 @@ static int bam_pause(struct dma_chan *chan)
> >   struct bam_device *bdev = bchan->bdev;
> >   unsigned long flag;
> >
> > + pm_runtime_get_sync(bdev->dev);
> >   spin_lock_irqsave(>vc.lock, flag);
> >   writel_relaxed(1, bam_addr(bdev, bchan->id, BAM_P_HALT));
> >   bchan->paused = 1;
> >   spin_unlock_irqrestore(>vc.lock, flag);
> > + pm_runtime_mark_last_busy(bdev->dev);
> > + pm_runtime_put_autosuspend(bdev->dev);
> >
> >   return 0;
> >  }
> > @@ -715,10 +724,13 @@ static int bam_resume(struct dma_chan *chan)
> >   struct bam_device *bdev = bchan->bdev;
> >   unsigned long flag;
> >
> > + pm_runtime_get_sync(bdev->dev);
> >   spin_lock_irqsave(>vc.lock, flag);
> >   writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
> >   bchan->paused = 0;
> >   spin_unlock_irqrestore(>vc.lock, flag);
> > + pm_runtime_mark_last_busy(bdev->dev);
> > + pm_runtime_put_autosuspend(bdev->dev);
> >
> >   return 0;
> >  }
> > @@ -943,6 +955,7 @@ static void bam_start_dma(struct bam_chan *bchan)
> >   wmb();
> >   writel_relaxed(bchan->tail * sizeof(struct bam_desc_hw),
> >   bam_addr(bdev, bchan->id, BAM_P_EVNT_REG));
> > +
>
> Unrelated change...
>
> >  }
> >
> >  /**
> > @@ -967,6 +980,9 @@ static void dma_tasklet(unsigned long data)
> >   bam_start_dma(bchan);
> >   spin_unlock_irqrestore(>vc.lock, flags);
> >   }
> > +
> > + pm_runtime_mark_last_busy(bdev->dev);
> > + pm_runtime_put_autosuspend(bdev->dev);
> >  }
> >
> >  /**
> > @@ -978,8 +994,12 @@ static void dma_tasklet(unsigned long data)
> >  static void bam_issue_pending(struct dma_chan *chan)
> >  {
> >   struct bam_chan *bchan = to_bam_chan(chan);
> > + struct bam_device *bdev = bchan->bdev;
> >   unsigned long flags;
> >
> > + if (pm_runtime_status_suspended(bdev->dev))
> > + pm_runtime_get_sync(bdev->dev);
> > +
> >   spin_lock_irqs

[PATCH 2/2] MAINTAINERS: Update the files to include the Qualcomm DMA folder

[Pramod Gurav]

Recently all qcom dma drivers were moved a separate directory. Update
the files to include the same

Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d826f1b..2327f92 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1407,6 +1407,7 @@ L:linux-arm-...@vger.kernel.org
 L: linux-...@vger.kernel.org
 S: Maintained
 F: arch/arm/mach-qcom/
+F: drivers/dma/qcom/
 F: drivers/soc/qcom/
 F: drivers/tty/serial/msm_serial.h
 F: drivers/tty/serial/msm_serial.c
-- 
1.8.2.1

[PATCH 2/2] MAINTAINERS: Update the files to include the Qualcomm DMA folder

[Pramod Gurav]

Recently all qcom dma drivers were moved a separate directory. Update
the files to include the same

Signed-off-by: Pramod Gurav 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d826f1b..2327f92 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1407,6 +1407,7 @@ L:linux-arm-...@vger.kernel.org
 L: linux-...@vger.kernel.org
 S: Maintained
 F: arch/arm/mach-qcom/
+F: drivers/dma/qcom/
 F: drivers/soc/qcom/
 F: drivers/tty/serial/msm_serial.h
 F: drivers/tty/serial/msm_serial.c
-- 
1.8.2.1

[PATCH 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support

[Pramod Gurav]

Adds pm_runtime support for BAM DMA so that clock
is enabled only when there is a transaction going on to help
save power.

Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
---
 drivers/dma/qcom/bam_dma.c | 88 +-
 1 file changed, 87 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 5b427c4..577f323 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -48,6 +48,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "../dmaengine.h"
 #include "../virt-dma.h"
@@ -58,6 +59,8 @@ struct bam_desc_hw {
u16 flags;
 };
 
+#define BAM_DMA_AUTOSUSPEND_DELAY 100
+
 #define DESC_FLAG_INT BIT(15)
 #define DESC_FLAG_EOT BIT(14)
 #define DESC_FLAG_EOB BIT(13)
@@ -535,6 +538,7 @@ static void bam_free_chan(struct dma_chan *chan)
return;
}
 
+   pm_runtime_get_sync(bdev->dev);
spin_lock_irqsave(>vc.lock, flags);
bam_reset_channel(bchan);
spin_unlock_irqrestore(>vc.lock, flags);
@@ -550,6 +554,8 @@ static void bam_free_chan(struct dma_chan *chan)
 
/* disable irq */
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_IRQ_EN));
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 }
 
 /**
@@ -696,10 +702,13 @@ static int bam_pause(struct dma_chan *chan)
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
 
+   pm_runtime_get_sync(bdev->dev);
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(1, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 1;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -715,10 +724,13 @@ static int bam_resume(struct dma_chan *chan)
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
 
+   pm_runtime_get_sync(bdev->dev);
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 0;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -943,6 +955,7 @@ static void bam_start_dma(struct bam_chan *bchan)
wmb();
writel_relaxed(bchan->tail * sizeof(struct bam_desc_hw),
bam_addr(bdev, bchan->id, BAM_P_EVNT_REG));
+
 }
 
 /**
@@ -967,6 +980,9 @@ static void dma_tasklet(unsigned long data)
bam_start_dma(bchan);
spin_unlock_irqrestore(>vc.lock, flags);
}
+
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 }
 
 /**
@@ -978,8 +994,12 @@ static void dma_tasklet(unsigned long data)
 static void bam_issue_pending(struct dma_chan *chan)
 {
struct bam_chan *bchan = to_bam_chan(chan);
+   struct bam_device *bdev = bchan->bdev;
unsigned long flags;
 
+   if (pm_runtime_status_suspended(bdev->dev))
+   pm_runtime_get_sync(bdev->dev);
+
spin_lock_irqsave(>vc.lock, flags);
 
/* if work pending and idle, start a transaction */
@@ -1210,6 +1230,13 @@ static int bam_dma_probe(struct platform_device *pdev)
if (ret)
goto err_unregister_dma;
 
+   pm_runtime_irq_safe(>dev);
+   pm_runtime_set_autosuspend_delay(>dev, BAM_DMA_AUTOSUSPEND_DELAY);
+   pm_runtime_use_autosuspend(>dev);
+   pm_runtime_mark_last_busy(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+
return 0;
 
 err_unregister_dma:
@@ -1221,7 +1248,6 @@ err_tasklet_kill:
tasklet_kill(>task);
 err_disable_clk:
clk_disable_unprepare(bdev->bamclk);
-
return ret;
 }
 
@@ -1252,16 +1278,76 @@ static int bam_dma_remove(struct platform_device *pdev)
 
tasklet_kill(>task);
 
+   pm_runtime_get_sync(>dev);
clk_disable_unprepare(bdev->bamclk);
+   pm_runtime_disable(>dev);
+   pm_runtime_put_noidle(>dev);
+   pm_runtime_set_suspended(>dev);
+
+   return 0;
+}
+
+static int bam_dma_runtime_suspend(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+
+   clk_disable(bdev->bamclk);
+
+   return 0;
+}
+
+static int bam_dma_runtime_resume(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+   int ret;
+
+   ret = clk_enable(bdev->bamclk);
+   if (ret < 0) {
+   dev_err(dev, "clk_enable failed: %d\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+#ifdef CONFIG_PM_SLEEP
+static int bam_dma_suspend(struct device *dev)
+{
+   struct bam_device 

[PATCH 1/2] dmaengine: qcom-bam-dma: Add pm_runtime support

[Pramod Gurav]

Adds pm_runtime support for BAM DMA so that clock
is enabled only when there is a transaction going on to help
save power.

Signed-off-by: Pramod Gurav 
---
 drivers/dma/qcom/bam_dma.c | 88 +-
 1 file changed, 87 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 5b427c4..577f323 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -48,6 +48,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "../dmaengine.h"
 #include "../virt-dma.h"
@@ -58,6 +59,8 @@ struct bam_desc_hw {
u16 flags;
 };
 
+#define BAM_DMA_AUTOSUSPEND_DELAY 100
+
 #define DESC_FLAG_INT BIT(15)
 #define DESC_FLAG_EOT BIT(14)
 #define DESC_FLAG_EOB BIT(13)
@@ -535,6 +538,7 @@ static void bam_free_chan(struct dma_chan *chan)
return;
}
 
+   pm_runtime_get_sync(bdev->dev);
spin_lock_irqsave(>vc.lock, flags);
bam_reset_channel(bchan);
spin_unlock_irqrestore(>vc.lock, flags);
@@ -550,6 +554,8 @@ static void bam_free_chan(struct dma_chan *chan)
 
/* disable irq */
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_IRQ_EN));
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 }
 
 /**
@@ -696,10 +702,13 @@ static int bam_pause(struct dma_chan *chan)
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
 
+   pm_runtime_get_sync(bdev->dev);
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(1, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 1;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -715,10 +724,13 @@ static int bam_resume(struct dma_chan *chan)
struct bam_device *bdev = bchan->bdev;
unsigned long flag;
 
+   pm_runtime_get_sync(bdev->dev);
spin_lock_irqsave(>vc.lock, flag);
writel_relaxed(0, bam_addr(bdev, bchan->id, BAM_P_HALT));
bchan->paused = 0;
spin_unlock_irqrestore(>vc.lock, flag);
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 
return 0;
 }
@@ -943,6 +955,7 @@ static void bam_start_dma(struct bam_chan *bchan)
wmb();
writel_relaxed(bchan->tail * sizeof(struct bam_desc_hw),
bam_addr(bdev, bchan->id, BAM_P_EVNT_REG));
+
 }
 
 /**
@@ -967,6 +980,9 @@ static void dma_tasklet(unsigned long data)
bam_start_dma(bchan);
spin_unlock_irqrestore(>vc.lock, flags);
}
+
+   pm_runtime_mark_last_busy(bdev->dev);
+   pm_runtime_put_autosuspend(bdev->dev);
 }
 
 /**
@@ -978,8 +994,12 @@ static void dma_tasklet(unsigned long data)
 static void bam_issue_pending(struct dma_chan *chan)
 {
struct bam_chan *bchan = to_bam_chan(chan);
+   struct bam_device *bdev = bchan->bdev;
unsigned long flags;
 
+   if (pm_runtime_status_suspended(bdev->dev))
+   pm_runtime_get_sync(bdev->dev);
+
spin_lock_irqsave(>vc.lock, flags);
 
/* if work pending and idle, start a transaction */
@@ -1210,6 +1230,13 @@ static int bam_dma_probe(struct platform_device *pdev)
if (ret)
goto err_unregister_dma;
 
+   pm_runtime_irq_safe(>dev);
+   pm_runtime_set_autosuspend_delay(>dev, BAM_DMA_AUTOSUSPEND_DELAY);
+   pm_runtime_use_autosuspend(>dev);
+   pm_runtime_mark_last_busy(>dev);
+   pm_runtime_set_active(>dev);
+   pm_runtime_enable(>dev);
+
return 0;
 
 err_unregister_dma:
@@ -1221,7 +1248,6 @@ err_tasklet_kill:
tasklet_kill(>task);
 err_disable_clk:
clk_disable_unprepare(bdev->bamclk);
-
return ret;
 }
 
@@ -1252,16 +1278,76 @@ static int bam_dma_remove(struct platform_device *pdev)
 
tasklet_kill(>task);
 
+   pm_runtime_get_sync(>dev);
clk_disable_unprepare(bdev->bamclk);
+   pm_runtime_disable(>dev);
+   pm_runtime_put_noidle(>dev);
+   pm_runtime_set_suspended(>dev);
+
+   return 0;
+}
+
+static int bam_dma_runtime_suspend(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+
+   clk_disable(bdev->bamclk);
+
+   return 0;
+}
+
+static int bam_dma_runtime_resume(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+   int ret;
+
+   ret = clk_enable(bdev->bamclk);
+   if (ret < 0) {
+   dev_err(dev, "clk_enable failed: %d\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+#ifdef CONFIG_PM_SLEEP
+static int bam_dma_suspend(struct device *dev)
+{
+   struct bam_device *bdev = dev_get_drvdata(dev);
+
+   pm

[PATCH 1/2] spi: qup: Handle clocks in pm_runtime suspend and resume

[Pramod Gurav]

Clocks must ne disabled in pm_runtime to achieve some power saving.
Enable the clocks when the device is runtime resumed during a transfer.

Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
---
 drivers/spi/spi-qup.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index 810a7fa..e42ff61 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -937,6 +937,10 @@ static int spi_qup_pm_suspend_runtime(struct device 
*device)
config = readl(controller->base + QUP_CONFIG);
config |= QUP_CONFIG_CLOCK_AUTO_GATE;
writel_relaxed(config, controller->base + QUP_CONFIG);
+
+   clk_disable_unprepare(controller->cclk);
+   clk_disable_unprepare(controller->iclk);
+
return 0;
 }
 
@@ -945,6 +949,15 @@ static int spi_qup_pm_resume_runtime(struct device *device)
struct spi_master *master = dev_get_drvdata(device);
struct spi_qup *controller = spi_master_get_devdata(master);
u32 config;
+   int ret;
+
+   ret = clk_prepare_enable(controller->iclk);
+   if (ret)
+   return ret;
+
+   ret = clk_prepare_enable(controller->cclk);
+   if (ret)
+   return ret;
 
/* Disable clocks auto gaiting */
config = readl_relaxed(controller->base + QUP_CONFIG);
-- 
1.8.2.1

[PATCH 1/2] spi: qup: Handle clocks in pm_runtime suspend and resume

[Pramod Gurav]

Clocks must ne disabled in pm_runtime to achieve some power saving.
Enable the clocks when the device is runtime resumed during a transfer.

Signed-off-by: Pramod Gurav 
---
 drivers/spi/spi-qup.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index 810a7fa..e42ff61 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -937,6 +937,10 @@ static int spi_qup_pm_suspend_runtime(struct device 
*device)
config = readl(controller->base + QUP_CONFIG);
config |= QUP_CONFIG_CLOCK_AUTO_GATE;
writel_relaxed(config, controller->base + QUP_CONFIG);
+
+   clk_disable_unprepare(controller->cclk);
+   clk_disable_unprepare(controller->iclk);
+
return 0;
 }
 
@@ -945,6 +949,15 @@ static int spi_qup_pm_resume_runtime(struct device *device)
struct spi_master *master = dev_get_drvdata(device);
struct spi_qup *controller = spi_master_get_devdata(master);
u32 config;
+   int ret;
+
+   ret = clk_prepare_enable(controller->iclk);
+   if (ret)
+   return ret;
+
+   ret = clk_prepare_enable(controller->cclk);
+   if (ret)
+   return ret;
 
/* Disable clocks auto gaiting */
config = readl_relaxed(controller->base + QUP_CONFIG);
-- 
1.8.2.1

[PATCH 2/2] spi: qup: Add spi_master_put in remove function

[Pramod Gurav]

Release memory allocated for spi master by calling spi_master_put in
.remove function.

Signed-off-by: Pramod Gurav <pramod.gu...@linaro.org>
---
 drivers/spi/spi-qup.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index e42ff61..c338ef1 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -1030,6 +1030,8 @@ static int spi_qup_remove(struct platform_device *pdev)
 
pm_runtime_put_noidle(>dev);
pm_runtime_disable(>dev);
+   spi_master_put(master);
+
return 0;
 }
 
-- 
1.8.2.1

[PATCH 2/2] spi: qup: Add spi_master_put in remove function

[Pramod Gurav]

Release memory allocated for spi master by calling spi_master_put in
.remove function.

Signed-off-by: Pramod Gurav 
---
 drivers/spi/spi-qup.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c
index e42ff61..c338ef1 100644
--- a/drivers/spi/spi-qup.c
+++ b/drivers/spi/spi-qup.c
@@ -1030,6 +1030,8 @@ static int spi_qup_remove(struct platform_device *pdev)
 
pm_runtime_put_noidle(>dev);
pm_runtime_disable(>dev);
+   spi_master_put(master);
+
return 0;
 }
 
-- 
1.8.2.1

[PATCH 2/2] dmaengine: qcom_bam_dma: Bypass BAM init if not managed locally

[Pramod Gurav]

On some QOCM platforms BAM control registers are managed remotely
hence can not be accessed by application processor for writes. Pass
a DT property qcom,bam_ctrl_remote to declare the same to skip bam_init.

Move the pipe number initialisation from bam_init to probe functiom
as it should be done for all platforms.

Signed-off-by: Pramod Gurav <gpra...@codeaurora.org>
---
 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt |  3 +++
 drivers/dma/qcom/bam_dma.c | 18 +++---
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt 
b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
index 1c9d48e..46e33ae 100644
--- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
+++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
@@ -13,6 +13,8 @@ Required properties:
 - clock-names: must contain "bam_clk" entry
 - qcom,ee : indicates the active Execution Environment identifier (0-7) used in
   the secure world.
+- qcom,bam_ctrl_remote: Use when BAM global device control is managed remotely
+  and not locally by the application processor.
 
 Example:
 
@@ -24,6 +26,7 @@ Example:
clock-names = "bam_clk";
#dma-cells = <1>;
qcom,ee = <0>;
+   qcom,bam_ctrl_remote;
};
 
 DMA clients must use the format described in the dma.txt file, using a two cell
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 0880345..04dd446 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -384,6 +384,7 @@ struct bam_device {
struct device_dma_parameters dma_parms;
struct bam_chan *channels;
u32 num_channels;
+   bool bam_ctrl_remote;
 
/* execution environment ID, from DT */
u32 ee;
@@ -1036,9 +1037,6 @@ static int bam_init(struct bam_device *bdev)
if (bdev->ee >= val)
return -EINVAL;
 
-   val = readl_relaxed(bam_addr(bdev, 0, BAM_NUM_PIPES));
-   bdev->num_channels = val & BAM_NUM_PIPES_MASK;
-
/* s/w reset bam */
/* after reset all pipes are disabled and idle */
val = readl_relaxed(bam_addr(bdev, 0, BAM_CTRL));
@@ -1095,7 +1093,7 @@ static int bam_dma_probe(struct platform_device *pdev)
struct bam_device *bdev;
const struct of_device_id *match;
struct resource *iores;
-   int ret, i;
+   int ret, i, val;
 
bdev = devm_kzalloc(>dev, sizeof(*bdev), GFP_KERNEL);
if (!bdev)
@@ -1136,9 +1134,15 @@ static int bam_dma_probe(struct platform_device *pdev)
return ret;
}
 
-   ret = bam_init(bdev);
-   if (ret)
-   goto err_disable_clk;
+   val = readl_relaxed(bam_addr(bdev, 0, BAM_NUM_PIPES));
+   bdev->num_channels = val & BAM_NUM_PIPES_MASK;
+
+   bdev->bam_ctrl_remote = of_property_read_bool(pdev->dev.of_node,
+   "qcom,bam_ctrl_remote");
+   if (bdev->bam_ctrl_remote != true)
+   ret = bam_init(bdev);
+   if (ret)
+   goto err_disable_clk;
 
tasklet_init(>task, dma_tasklet, (unsigned long)bdev);
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 2/2] dmaengine: qcom_bam_dma: Bypass BAM init if not managed locally

[Pramod Gurav]

On some QOCM platforms BAM control registers are managed remotely
hence can not be accessed by application processor for writes. Pass
a DT property qcom,bam_ctrl_remote to declare the same to skip bam_init.

Move the pipe number initialisation from bam_init to probe functiom
as it should be done for all platforms.

Signed-off-by: Pramod Gurav 
---
 Documentation/devicetree/bindings/dma/qcom_bam_dma.txt |  3 +++
 drivers/dma/qcom/bam_dma.c | 18 +++---
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt 
b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
index 1c9d48e..46e33ae 100644
--- a/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
+++ b/Documentation/devicetree/bindings/dma/qcom_bam_dma.txt
@@ -13,6 +13,8 @@ Required properties:
 - clock-names: must contain "bam_clk" entry
 - qcom,ee : indicates the active Execution Environment identifier (0-7) used in
   the secure world.
+- qcom,bam_ctrl_remote: Use when BAM global device control is managed remotely
+  and not locally by the application processor.
 
 Example:
 
@@ -24,6 +26,7 @@ Example:
clock-names = "bam_clk";
#dma-cells = <1>;
qcom,ee = <0>;
+   qcom,bam_ctrl_remote;
};
 
 DMA clients must use the format described in the dma.txt file, using a two cell
diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index 0880345..04dd446 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -384,6 +384,7 @@ struct bam_device {
struct device_dma_parameters dma_parms;
struct bam_chan *channels;
u32 num_channels;
+   bool bam_ctrl_remote;
 
/* execution environment ID, from DT */
u32 ee;
@@ -1036,9 +1037,6 @@ static int bam_init(struct bam_device *bdev)
if (bdev->ee >= val)
return -EINVAL;
 
-   val = readl_relaxed(bam_addr(bdev, 0, BAM_NUM_PIPES));
-   bdev->num_channels = val & BAM_NUM_PIPES_MASK;
-
/* s/w reset bam */
/* after reset all pipes are disabled and idle */
val = readl_relaxed(bam_addr(bdev, 0, BAM_CTRL));
@@ -1095,7 +1093,7 @@ static int bam_dma_probe(struct platform_device *pdev)
struct bam_device *bdev;
const struct of_device_id *match;
struct resource *iores;
-   int ret, i;
+   int ret, i, val;
 
bdev = devm_kzalloc(>dev, sizeof(*bdev), GFP_KERNEL);
if (!bdev)
@@ -1136,9 +1134,15 @@ static int bam_dma_probe(struct platform_device *pdev)
return ret;
}
 
-   ret = bam_init(bdev);
-   if (ret)
-   goto err_disable_clk;
+   val = readl_relaxed(bam_addr(bdev, 0, BAM_NUM_PIPES));
+   bdev->num_channels = val & BAM_NUM_PIPES_MASK;
+
+   bdev->bam_ctrl_remote = of_property_read_bool(pdev->dev.of_node,
+   "qcom,bam_ctrl_remote");
+   if (bdev->bam_ctrl_remote != true)
+   ret = bam_init(bdev);
+   if (ret)
+   goto err_disable_clk;
 
tasklet_init(>task, dma_tasklet, (unsigned long)bdev);
 
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 1/2] dmaengine: qcom_bam_dma: Clear IRQ only if its set

[Pramod Gurav]

Clear the BAM IRQ bit only if there is a BAM interrupt.

Signed-off-by: Pramod Gurav <gpra...@codeaurora.org>
---
 drivers/dma/qcom/bam_dma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index d5e0a9c..0880345 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -807,7 +807,8 @@ static irqreturn_t bam_dma_irq(int irq, void *data)
/* don't allow reorder of the various accesses to the BAM registers */
mb();
 
-   writel_relaxed(clr_mask, bam_addr(bdev, 0, BAM_IRQ_CLR));
+   if (srcs & BAM_IRQ)
+   writel_relaxed(clr_mask, bam_addr(bdev, 0, BAM_IRQ_CLR));
 
return IRQ_HANDLED;
 }
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 1/2] dmaengine: qcom_bam_dma: Clear IRQ only if its set

[Pramod Gurav]

Clear the BAM IRQ bit only if there is a BAM interrupt.

Signed-off-by: Pramod Gurav 
---
 drivers/dma/qcom/bam_dma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/qcom/bam_dma.c b/drivers/dma/qcom/bam_dma.c
index d5e0a9c..0880345 100644
--- a/drivers/dma/qcom/bam_dma.c
+++ b/drivers/dma/qcom/bam_dma.c
@@ -807,7 +807,8 @@ static irqreturn_t bam_dma_irq(int irq, void *data)
/* don't allow reorder of the various accesses to the BAM registers */
mb();
 
-   writel_relaxed(clr_mask, bam_addr(bdev, 0, BAM_IRQ_CLR));
+   if (srcs & BAM_IRQ)
+   writel_relaxed(clr_mask, bam_addr(bdev, 0, BAM_IRQ_CLR));
 
return IRQ_HANDLED;
 }
-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 0/2] Bypass BAM init if Remotely controlled

[Pramod Gurav]

On some QOCM platforms(eg 8996) BAM control registers are managed remotely
hence can not be accessed by application processor for writes. So skip the 
bam_init
for any such platform if DT property is set.

Tested on 8996 (BAM Global control is through remote) and DB410C boards.
Tested with i2c DMA on these targets which uses BAM as DMA controller.

Pramod Gurav (2):
  dmaengine: qcom_bam_dma: Clear IRQ only if its set
  dmaengine: qcom_bam_dma: Bypass BAM init if not managed locally

 .../devicetree/bindings/dma/qcom_bam_dma.txt|  3 +++
 drivers/dma/qcom/bam_dma.c  | 21 +
 2 files changed, 16 insertions(+), 8 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

[PATCH 0/2] Bypass BAM init if Remotely controlled

[Pramod Gurav]

On some QOCM platforms(eg 8996) BAM control registers are managed remotely
hence can not be accessed by application processor for writes. So skip the 
bam_init
for any such platform if DT property is set.

Tested on 8996 (BAM Global control is through remote) and DB410C boards.
Tested with i2c DMA on these targets which uses BAM as DMA controller.

Pramod Gurav (2):
  dmaengine: qcom_bam_dma: Clear IRQ only if its set
  dmaengine: qcom_bam_dma: Bypass BAM init if not managed locally

 .../devicetree/bindings/dma/qcom_bam_dma.txt|  3 +++
 drivers/dma/qcom/bam_dma.c  | 21 +
 2 files changed, 16 insertions(+), 8 deletions(-)

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation

Re: [PATCH] drivers: i2c: Fix qup fifo handling

[Pramod Gurav]

On Fri, February 26, 2016 9:28 pm, Sricharan R wrote:
> After the addition of V2 support, there was a regression observed
> when testing it on MSM8996. The reason is driver puts the controller
> in to RUN state and writes the data to be 'tx' ed in fifo. But controller
> has to be put in to 'PAUSE' state and data has to written to fifo. Then
> should be put in to 'RUN' state separately.
>
> Signed-off-by: Sricharan R <sricha...@codeaurora.org>

Tested-by: Pramod Gurav <gpra...@codeaurora.org>

Regards,
Pramod
> ---
>  drivers/i2c/busses/i2c-qup.c | 46
> +---
>  1 file changed, 39 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
> index 30f3a2b..23eaabb 100644
> --- a/drivers/i2c/busses/i2c-qup.c
> +++ b/drivers/i2c/busses/i2c-qup.c
> @@ -372,6 +372,38 @@ static void qup_i2c_set_write_mode(struct qup_i2c_dev
> *qup, struct i2c_msg *msg)
>   }
>  }
>
> +static int check_for_fifo_space(struct qup_i2c_dev *qup)
> +{
> + int ret;
> +
> + ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
> + if (ret)
> + goto out;
> +
> + ret = qup_i2c_wait_ready(qup, QUP_OUT_FULL,
> +  RESET_BIT, 4 * ONE_BYTE);
> + if (ret) {
> + /* Fifo is full. Drain out the fifo */
> + ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> + if (ret)
> + goto out;
> +
> + ret = qup_i2c_wait_ready(qup, QUP_OUT_NOT_EMPTY,
> +  RESET_BIT, 256 * ONE_BYTE);
> + if (ret) {
> + dev_err(qup->dev, "timeout for fifo out full");
> + goto out;
> + }
> +
> + ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
> + if (ret)
> + goto out;
> + }
> +
> +out:
> + return ret;
> +}
> +
>  static int qup_i2c_issue_write(struct qup_i2c_dev *qup, struct i2c_msg
> *msg)
>  {
>   u32 addr = msg->addr << 1;
> @@ -390,8 +422,7 @@ static int qup_i2c_issue_write(struct qup_i2c_dev
> *qup, struct i2c_msg *msg)
>
>   while (qup->pos < msg->len) {
>   /* Check that there's space in the FIFO for our pair */
> - ret = qup_i2c_wait_ready(qup, QUP_OUT_FULL, RESET_BIT,
> -  4 * ONE_BYTE);
> + ret = check_for_fifo_space(qup);
>   if (ret)
>   return ret;
>
> @@ -413,6 +444,8 @@ static int qup_i2c_issue_write(struct qup_i2c_dev
> *qup, struct i2c_msg *msg)
>   idx++;
>   }
>
> + ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> +
>   return ret;
>  }
>
> @@ -441,12 +474,9 @@ static int qup_i2c_send_data(struct qup_i2c_dev *qup,
> int tlen, u8 *tbuf,
>   int ret = 0;
>
>   while (len > 0) {
> - ret = qup_i2c_wait_ready(qup, QUP_OUT_FULL,
> -  RESET_BIT, 4 * ONE_BYTE);
> - if (ret) {
> - dev_err(qup->dev, "timeout for fifo out full");
> + ret = check_for_fifo_space(qup);
> + if (ret)
>   return ret;
> - }
>
>   t = (len >= 4) ? 4 : len;
>
> @@ -465,6 +495,8 @@ static int qup_i2c_send_data(struct qup_i2c_dev *qup,
> int tlen, u8 *tbuf,
>   len -= 4;
>   }
>
> + ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> +
>   return ret;
>  }
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>


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

Re: [PATCH] drivers: i2c: Fix qup fifo handling

[Pramod Gurav]

On Fri, February 26, 2016 9:28 pm, Sricharan R wrote:
> After the addition of V2 support, there was a regression observed
> when testing it on MSM8996. The reason is driver puts the controller
> in to RUN state and writes the data to be 'tx' ed in fifo. But controller
> has to be put in to 'PAUSE' state and data has to written to fifo. Then
> should be put in to 'RUN' state separately.
>
> Signed-off-by: Sricharan R 

Tested-by: Pramod Gurav 

Regards,
Pramod
> ---
>  drivers/i2c/busses/i2c-qup.c | 46
> +---
>  1 file changed, 39 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
> index 30f3a2b..23eaabb 100644
> --- a/drivers/i2c/busses/i2c-qup.c
> +++ b/drivers/i2c/busses/i2c-qup.c
> @@ -372,6 +372,38 @@ static void qup_i2c_set_write_mode(struct qup_i2c_dev
> *qup, struct i2c_msg *msg)
>   }
>  }
>
> +static int check_for_fifo_space(struct qup_i2c_dev *qup)
> +{
> + int ret;
> +
> + ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
> + if (ret)
> + goto out;
> +
> + ret = qup_i2c_wait_ready(qup, QUP_OUT_FULL,
> +  RESET_BIT, 4 * ONE_BYTE);
> + if (ret) {
> + /* Fifo is full. Drain out the fifo */
> + ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> + if (ret)
> + goto out;
> +
> + ret = qup_i2c_wait_ready(qup, QUP_OUT_NOT_EMPTY,
> +  RESET_BIT, 256 * ONE_BYTE);
> + if (ret) {
> + dev_err(qup->dev, "timeout for fifo out full");
> + goto out;
> + }
> +
> + ret = qup_i2c_change_state(qup, QUP_PAUSE_STATE);
> + if (ret)
> + goto out;
> + }
> +
> +out:
> + return ret;
> +}
> +
>  static int qup_i2c_issue_write(struct qup_i2c_dev *qup, struct i2c_msg
> *msg)
>  {
>   u32 addr = msg->addr << 1;
> @@ -390,8 +422,7 @@ static int qup_i2c_issue_write(struct qup_i2c_dev
> *qup, struct i2c_msg *msg)
>
>   while (qup->pos < msg->len) {
>   /* Check that there's space in the FIFO for our pair */
> - ret = qup_i2c_wait_ready(qup, QUP_OUT_FULL, RESET_BIT,
> -  4 * ONE_BYTE);
> + ret = check_for_fifo_space(qup);
>   if (ret)
>   return ret;
>
> @@ -413,6 +444,8 @@ static int qup_i2c_issue_write(struct qup_i2c_dev
> *qup, struct i2c_msg *msg)
>   idx++;
>   }
>
> + ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> +
>   return ret;
>  }
>
> @@ -441,12 +474,9 @@ static int qup_i2c_send_data(struct qup_i2c_dev *qup,
> int tlen, u8 *tbuf,
>   int ret = 0;
>
>   while (len > 0) {
> - ret = qup_i2c_wait_ready(qup, QUP_OUT_FULL,
> -  RESET_BIT, 4 * ONE_BYTE);
> - if (ret) {
> - dev_err(qup->dev, "timeout for fifo out full");
> + ret = check_for_fifo_space(qup);
> + if (ret)
>   return ret;
> - }
>
>   t = (len >= 4) ? 4 : len;
>
> @@ -465,6 +495,8 @@ static int qup_i2c_send_data(struct qup_i2c_dev *qup,
> int tlen, u8 *tbuf,
>   len -= 4;
>   }
>
> + ret = qup_i2c_change_state(qup, QUP_RUN_STATE);
> +
>   return ret;
>  }
>
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
>
>
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>


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

Re: [PATCH v3 2/3] tty: serial: msm: Remove duplicate operations on clocks in startup/shutdown

[Pramod Gurav]

Thanks Stephen for review.

On Fri, April 10, 2015 11:33 pm, Stephen Boyd wrote:
> On 04/10/15 05:19, Pramod Gurav wrote:
>> @@ -683,8 +679,7 @@ static void msm_power(struct uart_port *port,
>> unsigned int state,
>>
>>  switch (state) {
>>  case 0:
>> -clk_prepare_enable(msm_port->clk);
>> -clk_prepare_enable(msm_port->pclk);
>> +msm_init_clock(port);
>
> Hm... now we would call msm_serial_set_mnd_regs() whenever we power on
> the port? Presumably we only need to do that once when we probe (or when
> we resume from a sleep state that resets the registers, i.e.
> hibernation) but I guess we're getting saved by the fact that the
> if/else if pair in msm_serial_set_mnd_regs_from_uartclk would never be
> true after the first time we call it?

I tried replacing msm_init_clock() call with msm_serial_set_mnd_regs() in
msm_startup() as msm_startup gets called just after msm_power() so that
clk_prepare_enable() is followed by mnd settings. But it does not get the
kernel booted for some reason.

So, can I get a acked-by for this patch or you still think it can be done
in a better way?
>
>>  break;
>>  case 3:
>>  clk_disable_unprepare(msm_port->clk);
>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm"
> in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 2/3] tty: serial: msm: Remove duplicate operations on clocks in startup/shutdown

[Pramod Gurav]

Thanks Stephen for review.

On Fri, April 10, 2015 11:33 pm, Stephen Boyd wrote:
 On 04/10/15 05:19, Pramod Gurav wrote:
 @@ -683,8 +679,7 @@ static void msm_power(struct uart_port *port,
 unsigned int state,

  switch (state) {
  case 0:
 -clk_prepare_enable(msm_port-clk);
 -clk_prepare_enable(msm_port-pclk);
 +msm_init_clock(port);

 Hm... now we would call msm_serial_set_mnd_regs() whenever we power on
 the port? Presumably we only need to do that once when we probe (or when
 we resume from a sleep state that resets the registers, i.e.
 hibernation) but I guess we're getting saved by the fact that the
 if/else if pair in msm_serial_set_mnd_regs_from_uartclk would never be
 true after the first time we call it?

I tried replacing msm_init_clock() call with msm_serial_set_mnd_regs() in
msm_startup() as msm_startup gets called just after msm_power() so that
clk_prepare_enable() is followed by mnd settings. But it does not get the
kernel booted for some reason.

So, can I get a acked-by for this patch or you still think it can be done
in a better way?

  break;
  case 3:
  clk_disable_unprepare(msm_port-clk);


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

 --
 To unsubscribe from this list: send the line unsubscribe linux-arm-msm
 in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html



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

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] tty: serial: msm: Disable pclk when port is closed

[Pramod Gurav]

On Thu, April 9, 2015 5:21 am, Stephen Boyd wrote:
> On 04/08/15 06:28, Pramod Gurav wrote:
>> Disable the pclk when tty port is closed by user space.
>>
>> Signed-off-by: Pramod Gurav 
>> ---
>>  drivers/tty/serial/msm_serial.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/tty/serial/msm_serial.c
>> b/drivers/tty/serial/msm_serial.c
>> index 4c1e9ea..f38565c 100644
>> --- a/drivers/tty/serial/msm_serial.c
>> +++ b/drivers/tty/serial/msm_serial.c
>> @@ -523,6 +523,7 @@ static void msm_shutdown(struct uart_port *port)
>>  msm_write(port, 0, UART_IMR); /* disable interrupts */
>>
>>  clk_disable_unprepare(msm_port->clk);
>> +clk_disable_unprepare(msm_port->pclk);
>>
>>  free_irq(port->irq, port);
>>  }
>
> It's not clear to me at all when this clock is enabled and when it's
> disabled during the lifetime of this driver. For example, why do we have
> a .pm op to turn clocks on and off? Shouldn't they already be on? Can
> you please explain when the clocks are turned on and off and what
> userspace actions cause that to happen? Looking at drivers like
> amba-pl010.c I don't see any .pm op, just a
> clk_prepare_enable/clk_disable_unprepare pair in the startup and
> shutdown ops.


When a userspce opens a serial port the uart_startup (in serial_core)
function is executed which changes the uart_pm state to UART_PM_STATE_ON.
So when this port is release/closed by the application
uart_close(serial_core) changes the uart_pm state to UART_PM_STATE_OFF if
its not console. But it is not done in uart_shutdown function.

So ideally clk_prepare_enable/clk_disable_unprepare must be called in .pm
only. So, we can get rid of these operations from msm_startup function as
these will be called anyway using .pm ops.

About .pm in uart_ops, there are few drivers which have it for an example,
atmel_serial, sh-sci etc. That is where they do
clk_prepare_enable/clk_disable_unprepare. And moreover when there is
suspend across system these function will handled through .pm.


>
> Minus my confusion of why our clocking is complicated, it looks correct
> to me to do this, so
>
> Reviewed-by: Stephen Boyd 

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

Pramod

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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 2/2] tty: serial: msm: Disable pclk when port is closed

[Pramod Gurav]

Disable the pclk when tty port is closed by user space.

Signed-off-by: Pramod Gurav 
---
 drivers/tty/serial/msm_serial.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index 4c1e9ea..f38565c 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -523,6 +523,7 @@ static void msm_shutdown(struct uart_port *port)
msm_write(port, 0, UART_IMR); /* disable interrupts */
 
clk_disable_unprepare(msm_port->clk);
+   clk_disable_unprepare(msm_port->pclk);
 
free_irq(port->irq, port);
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 1/2] tty: serial: msm: Add mask value for UART_DM registers

[Pramod Gurav]

The bit masks for RFR_LEVEL1 and STALE_TIMEOUT_MSB values in MR1 and
IPR registers respectively are different for UART and UART_DM hardware
cores. We have been using UART core mask values for these. Add the same
for UART_DM core.

There is no bit setting as UART_IPR_RXSTALE_LAST for UART_DM core so do
it only for UART core.

Signed-off-by: Pramod Gurav 

---
Changes since last version:
 - Added new macro fo UART_DM_MR1_AUTO_RFR_LEVEL1 instead of modifying existing.
 - Added a new macro for IPR register as it is also different in UART_DM
 - Changed subject line
 - Removed change log from message 

 drivers/tty/serial/msm_serial.c | 19 +++
 drivers/tty/serial/msm_serial.h |  2 ++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index b73889c..4c1e9ea 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -432,8 +432,13 @@ static int msm_set_baud_rate(struct uart_port *port, 
unsigned int baud)
/* RX stale watermark */
rxstale = entry->rxstale;
watermark = UART_IPR_STALE_LSB & rxstale;
-   watermark |= UART_IPR_RXSTALE_LAST;
-   watermark |= UART_IPR_STALE_TIMEOUT_MSB & (rxstale << 2);
+   if (msm_port->is_uartdm)
+   watermark |= UART_DM_IPR_STALE_TIMEOUT_MSB & (rxstale << 2);
+   else {
+   watermark |= UART_IPR_RXSTALE_LAST;
+   watermark |= UART_IPR_STALE_TIMEOUT_MSB & (rxstale << 2);
+   }
+
msm_write(port, watermark, UART_IPR);
 
/* set RX watermark */
@@ -496,9 +501,15 @@ static int msm_startup(struct uart_port *port)
 
/* set automatic RFR level */
data = msm_read(port, UART_MR1);
-   data &= ~UART_MR1_AUTO_RFR_LEVEL1;
+   if (msm_port->is_uartdm) {
+   data &= ~UART_DM_MR1_AUTO_RFR_LEVEL1;
+   data |= UART_DM_MR1_AUTO_RFR_LEVEL1 & (rfr_level << 2);
+   } else {
+   data &= ~UART_MR1_AUTO_RFR_LEVEL1;
+   data |= UART_MR1_AUTO_RFR_LEVEL1 & (rfr_level << 2);
+   }
+
data &= ~UART_MR1_AUTO_RFR_LEVEL0;
-   data |= UART_MR1_AUTO_RFR_LEVEL1 & (rfr_level << 2);
data |= UART_MR1_AUTO_RFR_LEVEL0 & rfr_level;
msm_write(port, data, UART_MR1);
return 0;
diff --git a/drivers/tty/serial/msm_serial.h b/drivers/tty/serial/msm_serial.h
index 3e1c713..caf5363 100644
--- a/drivers/tty/serial/msm_serial.h
+++ b/drivers/tty/serial/msm_serial.h
@@ -20,6 +20,7 @@
 
 #define UART_MR1_AUTO_RFR_LEVEL0   0x3F
 #define UART_MR1_AUTO_RFR_LEVEL1   0x3FF00
+#define UART_DM_MR1_AUTO_RFR_LEVEL10xFF00
 #define UART_MR1_RX_RDY_CTL(1 << 7)
 #define UART_MR1_CTS_CTL   (1 << 6)
 
@@ -78,6 +79,7 @@
 #define UART_IPR_RXSTALE_LAST  0x20
 #define UART_IPR_STALE_LSB 0x1F
 #define UART_IPR_STALE_TIMEOUT_MSB 0x3FF80
+#define UART_DM_IPR_STALE_TIMEOUT_MSB  0xFF80
 
 #define UART_IPR   0x0018
 #define UART_TFWR  0x001C
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/2] tty: serial: msm: Disable pclk when port is closed

[Pramod Gurav]

Disable the pclk when tty port is closed by user space.

Signed-off-by: Pramod Gurav 
---
 drivers/tty/serial/msm_serial.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index 4c1e9ea..f38565c 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -523,6 +523,7 @@ static void msm_shutdown(struct uart_port *port)
msm_write(port, 0, UART_IMR); /* disable interrupts */
 
clk_disable_unprepare(msm_port->clk);
+   clk_disable_unprepare(msm_port->pclk);
 
free_irq(port->irq, port);
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/2] tty: serial: msm: Add mask value for UART_DM registers

[Pramod Gurav]

The bit masks for RFR_LEVEL1 and STALE_TIMEOUT_MSB values in MR1 and
IPR registers respectively are different for UART and UART_DM hardware
cores. We have been using UART core mask values for these. Add the same
for UART_DM core.

There is no bit setting as UART_IPR_RXSTALE_LAST for UART_DM core so do
it only for UART core.

Signed-off-by: Pramod Gurav 
--

Changes since last version:
 - Added new macro fo UART_DM_MR1_AUTO_RFR_LEVEL1 instead of modifying existing.
 - Added a new macro for IPR register as it is also different in UART_DM
 - Changed subject line

---
 drivers/tty/serial/msm_serial.c | 19 +++
 drivers/tty/serial/msm_serial.h |  2 ++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index b73889c..4c1e9ea 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -432,8 +432,13 @@ static int msm_set_baud_rate(struct uart_port *port, 
unsigned int baud)
/* RX stale watermark */
rxstale = entry->rxstale;
watermark = UART_IPR_STALE_LSB & rxstale;
-   watermark |= UART_IPR_RXSTALE_LAST;
-   watermark |= UART_IPR_STALE_TIMEOUT_MSB & (rxstale << 2);
+   if (msm_port->is_uartdm)
+   watermark |= UART_DM_IPR_STALE_TIMEOUT_MSB & (rxstale << 2);
+   else {
+   watermark |= UART_IPR_RXSTALE_LAST;
+   watermark |= UART_IPR_STALE_TIMEOUT_MSB & (rxstale << 2);
+   }
+
msm_write(port, watermark, UART_IPR);
 
/* set RX watermark */
@@ -496,9 +501,15 @@ static int msm_startup(struct uart_port *port)
 
/* set automatic RFR level */
data = msm_read(port, UART_MR1);
-   data &= ~UART_MR1_AUTO_RFR_LEVEL1;
+   if (msm_port->is_uartdm) {
+   data &= ~UART_DM_MR1_AUTO_RFR_LEVEL1;
+   data |= UART_DM_MR1_AUTO_RFR_LEVEL1 & (rfr_level << 2);
+   } else {
+   data &= ~UART_MR1_AUTO_RFR_LEVEL1;
+   data |= UART_MR1_AUTO_RFR_LEVEL1 & (rfr_level << 2);
+   }
+
data &= ~UART_MR1_AUTO_RFR_LEVEL0;
-   data |= UART_MR1_AUTO_RFR_LEVEL1 & (rfr_level << 2);
data |= UART_MR1_AUTO_RFR_LEVEL0 & rfr_level;
msm_write(port, data, UART_MR1);
return 0;
diff --git a/drivers/tty/serial/msm_serial.h b/drivers/tty/serial/msm_serial.h
index 3e1c713..caf5363 100644
--- a/drivers/tty/serial/msm_serial.h
+++ b/drivers/tty/serial/msm_serial.h
@@ -20,6 +20,7 @@
 
 #define UART_MR1_AUTO_RFR_LEVEL0   0x3F
 #define UART_MR1_AUTO_RFR_LEVEL1   0x3FF00
+#define UART_DM_MR1_AUTO_RFR_LEVEL10xFF00
 #define UART_MR1_RX_RDY_CTL(1 << 7)
 #define UART_MR1_CTS_CTL   (1 << 6)
 
@@ -78,6 +79,7 @@
 #define UART_IPR_RXSTALE_LAST  0x20
 #define UART_IPR_STALE_LSB 0x1F
 #define UART_IPR_STALE_TIMEOUT_MSB 0x3FF80
+#define UART_DM_IPR_STALE_TIMEOUT_MSB  0xFF80
 
 #define UART_IPR   0x0018
 #define UART_TFWR  0x001C
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 1/2] tty: serial: msm: Add mask value for UART_DM registers

[Pramod Gurav]

The bit masks for RFR_LEVEL1 and STALE_TIMEOUT_MSB values in MR1 and
IPR registers respectively are different for UART and UART_DM hardware
cores. We have been using UART core mask values for these. Add the same
for UART_DM core.

There is no bit setting as UART_IPR_RXSTALE_LAST for UART_DM core so do
it only for UART core.

Signed-off-by: Pramod Gurav gpra...@codeaurora.org

---
Changes since last version:
 - Added new macro fo UART_DM_MR1_AUTO_RFR_LEVEL1 instead of modifying existing.
 - Added a new macro for IPR register as it is also different in UART_DM
 - Changed subject line
 - Removed change log from message 

 drivers/tty/serial/msm_serial.c | 19 +++
 drivers/tty/serial/msm_serial.h |  2 ++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index b73889c..4c1e9ea 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -432,8 +432,13 @@ static int msm_set_baud_rate(struct uart_port *port, 
unsigned int baud)
/* RX stale watermark */
rxstale = entry-rxstale;
watermark = UART_IPR_STALE_LSB  rxstale;
-   watermark |= UART_IPR_RXSTALE_LAST;
-   watermark |= UART_IPR_STALE_TIMEOUT_MSB  (rxstale  2);
+   if (msm_port-is_uartdm)
+   watermark |= UART_DM_IPR_STALE_TIMEOUT_MSB  (rxstale  2);
+   else {
+   watermark |= UART_IPR_RXSTALE_LAST;
+   watermark |= UART_IPR_STALE_TIMEOUT_MSB  (rxstale  2);
+   }
+
msm_write(port, watermark, UART_IPR);
 
/* set RX watermark */
@@ -496,9 +501,15 @@ static int msm_startup(struct uart_port *port)
 
/* set automatic RFR level */
data = msm_read(port, UART_MR1);
-   data = ~UART_MR1_AUTO_RFR_LEVEL1;
+   if (msm_port-is_uartdm) {
+   data = ~UART_DM_MR1_AUTO_RFR_LEVEL1;
+   data |= UART_DM_MR1_AUTO_RFR_LEVEL1  (rfr_level  2);
+   } else {
+   data = ~UART_MR1_AUTO_RFR_LEVEL1;
+   data |= UART_MR1_AUTO_RFR_LEVEL1  (rfr_level  2);
+   }
+
data = ~UART_MR1_AUTO_RFR_LEVEL0;
-   data |= UART_MR1_AUTO_RFR_LEVEL1  (rfr_level  2);
data |= UART_MR1_AUTO_RFR_LEVEL0  rfr_level;
msm_write(port, data, UART_MR1);
return 0;
diff --git a/drivers/tty/serial/msm_serial.h b/drivers/tty/serial/msm_serial.h
index 3e1c713..caf5363 100644
--- a/drivers/tty/serial/msm_serial.h
+++ b/drivers/tty/serial/msm_serial.h
@@ -20,6 +20,7 @@
 
 #define UART_MR1_AUTO_RFR_LEVEL0   0x3F
 #define UART_MR1_AUTO_RFR_LEVEL1   0x3FF00
+#define UART_DM_MR1_AUTO_RFR_LEVEL10xFF00
 #define UART_MR1_RX_RDY_CTL(1  7)
 #define UART_MR1_CTS_CTL   (1  6)
 
@@ -78,6 +79,7 @@
 #define UART_IPR_RXSTALE_LAST  0x20
 #define UART_IPR_STALE_LSB 0x1F
 #define UART_IPR_STALE_TIMEOUT_MSB 0x3FF80
+#define UART_DM_IPR_STALE_TIMEOUT_MSB  0xFF80
 
 #define UART_IPR   0x0018
 #define UART_TFWR  0x001C
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v2 2/2] tty: serial: msm: Disable pclk when port is closed

[Pramod Gurav]

Disable the pclk when tty port is closed by user space.

Signed-off-by: Pramod Gurav gpra...@codeaurora.org
---
 drivers/tty/serial/msm_serial.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index 4c1e9ea..f38565c 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -523,6 +523,7 @@ static void msm_shutdown(struct uart_port *port)
msm_write(port, 0, UART_IMR); /* disable interrupts */
 
clk_disable_unprepare(msm_port-clk);
+   clk_disable_unprepare(msm_port-pclk);
 
free_irq(port-irq, port);
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 2/2] tty: serial: msm: Disable pclk when port is closed

[Pramod Gurav]

Disable the pclk when tty port is closed by user space.

Signed-off-by: Pramod Gurav gpra...@codeaurora.org
---
 drivers/tty/serial/msm_serial.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index 4c1e9ea..f38565c 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -523,6 +523,7 @@ static void msm_shutdown(struct uart_port *port)
msm_write(port, 0, UART_IMR); /* disable interrupts */
 
clk_disable_unprepare(msm_port-clk);
+   clk_disable_unprepare(msm_port-pclk);
 
free_irq(port-irq, port);
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 1/2] tty: serial: msm: Add mask value for UART_DM registers

[Pramod Gurav]

The bit masks for RFR_LEVEL1 and STALE_TIMEOUT_MSB values in MR1 and
IPR registers respectively are different for UART and UART_DM hardware
cores. We have been using UART core mask values for these. Add the same
for UART_DM core.

There is no bit setting as UART_IPR_RXSTALE_LAST for UART_DM core so do
it only for UART core.

Signed-off-by: Pramod Gurav gpra...@codeaurora.org
--

Changes since last version:
 - Added new macro fo UART_DM_MR1_AUTO_RFR_LEVEL1 instead of modifying existing.
 - Added a new macro for IPR register as it is also different in UART_DM
 - Changed subject line

---
 drivers/tty/serial/msm_serial.c | 19 +++
 drivers/tty/serial/msm_serial.h |  2 ++
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/msm_serial.c b/drivers/tty/serial/msm_serial.c
index b73889c..4c1e9ea 100644
--- a/drivers/tty/serial/msm_serial.c
+++ b/drivers/tty/serial/msm_serial.c
@@ -432,8 +432,13 @@ static int msm_set_baud_rate(struct uart_port *port, 
unsigned int baud)
/* RX stale watermark */
rxstale = entry-rxstale;
watermark = UART_IPR_STALE_LSB  rxstale;
-   watermark |= UART_IPR_RXSTALE_LAST;
-   watermark |= UART_IPR_STALE_TIMEOUT_MSB  (rxstale  2);
+   if (msm_port-is_uartdm)
+   watermark |= UART_DM_IPR_STALE_TIMEOUT_MSB  (rxstale  2);
+   else {
+   watermark |= UART_IPR_RXSTALE_LAST;
+   watermark |= UART_IPR_STALE_TIMEOUT_MSB  (rxstale  2);
+   }
+
msm_write(port, watermark, UART_IPR);
 
/* set RX watermark */
@@ -496,9 +501,15 @@ static int msm_startup(struct uart_port *port)
 
/* set automatic RFR level */
data = msm_read(port, UART_MR1);
-   data = ~UART_MR1_AUTO_RFR_LEVEL1;
+   if (msm_port-is_uartdm) {
+   data = ~UART_DM_MR1_AUTO_RFR_LEVEL1;
+   data |= UART_DM_MR1_AUTO_RFR_LEVEL1  (rfr_level  2);
+   } else {
+   data = ~UART_MR1_AUTO_RFR_LEVEL1;
+   data |= UART_MR1_AUTO_RFR_LEVEL1  (rfr_level  2);
+   }
+
data = ~UART_MR1_AUTO_RFR_LEVEL0;
-   data |= UART_MR1_AUTO_RFR_LEVEL1  (rfr_level  2);
data |= UART_MR1_AUTO_RFR_LEVEL0  rfr_level;
msm_write(port, data, UART_MR1);
return 0;
diff --git a/drivers/tty/serial/msm_serial.h b/drivers/tty/serial/msm_serial.h
index 3e1c713..caf5363 100644
--- a/drivers/tty/serial/msm_serial.h
+++ b/drivers/tty/serial/msm_serial.h
@@ -20,6 +20,7 @@
 
 #define UART_MR1_AUTO_RFR_LEVEL0   0x3F
 #define UART_MR1_AUTO_RFR_LEVEL1   0x3FF00
+#define UART_DM_MR1_AUTO_RFR_LEVEL10xFF00
 #define UART_MR1_RX_RDY_CTL(1  7)
 #define UART_MR1_CTS_CTL   (1  6)
 
@@ -78,6 +79,7 @@
 #define UART_IPR_RXSTALE_LAST  0x20
 #define UART_IPR_STALE_LSB 0x1F
 #define UART_IPR_STALE_TIMEOUT_MSB 0x3FF80
+#define UART_DM_IPR_STALE_TIMEOUT_MSB  0xFF80
 
 #define UART_IPR   0x0018
 #define UART_TFWR  0x001C
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 2/2] tty: serial: msm: Disable pclk when port is closed

[Pramod Gurav]

On Thu, April 9, 2015 5:21 am, Stephen Boyd wrote:
 On 04/08/15 06:28, Pramod Gurav wrote:
 Disable the pclk when tty port is closed by user space.

 Signed-off-by: Pramod Gurav gpra...@codeaurora.org
 ---
  drivers/tty/serial/msm_serial.c | 1 +
  1 file changed, 1 insertion(+)

 diff --git a/drivers/tty/serial/msm_serial.c
 b/drivers/tty/serial/msm_serial.c
 index 4c1e9ea..f38565c 100644
 --- a/drivers/tty/serial/msm_serial.c
 +++ b/drivers/tty/serial/msm_serial.c
 @@ -523,6 +523,7 @@ static void msm_shutdown(struct uart_port *port)
  msm_write(port, 0, UART_IMR); /* disable interrupts */

  clk_disable_unprepare(msm_port-clk);
 +clk_disable_unprepare(msm_port-pclk);

  free_irq(port-irq, port);
  }

 It's not clear to me at all when this clock is enabled and when it's
 disabled during the lifetime of this driver. For example, why do we have
 a .pm op to turn clocks on and off? Shouldn't they already be on? Can
 you please explain when the clocks are turned on and off and what
 userspace actions cause that to happen? Looking at drivers like
 amba-pl010.c I don't see any .pm op, just a
 clk_prepare_enable/clk_disable_unprepare pair in the startup and
 shutdown ops.


When a userspce opens a serial port the uart_startup (in serial_core)
function is executed which changes the uart_pm state to UART_PM_STATE_ON.
So when this port is release/closed by the application
uart_close(serial_core) changes the uart_pm state to UART_PM_STATE_OFF if
its not console. But it is not done in uart_shutdown function.

So ideally clk_prepare_enable/clk_disable_unprepare must be called in .pm
only. So, we can get rid of these operations from msm_startup function as
these will be called anyway using .pm ops.

About .pm in uart_ops, there are few drivers which have it for an example,
atmel_serial, sh-sci etc. That is where they do
clk_prepare_enable/clk_disable_unprepare. And moreover when there is
suspend across system these function will handled through .pm.



 Minus my confusion of why our clocking is complicated, it looks correct
 to me to do this, so

 Reviewed-by: Stephen Boyd sb...@codeaurora.org


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



Pramod

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


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/