On Wed, Apr 27, 2011 at 2:48 PM, Rafael J. Wysocki <rjw@xxxxxxx> wrote:
> From: Rafael J. Wysocki <rjw@xxxxxxx>
>
> Many different platforms and subsystems may want to disable device
> clocks during suspend and enable them during resume which is going to
> be done in a very similar way in all those cases. For this reason,
> provide generic routines for the manipulation of device clocks during
> suspend and resume.
>
> Convert the ARM shmobile platform to using the new routines.
>
> Signed-off-by: Rafael J. Wysocki <rjw@xxxxxxx>
> ---
>
> Hi,
>
> This (hopefully final) version of the patch has a couple of bugs fixed in
> clock_ops.c.
>
> Thanks,
> Rafael
>
> ---
> arch/arm/mach-shmobile/pm_runtime.c | 140 -----------
> drivers/base/power/Makefile | 1
> drivers/base/power/clock_ops.c | 423 ++++++++++++++++++++++++++++++++++++
> include/linux/pm_runtime.h | 42 +++
> kernel/power/Kconfig | 4
> 5 files changed, 479 insertions(+), 131 deletions(-)
>
<snip>
> +void pm_runtime_clk_remove(struct device *dev, const char *con_id)
> +{
> + struct pm_runtime_clk_data *prd = __to_prd(dev);
> + struct pm_clock_entry *ce;
> +
> + if (!prd)
> + return;
> +
> + mutex_lock(&prd->lock);
> +
> + list_for_each_entry(ce, &prd->clock_list, node)
Braces
> + if (!con_id && !ce->con_id) {
> + __pm_runtime_clk_remove(ce);
> + break;
> + } else if (!con_id || !ce->con_id) {
> + continue;
> + } else if (!strcmp(con_id, ce->con_id)) {
> + __pm_runtime_clk_remove(ce);
> + break;
> + }
> +
> + mutex_unlock(&prd->lock);
> +}
>
> +/**
> + * pm_runtime_clk_acquire - Acquire a device clock.
> + * @dev: Device whose clock is to be acquired.
> + * @con_id: Connection ID of the clock.
> + */
> +static void pm_runtime_clk_acquire(struct device *dev,
> + struct pm_clock_entry *ce)
> +{
> + ce->clk = clk_get(dev, ce->con_id);
> + if (!IS_ERR(ce->clk)) {
> + ce->clock_active = true;
> + dev_dbg(dev, "Clock %s managed by runtime PM.\n", ce->con_id);
> + }
> +}
> +
> +/**
> + * pm_runtime_clk_suspend - Disable clocks in a device's runtime PM clock list.
> + * @dev: Device to disable the clocks for.
> + */
> +int pm_runtime_clk_suspend(struct device *dev)
> +{
> + struct pm_runtime_clk_data *prd = __to_prd(dev);
> + struct pm_clock_entry *ce;
> +
> + dev_dbg(dev, "%s()\n", __func__);
> +
> + if (!prd)
> + return 0;
> +
> + mutex_lock(&prd->lock);
> +
> + list_for_each_entry_reverse(ce, &prd->clock_list, node) {
> + if (!ce->clk) {
> + dev_err(dev, "Clock is not ready for runtime PM\n");
> + pm_runtime_clk_acquire(dev, ce);
Why delay the call to clk_get until the first suspend? Also, this
will always print an error during the first call to suspend.
> + }
> +
> + if (ce->clock_active) {
I don't think clock_active is necessary, and the name is misleading.
Why not use if (ce->clk)?
> + clk_disable(ce->clk);
> + ce->clock_enabled = false;
Clock enables are already refcounted, do you really need a flag as
well? In what situations would pm_runtime_clk_remove get called,
which currently needs to know when the clock is enabled?
> + }
> + }
> +
> + mutex_unlock(&prd->lock);
> +
> + return 0;
> +}
> +
> +/**
> + * pm_runtime_clk_resume - Enable clocks in a device's runtime PM clock list.
> + * @dev: Device to enable the clocks for.
> + */
> +int pm_runtime_clk_resume(struct device *dev)
> +{
> + struct pm_runtime_clk_data *prd = __to_prd(dev);
> + struct pm_clock_entry *ce;
> +
> + dev_dbg(dev, "%s()\n", __func__);
> +
> + if (!prd)
> + return 0;
> +
> + mutex_lock(&prd->lock);
> +
> + list_for_each_entry(ce, &prd->clock_list, node) {
> + if (!ce->clk)
> + pm_runtime_clk_acquire(dev, ce);
If the clock was not present during suspend, should you be enabling it
during resume?
> +
> + if (ce->clock_active) {
> + clk_enable(ce->clk);
> + ce->clock_enabled = true;
> + }
> + }
> +
> + mutex_unlock(&prd->lock);
> +
> + return 0;
> +}
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
