| Message ID | 1348658647-25975-3-git-send-email-vipulkumar.samar@st.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wed, Sep 26, 2012 at 1:24 PM, Vipul Kumar Samar <vipulkumar.samar@st.com> 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 ------------------------------------------------------------------------------ Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
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)
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 <vipulkumar.samar@st.com> --- drivers/amba/bus.c | 42 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-)