[2/2] ARM: ABMA: Disable/Enable interface clock from suspend/resume

Commit Message

vipul kumar samar Sept. 26, 2012, 11:24 a.m. UTC

AMBA devices interface clock is disabled in RTPM suspend/resume hooks
but not in conventional hooks.

This patch adds support to disable/enable clock for conventional
suspend/resume calls.

Signed-off-by: Vipul Kumar Samar <[e-mail]>
---
 drivers/amba/bus.c |   42 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 39 insertions(+), 3 deletions(-)

Comments

Linus Walleij Sept. 26, 2012, 12:38 p.m. UTC | #1

On Wed, Sep 26, 2012 at 1:24 PM, Vipul Kumar Samar
<[e-mail]> wrote:

> AMBA devices interface clock is disabled in RTPM suspend/resume hooks
> but not in conventional hooks.
>
> This patch adds support to disable/enable clock for conventional
> suspend/resume calls.
(...)
> +       struct amba_device *pcdev = to_amba_device(dev);
>         int ret = 0;
>
>         if (!drv)
> @@ -132,16 +133,27 @@ static int amba_pm_suspend(struct device *dev)
>                 ret = amba_legacy_suspend(dev, PMSG_SUSPEND);
>         }
>
> +       if (!ret)
> +               clk_disable(pcdev->pclk);
> +
>         return ret;
>  }

You're not accounting for the case where pcdev->pclk
is an error pointer (as happens if pclk lookup fails at
probe).

I think you can simplify some of the code using
the external accessors from <linux/amba/bus.h>:

amba_pclk_enable();
amba_pclk_disable();

But:

Again this is a case where you have a race between runtime
suspend/resume and ordinary suspend/resume.

These ordinary suspend/resume operations should probably
wait for runtime suspend to happen *first* if and only if
runtime PM is enabled, and then the block will be gated
off. So we really need to figure out how we can make sure
that this happens.

If just ordinary PM is enabled, the above makes sense but in
that case I think we should have that #ifdef:ed as an alternative
or something.

So something like:

suspend():
#ifdef CONFIG_PM_RUNTIME
   /* Wait for runtime PM to hammer down the pclk */
#elif CONFIG_PM
   amba_pclk_disable();
#endif

resume():
#if defined(CONFIG_PM) && !defined(CONFIG_PM_RUNTIME)
  amba_pclk_enable();
#endif
  /* Let runtime PM lazily enable the clock when needed */

To complicate things further the bus operations can be
overridden by e.g. voltage domains. In this case we (ux500)
have choosen to call out to the AMBA level to make sure
semantics are preserved.

Yours,
Linus Walleij

Patch

diff --git a/drivers/amba/bus.c b/drivers/amba/bus.c
index b7e7285..ded3d98 100644
--- a/drivers/amba/bus.c
+++ b/drivers/amba/bus.c
@@ -120,6 +120,7 @@  static int amba_legacy_resume(struct device *dev)
 static int amba_pm_suspend(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct amba_device *pcdev = to_amba_device(dev);
 	int ret = 0;
 
 	if (!drv)
@@ -132,16 +133,27 @@  static int amba_pm_suspend(struct device *dev)
 		ret = amba_legacy_suspend(dev, PMSG_SUSPEND);
 	}
 
+	if (!ret)
+		clk_disable(pcdev->pclk);
+
 	return ret;
 }
 
 static int amba_pm_resume(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct amba_device *pcdev = to_amba_device(dev);
 	int ret = 0;
 
-	if (!drv)
+	if (!drv) {
 		return 0;
+	} else {
+		ret = clk_enable(pcdev->pclk);
+		if (ret) {
+			dev_err(dev, "Resume: Clk enable failed");
+			return ret;
+		}
+	}
 
 	if (drv->pm) {
 		if (drv->pm->resume)
@@ -165,6 +177,7 @@  static int amba_pm_resume(struct device *dev)
 static int amba_pm_freeze(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct amba_device *pcdev = to_amba_device(dev);
 	int ret = 0;
 
 	if (!drv)
@@ -177,16 +190,27 @@  static int amba_pm_freeze(struct device *dev)
 		ret = amba_legacy_suspend(dev, PMSG_FREEZE);
 	}
 
+	if (!ret)
+		clk_disable(pcdev->pclk);
+
 	return ret;
 }
 
 static int amba_pm_thaw(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct amba_device *pcdev = to_amba_device(dev);
 	int ret = 0;
 
-	if (!drv)
+	if (!drv) {
 		return 0;
+	} else {
+		ret = clk_enable(pcdev->pclk);
+		if (ret) {
+			dev_err(dev, "Thaw: Clk enable failed");
+			return ret;
+		}
+	}
 
 	if (drv->pm) {
 		if (drv->pm->thaw)
@@ -201,6 +225,7 @@  static int amba_pm_thaw(struct device *dev)
 static int amba_pm_poweroff(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct amba_device *pcdev = to_amba_device(dev);
 	int ret = 0;
 
 	if (!drv)
@@ -213,16 +238,27 @@  static int amba_pm_poweroff(struct device *dev)
 		ret = amba_legacy_suspend(dev, PMSG_HIBERNATE);
 	}
 
+	if (!ret)
+		clk_disable(pcdev->pclk);
+
 	return ret;
 }
 
 static int amba_pm_restore(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
+	struct amba_device *pcdev = to_amba_device(dev);
 	int ret = 0;
 
-	if (!drv)
+	if (!drv) {
 		return 0;
+	} else {
+		ret = clk_enable(pcdev->pclk);
+		if (ret) {
+			dev_err(dev, "Restore: Clk enable failed");
+			return ret;
+		}
+	}
 
 	if (drv->pm) {
 		if (drv->pm->restore)