2011-06-11 20:42:00

by Rafael J. Wysocki

Subject: [PATCH 0/8] PM / Domains: Support for generic I/O PM domains (v5)

Hi,

This is the 4th update of the patchset adding support for generic I/O PM
domains. The patches have been reworked quite a bit to take feedback into
account, but I left the Greg's ACK in [4/8] in the hope it still applies
(Greg, please let me know in case it doesn't :-)).

The model here is that a bunch of devices share a common power resource
that can be turned on and off by software. In addition to that, there
are means to start and stop the activity of each device, for example
by manipulating their clocks. Moreover, there may be hierarchy of
such things, for example power resource A may be necessary for devices
a, b, c, which don't rely on any other power resources, and for devices
x, y, z that also rely on power resource X. In that case there one PM
domain object representing devices a, b, c and power resource A, and
another PM domain object will represent devices x, y, z with power
resource X, plus the first object will be the second one's parent.

Note to Kevin: I know you'd like each PM domain to be able to go into several
different states, but the situation will always be that in some of those
states the devices' registers will remain intact, while in the rest of those
states they will be reset. Say, there are states 1, 2, 3, 4 and states
1-3 preserve device registers. Then it is not necessary to save device
registers for "domain" states 1-3 and it only is necessary to save them
when going to state 4. In that case, .power_off() may map to the "go to
state 4" operation (and analogously .power_on()), while the rest may be
done by .stop_device() and .start_device(). IOW, .power_is_off == true
means "the devices' registers have to be restored", so it need not map to
any particular physical state of a (hardware) power domain.

Note to Magnus and Paul: I didn't use a global lock as suggested, because
I think it may lead to completely unnecessary congestion in situations in
which there are no hierarchies of PM domains. It is quite easy to show that
the code doesn't deadlock, because (1) no more than 2 locks are held by the
same thread at a time (parent lock and child lock) and (2) they are always
acquired in the same order (parent before the child).

Overall, I think I've taken all of the important dependencies into
consideration, but if you spot something suspicious, please let me know. :-)
Wakeup is not covered at this point, because it's not necessary for the
SH7372's A4LC power domain that's the first user of the new code, but it
is quite clear how add the support for it. Also, for more complicated
cases it is necessary to take QoS requirements (latencies) into account,
which is in the works (kind of).

[1/8] - Update documentation to reflect the fact that struct dev_power_domain
callbacks take precedence over subsystem PM callbacks.

[2/8] - Rename struct dev_power_domain to struct dev_pm_domain to reflect the
fact that those objects need not correspond to hardware power domains
directly.

[3/8] - Move subsys_data in struct dev_pm_info out of #ifdef CONFIG_PM_RUNTIME

[4/8] - Introduce runtime PM support for generic I/O PM domains.

[5/8] - Introduce generic "noirq" callbacks for system suspend/hibernation
(that's necessary for the next patches).

[6/8] - Move some PM domains support code fro under #ifdef CONFIG_PM_RUNTIME

[7/8] - Add system-wide PM support for generic I/O PM domains.

[8/8] - Use the new code to represent the SH7372's A4MP power domain.

The patchset has been tested on SH7372 Mackerel board and appears to work
correctly.

I'd like to push [1/8] for 3.0 (it may be regarded as a fix), but I _think_
that it may be a good idea to push [2/8] for 3.0 too, to limit the time in
which people may possibly use the naming that's going to change in their new
code. If you agree with that, please let me know, I'll need some serious
ACKs below that patch if it's to be pushed for 3.0. ;-)

Thanks,
Rafael

2011-06-11 20:40:37

by Rafael J. Wysocki

Subject: [PATCH 1/8] PM / Domains: Update documentation

From: Rafael J. Wysocki <[email protected]>

Commit 4d27e9dcff00a6425d779b065ec8892e4f391661 (PM: Make power
domain callbacks take precedence over subsystem ones) forgot to
update the device power management documentation to take changes
made by it into account. Correct that mistake.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
Documentation/power/devices.txt | 39 +++++++++++++--------------------------
1 file changed, 13 insertions(+), 26 deletions(-)

Index: linux-2.6/Documentation/power/devices.txt
===================================================================
--- linux-2.6.orig/Documentation/power/devices.txt
+++ linux-2.6/Documentation/power/devices.txt
@@ -520,33 +520,20 @@ Support for power domains is provided th
device. This field is a pointer to an object of type struct dev_power_domain,
defined in include/linux/pm.h, providing a set of power management callbacks
analogous to the subsystem-level and device driver callbacks that are executed
-for the given device during all power transitions, in addition to the respective
-subsystem-level callbacks. Specifically, the power domain "suspend" callbacks
-(i.e. ->runtime_suspend(), ->suspend(), ->freeze(), ->poweroff(), etc.) are
-executed after the analogous subsystem-level callbacks, while the power domain
-"resume" callbacks (i.e. ->runtime_resume(), ->resume(), ->thaw(), ->restore,
-etc.) are executed before the analogous subsystem-level callbacks. Error codes
-returned by the "suspend" and "resume" power domain callbacks are ignored.
+for the given device during all power transitions, instead of the respective
+subsystem-level callbacks. Specifically, if a device's pm_domain pointer is
+not NULL, the ->suspend() callback from the object pointed to by it will be
+executed instead of its subsystem's (e.g. bus type's) ->suspend() callback and
+anlogously for all of the remaining callbacks. In other words, power management
+domain callbacks, if defined for the given device, always take precedence over
+the callbacks provided by the device's subsystem (e.g. bus type).

-Power domain ->runtime_idle() callback is executed before the subsystem-level
-->runtime_idle() callback and the result returned by it is not ignored. Namely,
-if it returns error code, the subsystem-level ->runtime_idle() callback will not
-be called and the helper function rpm_idle() executing it will return error
-code. This mechanism is intended to help platforms where saving device state
-is a time consuming operation and should only be carried out if all devices
-in the power domain are idle, before turning off the shared power resource(s).
-Namely, the power domain ->runtime_idle() callback may return error code until
-the pm_runtime_idle() helper (or its asychronous version) has been called for
-all devices in the power domain (it is recommended that the returned error code
-be -EBUSY in those cases), preventing the subsystem-level ->runtime_idle()
-callback from being run prematurely.
-
-The support for device power domains is only relevant to platforms needing to
-use the same subsystem-level (e.g. platform bus type) and device driver power
-management callbacks in many different power domain configurations and wanting
-to avoid incorporating the support for power domains into the subsystem-level
-callbacks. The other platforms need not implement it or take it into account
-in any way.
+The support for device power management domains is only relevant to platforms
+needing to use the same device driver power management callbacks in many
+different power domain configurations and wanting to avoid incorporating the
+support for power domains into subsystem-level callbacks, for example by
+modifying the platform bus type. Other platforms need not implement it or take
+it into account in any way.


Device Low Power (suspend) States

2011-06-11 20:40:50

by Rafael J. Wysocki

Subject: [PATCH 2/8] PM / Domains: Rename struct dev_power_domain to struct dev_pm_domain

From: Rafael J. Wysocki <[email protected]>

The naming convention used by commit 7538e3db6e015e890825fbd9f86599b
(PM: Add support for device power domains), which introduced the
struct dev_power_domain type for representing device power domains,
evidently confuses some developers who tend to think that objects
of this type must correspond to "power domains" as defined by
hardware, which is not the case. Namely, at the kernel level, a
struct dev_power_domain object can represent arbitrary set of devices
that are mutually dependent power management-wise and need not belong
to one hardware power domain. To avoid that confusion, rename struct
dev_power_domain to struct dev_pm_domain and rename the related
pointers in struct device and struct pm_clk_notifier_block from
pwr_domain to pm_domain.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
Documentation/power/devices.txt | 8 ++++----
arch/arm/mach-omap1/pm_bus.c | 4 ++--
arch/arm/mach-shmobile/pm_runtime.c | 8 ++++----
arch/arm/plat-omap/omap_device.c | 4 ++--
arch/sh/kernel/cpu/shmobile/pm_runtime.c | 6 +++---
drivers/base/power/clock_ops.c | 14 +++++++-------
drivers/base/power/main.c | 30 +++++++++++++++---------------
drivers/base/power/runtime.c | 12 ++++++------
include/linux/device.h | 4 ++--
include/linux/pm.h | 2 +-
include/linux/pm_runtime.h | 2 +-
11 files changed, 47 insertions(+), 47 deletions(-)

Index: linux-2.6/arch/arm/mach-omap1/pm_bus.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-omap1/pm_bus.c
+++ linux-2.6/arch/arm/mach-omap1/pm_bus.c
@@ -49,7 +49,7 @@ static int omap1_pm_runtime_resume(struc
return pm_generic_runtime_resume(dev);
}

-static struct dev_power_domain default_power_domain = {
+static struct dev_pm_domain default_pm_domain = {
.ops = {
.runtime_suspend = omap1_pm_runtime_suspend,
.runtime_resume = omap1_pm_runtime_resume,
@@ -58,7 +58,7 @@ static struct dev_power_domain default_p
};

static struct pm_clk_notifier_block platform_bus_notifier = {
- .pwr_domain = &default_power_domain,
+ .pm_domain = &default_pm_domain,
.con_ids = { "ick", "fck", NULL, },
};

Index: linux-2.6/arch/arm/mach-shmobile/pm_runtime.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/pm_runtime.c
+++ linux-2.6/arch/arm/mach-shmobile/pm_runtime.c
@@ -28,7 +28,7 @@ static int default_platform_runtime_idle
return pm_runtime_suspend(dev);
}

-static struct dev_power_domain default_power_domain = {
+static struct dev_pm_domain default_pm_domain = {
.ops = {
.runtime_suspend = pm_runtime_clk_suspend,
.runtime_resume = pm_runtime_clk_resume,
@@ -37,16 +37,16 @@ static struct dev_power_domain default_p
},
};

-#define DEFAULT_PWR_DOMAIN_PTR (&default_power_domain)
+#define DEFAULT_PM_DOMAIN_PTR (&default_pm_domain)

#else

-#define DEFAULT_PWR_DOMAIN_PTR NULL
+#define DEFAULT_PM_DOMAIN_PTR NULL

#endif /* CONFIG_PM_RUNTIME */

static struct pm_clk_notifier_block platform_bus_notifier = {
- .pwr_domain = DEFAULT_PWR_DOMAIN_PTR,
+ .pm_domain = DEFAULT_PM_DOMAIN_PTR,
.con_ids = { NULL, },
};

Index: linux-2.6/arch/arm/plat-omap/omap_device.c
===================================================================
--- linux-2.6.orig/arch/arm/plat-omap/omap_device.c
+++ linux-2.6/arch/arm/plat-omap/omap_device.c
@@ -550,7 +550,7 @@ static int _od_runtime_resume(struct dev
return omap_device_enable(pdev);
}

-static struct dev_power_domain omap_device_power_domain = {
+static struct dev_pm_domain omap_device_pm_domain = {
.ops = {
.runtime_suspend = _od_runtime_suspend,
.runtime_resume = _od_runtime_resume,
@@ -571,7 +571,7 @@ int omap_device_register(struct omap_dev
pr_debug("omap_device: %s: registering\n", od->pdev.name);

od->pdev.dev.parent = &omap_device_parent;
- od->pdev.dev.pwr_domain = &omap_device_power_domain;
+ od->pdev.dev.pwr_domain = &omap_device_pm_domain;
return platform_device_register(&od->pdev);
}

Index: linux-2.6/arch/sh/kernel/cpu/shmobile/pm_runtime.c
===================================================================
--- linux-2.6.orig/arch/sh/kernel/cpu/shmobile/pm_runtime.c
+++ linux-2.6/arch/sh/kernel/cpu/shmobile/pm_runtime.c
@@ -256,7 +256,7 @@ out:
return ret;
}

-static struct dev_power_domain default_power_domain = {
+static struct dev_pm_domain default_power_domain = {
.ops = {
.runtime_suspend = default_platform_runtime_suspend,
.runtime_resume = default_platform_runtime_resume,
@@ -285,7 +285,7 @@ static int platform_bus_notify(struct no
hwblk_disable(hwblk_info, hwblk);
/* make sure driver re-inits itself once */
__set_bit(PDEV_ARCHDATA_FLAG_INIT, &pdev->archdata.flags);
- dev->pwr_domain = &default_power_domain;
+ dev->pm_domain = &default_power_domain;
break;
/* TODO: add BUS_NOTIFY_BIND_DRIVER and increase idle count */
case BUS_NOTIFY_BOUND_DRIVER:
@@ -299,7 +299,7 @@ static int platform_bus_notify(struct no
__set_bit(PDEV_ARCHDATA_FLAG_INIT, &pdev->archdata.flags);
break;
case BUS_NOTIFY_DEL_DEVICE:
- dev->pwr_domain = NULL;
+ dev->pm_domain = NULL;
break;
}
return 0;
Index: linux-2.6/Documentation/power/devices.txt
===================================================================
--- linux-2.6.orig/Documentation/power/devices.txt
+++ linux-2.6/Documentation/power/devices.txt
@@ -506,8 +506,8 @@ routines. Nevertheless, different callb
situation where it actually matters.


-Device Power Domains
---------------------
+Device Power Management Domains
+-------------------------------
Sometimes devices share reference clocks or other power resources. In those
cases it generally is not possible to put devices into low-power states
individually. Instead, a set of devices sharing a power resource can be put
@@ -516,8 +516,8 @@ power resource. Of course, they also ne
together, by turning the shared power resource on. A set of devices with this
property is often referred to as a power domain.

-Support for power domains is provided through the pwr_domain field of struct
-device. This field is a pointer to an object of type struct dev_power_domain,
+Support for power domains is provided through the pm_domain field of struct
+device. This field is a pointer to an object of type struct dev_pm_domain,
defined in include/linux/pm.h, providing a set of power management callbacks
analogous to the subsystem-level and device driver callbacks that are executed
for the given device during all power transitions, instead of the respective
Index: linux-2.6/include/linux/device.h
===================================================================
--- linux-2.6.orig/include/linux/device.h
+++ linux-2.6/include/linux/device.h
@@ -516,7 +516,7 @@ struct device_dma_parameters {
* minimizes board-specific #ifdefs in drivers.
* @power: For device power management.
* See Documentation/power/devices.txt for details.
- * @pwr_domain: Provide callbacks that are executed during system suspend,
+ * @pm_domain: Provide callbacks that are executed during system suspend,
* hibernation, system resume and during runtime PM transitions
* along with subsystem-level and driver-level callbacks.
* @numa_node: NUMA node this device is close to.
@@ -568,7 +568,7 @@ struct device {
void *platform_data; /* Platform specific data, device
core doesn't touch it */
struct dev_pm_info power;
- struct dev_power_domain *pwr_domain;
+ struct dev_pm_domain *pm_domain;

#ifdef CONFIG_NUMA
int numa_node; /* NUMA node this device is close to */
Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -471,7 +471,7 @@ extern void update_pm_runtime_accounting
* hibernation, system resume and during runtime PM transitions along with
* subsystem-level and driver-level callbacks.
*/
-struct dev_power_domain {
+struct dev_pm_domain {
struct dev_pm_ops ops;
};

Index: linux-2.6/include/linux/pm_runtime.h
===================================================================
--- linux-2.6.orig/include/linux/pm_runtime.h
+++ linux-2.6/include/linux/pm_runtime.h
@@ -247,7 +247,7 @@ static inline void pm_runtime_dont_use_a

struct pm_clk_notifier_block {
struct notifier_block nb;
- struct dev_power_domain *pwr_domain;
+ struct dev_pm_domain *pm_domain;
char *con_ids[];
};

Index: linux-2.6/drivers/base/power/clock_ops.c
===================================================================
--- linux-2.6.orig/drivers/base/power/clock_ops.c
+++ linux-2.6/drivers/base/power/clock_ops.c
@@ -278,11 +278,11 @@ int pm_runtime_clk_resume(struct device
*
* For this function to work, @nb must be a member of an object of type
* struct pm_clk_notifier_block containing all of the requisite data.
- * Specifically, the pwr_domain member of that object is copied to the device's
- * pwr_domain field and its con_ids member is used to populate the device's list
+ * Specifically, the pm_domain member of that object is copied to the device's
+ * pm_domain field and its con_ids member is used to populate the device's list
* of runtime PM clocks, depending on @action.
*
- * If the device's pwr_domain field is already populated with a value different
+ * If the device's pm_domain field is already populated with a value different
* from the one stored in the struct pm_clk_notifier_block object, the function
* does nothing.
*/
@@ -300,14 +300,14 @@ static int pm_runtime_clk_notify(struct

switch (action) {
case BUS_NOTIFY_ADD_DEVICE:
- if (dev->pwr_domain)
+ if (dev->pm_domain)
break;

error = pm_runtime_clk_init(dev);
if (error)
break;

- dev->pwr_domain = clknb->pwr_domain;
+ dev->pm_domain = clknb->pm_domain;
if (clknb->con_ids[0]) {
for (con_id = clknb->con_ids; *con_id; con_id++)
pm_runtime_clk_add(dev, *con_id);
@@ -317,10 +317,10 @@ static int pm_runtime_clk_notify(struct

break;
case BUS_NOTIFY_DEL_DEVICE:
- if (dev->pwr_domain != clknb->pwr_domain)
+ if (dev->pm_domain != clknb->pm_domain)
break;

- dev->pwr_domain = NULL;
+ dev->pm_domain = NULL;
pm_runtime_clk_destroy(dev);
break;
}
Index: linux-2.6/drivers/base/power/main.c
===================================================================
--- linux-2.6.orig/drivers/base/power/main.c
+++ linux-2.6/drivers/base/power/main.c
@@ -424,9 +424,9 @@ static int device_resume_noirq(struct de
TRACE_DEVICE(dev);
TRACE_RESUME(0);

- if (dev->pwr_domain) {
+ if (dev->pm_domain) {
pm_dev_dbg(dev, state, "EARLY power domain ");
- error = pm_noirq_op(dev, &dev->pwr_domain->ops, state);
+ error = pm_noirq_op(dev, &dev->pm_domain->ops, state);
} else if (dev->type && dev->type->pm) {
pm_dev_dbg(dev, state, "EARLY type ");
error = pm_noirq_op(dev, dev->type->pm, state);
@@ -513,9 +513,9 @@ static int device_resume(struct device *

dev->power.in_suspend = false;

- if (dev->pwr_domain) {
+ if (dev->pm_domain) {
pm_dev_dbg(dev, state, "power domain ");
- error = pm_op(dev, &dev->pwr_domain->ops, state);
+ error = pm_op(dev, &dev->pm_domain->ops, state);
goto End;
}

@@ -630,10 +630,10 @@ static void device_complete(struct devic
{
device_lock(dev);

- if (dev->pwr_domain) {
+ if (dev->pm_domain) {
pm_dev_dbg(dev, state, "completing power domain ");
- if (dev->pwr_domain->ops.complete)
- dev->pwr_domain->ops.complete(dev);
+ if (dev->pm_domain->ops.complete)
+ dev->pm_domain->ops.complete(dev);
} else if (dev->type && dev->type->pm) {
pm_dev_dbg(dev, state, "completing type ");
if (dev->type->pm->complete)
@@ -733,9 +733,9 @@ static int device_suspend_noirq(struct d
{
int error;

- if (dev->pwr_domain) {
+ if (dev->pm_domain) {
pm_dev_dbg(dev, state, "LATE power domain ");
- error = pm_noirq_op(dev, &dev->pwr_domain->ops, state);
+ error = pm_noirq_op(dev, &dev->pm_domain->ops, state);
if (error)
return error;
} else if (dev->type && dev->type->pm) {
@@ -842,9 +842,9 @@ static int __device_suspend(struct devic
goto End;
}

- if (dev->pwr_domain) {
+ if (dev->pm_domain) {
pm_dev_dbg(dev, state, "power domain ");
- error = pm_op(dev, &dev->pwr_domain->ops, state);
+ error = pm_op(dev, &dev->pm_domain->ops, state);
goto End;
}

@@ -968,11 +968,11 @@ static int device_prepare(struct device

device_lock(dev);

- if (dev->pwr_domain) {
+ if (dev->pm_domain) {
pm_dev_dbg(dev, state, "preparing power domain ");
- if (dev->pwr_domain->ops.prepare)
- error = dev->pwr_domain->ops.prepare(dev);
- suspend_report_result(dev->pwr_domain->ops.prepare, error);
+ if (dev->pm_domain->ops.prepare)
+ error = dev->pm_domain->ops.prepare(dev);
+ suspend_report_result(dev->pm_domain->ops.prepare, error);
if (error)
goto End;
} else if (dev->type && dev->type->pm) {
Index: linux-2.6/drivers/base/power/runtime.c
===================================================================
--- linux-2.6.orig/drivers/base/power/runtime.c
+++ linux-2.6/drivers/base/power/runtime.c
@@ -213,8 +213,8 @@ static int rpm_idle(struct device *dev,

dev->power.idle_notification = true;

- if (dev->pwr_domain)
- callback = dev->pwr_domain->ops.runtime_idle;
+ if (dev->pm_domain)
+ callback = dev->pm_domain->ops.runtime_idle;
else if (dev->type && dev->type->pm)
callback = dev->type->pm->runtime_idle;
else if (dev->class && dev->class->pm)
@@ -374,8 +374,8 @@ static int rpm_suspend(struct device *de

__update_runtime_status(dev, RPM_SUSPENDING);

- if (dev->pwr_domain)
- callback = dev->pwr_domain->ops.runtime_suspend;
+ if (dev->pm_domain)
+ callback = dev->pm_domain->ops.runtime_suspend;
else if (dev->type && dev->type->pm)
callback = dev->type->pm->runtime_suspend;
else if (dev->class && dev->class->pm)
@@ -573,8 +573,8 @@ static int rpm_resume(struct device *dev

__update_runtime_status(dev, RPM_RESUMING);

- if (dev->pwr_domain)
- callback = dev->pwr_domain->ops.runtime_resume;
+ if (dev->pm_domain)
+ callback = dev->pm_domain->ops.runtime_resume;
else if (dev->type && dev->type->pm)
callback = dev->type->pm->runtime_resume;
else if (dev->class && dev->class->pm)

2011-06-11 20:40:45

by Rafael J. Wysocki

Subject: [PATCH 3/8] PM: subsys_data in struct dev_pm_info need not depend on RM_RUNTIME

From: Rafael J. Wysocki <[email protected]>

The subsys_data field of struct dev_pm_info, introduced by commit
1d2b71f61b6a10216274e27b717becf9ae101fc7 (PM / Runtime: Add subsystem
data field to struct dev_pm_info), is going to be used even if
CONFIG_PM_RUNTIME is not set, so move it from under the #ifdef.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
include/linux/pm.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -460,8 +460,8 @@ struct dev_pm_info {
unsigned long active_jiffies;
unsigned long suspended_jiffies;
unsigned long accounting_timestamp;
- void *subsys_data; /* Owned by the subsystem. */
#endif
+ void *subsys_data; /* Owned by the subsystem. */
};

extern void update_pm_runtime_accounting(struct device *dev);

2011-06-11 20:40:48

by Rafael J. Wysocki

Subject: [PATCH 4/8] PM / Domains: Support for generic I/O PM domains (v5)

From: Rafael J. Wysocki <[email protected]>

Introduce common headers, helper functions and callbacks allowing
platforms to use simple generic power domains for runtime power
management.

Introduce struct generic_pm_domain to be used for representing
power domains that each contain a number of devices and may be
parent domains or subdomains with respect to other power domains.
Among other things, this structure includes callbacks to be
provided by platforms for performing specific tasks related to
power management (i.e. ->stop_device() may disable a device's
clocks, while ->start_device() may enable them, ->power_off() is
supposed to remove power from the entire power domain
and ->power_on() is supposed to restore it).

Introduce functions that can be used as power domain runtime PM
callbacks, pm_genpd_runtime_suspend() and pm_genpd_runtime_resume(),
as well as helper functions for the initialization of a power
domain represented by a struct generic_power_domain object,
adding a device to or removing a device from it and adding or
removing subdomains.

Introduce configuration option CONFIG_PM_GENERIC_DOMAINS to be
selected by the platforms that want to use the new code.

Signed-off-by: Rafael J. Wysocki <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
---
drivers/base/power/Makefile | 1
drivers/base/power/domain.c | 467 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/pm.h | 3
include/linux/pm_domain.h | 78 +++++++
kernel/power/Kconfig | 4
5 files changed, 552 insertions(+), 1 deletion(-)

Index: linux-2.6/include/linux/pm_domain.h
===================================================================
--- /dev/null
+++ linux-2.6/include/linux/pm_domain.h
@@ -0,0 +1,78 @@
+/*
+ * pm_domain.h - Definitions and headers related to device power domains.
+ *
+ * Copyright (C) 2011 Rafael J. Wysocki <[email protected]>, Renesas Electronics Corp.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#ifndef _LINUX_PM_DOMAIN_H
+#define _LINUX_PM_DOMAIN_H
+
+#include <linux/device.h>
+
+struct dev_power_governor {
+ bool (*power_down_ok)(struct dev_pm_domain *domain);
+};
+
+struct generic_pm_domain {
+ struct dev_pm_domain domain; /* PM domain operations */
+ struct list_head sd_node; /* Node in the parent's subdomain list */
+ struct generic_pm_domain *parent; /* Parent PM domain */
+ struct list_head sd_list; /* List of dubdomains */
+ struct list_head dev_list; /* List of devices */
+ struct mutex lock;
+ struct dev_power_governor *gov;
+ struct work_struct power_off_work;
+ unsigned int in_progress; /* Number of devices being suspended now */
+ unsigned int sd_count; /* Number of subdomains with power "on" */
+ bool power_is_off; /* Whether or not power has been removed */
+ int (*power_off)(struct dev_pm_domain *domain);
+ int (*power_on)(struct dev_pm_domain *domain);
+ int (*start_device)(struct device *dev);
+ int (*stop_device)(struct device *dev);
+};
+
+struct dev_list_entry {
+ struct list_head node;
+ struct device *dev;
+ bool need_restore;
+};
+
+#ifdef CONFIG_PM_GENERIC_DOMAINS
+extern int pm_genpd_add_device(struct generic_pm_domain *genpd,
+ struct device *dev);
+extern int pm_genpd_remove_device(struct generic_pm_domain *genpd,
+ struct device *dev);
+extern int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
+ struct generic_pm_domain *new_subdomain);
+extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
+ struct generic_pm_domain *target);
+extern void pm_genpd_init(struct generic_pm_domain *genpd,
+ struct dev_power_governor *gov, bool is_off);
+#else
+static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
+ struct device *dev)
+{
+ return -ENOSYS;
+}
+static inline int pm_genpd_remove_device(struct generic_pm_domain *genpd,
+ struct device *dev)
+{
+ return -ENOSYS;
+}
+static inline int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
+ struct generic_pm_domain *new_sd)
+{
+ return -ENOSYS;
+}
+static inline int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
+ struct generic_pm_domain *target)
+{
+ return -ENOSYS;
+}
+static inline void pm_genpd_init(struct generic_pm_domain *genpd,
+ struct dev_power_governor *gov, bool is_off) {}
+#endif
+
+#endif /* _LINUX_PM_DOMAIN_H */
Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -472,7 +472,8 @@ extern void update_pm_runtime_accounting
* subsystem-level and driver-level callbacks.
*/
struct dev_pm_domain {
- struct dev_pm_ops ops;
+ struct dev_pm_ops ops;
+ void *platform_data;
};

/*
Index: linux-2.6/drivers/base/power/Makefile
===================================================================
--- linux-2.6.orig/drivers/base/power/Makefile
+++ linux-2.6/drivers/base/power/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_PM_SLEEP) += main.o wakeup.
obj-$(CONFIG_PM_RUNTIME) += runtime.o
obj-$(CONFIG_PM_TRACE_RTC) += trace.o
obj-$(CONFIG_PM_OPP) += opp.o
+obj-$(CONFIG_PM_GENERIC_DOMAINS) += domain.o
obj-$(CONFIG_HAVE_CLK) += clock_ops.o

ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
\ No newline at end of file
Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/base/power/domain.c
@@ -0,0 +1,467 @@
+/*
+ * drivers/base/power/domain.c - Common code related to device power domains.
+ *
+ * Copyright (C) 2011 Rafael J. Wysocki <[email protected]>, Renesas Electronics Corp.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/io.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm_domain.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+
+#ifdef CONFIG_PM_RUNTIME
+
+static void genpd_sd_counter_dec(struct generic_pm_domain *genpd)
+{
+ if (!WARN_ON(genpd->sd_count == 0))
+ genpd->sd_count--;
+}
+
+/**
+ * __pm_genpd_restore_device - Restore the pre-suspend state of a device.
+ * @dle: Device list entry of the device to restore the state of.
+ * @genpd: PM domain the device belongs to.
+ */
+static void __pm_genpd_restore_device(struct dev_list_entry *dle,
+ struct generic_pm_domain *genpd)
+{
+ struct device *dev = dle->dev;
+ struct device_driver *drv = dev->driver;
+
+ if (!dle->need_restore)
+ return;
+
+ if (genpd->start_device)
+ genpd->start_device(dev);
+
+ if (drv && drv->pm && drv->pm->runtime_resume)
+ drv->pm->runtime_resume(dev);
+
+ if (genpd->stop_device)
+ genpd->stop_device(dev);
+
+ dle->need_restore = false;
+}
+
+/**
+ * pm_genpd_poweroff - Remove power from a given PM domain.
+ * @genpd: PM domain to power down.
+ *
+ * If all of the @genpd's devices have been suspended and all of its subdomains
+ * have been powered down, run the runtime suspend callbacks provided by all of
+ * the @genpd's devices' drivers and remove power from @genpd.
+ */
+static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
+{
+ struct generic_pm_domain *parent;
+ struct dev_list_entry *dle;
+ unsigned int not_suspended;
+ int ret;
+
+ if (genpd->power_is_off)
+ return 0;
+
+ if (genpd->sd_count > 0)
+ return -EBUSY;
+
+ not_suspended = 0;
+ list_for_each_entry(dle, &genpd->dev_list, node)
+ if (dle->dev->driver && !pm_runtime_suspended(dle->dev))
+ not_suspended++;
+
+ if (not_suspended > genpd->in_progress)
+ return -EBUSY;
+
+ if (genpd->gov && genpd->gov->power_down_ok) {
+ if (!genpd->gov->power_down_ok(&genpd->domain))
+ return -EAGAIN;
+ }
+
+ list_for_each_entry_reverse(dle, &genpd->dev_list, node) {
+ struct device *dev = dle->dev;
+ struct device_driver *drv = dev->driver;
+
+ if (genpd->start_device)
+ genpd->start_device(dev);
+
+ if (drv && drv->pm && drv->pm->runtime_suspend)
+ ret = drv->pm->runtime_suspend(dev);
+
+ if (genpd->stop_device)
+ genpd->stop_device(dev);
+
+ if (ret)
+ goto err_dev;
+ else
+ dle->need_restore = true;
+ }
+
+ if (genpd->power_off)
+ genpd->power_off(&genpd->domain);
+
+ genpd->power_is_off = true;
+
+ parent = genpd->parent;
+ if (parent) {
+ genpd_sd_counter_dec(parent);
+ if (parent->sd_count == 0)
+ queue_work(pm_wq, &parent->power_off_work);
+ }
+
+ return 0;
+
+ err_dev:
+ list_for_each_entry_continue(dle, &genpd->dev_list, node)
+ __pm_genpd_restore_device(dle, genpd);
+
+ return ret;
+}
+
+/**
+ * genpd_power_off_work_fn - Power off PM domain whose subdomain count is 0.
+ * @work: Work structure used for scheduling the execution of this function.
+ */
+static void genpd_power_off_work_fn(struct work_struct *work)
+{
+ struct generic_pm_domain *genpd;
+
+ genpd = container_of(work, struct generic_pm_domain, power_off_work);
+
+ if (genpd->parent)
+ mutex_lock(&genpd->parent->lock);
+ mutex_lock(&genpd->lock);
+ pm_genpd_poweroff(genpd);
+ mutex_unlock(&genpd->lock);
+ if (genpd->parent)
+ mutex_unlock(&genpd->parent->lock);
+}
+
+/**
+ * pm_genpd_runtime_suspend - Suspend a device belonging to I/O PM domain.
+ * @dev: Device to suspend.
+ *
+ * Carry out a runtime suspend of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_runtime_suspend(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ if (IS_ERR_OR_NULL(dev->pm_domain))
+ return -EINVAL;
+
+ genpd = container_of(dev->pm_domain, struct generic_pm_domain, domain);
+
+ if (genpd->parent)
+ mutex_lock(&genpd->parent->lock);
+ mutex_lock(&genpd->lock);
+
+ if (genpd->stop_device) {
+ int ret = genpd->stop_device(dev);
+ if (ret)
+ goto out;
+ }
+ genpd->in_progress++;
+ pm_genpd_poweroff(genpd);
+ genpd->in_progress--;
+
+ out:
+ mutex_unlock(&genpd->lock);
+ if (genpd->parent)
+ mutex_unlock(&genpd->parent->lock);
+
+ return 0;
+}
+
+/**
+ * pm_genpd_poweron - Restore power to a given PM domain and its parents.
+ * @genpd: PM domain to power up.
+ *
+ * Restore power to @genpd and all of its parents so that it is possible to
+ * resume a device belonging to it.
+ */
+static int pm_genpd_poweron(struct generic_pm_domain *genpd)
+{
+ int ret = 0;
+
+ start:
+ if (genpd->parent)
+ mutex_lock(&genpd->parent->lock);
+ mutex_lock(&genpd->lock);
+
+ if (!genpd->power_is_off)
+ goto out;
+
+ if (genpd->parent && genpd->parent->power_is_off) {
+ mutex_unlock(&genpd->lock);
+ mutex_unlock(&genpd->parent->lock);
+
+ ret = pm_genpd_poweron(genpd->parent);
+ if (ret)
+ return ret;
+
+ goto start;
+ }
+
+ if (genpd->power_on) {
+ int ret = genpd->power_on(&genpd->domain);
+ if (ret)
+ goto out;
+ }
+
+ genpd->power_is_off = false;
+ if (genpd->parent)
+ genpd->parent->sd_count++;
+
+ out:
+ mutex_unlock(&genpd->lock);
+ if (genpd->parent)
+ mutex_unlock(&genpd->parent->lock);
+
+ return ret;
+}
+
+/**
+ * pm_genpd_runtime_resume - Resume a device belonging to I/O PM domain.
+ * @dev: Device to resume.
+ *
+ * Carry out a runtime resume of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_runtime_resume(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ struct dev_list_entry *dle;
+ int ret;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ if (IS_ERR_OR_NULL(dev->pm_domain))
+ return -EINVAL;
+
+ genpd = container_of(dev->pm_domain, struct generic_pm_domain, domain);
+
+ ret = pm_genpd_poweron(genpd);
+ if (ret)
+ return ret;
+
+ mutex_lock(&genpd->lock);
+
+ list_for_each_entry(dle, &genpd->dev_list, node) {
+ if (dle->dev == dev) {
+ __pm_genpd_restore_device(dle, genpd);
+ break;
+ }
+ }
+
+ if (genpd->start_device)
+ genpd->start_device(dev);
+
+ mutex_unlock(&genpd->lock);
+
+ return 0;
+}
+
+#else
+
+#define pm_genpd_runtime_suspend NULL
+#define pm_genpd_runtime_resume NULL
+
+#endif /* CONFIG_PM_RUNTIME */
+
+/**
+ * pm_genpd_add_device - Add a device to an I/O PM domain.
+ * @genpd: PM domain to add the device to.
+ * @dev: Device to be added.
+ */
+int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
+{
+ struct dev_list_entry *dle;
+ int ret = 0;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
+ return -EINVAL;
+
+ mutex_lock(&genpd->lock);
+
+ list_for_each_entry(dle, &genpd->dev_list, node)
+ if (dle->dev == dev) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ dle = kzalloc(sizeof(*dle), GFP_KERNEL);
+ if (!dle) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ dle->dev = dev;
+ list_add_tail(&dle->node, &genpd->dev_list);
+
+ spin_lock_irq(&dev->power.lock);
+ dev->pm_domain = &genpd->domain;
+ spin_unlock_irq(&dev->power.lock);
+
+ out:
+ mutex_unlock(&genpd->lock);
+
+ return ret;
+}
+
+/**
+ * pm_genpd_remove_device - Remove a device from an I/O PM domain.
+ * @genpd: PM domain to remove the device from.
+ * @dev: Device to be removed.
+ */
+int pm_genpd_remove_device(struct generic_pm_domain *genpd,
+ struct device *dev)
+{
+ struct dev_list_entry *dle;
+ int ret = -EINVAL;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
+ return -EINVAL;
+
+ mutex_lock(&genpd->lock);
+
+ list_for_each_entry(dle, &genpd->dev_list, node) {
+ if (dle->dev != dev)
+ continue;
+
+ spin_lock_irq(&dev->power.lock);
+ dev->pm_domain = NULL;
+ spin_unlock_irq(&dev->power.lock);
+
+ list_del(&dle->node);
+ kfree(dle);
+
+ ret = 0;
+ break;
+ }
+
+ mutex_unlock(&genpd->lock);
+
+ return ret;
+}
+
+/**
+ * pm_genpd_add_subdomain - Add a subdomain to an I/O PM domain.
+ * @genpd: Master PM domain to add the subdomain to.
+ * @new_subdomain: Subdomain to be added.
+ */
+int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
+ struct generic_pm_domain *new_subdomain)
+{
+ struct generic_pm_domain *subdomain;
+ int ret = 0;
+
+ if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(new_subdomain))
+ return -EINVAL;
+
+ mutex_lock(&genpd->lock);
+
+ if (genpd->power_is_off && !new_subdomain->power_is_off) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ list_for_each_entry(subdomain, &genpd->sd_list, sd_node) {
+ if (subdomain == new_subdomain) {
+ ret = -EINVAL;
+ goto out;
+ }
+ }
+
+ mutex_lock(&new_subdomain->lock);
+
+ list_add_tail(&new_subdomain->sd_node, &genpd->sd_list);
+ new_subdomain->parent = genpd;
+ if (!subdomain->power_is_off)
+ genpd->sd_count++;
+
+ mutex_unlock(&new_subdomain->lock);
+
+ out:
+ mutex_unlock(&genpd->lock);
+
+ return ret;
+}
+
+/**
+ * pm_genpd_remove_subdomain - Remove a subdomain from an I/O PM domain.
+ * @genpd: Master PM domain to remove the subdomain from.
+ * @target: Subdomain to be removed.
+ */
+int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
+ struct generic_pm_domain *target)
+{
+ struct generic_pm_domain *subdomain;
+ int ret = -EINVAL;
+
+ if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(target))
+ return -EINVAL;
+
+ mutex_lock(&genpd->lock);
+
+ list_for_each_entry(subdomain, &genpd->sd_list, sd_node) {
+ if (subdomain != target)
+ continue;
+
+ mutex_lock(&subdomain->lock);
+
+ list_del(&subdomain->sd_node);
+ subdomain->parent = NULL;
+ if (!subdomain->power_is_off)
+ genpd_sd_counter_dec(genpd);
+
+ mutex_unlock(&subdomain->lock);
+
+ ret = 0;
+ break;
+ }
+
+ mutex_unlock(&genpd->lock);
+
+ return ret;
+}
+
+/**
+ * pm_genpd_init - Initialize a generic I/O PM domain object.
+ * @genpd: PM domain object to initialize.
+ * @gov: PM domain governor to associate with the domain (may be NULL).
+ * @is_off: Initial value of the domain's power_is_off field.
+ */
+void pm_genpd_init(struct generic_pm_domain *genpd,
+ struct dev_power_governor *gov, bool is_off)
+{
+ if (IS_ERR_OR_NULL(genpd))
+ return;
+
+ INIT_LIST_HEAD(&genpd->sd_node);
+ genpd->parent = NULL;
+ INIT_LIST_HEAD(&genpd->dev_list);
+ INIT_LIST_HEAD(&genpd->sd_list);
+ mutex_init(&genpd->lock);
+ genpd->gov = gov;
+ INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn);
+ genpd->in_progress = 0;
+ genpd->sd_count = 0;
+ genpd->power_is_off = is_off;
+ genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
+ genpd->domain.ops.runtime_resume = pm_genpd_runtime_resume;
+ genpd->domain.ops.runtime_idle = pm_generic_runtime_idle;
+}
Index: linux-2.6/kernel/power/Kconfig
===================================================================
--- linux-2.6.orig/kernel/power/Kconfig
+++ linux-2.6/kernel/power/Kconfig
@@ -227,3 +227,7 @@ config PM_OPP
config PM_RUNTIME_CLK
def_bool y
depends on PM_RUNTIME && HAVE_CLK
+
+config PM_GENERIC_DOMAINS
+ bool
+ depends on PM

2011-06-11 20:42:01

by Rafael J. Wysocki

Subject: [PATCH 5/8] PM: Introduce generic "noirq" callback routines for subsystems

From: Rafael J. Wysocki <[email protected]>

Introduce generic "noirq" power management callback routines for
subsystems in addition to the "regular" generic PM callback routines.

The new routines will be used, among other things, for implementing
system-wide PM transitions support for generic PM domains.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/base/power/generic_ops.c | 90 ++++++++++++++++++++++++++++++++-------
include/linux/pm.h | 6 ++
2 files changed, 82 insertions(+), 14 deletions(-)

Index: linux-2.6/drivers/base/power/generic_ops.c
===================================================================
--- linux-2.6.orig/drivers/base/power/generic_ops.c
+++ linux-2.6/drivers/base/power/generic_ops.c
@@ -94,12 +94,13 @@ int pm_generic_prepare(struct device *de
* __pm_generic_call - Generic suspend/freeze/poweroff/thaw subsystem callback.
* @dev: Device to handle.
* @event: PM transition of the system under way.
+ * @bool: Whether or not this is the "noirq" stage.
*
* If the device has not been suspended at run time, execute the
* suspend/freeze/poweroff/thaw callback provided by its driver, if defined, and
* return its error code. Otherwise, return zero.
*/
-static int __pm_generic_call(struct device *dev, int event)
+static int __pm_generic_call(struct device *dev, int event, bool noirq)
{
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
int (*callback)(struct device *);
@@ -109,16 +110,16 @@ static int __pm_generic_call(struct devi

switch (event) {
case PM_EVENT_SUSPEND:
- callback = pm->suspend;
+ callback = noirq ? pm->suspend_noirq : pm->suspend;
break;
case PM_EVENT_FREEZE:
- callback = pm->freeze;
+ callback = noirq ? pm->freeze_noirq : pm->freeze;
break;
case PM_EVENT_HIBERNATE:
- callback = pm->poweroff;
+ callback = noirq ? pm->poweroff_noirq : pm->poweroff;
break;
case PM_EVENT_THAW:
- callback = pm->thaw;
+ callback = noirq ? pm->thaw_noirq : pm->thaw;
break;
default:
callback = NULL;
@@ -129,42 +130,82 @@ static int __pm_generic_call(struct devi
}

/**
+ * pm_generic_suspend_noirq - Generic suspend_noirq callback for subsystems.
+ * @dev: Device to suspend.
+ */
+int pm_generic_suspend_noirq(struct device *dev)
+{
+ return __pm_generic_call(dev, PM_EVENT_SUSPEND, true);
+}
+EXPORT_SYMBOL_GPL(pm_generic_suspend_noirq);
+
+/**
* pm_generic_suspend - Generic suspend callback for subsystems.
* @dev: Device to suspend.
*/
int pm_generic_suspend(struct device *dev)
{
- return __pm_generic_call(dev, PM_EVENT_SUSPEND);
+ return __pm_generic_call(dev, PM_EVENT_SUSPEND, false);
}
EXPORT_SYMBOL_GPL(pm_generic_suspend);

/**
+ * pm_generic_freeze_noirq - Generic freeze_noirq callback for subsystems.
+ * @dev: Device to freeze.
+ */
+int pm_generic_freeze_noirq(struct device *dev)
+{
+ return __pm_generic_call(dev, PM_EVENT_FREEZE, true);
+}
+EXPORT_SYMBOL_GPL(pm_generic_freeze_noirq);
+
+/**
* pm_generic_freeze - Generic freeze callback for subsystems.
* @dev: Device to freeze.
*/
int pm_generic_freeze(struct device *dev)
{
- return __pm_generic_call(dev, PM_EVENT_FREEZE);
+ return __pm_generic_call(dev, PM_EVENT_FREEZE, false);
}
EXPORT_SYMBOL_GPL(pm_generic_freeze);

/**
+ * pm_generic_poweroff_noirq - Generic poweroff_noirq callback for subsystems.
+ * @dev: Device to handle.
+ */
+int pm_generic_poweroff_noirq(struct device *dev)
+{
+ return __pm_generic_call(dev, PM_EVENT_HIBERNATE, true);
+}
+EXPORT_SYMBOL_GPL(pm_generic_poweroff_noirq);
+
+/**
* pm_generic_poweroff - Generic poweroff callback for subsystems.
* @dev: Device to handle.
*/
int pm_generic_poweroff(struct device *dev)
{
- return __pm_generic_call(dev, PM_EVENT_HIBERNATE);
+ return __pm_generic_call(dev, PM_EVENT_HIBERNATE, false);
}
EXPORT_SYMBOL_GPL(pm_generic_poweroff);

/**
+ * pm_generic_thaw_noirq - Generic thaw_noirq callback for subsystems.
+ * @dev: Device to thaw.
+ */
+int pm_generic_thaw_noirq(struct device *dev)
+{
+ return __pm_generic_call(dev, PM_EVENT_THAW, true);
+}
+EXPORT_SYMBOL_GPL(pm_generic_thaw_noirq);
+
+/**
* pm_generic_thaw - Generic thaw callback for subsystems.
* @dev: Device to thaw.
*/
int pm_generic_thaw(struct device *dev)
{
- return __pm_generic_call(dev, PM_EVENT_THAW);
+ return __pm_generic_call(dev, PM_EVENT_THAW, false);
}
EXPORT_SYMBOL_GPL(pm_generic_thaw);

@@ -172,12 +213,13 @@ EXPORT_SYMBOL_GPL(pm_generic_thaw);
* __pm_generic_resume - Generic resume/restore callback for subsystems.
* @dev: Device to handle.
* @event: PM transition of the system under way.
+ * @bool: Whether or not this is the "noirq" stage.
*
* Execute the resume/resotre callback provided by the @dev's driver, if
* defined. If it returns 0, change the device's runtime PM status to 'active'.
* Return the callback's error code.
*/
-static int __pm_generic_resume(struct device *dev, int event)
+static int __pm_generic_resume(struct device *dev, int event, bool noirq)
{
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
int (*callback)(struct device *);
@@ -188,10 +230,10 @@ static int __pm_generic_resume(struct de

switch (event) {
case PM_EVENT_RESUME:
- callback = pm->resume;
+ callback = noirq ? pm->resume_noirq : pm->resume;
break;
case PM_EVENT_RESTORE:
- callback = pm->restore;
+ callback = noirq ? pm->restore_noirq : pm->restore;
break;
default:
callback = NULL;
@@ -212,22 +254,42 @@ static int __pm_generic_resume(struct de
}

/**
+ * pm_generic_resume_noirq - Generic resume_noirq callback for subsystems.
+ * @dev: Device to resume.
+ */
+int pm_generic_resume_noirq(struct device *dev)
+{
+ return __pm_generic_resume(dev, PM_EVENT_RESUME, true);
+}
+EXPORT_SYMBOL_GPL(pm_generic_resume_noirq);
+
+/**
* pm_generic_resume - Generic resume callback for subsystems.
* @dev: Device to resume.
*/
int pm_generic_resume(struct device *dev)
{
- return __pm_generic_resume(dev, PM_EVENT_RESUME);
+ return __pm_generic_resume(dev, PM_EVENT_RESUME, false);
}
EXPORT_SYMBOL_GPL(pm_generic_resume);

/**
+ * pm_generic_restore_noirq - Generic restore_noirq callback for subsystems.
+ * @dev: Device to restore.
+ */
+int pm_generic_restore_noirq(struct device *dev)
+{
+ return __pm_generic_resume(dev, PM_EVENT_RESTORE, true);
+}
+EXPORT_SYMBOL_GPL(pm_generic_restore_noirq);
+
+/**
* pm_generic_restore - Generic restore callback for subsystems.
* @dev: Device to restore.
*/
int pm_generic_restore(struct device *dev)
{
- return __pm_generic_resume(dev, PM_EVENT_RESTORE);
+ return __pm_generic_resume(dev, PM_EVENT_RESTORE, false);
}
EXPORT_SYMBOL_GPL(pm_generic_restore);

Index: linux-2.6/include/linux/pm.h
===================================================================
--- linux-2.6.orig/include/linux/pm.h
+++ linux-2.6/include/linux/pm.h
@@ -553,11 +553,17 @@ extern void __suspend_report_result(cons
extern int device_pm_wait_for_dev(struct device *sub, struct device *dev);

extern int pm_generic_prepare(struct device *dev);
+extern int pm_generic_suspend_noirq(struct device *dev);
extern int pm_generic_suspend(struct device *dev);
+extern int pm_generic_resume_noirq(struct device *dev);
extern int pm_generic_resume(struct device *dev);
+extern int pm_generic_freeze_noirq(struct device *dev);
extern int pm_generic_freeze(struct device *dev);
+extern int pm_generic_thaw_noirq(struct device *dev);
extern int pm_generic_thaw(struct device *dev);
+extern int pm_generic_restore_noirq(struct device *dev);
extern int pm_generic_restore(struct device *dev);
+extern int pm_generic_poweroff_noirq(struct device *dev);
extern int pm_generic_poweroff(struct device *dev);
extern void pm_generic_complete(struct device *dev);

2011-06-11 20:40:53

by Rafael J. Wysocki

Subject: [PATCH 6/8] PM / Domains: Move code from under #ifdef CONFIG_PM_RUNTIME

From: Rafael J. Wysocki <[email protected]>

There is some code in drivers/base/power/domain.c that will be useful
for both runtime PM and system-wide power transitions, so make it
depend on CONFIG_PM instead of CONFIG_PM_RUNTIME.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/base/power/domain.c | 118 +++++++++++++++++++++++---------------------
1 file changed, 64 insertions(+), 54 deletions(-)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -14,6 +14,66 @@
#include <linux/slab.h>
#include <linux/err.h>

+#ifdef CONFIG_PM
+
+static struct generic_pm_domain *dev_to_genpd(struct device *dev)
+{
+ if (IS_ERR_OR_NULL(dev->pm_domain))
+ return ERR_PTR(-EINVAL);
+
+ return container_of(dev->pm_domain, struct generic_pm_domain, domain);
+}
+
+/**
+ * pm_genpd_poweron - Restore power to a given PM domain and its parents.
+ * @genpd: PM domain to power up.
+ *
+ * Restore power to @genpd and all of its parents so that it is possible to
+ * resume a device belonging to it.
+ */
+static int pm_genpd_poweron(struct generic_pm_domain *genpd)
+{
+ int ret = 0;
+
+ start:
+ if (genpd->parent)
+ mutex_lock(&genpd->parent->lock);
+ mutex_lock(&genpd->lock);
+
+ if (!genpd->power_is_off)
+ goto out;
+
+ if (genpd->parent && genpd->parent->power_is_off) {
+ mutex_unlock(&genpd->lock);
+ mutex_unlock(&genpd->parent->lock);
+
+ ret = pm_genpd_poweron(genpd->parent);
+ if (ret)
+ return ret;
+
+ goto start;
+ }
+
+ if (genpd->power_on) {
+ int ret = genpd->power_on(&genpd->domain);
+ if (ret)
+ goto out;
+ }
+
+ genpd->power_is_off = false;
+ if (genpd->parent)
+ genpd->parent->sd_count++;
+
+ out:
+ mutex_unlock(&genpd->lock);
+ if (genpd->parent)
+ mutex_unlock(&genpd->parent->lock);
+
+ return ret;
+}
+
+#endif /* CONFIG_PM */
+
#ifdef CONFIG_PM_RUNTIME

static void genpd_sd_counter_dec(struct generic_pm_domain *genpd)
@@ -155,11 +215,10 @@ static int pm_genpd_runtime_suspend(stru

dev_dbg(dev, "%s()\n", __func__);

- if (IS_ERR_OR_NULL(dev->pm_domain))
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
return -EINVAL;

- genpd = container_of(dev->pm_domain, struct generic_pm_domain, domain);
-
if (genpd->parent)
mutex_lock(&genpd->parent->lock);
mutex_lock(&genpd->lock);
@@ -182,54 +241,6 @@ static int pm_genpd_runtime_suspend(stru
}

/**
- * pm_genpd_poweron - Restore power to a given PM domain and its parents.
- * @genpd: PM domain to power up.
- *
- * Restore power to @genpd and all of its parents so that it is possible to
- * resume a device belonging to it.
- */
-static int pm_genpd_poweron(struct generic_pm_domain *genpd)
-{
- int ret = 0;
-
- start:
- if (genpd->parent)
- mutex_lock(&genpd->parent->lock);
- mutex_lock(&genpd->lock);
-
- if (!genpd->power_is_off)
- goto out;
-
- if (genpd->parent && genpd->parent->power_is_off) {
- mutex_unlock(&genpd->lock);
- mutex_unlock(&genpd->parent->lock);
-
- ret = pm_genpd_poweron(genpd->parent);
- if (ret)
- return ret;
-
- goto start;
- }
-
- if (genpd->power_on) {
- int ret = genpd->power_on(&genpd->domain);
- if (ret)
- goto out;
- }
-
- genpd->power_is_off = false;
- if (genpd->parent)
- genpd->parent->sd_count++;
-
- out:
- mutex_unlock(&genpd->lock);
- if (genpd->parent)
- mutex_unlock(&genpd->parent->lock);
-
- return ret;
-}
-
-/**
* pm_genpd_runtime_resume - Resume a device belonging to I/O PM domain.
* @dev: Device to resume.
*
@@ -245,11 +256,10 @@ static int pm_genpd_runtime_resume(struc

dev_dbg(dev, "%s()\n", __func__);

- if (IS_ERR_OR_NULL(dev->pm_domain))
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
return -EINVAL;

- genpd = container_of(dev->pm_domain, struct generic_pm_domain, domain);
-
ret = pm_genpd_poweron(genpd);
if (ret)
return ret;

2011-06-11 20:41:33

by Rafael J. Wysocki

Subject: [PATCH 7/8] PM / Domains: System-wide transitions support for generic PM domains

From: Rafael J. Wysocki <[email protected]>

Make generic PM domains support system-wide power transitions
(system suspend and hibernation). Add suspend, resume, freeze, thaw,
poweroff and restore callbacks to be associated with struct
generic_pm_domain objects and make pm_genpd_init() use them as
appropriate.

The new callbacks do nothing for devices belonging to power domains
that were powered down at run time (before the transition). For the
other devices the action carried out depends on the type of the
transition. During system suspend the power domain .suspend()
callback executes pm_generic_suspend() for the device, while the
PM domain .suspend_noirq() callback runs pm_generic_suspend_noirq()
for it, stops it and eventually removes power from the PM domain it
belongs to (after all devices in the domain have been stopped and
its subdomains have been powered off).

During system resume the PM domain .resume_noirq() callback
restores power to the PM domain (when executed for it first time),
starts the device and executes pm_generic_resume_noirq() for it,
while the .resume() callback executes pm_generic_resume() for the
device. Finally, the .complete() callback executes pm_runtime_idle()
for the device which should put it back into the suspended state if
its runtime PM usage count is equal to zero at that time.

The actions carried out during hibernation and resume from it are
analogous to the ones described above.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/base/power/domain.c | 544 +++++++++++++++++++++++++++++++++++++++++++-
include/linux/pm_domain.h | 7
2 files changed, 539 insertions(+), 12 deletions(-)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -40,7 +40,8 @@ static int pm_genpd_poweron(struct gener
mutex_lock(&genpd->parent->lock);
mutex_lock(&genpd->lock);

- if (!genpd->power_is_off)
+ if (!genpd->power_is_off
+ || (genpd->prepared_count > 0 && genpd->suspend_power_off))
goto out;

if (genpd->parent && genpd->parent->power_is_off) {
@@ -123,7 +124,7 @@ static int pm_genpd_poweroff(struct gene
unsigned int not_suspended;
int ret;

- if (genpd->power_is_off)
+ if (genpd->power_is_off || genpd->prepared_count > 0)
return 0;

if (genpd->sd_count > 0)
@@ -241,6 +242,27 @@ static int pm_genpd_runtime_suspend(stru
}

/**
+ * __pm_genpd_runtime_resume - Resume a device belonging to I/O PM domain.
+ * @dev: Device to resume.
+ * @genpd: PM domain the device belongs to.
+ */
+static void __pm_genpd_runtime_resume(struct device *dev,
+ struct generic_pm_domain *genpd)
+{
+ struct dev_list_entry *dle;
+
+ list_for_each_entry(dle, &genpd->dev_list, node) {
+ if (dle->dev == dev) {
+ __pm_genpd_restore_device(dle, genpd);
+ break;
+ }
+ }
+
+ if (genpd->start_device)
+ genpd->start_device(dev);
+}
+
+/**
* pm_genpd_runtime_resume - Resume a device belonging to I/O PM domain.
* @dev: Device to resume.
*
@@ -251,7 +273,6 @@ static int pm_genpd_runtime_suspend(stru
static int pm_genpd_runtime_resume(struct device *dev)
{
struct generic_pm_domain *genpd;
- struct dev_list_entry *dle;
int ret;

dev_dbg(dev, "%s()\n", __func__);
@@ -265,28 +286,498 @@ static int pm_genpd_runtime_resume(struc
return ret;

mutex_lock(&genpd->lock);
+ __pm_genpd_runtime_resume(dev, genpd);
+ mutex_unlock(&genpd->lock);

- list_for_each_entry(dle, &genpd->dev_list, node) {
- if (dle->dev == dev) {
- __pm_genpd_restore_device(dle, genpd);
- break;
- }
+ return 0;
+}
+
+#else
+
+static inline void __pm_genpd_runtime_resume(struct device *dev,
+ struct generic_pm_domain *genpd) {}
+
+#define pm_genpd_runtime_suspend NULL
+#define pm_genpd_runtime_resume NULL
+
+#endif /* CONFIG_PM_RUNTIME */
+
+#ifdef CONFIG_PM_SLEEP
+
+/**
+ * pm_genpd_sync_poweroff - Synchronously power off a PM domain and its parents.
+ * @genpd: PM domain to power off, if possible.
+ *
+ * Check if the given PM domain can be powered off (during system suspend or
+ * hibernation) and do that if so. Also, in that case propagate to its parent.
+ *
+ * This function is only called in "noirq" stages of system power transitions,
+ * so it need not acquire locks (all of the "noirq" callbacks are executed
+ * sequentially, so it is guaranteed that it will never run twice in parallel).
+ */
+static void pm_genpd_sync_poweroff(struct generic_pm_domain *genpd)
+{
+ struct generic_pm_domain *parent = genpd->parent;
+
+ if (genpd->power_is_off)
+ return;
+
+ if (genpd->suspended_count != genpd->device_count || genpd->sd_count > 0)
+ return;
+
+ if (genpd->power_off)
+ genpd->power_off(&genpd->domain);
+
+ genpd->power_is_off = true;
+ if (parent) {
+ genpd_sd_counter_dec(parent);
+ pm_genpd_sync_poweroff(parent);
}
+}

+/**
+ * pm_genpd_prepare - Start power transition of a device in a PM domain.
+ * @dev: Device to start the transition of.
+ *
+ * Start a power transition of a device (during a system-wide power transition)
+ * under the assumption that its pm_domain field points to the domain member of
+ * an object of type struct generic_pm_domain representing a PM domain
+ * consisting of I/O devices.
+ */
+static int pm_genpd_prepare(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ int ret;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ mutex_lock(&genpd->lock);
+
+ if (genpd->prepared_count++ == 0)
+ genpd->suspend_power_off = genpd->power_is_off;
+
+ if (genpd->suspend_power_off) {
+ mutex_unlock(&genpd->lock);
+ return 0;
+ }
+
+ /*
+ * If the device is in the (runtime) "suspended" state, call
+ * .start_device() for it, if defined.
+ */
+ if (pm_runtime_suspended(dev))
+ __pm_genpd_runtime_resume(dev, genpd);
+
+ mutex_unlock(&genpd->lock);
+
+ ret = pm_generic_prepare(dev);
+ /*
+ * This should be done by the core, after calling the subsystem-level
+ * .prepare(), but moving it to the core at this point would break a
+ * number of platform drivers.
+ */
+ pm_runtime_disable(dev);
+
+ return ret;
+}
+
+/**
+ * pm_genpd_suspend - Suspend a device belonging to an I/O PM domain.
+ * @dev: Device to suspend.
+ *
+ * Suspend a device under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_suspend(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_suspend(dev);
+}
+
+/**
+ * pm_genpd_suspend_noirq - Late suspend of a device from an I/O PM domain.
+ * @dev: Device to suspend.
+ *
+ * Carry out a late suspend of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_suspend_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ int ret;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ if (genpd->suspend_power_off)
+ return 0;
+
+ ret = pm_generic_suspend_noirq(dev);
+ if (ret)
+ return ret;
+
+ if (genpd->stop_device)
+ genpd->stop_device(dev);
+
+ /*
+ * Since all of the "noirq" callbacks are executed sequentially, it is
+ * guaranteed that this function will never run twice in parallel for
+ * the same PM domain, so it is not necessary to use locking here.
+ */
+ genpd->suspended_count++;
+ pm_genpd_sync_poweroff(genpd);
+
+ return 0;
+}
+
+/**
+ * pm_genpd_resume_noirq - Early resume of a device from an I/O power domain.
+ * @dev: Device to resume.
+ *
+ * Carry out an early resume of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_resume_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ if (genpd->suspend_power_off)
+ return 0;
+
+ /*
+ * Since all of the "noirq" callbacks are executed sequentially, it is
+ * guaranteed that this function will never run twice in parallel for
+ * the same PM domain, so it is not necessary to use locking here.
+ */
+ pm_genpd_poweron(genpd);
+ genpd->suspended_count--;
if (genpd->start_device)
genpd->start_device(dev);

- mutex_unlock(&genpd->lock);
+ return pm_generic_resume_noirq(dev);
+}
+
+/**
+ * pm_genpd_resume - Resume a device belonging to an I/O power domain.
+ * @dev: Device to resume.
+ *
+ * Resume a device under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_resume(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_resume(dev);
+}
+
+/**
+ * pm_genpd_freeze - Freeze a device belonging to an I/O power domain.
+ * @dev: Device to freeze.
+ *
+ * Freeze a device under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_freeze(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_freeze(dev);
+}
+
+/**
+ * pm_genpd_freeze_noirq - Late freeze of a device from an I/O power domain.
+ * @dev: Device to freeze.
+ *
+ * Carry out a late freeze of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_freeze_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ int ret;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ if (genpd->suspend_power_off)
+ return 0;
+
+ ret = pm_generic_freeze_noirq(dev);
+ if (ret)
+ return ret;
+
+ if (genpd->stop_device)
+ genpd->stop_device(dev);
+
+ return 0;
+}
+
+/**
+ * pm_genpd_thaw_noirq - Early thaw of a device from an I/O power domain.
+ * @dev: Device to thaw.
+ *
+ * Carry out an early thaw of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_thaw_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ if (genpd->suspend_power_off)
+ return 0;
+
+ if (genpd->start_device)
+ genpd->start_device(dev);
+
+ return pm_generic_thaw_noirq(dev);
+}
+
+/**
+ * pm_genpd_thaw - Thaw a device belonging to an I/O power domain.
+ * @dev: Device to thaw.
+ *
+ * Thaw a device under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_thaw(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_thaw(dev);
+}
+
+/**
+ * pm_genpd_dev_poweroff - Power off a device belonging to an I/O PM domain.
+ * @dev: Device to suspend.
+ *
+ * Power off a device under the assumption that its pm_domain field points to
+ * the domain member of an object of type struct generic_pm_domain representing
+ * a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_dev_poweroff(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_poweroff(dev);
+}
+
+/**
+ * pm_genpd_dev_poweroff_noirq - Late power off of a device from a PM domain.
+ * @dev: Device to suspend.
+ *
+ * Carry out a late powering off of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_dev_poweroff_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ int ret;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ if (genpd->suspend_power_off)
+ return 0;
+
+ ret = pm_generic_poweroff_noirq(dev);
+ if (ret)
+ return ret;
+
+ if (genpd->stop_device)
+ genpd->stop_device(dev);
+
+ /*
+ * Since all of the "noirq" callbacks are executed sequentially, it is
+ * guaranteed that this function will never run twice in parallel for
+ * the same PM domain, so it is not necessary to use locking here.
+ */
+ genpd->suspended_count++;
+ pm_genpd_sync_poweroff(genpd);

return 0;
}

+/**
+ * pm_genpd_restore_noirq - Early restore of a device from an I/O power domain.
+ * @dev: Device to resume.
+ *
+ * Carry out an early restore of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_restore_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ /*
+ * Since all of the "noirq" callbacks are executed sequentially, it is
+ * guaranteed that this function will never run twice in parallel for
+ * the same PM domain, so it is not necessary to use locking here.
+ */
+ genpd->power_is_off = true;
+ if (genpd->suspend_power_off) {
+ /*
+ * The boot kernel might put the domain into the power on state,
+ * so make sure it really is powered off.
+ */
+ if (genpd->power_off)
+ genpd->power_off(&genpd->domain);
+ return 0;
+ }
+
+ pm_genpd_poweron(genpd);
+ genpd->suspended_count--;
+ if (genpd->start_device)
+ genpd->start_device(dev);
+
+ return pm_generic_restore_noirq(dev);
+}
+
+/**
+ * pm_genpd_restore - Restore a device belonging to an I/O power domain.
+ * @dev: Device to resume.
+ *
+ * Restore a device under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_restore(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_restore(dev);
+}
+
+/**
+ * pm_genpd_complete - Complete power transition of a device in a power domain.
+ * @dev: Device to complete the transition of.
+ *
+ * Complete a power transition of a device (during a system-wide power
+ * transition) under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static void pm_genpd_complete(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ bool run_complete;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return;
+
+ pm_runtime_enable(dev);
+
+ mutex_lock(&genpd->lock);
+
+ run_complete = !genpd->suspend_power_off;
+ if (--genpd->prepared_count == 0)
+ genpd->suspend_power_off = false;
+
+ mutex_unlock(&genpd->lock);
+
+ if (run_complete)
+ pm_generic_complete(dev);
+}
+
#else

-#define pm_genpd_runtime_suspend NULL
-#define pm_genpd_runtime_resume NULL
+#define pm_genpd_prepare NULL
+#define pm_genpd_suspend NULL
+#define pm_genpd_suspend_noirq NULL
+#define pm_genpd_resume_noirq NULL
+#define pm_genpd_resume NULL
+#define pm_genpd_freeze NULL
+#define pm_genpd_freeze_noirq NULL
+#define pm_genpd_thaw_noirq NULL
+#define pm_genpd_thaw NULL
+#define pm_genpd_complete NULL

-#endif /* CONFIG_PM_RUNTIME */
+#endif /* CONFIG_PM_SLEEP */

/**
* pm_genpd_add_device - Add a device to an I/O PM domain.
@@ -305,6 +796,11 @@ int pm_genpd_add_device(struct generic_p

mutex_lock(&genpd->lock);

+ if (genpd->prepared_count > 0) {
+ ret = -EAGAIN;
+ goto out;
+ }
+
list_for_each_entry(dle, &genpd->dev_list, node)
if (dle->dev == dev) {
ret = -EINVAL;
@@ -319,6 +815,7 @@ int pm_genpd_add_device(struct generic_p

dle->dev = dev;
list_add_tail(&dle->node, &genpd->dev_list);
+ genpd->device_count++;

spin_lock_irq(&dev->power.lock);
dev->pm_domain = &genpd->domain;
@@ -348,6 +845,11 @@ int pm_genpd_remove_device(struct generi

mutex_lock(&genpd->lock);

+ if (genpd->prepared_count > 0) {
+ ret = -EAGAIN;
+ goto out;
+ }
+
list_for_each_entry(dle, &genpd->dev_list, node) {
if (dle->dev != dev)
continue;
@@ -356,6 +858,7 @@ int pm_genpd_remove_device(struct generi
dev->pm_domain = NULL;
spin_unlock_irq(&dev->power.lock);

+ genpd->device_count--;
list_del(&dle->node);
kfree(dle);

@@ -363,6 +866,7 @@ int pm_genpd_remove_device(struct generi
break;
}

+ out:
mutex_unlock(&genpd->lock);

return ret;
@@ -471,7 +975,23 @@ void pm_genpd_init(struct generic_pm_dom
genpd->in_progress = 0;
genpd->sd_count = 0;
genpd->power_is_off = is_off;
+ genpd->device_count = 0;
+ genpd->suspended_count = 0;
genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
genpd->domain.ops.runtime_resume = pm_genpd_runtime_resume;
genpd->domain.ops.runtime_idle = pm_generic_runtime_idle;
+ genpd->domain.ops.prepare = pm_genpd_prepare;
+ genpd->domain.ops.suspend = pm_genpd_suspend;
+ genpd->domain.ops.suspend_noirq = pm_genpd_suspend_noirq;
+ genpd->domain.ops.resume_noirq = pm_genpd_resume_noirq;
+ genpd->domain.ops.resume = pm_genpd_resume;
+ genpd->domain.ops.freeze = pm_genpd_freeze;
+ genpd->domain.ops.freeze_noirq = pm_genpd_freeze_noirq;
+ genpd->domain.ops.thaw_noirq = pm_genpd_thaw_noirq;
+ genpd->domain.ops.thaw = pm_genpd_thaw;
+ genpd->domain.ops.poweroff = pm_genpd_dev_poweroff;
+ genpd->domain.ops.poweroff_noirq = pm_genpd_dev_poweroff_noirq;
+ genpd->domain.ops.restore_noirq = pm_genpd_restore_noirq;
+ genpd->domain.ops.restore = pm_genpd_restore;
+ genpd->domain.ops.complete = pm_genpd_complete;
}
Index: linux-2.6/include/linux/pm_domain.h
===================================================================
--- linux-2.6.orig/include/linux/pm_domain.h
+++ linux-2.6/include/linux/pm_domain.h
@@ -11,6 +11,9 @@

#include <linux/device.h>

+#define GPD_IN_SUSPEND 1
+#define GPD_POWER_OFF 2
+
struct dev_power_governor {
bool (*power_down_ok)(struct dev_pm_domain *domain);
};
@@ -27,6 +30,10 @@ struct generic_pm_domain {
unsigned int in_progress; /* Number of devices being suspended now */
unsigned int sd_count; /* Number of subdomains with power "on" */
bool power_is_off; /* Whether or not power has been removed */
+ unsigned int device_count; /* Number of devices */
+ unsigned int suspended_count; /* System suspend device counter */
+ unsigned int prepared_count; /* Suspend counter of prepared devices */
+ bool suspend_power_off; /* Power status before system suspend */
int (*power_off)(struct dev_pm_domain *domain);
int (*power_on)(struct dev_pm_domain *domain);
int (*start_device)(struct device *dev);

2011-06-11 20:41:35

by Rafael J. Wysocki

Subject: [PATCH 8/8] ARM / shmobile: Support for I/O PM domains for SH7372 (v5)

From: Rafael J. Wysocki <[email protected]>

Use the generic power domains support introduced by the previous
patch to implement support for power domains on SH7372.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
arch/arm/mach-shmobile/Kconfig | 1
arch/arm/mach-shmobile/Makefile | 4 +
arch/arm/mach-shmobile/board-mackerel.c | 3 +
arch/arm/mach-shmobile/include/mach/sh7372.h | 16 +++++
arch/arm/mach-shmobile/pm-sh7372.c | 81 +++++++++++++++++++++++++++
5 files changed, 105 insertions(+)

Index: linux-2.6/arch/arm/mach-shmobile/board-mackerel.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/board-mackerel.c
+++ linux-2.6/arch/arm/mach-shmobile/board-mackerel.c
@@ -1496,6 +1496,9 @@ static void __init mackerel_init(void)

platform_add_devices(mackerel_devices, ARRAY_SIZE(mackerel_devices));

+ sh7372_add_device_to_domain(SH7372_A4LC, &lcdc_device);
+ sh7372_add_device_to_domain(SH7372_A4LC, &hdmi_lcdc_device);
+
hdmi_init_pm_clock();
sh7372_pm_init();
}
Index: linux-2.6/arch/arm/mach-shmobile/include/mach/sh7372.h
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/include/mach/sh7372.h
+++ linux-2.6/arch/arm/mach-shmobile/include/mach/sh7372.h
@@ -12,6 +12,7 @@
#define __ASM_SH7372_H__

#include <linux/sh_clk.h>
+#include <linux/pm_domain.h>

/*
* Pin Function Controller:
@@ -470,4 +471,19 @@ extern struct clk sh7372_fsibck_clk;
extern struct clk sh7372_fsidiva_clk;
extern struct clk sh7372_fsidivb_clk;

+struct platform_device;
+
+#ifdef CONFIG_PM
+extern struct generic_pm_domain sh7372_a4lc_domain;
+#define SH7372_A4LC (&sh7372_a4lc_domain)
+
+extern void sh7372_add_device_to_domain(struct generic_pm_domain *domain,
+ struct platform_device *pdev);
+#else
+#define SH7372_A4LC NULL
+
+static inline void sh7372_add_device_to_domain(struct generic_pm_domain *dom,
+ struct platform_device *pd) {}
+#endif /* CONFIG_PM */
+
#endif /* __ASM_SH7372_H__ */
Index: linux-2.6/arch/arm/mach-shmobile/Makefile
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/Makefile
+++ linux-2.6/arch/arm/mach-shmobile/Makefile
@@ -42,6 +42,10 @@ obj-$(CONFIG_MACH_AP4EVB) += board-ap4ev
obj-$(CONFIG_MACH_AG5EVM) += board-ag5evm.o
obj-$(CONFIG_MACH_MACKEREL) += board-mackerel.o

+# PM objects
+pm-$(CONFIG_ARCH_SH7372) += pm-sh7372.o
+
# Framework support
obj-$(CONFIG_SMP) += $(smp-y)
obj-$(CONFIG_GENERIC_GPIO) += $(pfc-y)
+obj-$(CONFIG_PM) += $(pm-y)
Index: linux-2.6/arch/arm/mach-shmobile/Kconfig
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/Kconfig
+++ linux-2.6/arch/arm/mach-shmobile/Kconfig
@@ -19,6 +19,7 @@ config ARCH_SH7372
select CPU_V7
select SH_CLK_CPG
select ARCH_WANT_OPTIONAL_GPIOLIB
+ select PM_GENERIC_DOMAINS

config ARCH_SH73A0
bool "SH-Mobile AG5 (R8A73A00)"
Index: linux-2.6/arch/arm/mach-shmobile/pm-sh7372.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux-2.6/arch/arm/mach-shmobile/pm-sh7372.c
@@ -15,16 +15,97 @@
#include <linux/list.h>
#include <linux/err.h>
#include <linux/slab.h>
+#include <linux/pm_runtime.h>
+#include <linux/platform_device.h>
#include <asm/system.h>
#include <asm/io.h>
#include <asm/tlbflush.h>
#include <mach/common.h>
+#include <mach/sh7372.h>

#define SMFRAM 0xe6a70000
#define SYSTBCR 0xe6150024
#define SBAR 0xe6180020
#define APARMBAREA 0xe6f10020

+#define SPDCR 0xe6180008
+#define SWUCR 0xe6180014
+#define PSTR 0xe6180080
+
+struct sh7372_domain_data {
+ unsigned int bit_shift;
+};
+
+static int pd_power_down(struct dev_pm_domain *domain)
+{
+ struct sh7372_domain_data *dd = domain->platform_data;
+ unsigned int mask = 1 << dd->bit_shift;
+
+ if (__raw_readl(PSTR) & mask) {
+ __raw_writel(mask, SPDCR);
+
+ while (__raw_readl(SPDCR) & mask) {}
+
+ pr_debug("sh7372 power domain down 0x%08x -> PSTR = 0x%08x\n",
+ mask, __raw_readl(PSTR));
+ }
+
+ return 0;
+}
+
+static int pd_power_up(struct dev_pm_domain *domain)
+{
+ struct sh7372_domain_data *dd = domain->platform_data;
+ unsigned int mask = 1 << dd->bit_shift;
+
+ if (!(__raw_readl(PSTR) & mask)) {
+ __raw_writel(mask, SWUCR);
+
+ while (__raw_readl(SWUCR) & mask) {}
+
+ pr_debug("sh7372 power domain up 0x%08x -> PSTR = 0x%08x\n",
+ mask, __raw_readl(PSTR));
+ }
+
+ return 0;
+}
+
+static void sh7372_init_domain(struct generic_pm_domain *domain,
+ struct sh7372_domain_data *pdata)
+{
+ pm_genpd_init(domain, NULL, false);
+ domain->domain.platform_data = pdata;
+ domain->stop_device = pm_runtime_clk_suspend;
+ domain->start_device = pm_runtime_clk_resume;
+ domain->power_off = pd_power_down;
+ domain->power_on = pd_power_up;
+}
+
+void sh7372_add_device_to_domain(struct generic_pm_domain *domain,
+ struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+
+ if (!dev->power.subsys_data) {
+ pm_runtime_clk_init(dev);
+ pm_runtime_clk_add(dev, NULL);
+ }
+ pm_genpd_add_device(domain, dev);
+}
+
+static struct sh7372_domain_data sh7372_a4lc_domain_data = {
+ .bit_shift = 1,
+};
+
+struct generic_pm_domain sh7372_a4lc_domain;
+
+static int __init sh7372_power_domains_init(void)
+{
+ sh7372_init_domain(&sh7372_a4lc_domain, &sh7372_a4lc_domain_data);
+ return 0;
+}
+core_initcall(sh7372_power_domains_init);
+
static void sh7372_enter_core_standby(void)
{
void __iomem *smfram = (void __iomem *)SMFRAM;

2011-06-11 20:57:21

by Greg KH

Subject: Re: [PATCH 0/8] PM / Domains: Support for generic I/O PM domains (v5)

On Sat, Jun 11, 2011 at 10:23:04PM +0200, Rafael J. Wysocki wrote:
> Hi,
>
> This is the 4th update of the patchset adding support for generic I/O PM
> domains. The patches have been reworked quite a bit to take feedback into
> account, but I left the Greg's ACK in [4/8] in the hope it still applies
> (Greg, please let me know in case it doesn't :-)).

It still applies, thanks for leaving it in.

2011-06-11 23:27:25

by Rafael J. Wysocki

Subject: [Update][PATCH 7/8] PM / Domains: System-wide transitions support for generic domains (v2)

From: Rafael J. Wysocki <[email protected]>

Make generic PM domains support system-wide power transitions
(system suspend and hibernation). Add suspend, resume, freeze, thaw,
poweroff and restore callbacks to be associated with struct
generic_pm_domain objects and make pm_genpd_init() use them as
appropriate.

The new callbacks do nothing for devices belonging to power domains
that were powered down at run time (before the transition). For the
other devices the action carried out depends on the type of the
transition. During system suspend the power domain .suspend()
callback executes pm_generic_suspend() for the device, while the
PM domain .suspend_noirq() callback runs pm_generic_suspend_noirq()
for it, stops it and eventually removes power from the PM domain it
belongs to (after all devices in the domain have been stopped and its
subdomains have been powered off).

During system resume the PM domain .resume_noirq() callback
restores power to the PM domain (when executed for it first time),
starts the device and executes pm_generic_resume_noirq() for it,
while the .resume() callback executes pm_generic_resume() for the
device. Finally, the .complete() callback executes pm_runtime_idle()
for the device which should put it back into the suspended state if
its runtime PM usage count is equal to zero at that time.

The actions carried out during hibernation and resume from it are
analogous to the ones described above.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---

One fix here. Namely, the pm_runtime_disable() in pm_genpd_prepare() has
to go under the genpd lock to prevent runtime PM from happening after we've
done __pm_genpd_runtime_resume(), because pm_genpd_suspend() assumes that
the device will be active. Of course, that causes pm_generic_prepare()
to be called after pm_runtime_disable(), so it makes sense to call
pm_generic_complete() before pm_runtime_enable() in pm_genpd_complete().

Thanks,
Rafael

---
drivers/base/power/domain.c | 537 +++++++++++++++++++++++++++++++++++++++++++-
include/linux/pm_domain.h | 7
2 files changed, 532 insertions(+), 12 deletions(-)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -40,7 +40,8 @@ static int pm_genpd_poweron(struct gener
mutex_lock(&genpd->parent->lock);
mutex_lock(&genpd->lock);

- if (!genpd->power_is_off)
+ if (!genpd->power_is_off
+ || (genpd->prepared_count > 0 && genpd->suspend_power_off))
goto out;

if (genpd->parent && genpd->parent->power_is_off) {
@@ -123,7 +124,7 @@ static int pm_genpd_poweroff(struct gene
unsigned int not_suspended;
int ret;

- if (genpd->power_is_off)
+ if (genpd->power_is_off || genpd->prepared_count > 0)
return 0;

if (genpd->sd_count > 0)
@@ -241,6 +242,27 @@ static int pm_genpd_runtime_suspend(stru
}

/**
+ * __pm_genpd_runtime_resume - Resume a device belonging to I/O PM domain.
+ * @dev: Device to resume.
+ * @genpd: PM domain the device belongs to.
+ */
+static void __pm_genpd_runtime_resume(struct device *dev,
+ struct generic_pm_domain *genpd)
+{
+ struct dev_list_entry *dle;
+
+ list_for_each_entry(dle, &genpd->dev_list, node) {
+ if (dle->dev == dev) {
+ __pm_genpd_restore_device(dle, genpd);
+ break;
+ }
+ }
+
+ if (genpd->start_device)
+ genpd->start_device(dev);
+}
+
+/**
* pm_genpd_runtime_resume - Resume a device belonging to I/O PM domain.
* @dev: Device to resume.
*
@@ -251,7 +273,6 @@ static int pm_genpd_runtime_suspend(stru
static int pm_genpd_runtime_resume(struct device *dev)
{
struct generic_pm_domain *genpd;
- struct dev_list_entry *dle;
int ret;

dev_dbg(dev, "%s()\n", __func__);
@@ -265,28 +286,491 @@ static int pm_genpd_runtime_resume(struc
return ret;

mutex_lock(&genpd->lock);
+ __pm_genpd_runtime_resume(dev, genpd);
+ mutex_unlock(&genpd->lock);

- list_for_each_entry(dle, &genpd->dev_list, node) {
- if (dle->dev == dev) {
- __pm_genpd_restore_device(dle, genpd);
- break;
- }
+ return 0;
+}
+
+#else
+
+static inline void __pm_genpd_runtime_resume(struct device *dev,
+ struct generic_pm_domain *genpd) {}
+
+#define pm_genpd_runtime_suspend NULL
+#define pm_genpd_runtime_resume NULL
+
+#endif /* CONFIG_PM_RUNTIME */
+
+#ifdef CONFIG_PM_SLEEP
+
+/**
+ * pm_genpd_sync_poweroff - Synchronously power off a PM domain and its parents.
+ * @genpd: PM domain to power off, if possible.
+ *
+ * Check if the given PM domain can be powered off (during system suspend or
+ * hibernation) and do that if so. Also, in that case propagate to its parent.
+ *
+ * This function is only called in "noirq" stages of system power transitions,
+ * so it need not acquire locks (all of the "noirq" callbacks are executed
+ * sequentially, so it is guaranteed that it will never run twice in parallel).
+ */
+static void pm_genpd_sync_poweroff(struct generic_pm_domain *genpd)
+{
+ struct generic_pm_domain *parent = genpd->parent;
+
+ if (genpd->power_is_off)
+ return;
+
+ if (genpd->suspended_count != genpd->device_count || genpd->sd_count > 0)
+ return;
+
+ if (genpd->power_off)
+ genpd->power_off(&genpd->domain);
+
+ genpd->power_is_off = true;
+ if (parent) {
+ genpd_sd_counter_dec(parent);
+ pm_genpd_sync_poweroff(parent);
+ }
+}
+
+/**
+ * pm_genpd_prepare - Start power transition of a device in a PM domain.
+ * @dev: Device to start the transition of.
+ *
+ * Start a power transition of a device (during a system-wide power transition)
+ * under the assumption that its pm_domain field points to the domain member of
+ * an object of type struct generic_pm_domain representing a PM domain
+ * consisting of I/O devices.
+ */
+static int pm_genpd_prepare(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ mutex_lock(&genpd->lock);
+
+ if (genpd->prepared_count++ == 0)
+ genpd->suspend_power_off = genpd->power_is_off;
+
+ if (genpd->suspend_power_off) {
+ mutex_unlock(&genpd->lock);
+ return 0;
}

+ /*
+ * If the device is in the (runtime) "suspended" state, call
+ * .start_device() for it, if defined.
+ */
+ if (pm_runtime_suspended(dev))
+ __pm_genpd_runtime_resume(dev, genpd);
+
+ pm_runtime_disable(dev);
+
+ mutex_unlock(&genpd->lock);
+
+ return pm_generic_prepare(dev);
+}
+
+/**
+ * pm_genpd_suspend - Suspend a device belonging to an I/O PM domain.
+ * @dev: Device to suspend.
+ *
+ * Suspend a device under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_suspend(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_suspend(dev);
+}
+
+/**
+ * pm_genpd_suspend_noirq - Late suspend of a device from an I/O PM domain.
+ * @dev: Device to suspend.
+ *
+ * Carry out a late suspend of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_suspend_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ int ret;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ if (genpd->suspend_power_off)
+ return 0;
+
+ ret = pm_generic_suspend_noirq(dev);
+ if (ret)
+ return ret;
+
+ if (genpd->stop_device)
+ genpd->stop_device(dev);
+
+ /*
+ * Since all of the "noirq" callbacks are executed sequentially, it is
+ * guaranteed that this function will never run twice in parallel for
+ * the same PM domain, so it is not necessary to use locking here.
+ */
+ genpd->suspended_count++;
+ pm_genpd_sync_poweroff(genpd);
+
+ return 0;
+}
+
+/**
+ * pm_genpd_resume_noirq - Early resume of a device from an I/O power domain.
+ * @dev: Device to resume.
+ *
+ * Carry out an early resume of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_resume_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ if (genpd->suspend_power_off)
+ return 0;
+
+ /*
+ * Since all of the "noirq" callbacks are executed sequentially, it is
+ * guaranteed that this function will never run twice in parallel for
+ * the same PM domain, so it is not necessary to use locking here.
+ */
+ pm_genpd_poweron(genpd);
+ genpd->suspended_count--;
if (genpd->start_device)
genpd->start_device(dev);

- mutex_unlock(&genpd->lock);
+ return pm_generic_resume_noirq(dev);
+}
+
+/**
+ * pm_genpd_resume - Resume a device belonging to an I/O power domain.
+ * @dev: Device to resume.
+ *
+ * Resume a device under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_resume(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_resume(dev);
+}
+
+/**
+ * pm_genpd_freeze - Freeze a device belonging to an I/O power domain.
+ * @dev: Device to freeze.
+ *
+ * Freeze a device under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_freeze(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_freeze(dev);
+}
+
+/**
+ * pm_genpd_freeze_noirq - Late freeze of a device from an I/O power domain.
+ * @dev: Device to freeze.
+ *
+ * Carry out a late freeze of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_freeze_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ int ret;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ if (genpd->suspend_power_off)
+ return 0;
+
+ ret = pm_generic_freeze_noirq(dev);
+ if (ret)
+ return ret;
+
+ if (genpd->stop_device)
+ genpd->stop_device(dev);

return 0;
}

+/**
+ * pm_genpd_thaw_noirq - Early thaw of a device from an I/O power domain.
+ * @dev: Device to thaw.
+ *
+ * Carry out an early thaw of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_thaw_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ if (genpd->suspend_power_off)
+ return 0;
+
+ if (genpd->start_device)
+ genpd->start_device(dev);
+
+ return pm_generic_thaw_noirq(dev);
+}
+
+/**
+ * pm_genpd_thaw - Thaw a device belonging to an I/O power domain.
+ * @dev: Device to thaw.
+ *
+ * Thaw a device under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_thaw(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_thaw(dev);
+}
+
+/**
+ * pm_genpd_dev_poweroff - Power off a device belonging to an I/O PM domain.
+ * @dev: Device to suspend.
+ *
+ * Power off a device under the assumption that its pm_domain field points to
+ * the domain member of an object of type struct generic_pm_domain representing
+ * a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_dev_poweroff(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_poweroff(dev);
+}
+
+/**
+ * pm_genpd_dev_poweroff_noirq - Late power off of a device from a PM domain.
+ * @dev: Device to suspend.
+ *
+ * Carry out a late powering off of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_dev_poweroff_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ int ret;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ if (genpd->suspend_power_off)
+ return 0;
+
+ ret = pm_generic_poweroff_noirq(dev);
+ if (ret)
+ return ret;
+
+ if (genpd->stop_device)
+ genpd->stop_device(dev);
+
+ /*
+ * Since all of the "noirq" callbacks are executed sequentially, it is
+ * guaranteed that this function will never run twice in parallel for
+ * the same PM domain, so it is not necessary to use locking here.
+ */
+ genpd->suspended_count++;
+ pm_genpd_sync_poweroff(genpd);
+
+ return 0;
+}
+
+/**
+ * pm_genpd_restore_noirq - Early restore of a device from an I/O power domain.
+ * @dev: Device to resume.
+ *
+ * Carry out an early restore of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_restore_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ /*
+ * Since all of the "noirq" callbacks are executed sequentially, it is
+ * guaranteed that this function will never run twice in parallel for
+ * the same PM domain, so it is not necessary to use locking here.
+ */
+ genpd->power_is_off = true;
+ if (genpd->suspend_power_off) {
+ /*
+ * The boot kernel might put the domain into the power on state,
+ * so make sure it really is powered off.
+ */
+ if (genpd->power_off)
+ genpd->power_off(&genpd->domain);
+ return 0;
+ }
+
+ pm_genpd_poweron(genpd);
+ genpd->suspended_count--;
+ if (genpd->start_device)
+ genpd->start_device(dev);
+
+ return pm_generic_restore_noirq(dev);
+}
+
+/**
+ * pm_genpd_restore - Restore a device belonging to an I/O power domain.
+ * @dev: Device to resume.
+ *
+ * Restore a device under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_restore(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_restore(dev);
+}
+
+/**
+ * pm_genpd_complete - Complete power transition of a device in a power domain.
+ * @dev: Device to complete the transition of.
+ *
+ * Complete a power transition of a device (during a system-wide power
+ * transition) under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static void pm_genpd_complete(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ bool run_complete;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return;
+
+ mutex_lock(&genpd->lock);
+
+ run_complete = !genpd->suspend_power_off;
+ if (--genpd->prepared_count == 0)
+ genpd->suspend_power_off = false;
+
+ mutex_unlock(&genpd->lock);
+
+ if (run_complete)
+ pm_generic_complete(dev);
+
+ pm_runtime_enable(dev);
+}
+
#else

-#define pm_genpd_runtime_suspend NULL
-#define pm_genpd_runtime_resume NULL
+#define pm_genpd_prepare NULL
+#define pm_genpd_suspend NULL
+#define pm_genpd_suspend_noirq NULL
+#define pm_genpd_resume_noirq NULL
+#define pm_genpd_resume NULL
+#define pm_genpd_freeze NULL
+#define pm_genpd_freeze_noirq NULL
+#define pm_genpd_thaw_noirq NULL
+#define pm_genpd_thaw NULL
+#define pm_genpd_complete NULL

-#endif /* CONFIG_PM_RUNTIME */
+#endif /* CONFIG_PM_SLEEP */

/**
* pm_genpd_add_device - Add a device to an I/O PM domain.
@@ -305,6 +789,11 @@ int pm_genpd_add_device(struct generic_p

mutex_lock(&genpd->lock);

+ if (genpd->prepared_count > 0) {
+ ret = -EAGAIN;
+ goto out;
+ }
+
list_for_each_entry(dle, &genpd->dev_list, node)
if (dle->dev == dev) {
ret = -EINVAL;
@@ -319,6 +808,7 @@ int pm_genpd_add_device(struct generic_p

dle->dev = dev;
list_add_tail(&dle->node, &genpd->dev_list);
+ genpd->device_count++;

spin_lock_irq(&dev->power.lock);
dev->pm_domain = &genpd->domain;
@@ -348,6 +838,11 @@ int pm_genpd_remove_device(struct generi

mutex_lock(&genpd->lock);

+ if (genpd->prepared_count > 0) {
+ ret = -EAGAIN;
+ goto out;
+ }
+
list_for_each_entry(dle, &genpd->dev_list, node) {
if (dle->dev != dev)
continue;
@@ -356,6 +851,7 @@ int pm_genpd_remove_device(struct generi
dev->pm_domain = NULL;
spin_unlock_irq(&dev->power.lock);

+ genpd->device_count--;
list_del(&dle->node);
kfree(dle);

@@ -363,6 +859,7 @@ int pm_genpd_remove_device(struct generi
break;
}

+ out:
mutex_unlock(&genpd->lock);

return ret;
@@ -471,7 +968,23 @@ void pm_genpd_init(struct generic_pm_dom
genpd->in_progress = 0;
genpd->sd_count = 0;
genpd->power_is_off = is_off;
+ genpd->device_count = 0;
+ genpd->suspended_count = 0;
genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
genpd->domain.ops.runtime_resume = pm_genpd_runtime_resume;
genpd->domain.ops.runtime_idle = pm_generic_runtime_idle;
+ genpd->domain.ops.prepare = pm_genpd_prepare;
+ genpd->domain.ops.suspend = pm_genpd_suspend;
+ genpd->domain.ops.suspend_noirq = pm_genpd_suspend_noirq;
+ genpd->domain.ops.resume_noirq = pm_genpd_resume_noirq;
+ genpd->domain.ops.resume = pm_genpd_resume;
+ genpd->domain.ops.freeze = pm_genpd_freeze;
+ genpd->domain.ops.freeze_noirq = pm_genpd_freeze_noirq;
+ genpd->domain.ops.thaw_noirq = pm_genpd_thaw_noirq;
+ genpd->domain.ops.thaw = pm_genpd_thaw;
+ genpd->domain.ops.poweroff = pm_genpd_dev_poweroff;
+ genpd->domain.ops.poweroff_noirq = pm_genpd_dev_poweroff_noirq;
+ genpd->domain.ops.restore_noirq = pm_genpd_restore_noirq;
+ genpd->domain.ops.restore = pm_genpd_restore;
+ genpd->domain.ops.complete = pm_genpd_complete;
}
Index: linux-2.6/include/linux/pm_domain.h
===================================================================
--- linux-2.6.orig/include/linux/pm_domain.h
+++ linux-2.6/include/linux/pm_domain.h
@@ -11,6 +11,9 @@

#include <linux/device.h>

+#define GPD_IN_SUSPEND 1
+#define GPD_POWER_OFF 2
+
struct dev_power_governor {
bool (*power_down_ok)(struct dev_pm_domain *domain);
};
@@ -27,6 +30,10 @@ struct generic_pm_domain {
unsigned int in_progress; /* Number of devices being suspended now */
unsigned int sd_count; /* Number of subdomains with power "on" */
bool power_is_off; /* Whether or not power has been removed */
+ unsigned int device_count; /* Number of devices */
+ unsigned int suspended_count; /* System suspend device counter */
+ unsigned int prepared_count; /* Suspend counter of prepared devices */
+ bool suspend_power_off; /* Power status before system suspend */
int (*power_off)(struct dev_pm_domain *domain);
int (*power_on)(struct dev_pm_domain *domain);
int (*start_device)(struct device *dev);

2011-06-14 13:12:44

by Magnus Damm

Subject: Re: [PATCH 8/8] ARM / shmobile: Support for I/O PM domains for SH7372 (v5)

Hi Rafael,

On Sun, Jun 12, 2011 at 5:40 AM, Rafael J. Wysocki <[email protected]> wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Use the generic power domains support introduced by the previous
> patch to implement support for power domains on SH7372.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---

Thanks for your work on this. I just tried this on my Mackerel board,
but I can't seem to get the pd_power_up() and pd_power_down()
callbacks to be executed. It is probably a misconfiguration from my
side.

Here's some feedback on the sh7372-specific code:

> --- linux-2.6.orig/arch/arm/mach-shmobile/Kconfig
> +++ linux-2.6/arch/arm/mach-shmobile/Kconfig
> @@ -19,6 +19,7 @@ config ARCH_SH7372
> ? ? ? ?select CPU_V7
> ? ? ? ?select SH_CLK_CPG
> ? ? ? ?select ARCH_WANT_OPTIONAL_GPIOLIB
> + ? ? ? select PM_GENERIC_DOMAINS

We want to support a single ARM binary for multiple boards, so this
should be enabled for all SoCs in mach-shmobile as a whole.

> --- linux-2.6.orig/arch/arm/mach-shmobile/pm-sh7372.c
> +++ linux-2.6/arch/arm/mach-shmobile/pm-sh7372.c
> @@ -15,16 +15,97 @@
> ?#include <linux/list.h>
> ?#include <linux/err.h>
> ?#include <linux/slab.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/platform_device.h>
> ?#include <asm/system.h>
> ?#include <asm/io.h>
> ?#include <asm/tlbflush.h>
> ?#include <mach/common.h>
> +#include <mach/sh7372.h>
>
> ?#define SMFRAM 0xe6a70000
> ?#define SYSTBCR 0xe6150024
> ?#define SBAR 0xe6180020
> ?#define APARMBAREA 0xe6f10020
>
> +#define SPDCR 0xe6180008
> +#define SWUCR 0xe6180014
> +#define PSTR 0xe6180080
> +
> +struct sh7372_domain_data {
> + ? ? ? unsigned int bit_shift;
> +};

Is it possible to make struct sh7372_domain_data include struct
generic_pm_domain? I suspect so since these two data types seem to be
linked together. I guess container_of() can be used for conversion
between the types?

> +
> +static int pd_power_down(struct dev_pm_domain *domain)
> +{
> + ? ? ? struct sh7372_domain_data *dd = domain->platform_data;
> + ? ? ? unsigned int mask = 1 << dd->bit_shift;
> +
> + ? ? ? if (__raw_readl(PSTR) & mask) {
> + ? ? ? ? ? ? ? __raw_writel(mask, SPDCR);
> +
> + ? ? ? ? ? ? ? while (__raw_readl(SPDCR) & mask) {}

This probably wants a cpu_relax() in the polling loop. I realize that
my prototype hack lacked that...

> + ? ? ? ? ? ? ? pr_debug("sh7372 power domain down 0x%08x -> PSTR = 0x%08x\n",
> + ? ? ? ? ? ? ? ? ? ? ? ?mask, __raw_readl(PSTR));
> + ? ? ? }
> +
> + ? ? ? return 0;
> +}
> +
> +static int pd_power_up(struct dev_pm_domain *domain)
> +{
> + ? ? ? struct sh7372_domain_data *dd = domain->platform_data;
> + ? ? ? unsigned int mask = 1 << dd->bit_shift;
> +
> + ? ? ? if (!(__raw_readl(PSTR) & mask)) {
> + ? ? ? ? ? ? ? __raw_writel(mask, SWUCR);
> +
> + ? ? ? ? ? ? ? while (__raw_readl(SWUCR) & mask) {}

Same cpu_relax() here.

> +static int __init sh7372_power_domains_init(void)
> +{
> + ? ? ? sh7372_init_domain(&sh7372_a4lc_domain, &sh7372_a4lc_domain_data);
> + ? ? ? return 0;
> +}
> +core_initcall(sh7372_power_domains_init);

This initcall is going to be executed regardless which SoC we're
running on. We only want it called for sh7372 though.

If you look at other SoC-specific code then you will notice that
initcalls are only used for functions in mach-shmobile/ that are
common for all SoCs implemented under mach-shmobile.

You most likely want to initialize from sh7372_pm_init(), but for that
to work you probably have to reorder your code inside mackerel_init().

Thanks,

/ magnus

2011-06-14 21:15:42

by Rafael J. Wysocki

Subject: Re: [PATCH 8/8] ARM / shmobile: Support for I/O PM domains for SH7372 (v5)

On Tuesday, June 14, 2011, Magnus Damm wrote:
> Hi Rafael,

Hi,

> On Sun, Jun 12, 2011 at 5:40 AM, Rafael J. Wysocki <[email protected]> wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > Use the generic power domains support introduced by the previous
> > patch to implement support for power domains on SH7372.
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
>
> Thanks for your work on this. I just tried this on my Mackerel board,
> but I can't seem to get the pd_power_up() and pd_power_down()
> callbacks to be executed. It is probably a misconfiguration from my
> side.

They trigger for me e.g. after doing

# echo 3 > /sys/devices/platform/sh_mobile_lcdc_fb.0/graphics/fb0/blank

Attached is the .config I've been using.

> Here's some feedback on the sh7372-specific code:
>
> > --- linux-2.6.orig/arch/arm/mach-shmobile/Kconfig
> > +++ linux-2.6/arch/arm/mach-shmobile/Kconfig
> > @@ -19,6 +19,7 @@ config ARCH_SH7372
> > select CPU_V7
> > select SH_CLK_CPG
> > select ARCH_WANT_OPTIONAL_GPIOLIB
> > + select PM_GENERIC_DOMAINS
>
> We want to support a single ARM binary for multiple boards,

Surely CONFIG_ARCH_SH7372 will be set in that binary?

> so this should be enabled for all SoCs in mach-shmobile as a whole.

OK, where exactly do you want me to move it?

> > --- linux-2.6.orig/arch/arm/mach-shmobile/pm-sh7372.c
> > +++ linux-2.6/arch/arm/mach-shmobile/pm-sh7372.c
> > @@ -15,16 +15,97 @@
> > #include <linux/list.h>
> > #include <linux/err.h>
> > #include <linux/slab.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/platform_device.h>
> > #include <asm/system.h>
> > #include <asm/io.h>
> > #include <asm/tlbflush.h>
> > #include <mach/common.h>
> > +#include <mach/sh7372.h>
> >
> > #define SMFRAM 0xe6a70000
> > #define SYSTBCR 0xe6150024
> > #define SBAR 0xe6180020
> > #define APARMBAREA 0xe6f10020
> >
> > +#define SPDCR 0xe6180008
> > +#define SWUCR 0xe6180014
> > +#define PSTR 0xe6180080
> > +
> > +struct sh7372_domain_data {
> > + unsigned int bit_shift;
> > +};
>
> Is it possible to make struct sh7372_domain_data include struct
> generic_pm_domain?

It should be possible to do that.

Do I understand it correctly that you want one structure definition per
power domain instead of the two?

> I suspect so since these two data types seem to be linked together.
> I guess container_of() can be used for conversion between the types?

I don't think that will be necessary. :-)

> > +
> > +static int pd_power_down(struct dev_pm_domain *domain)
> > +{
> > + struct sh7372_domain_data *dd = domain->platform_data;
> > + unsigned int mask = 1 << dd->bit_shift;
> > +
> > + if (__raw_readl(PSTR) & mask) {
> > + __raw_writel(mask, SPDCR);
> > +
> > + while (__raw_readl(SPDCR) & mask) {}
>
> This probably wants a cpu_relax() in the polling loop. I realize that
> my prototype hack lacked that...

OK

> > + pr_debug("sh7372 power domain down 0x%08x -> PSTR = 0x%08x\n",
> > + mask, __raw_readl(PSTR));
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int pd_power_up(struct dev_pm_domain *domain)
> > +{
> > + struct sh7372_domain_data *dd = domain->platform_data;
> > + unsigned int mask = 1 << dd->bit_shift;
> > +
> > + if (!(__raw_readl(PSTR) & mask)) {
> > + __raw_writel(mask, SWUCR);
> > +
> > + while (__raw_readl(SWUCR) & mask) {}
>
> Same cpu_relax() here.

OK

> > +static int __init sh7372_power_domains_init(void)
> > +{
> > + sh7372_init_domain(&sh7372_a4lc_domain, &sh7372_a4lc_domain_data);
> > + return 0;
> > +}
> > +core_initcall(sh7372_power_domains_init);
>
> This initcall is going to be executed regardless which SoC we're
> running on. We only want it called for sh7372 though.

OK

> If you look at other SoC-specific code then you will notice that
> initcalls are only used for functions in mach-shmobile/ that are
> common for all SoCs implemented under mach-shmobile.
>
> You most likely want to initialize from sh7372_pm_init(), but for that
> to work you probably have to reorder your code inside mackerel_init().

OK, I'll figure out how to do the initialization correctly.

Thanks,
Rafael

Attachments:

2011-06-15 14:17:21

by Magnus Damm

Subject: Re: [PATCH 8/8] ARM / shmobile: Support for I/O PM domains for SH7372 (v5)

On Wed, Jun 15, 2011 at 6:16 AM, Rafael J. Wysocki <[email protected]> wrote:
> On Tuesday, June 14, 2011, Magnus Damm wrote:
>> On Sun, Jun 12, 2011 at 5:40 AM, Rafael J. Wysocki <[email protected]> wrote:
>> > From: Rafael J. Wysocki <[email protected]>
>> >
>> > Use the generic power domains support introduced by the previous
>> > patch to implement support for power domains on SH7372.
>> >
>> > Signed-off-by: Rafael J. Wysocki <[email protected]>
>> > ---
>>
>> Thanks for your work on this. I just tried this on my Mackerel board,
>> but I can't seem to get the pd_power_up() and pd_power_down()
>> callbacks to be executed. It is probably a misconfiguration from my
>> side.
>
> They trigger for me e.g. after doing
>
> # echo 3 > /sys/devices/platform/sh_mobile_lcdc_fb.0/graphics/fb0/blank
>
> Attached is the .config I've been using.

Thanks, I can trigger using sysfs and your kernel configuration.

However, I assumed it also would work when the sceen saver kicked in.
I recall it being fbcon that controls the screen save, perhaps
something else. So just wait a bit and see if you also can reproduce
it. The console gets black but the power is still on...

Also forcing to go back to powered-on state (see below) doesn't work that well:
# echo 0 > /sys/devices/platform/sh_mobile_lcdc_fb.0/graphics/fb0/blank

It looks like we loose the panning information somehow. Most likely a
LCDC driver bug. Unless the driver callbacks are not being invoked as
expected.

Also, there is garbage in on the screen if FB_SH_MOBILE_MERAM is
enabled. The MERAM hardware is a 1.5 MiB memory block that can be used
as a LCD cache. It sits in the same hardware power domain as the
LCDCs. I don't think the MERAM software supports power down
unfortunately. Disabling MERAM support removes the garbage on the
screen.

>> Here's some feedback on the sh7372-specific code:
>>
>> > --- linux-2.6.orig/arch/arm/mach-shmobile/Kconfig
>> > +++ linux-2.6/arch/arm/mach-shmobile/Kconfig
>> > @@ -19,6 +19,7 @@ config ARCH_SH7372
>> > ? ? ? ?select CPU_V7
>> > ? ? ? ?select SH_CLK_CPG
>> > ? ? ? ?select ARCH_WANT_OPTIONAL_GPIOLIB
>> > + ? ? ? select PM_GENERIC_DOMAINS
>>
>> We want to support a single ARM binary for multiple boards,
>
> Surely CONFIG_ARCH_SH7372 will be set in that binary?
>
>> so this should be enabled for all SoCs in mach-shmobile as a whole.
>
> OK, where exactly do you want me to move it?

Ideally to ARCH_SHMOBILE in arch/arm/Kconfig.

>> > --- linux-2.6.orig/arch/arm/mach-shmobile/pm-sh7372.c
>> > +++ linux-2.6/arch/arm/mach-shmobile/pm-sh7372.c
>> > @@ -15,16 +15,97 @@
>> > ?#include <linux/list.h>
>> > ?#include <linux/err.h>
>> > ?#include <linux/slab.h>
>> > +#include <linux/pm_runtime.h>
>> > +#include <linux/platform_device.h>
>> > ?#include <asm/system.h>
>> > ?#include <asm/io.h>
>> > ?#include <asm/tlbflush.h>
>> > ?#include <mach/common.h>
>> > +#include <mach/sh7372.h>
>> >
>> > ?#define SMFRAM 0xe6a70000
>> > ?#define SYSTBCR 0xe6150024
>> > ?#define SBAR 0xe6180020
>> > ?#define APARMBAREA 0xe6f10020
>> >
>> > +#define SPDCR 0xe6180008
>> > +#define SWUCR 0xe6180014
>> > +#define PSTR 0xe6180080
>> > +
>> > +struct sh7372_domain_data {
>> > + ? ? ? unsigned int bit_shift;
>> > +};
>>
>> Is it possible to make struct sh7372_domain_data include struct
>> generic_pm_domain?
>
> It should be possible to do that.
>
> Do I understand it correctly that you want one structure definition per
> power domain instead of the two?

Yes, at least that's what I would do to keep the data together. I
don't care that much though, so feel free to implement it however
you'd like.

>> > +core_initcall(sh7372_power_domains_init);
>>
>> This initcall is going to be executed regardless which SoC we're
>> running on. We only want it called for sh7372 though.
>
> OK
>
>> If you look at other SoC-specific code then you will notice that
>> initcalls are only used for functions in mach-shmobile/ that are
>> common for all SoCs implemented under mach-shmobile.
>>
>> You most likely want to initialize from sh7372_pm_init(), but for that
>> to work you probably have to reorder your code inside mackerel_init().
>
> OK, I'll figure out how to do the initialization correctly.

Please do!

Thanks,

/ magnus

2011-06-15 23:06:17

by Rafael J. Wysocki

Subject: Re: [PATCH 8/8] ARM / shmobile: Support for I/O PM domains for SH7372 (v5)

On Wednesday, June 15, 2011, Magnus Damm wrote:
> On Wed, Jun 15, 2011 at 6:16 AM, Rafael J. Wysocki <[email protected]> wrote:
> > On Tuesday, June 14, 2011, Magnus Damm wrote:
> >> On Sun, Jun 12, 2011 at 5:40 AM, Rafael J. Wysocki <[email protected]> wrote:
> >> > From: Rafael J. Wysocki <[email protected]>
> >> >
> >> > Use the generic power domains support introduced by the previous
> >> > patch to implement support for power domains on SH7372.
> >> >
> >> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> >> > ---
> >>
> >> Thanks for your work on this. I just tried this on my Mackerel board,
> >> but I can't seem to get the pd_power_up() and pd_power_down()
> >> callbacks to be executed. It is probably a misconfiguration from my
> >> side.
> >
> > They trigger for me e.g. after doing
> >
> > # echo 3 > /sys/devices/platform/sh_mobile_lcdc_fb.0/graphics/fb0/blank
> >
> > Attached is the .config I've been using.
>
> Thanks, I can trigger using sysfs and your kernel configuration.

Good.

> However, I assumed it also would work when the sceen saver kicked in.
> I recall it being fbcon that controls the screen save, perhaps
> something else. So just wait a bit and see if you also can reproduce
> it. The console gets black but the power is still on...

I noticed that, but I think it simply means pm_runtime_put() isn't
called in that code path.

> Also forcing to go back to powered-on state (see below) doesn't work that well:
> # echo 0 > /sys/devices/platform/sh_mobile_lcdc_fb.0/graphics/fb0/blank
>
> It looks like we loose the panning information somehow. Most likely a
> LCDC driver bug. Unless the driver callbacks are not being invoked as
> expected.

That should be easy to verify, I'll do that.

> Also, there is garbage in on the screen if FB_SH_MOBILE_MERAM is
> enabled. The MERAM hardware is a 1.5 MiB memory block that can be used
> as a LCD cache. It sits in the same hardware power domain as the
> LCDCs. I don't think the MERAM software supports power down
> unfortunately. Disabling MERAM support removes the garbage on the
> screen.

Well, I can't really comment here.

> >> Here's some feedback on the sh7372-specific code:
> >>
> >> > --- linux-2.6.orig/arch/arm/mach-shmobile/Kconfig
> >> > +++ linux-2.6/arch/arm/mach-shmobile/Kconfig
> >> > @@ -19,6 +19,7 @@ config ARCH_SH7372
> >> > select CPU_V7
> >> > select SH_CLK_CPG
> >> > select ARCH_WANT_OPTIONAL_GPIOLIB
> >> > + select PM_GENERIC_DOMAINS
> >>
> >> We want to support a single ARM binary for multiple boards,
> >
> > Surely CONFIG_ARCH_SH7372 will be set in that binary?
> >
> >> so this should be enabled for all SoCs in mach-shmobile as a whole.
> >
> > OK, where exactly do you want me to move it?
>
> Ideally to ARCH_SHMOBILE in arch/arm/Kconfig.

OK

> >> > --- linux-2.6.orig/arch/arm/mach-shmobile/pm-sh7372.c
> >> > +++ linux-2.6/arch/arm/mach-shmobile/pm-sh7372.c
> >> > @@ -15,16 +15,97 @@
> >> > #include <linux/list.h>
> >> > #include <linux/err.h>
> >> > #include <linux/slab.h>
> >> > +#include <linux/pm_runtime.h>
> >> > +#include <linux/platform_device.h>
> >> > #include <asm/system.h>
> >> > #include <asm/io.h>
> >> > #include <asm/tlbflush.h>
> >> > #include <mach/common.h>
> >> > +#include <mach/sh7372.h>
> >> >
> >> > #define SMFRAM 0xe6a70000
> >> > #define SYSTBCR 0xe6150024
> >> > #define SBAR 0xe6180020
> >> > #define APARMBAREA 0xe6f10020
> >> >
> >> > +#define SPDCR 0xe6180008
> >> > +#define SWUCR 0xe6180014
> >> > +#define PSTR 0xe6180080
> >> > +
> >> > +struct sh7372_domain_data {
> >> > + unsigned int bit_shift;
> >> > +};
> >>
> >> Is it possible to make struct sh7372_domain_data include struct
> >> generic_pm_domain?
> >
> > It should be possible to do that.
> >
> > Do I understand it correctly that you want one structure definition per
> > power domain instead of the two?
>
> Yes, at least that's what I would do to keep the data together. I
> don't care that much though, so feel free to implement it however
> you'd like.

OK, I think I can merge them.

Thanks,
Rafael

2011-06-19 22:01:50

by Rafael J. Wysocki

Subject: [Update][PATCH 4/8] PM / Domains: Support for generic I/O PM domains (v6)

From: Rafael J. Wysocki <[email protected]>

Introduce common headers, helper functions and callbacks allowing
platforms to use simple generic power domains for runtime power
management.

Introduce struct generic_pm_domain to be used for representing
power domains that each contain a number of devices and may be
parent domains or subdomains with respect to other power domains.
Among other things, this structure includes callbacks to be
provided by platforms for performing specific tasks related to
power management (i.e. ->stop_device() may disable a device's
clocks, while ->start_device() may enable them, ->power_off() is
supposed to remove power from the entire power domain
and ->power_on() is supposed to restore it).

Introduce functions that can be used as power domain runtime PM
callbacks, pm_genpd_runtime_suspend() and pm_genpd_runtime_resume(),
as well as helper functions for the initialization of a power
domain represented by a struct generic_power_domain object,
adding a device to or removing a device from it and adding or
removing subdomains.

Introduce configuration option CONFIG_PM_GENERIC_DOMAINS to be
selected by the platforms that want to use the new code.

Signed-off-by: Rafael J. Wysocki <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
---

Hi,

I this version of the patch I made the following modifications:

* Removed the change adding platform_data to struct dev_pm_domain,
because that field is not going to be necessary in the near future.

* Moved the code calling drv->pm->runtime_suspend(dev) to a separate
function that returns immediately if dle->need_restore is set for the
given device (meaning that drv->pm->runtime_suspend(dev) has already
been called for it and the corresponding ->runtime_resume() hasn't).
This fixes a bug where drv->pm->runtime_suspend() could be called for
a device whose state hasn't been restored after power cycling its
PM domain.

* Made pm_genpd_add_device() return error code on an attempt to add a device
do a PM domain whose power_is_off is set (that complemets the previous
modification).

* Makde the .power_on() and .power_off() generic PM domain callbacks take
(struct generic_pm_domain *) arguments instead of (struct dev_pm_domain *).

Thanks,
Rafael

---
drivers/base/power/Makefile | 1
drivers/base/power/domain.c | 490 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/pm_domain.h | 78 +++++++
kernel/power/Kconfig | 4
4 files changed, 573 insertions(+)

Index: linux-2.6/include/linux/pm_domain.h
===================================================================
--- /dev/null
+++ linux-2.6/include/linux/pm_domain.h
@@ -0,0 +1,78 @@
+/*
+ * pm_domain.h - Definitions and headers related to device power domains.
+ *
+ * Copyright (C) 2011 Rafael J. Wysocki <[email protected]>, Renesas Electronics Corp.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#ifndef _LINUX_PM_DOMAIN_H
+#define _LINUX_PM_DOMAIN_H
+
+#include <linux/device.h>
+
+struct dev_power_governor {
+ bool (*power_down_ok)(struct dev_pm_domain *domain);
+};
+
+struct generic_pm_domain {
+ struct dev_pm_domain domain; /* PM domain operations */
+ struct list_head sd_node; /* Node in the parent's subdomain list */
+ struct generic_pm_domain *parent; /* Parent PM domain */
+ struct list_head sd_list; /* List of dubdomains */
+ struct list_head dev_list; /* List of devices */
+ struct mutex lock;
+ struct dev_power_governor *gov;
+ struct work_struct power_off_work;
+ unsigned int in_progress; /* Number of devices being suspended now */
+ unsigned int sd_count; /* Number of subdomains with power "on" */
+ bool power_is_off; /* Whether or not power has been removed */
+ int (*power_off)(struct generic_pm_domain *domain);
+ int (*power_on)(struct generic_pm_domain *domain);
+ int (*start_device)(struct device *dev);
+ int (*stop_device)(struct device *dev);
+};
+
+struct dev_list_entry {
+ struct list_head node;
+ struct device *dev;
+ bool need_restore;
+};
+
+#ifdef CONFIG_PM_GENERIC_DOMAINS
+extern int pm_genpd_add_device(struct generic_pm_domain *genpd,
+ struct device *dev);
+extern int pm_genpd_remove_device(struct generic_pm_domain *genpd,
+ struct device *dev);
+extern int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
+ struct generic_pm_domain *new_subdomain);
+extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
+ struct generic_pm_domain *target);
+extern void pm_genpd_init(struct generic_pm_domain *genpd,
+ struct dev_power_governor *gov, bool is_off);
+#else
+static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
+ struct device *dev)
+{
+ return -ENOSYS;
+}
+static inline int pm_genpd_remove_device(struct generic_pm_domain *genpd,
+ struct device *dev)
+{
+ return -ENOSYS;
+}
+static inline int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
+ struct generic_pm_domain *new_sd)
+{
+ return -ENOSYS;
+}
+static inline int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
+ struct generic_pm_domain *target)
+{
+ return -ENOSYS;
+}
+static inline void pm_genpd_init(struct generic_pm_domain *genpd,
+ struct dev_power_governor *gov, bool is_off) {}
+#endif
+
+#endif /* _LINUX_PM_DOMAIN_H */
Index: linux-2.6/drivers/base/power/Makefile
===================================================================
--- linux-2.6.orig/drivers/base/power/Makefile
+++ linux-2.6/drivers/base/power/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_PM_SLEEP) += main.o wakeup.
obj-$(CONFIG_PM_RUNTIME) += runtime.o
obj-$(CONFIG_PM_TRACE_RTC) += trace.o
obj-$(CONFIG_PM_OPP) += opp.o
+obj-$(CONFIG_PM_GENERIC_DOMAINS) += domain.o
obj-$(CONFIG_HAVE_CLK) += clock_ops.o

ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
\ No newline at end of file
Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- /dev/null
+++ linux-2.6/drivers/base/power/domain.c
@@ -0,0 +1,490 @@
+/*
+ * drivers/base/power/domain.c - Common code related to device power domains.
+ *
+ * Copyright (C) 2011 Rafael J. Wysocki <[email protected]>, Renesas Electronics Corp.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/io.h>
+#include <linux/pm_runtime.h>
+#include <linux/pm_domain.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+
+#ifdef CONFIG_PM_RUNTIME
+
+static void genpd_sd_counter_dec(struct generic_pm_domain *genpd)
+{
+ if (!WARN_ON(genpd->sd_count == 0))
+ genpd->sd_count--;
+}
+
+/**
+ * __pm_genpd_save_device - Save the pre-suspend state of a device.
+ * @dle: Device list entry of the device to save the state of.
+ * @genpd: PM domain the device belongs to.
+ */
+static int __pm_genpd_save_device(struct dev_list_entry *dle,
+ struct generic_pm_domain *genpd)
+{
+ struct device *dev = dle->dev;
+ struct device_driver *drv = dev->driver;
+ int ret = 0;
+
+ if (dle->need_restore)
+ return 0;
+
+ if (genpd->start_device)
+ genpd->start_device(dev);
+
+ if (drv && drv->pm && drv->pm->runtime_suspend)
+ ret = drv->pm->runtime_suspend(dev);
+
+ if (genpd->stop_device)
+ genpd->stop_device(dev);
+
+ if (!ret)
+ dle->need_restore = true;
+
+ return ret;
+}
+
+/**
+ * __pm_genpd_restore_device - Restore the pre-suspend state of a device.
+ * @dle: Device list entry of the device to restore the state of.
+ * @genpd: PM domain the device belongs to.
+ */
+static void __pm_genpd_restore_device(struct dev_list_entry *dle,
+ struct generic_pm_domain *genpd)
+{
+ struct device *dev = dle->dev;
+ struct device_driver *drv = dev->driver;
+
+ if (!dle->need_restore)
+ return;
+
+ if (genpd->start_device)
+ genpd->start_device(dev);
+
+ if (drv && drv->pm && drv->pm->runtime_resume)
+ drv->pm->runtime_resume(dev);
+
+ if (genpd->stop_device)
+ genpd->stop_device(dev);
+
+ dle->need_restore = false;
+}
+
+/**
+ * pm_genpd_poweroff - Remove power from a given PM domain.
+ * @genpd: PM domain to power down.
+ *
+ * If all of the @genpd's devices have been suspended and all of its subdomains
+ * have been powered down, run the runtime suspend callbacks provided by all of
+ * the @genpd's devices' drivers and remove power from @genpd.
+ */
+static int pm_genpd_poweroff(struct generic_pm_domain *genpd)
+{
+ struct generic_pm_domain *parent;
+ struct dev_list_entry *dle;
+ unsigned int not_suspended;
+ int ret;
+
+ if (genpd->power_is_off)
+ return 0;
+
+ if (genpd->sd_count > 0)
+ return -EBUSY;
+
+ not_suspended = 0;
+ list_for_each_entry(dle, &genpd->dev_list, node)
+ if (dle->dev->driver && !pm_runtime_suspended(dle->dev))
+ not_suspended++;
+
+ if (not_suspended > genpd->in_progress)
+ return -EBUSY;
+
+ if (genpd->gov && genpd->gov->power_down_ok) {
+ if (!genpd->gov->power_down_ok(&genpd->domain))
+ return -EAGAIN;
+ }
+
+ list_for_each_entry_reverse(dle, &genpd->dev_list, node) {
+ ret = __pm_genpd_save_device(dle, genpd);
+ if (ret)
+ goto err_dev;
+ }
+
+ if (genpd->power_off)
+ genpd->power_off(genpd);
+
+ genpd->power_is_off = true;
+
+ parent = genpd->parent;
+ if (parent) {
+ genpd_sd_counter_dec(parent);
+ if (parent->sd_count == 0)
+ queue_work(pm_wq, &parent->power_off_work);
+ }
+
+ return 0;
+
+ err_dev:
+ list_for_each_entry_continue(dle, &genpd->dev_list, node)
+ __pm_genpd_restore_device(dle, genpd);
+
+ return ret;
+}
+
+/**
+ * genpd_power_off_work_fn - Power off PM domain whose subdomain count is 0.
+ * @work: Work structure used for scheduling the execution of this function.
+ */
+static void genpd_power_off_work_fn(struct work_struct *work)
+{
+ struct generic_pm_domain *genpd;
+
+ genpd = container_of(work, struct generic_pm_domain, power_off_work);
+
+ if (genpd->parent)
+ mutex_lock(&genpd->parent->lock);
+ mutex_lock(&genpd->lock);
+ pm_genpd_poweroff(genpd);
+ mutex_unlock(&genpd->lock);
+ if (genpd->parent)
+ mutex_unlock(&genpd->parent->lock);
+}
+
+/**
+ * pm_genpd_runtime_suspend - Suspend a device belonging to I/O PM domain.
+ * @dev: Device to suspend.
+ *
+ * Carry out a runtime suspend of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_runtime_suspend(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ if (IS_ERR_OR_NULL(dev->pm_domain))
+ return -EINVAL;
+
+ genpd = container_of(dev->pm_domain, struct generic_pm_domain, domain);
+
+ if (genpd->parent)
+ mutex_lock(&genpd->parent->lock);
+ mutex_lock(&genpd->lock);
+
+ if (genpd->stop_device) {
+ int ret = genpd->stop_device(dev);
+ if (ret)
+ goto out;
+ }
+ genpd->in_progress++;
+ pm_genpd_poweroff(genpd);
+ genpd->in_progress--;
+
+ out:
+ mutex_unlock(&genpd->lock);
+ if (genpd->parent)
+ mutex_unlock(&genpd->parent->lock);
+
+ return 0;
+}
+
+/**
+ * pm_genpd_poweron - Restore power to a given PM domain and its parents.
+ * @genpd: PM domain to power up.
+ *
+ * Restore power to @genpd and all of its parents so that it is possible to
+ * resume a device belonging to it.
+ */
+static int pm_genpd_poweron(struct generic_pm_domain *genpd)
+{
+ int ret = 0;
+
+ start:
+ if (genpd->parent)
+ mutex_lock(&genpd->parent->lock);
+ mutex_lock(&genpd->lock);
+
+ if (!genpd->power_is_off)
+ goto out;
+
+ if (genpd->parent && genpd->parent->power_is_off) {
+ mutex_unlock(&genpd->lock);
+ mutex_unlock(&genpd->parent->lock);
+
+ ret = pm_genpd_poweron(genpd->parent);
+ if (ret)
+ return ret;
+
+ goto start;
+ }
+
+ if (genpd->power_on) {
+ int ret = genpd->power_on(genpd);
+ if (ret)
+ goto out;
+ }
+
+ genpd->power_is_off = false;
+ if (genpd->parent)
+ genpd->parent->sd_count++;
+
+ out:
+ mutex_unlock(&genpd->lock);
+ if (genpd->parent)
+ mutex_unlock(&genpd->parent->lock);
+
+ return ret;
+}
+
+/**
+ * pm_genpd_runtime_resume - Resume a device belonging to I/O PM domain.
+ * @dev: Device to resume.
+ *
+ * Carry out a runtime resume of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_runtime_resume(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ struct dev_list_entry *dle;
+ int ret;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ if (IS_ERR_OR_NULL(dev->pm_domain))
+ return -EINVAL;
+
+ genpd = container_of(dev->pm_domain, struct generic_pm_domain, domain);
+
+ ret = pm_genpd_poweron(genpd);
+ if (ret)
+ return ret;
+
+ mutex_lock(&genpd->lock);
+
+ list_for_each_entry(dle, &genpd->dev_list, node) {
+ if (dle->dev == dev) {
+ __pm_genpd_restore_device(dle, genpd);
+ break;
+ }
+ }
+
+ if (genpd->start_device)
+ genpd->start_device(dev);
+
+ mutex_unlock(&genpd->lock);
+
+ return 0;
+}
+
+#else
+
+#define pm_genpd_runtime_suspend NULL
+#define pm_genpd_runtime_resume NULL
+
+#endif /* CONFIG_PM_RUNTIME */
+
+/**
+ * pm_genpd_add_device - Add a device to an I/O PM domain.
+ * @genpd: PM domain to add the device to.
+ * @dev: Device to be added.
+ */
+int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
+{
+ struct dev_list_entry *dle;
+ int ret = 0;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
+ return -EINVAL;
+
+ mutex_lock(&genpd->lock);
+
+ if (genpd->power_is_off) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ list_for_each_entry(dle, &genpd->dev_list, node)
+ if (dle->dev == dev) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ dle = kzalloc(sizeof(*dle), GFP_KERNEL);
+ if (!dle) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ dle->dev = dev;
+ dle->need_restore = false;
+ list_add_tail(&dle->node, &genpd->dev_list);
+
+ spin_lock_irq(&dev->power.lock);
+ dev->pm_domain = &genpd->domain;
+ spin_unlock_irq(&dev->power.lock);
+
+ out:
+ mutex_unlock(&genpd->lock);
+
+ return ret;
+}
+
+/**
+ * pm_genpd_remove_device - Remove a device from an I/O PM domain.
+ * @genpd: PM domain to remove the device from.
+ * @dev: Device to be removed.
+ */
+int pm_genpd_remove_device(struct generic_pm_domain *genpd,
+ struct device *dev)
+{
+ struct dev_list_entry *dle;
+ int ret = -EINVAL;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(dev))
+ return -EINVAL;
+
+ mutex_lock(&genpd->lock);
+
+ list_for_each_entry(dle, &genpd->dev_list, node) {
+ if (dle->dev != dev)
+ continue;
+
+ spin_lock_irq(&dev->power.lock);
+ dev->pm_domain = NULL;
+ spin_unlock_irq(&dev->power.lock);
+
+ list_del(&dle->node);
+ kfree(dle);
+
+ ret = 0;
+ break;
+ }
+
+ mutex_unlock(&genpd->lock);
+
+ return ret;
+}
+
+/**
+ * pm_genpd_add_subdomain - Add a subdomain to an I/O PM domain.
+ * @genpd: Master PM domain to add the subdomain to.
+ * @new_subdomain: Subdomain to be added.
+ */
+int pm_genpd_add_subdomain(struct generic_pm_domain *genpd,
+ struct generic_pm_domain *new_subdomain)
+{
+ struct generic_pm_domain *subdomain;
+ int ret = 0;
+
+ if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(new_subdomain))
+ return -EINVAL;
+
+ mutex_lock(&genpd->lock);
+
+ if (genpd->power_is_off && !new_subdomain->power_is_off) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ list_for_each_entry(subdomain, &genpd->sd_list, sd_node) {
+ if (subdomain == new_subdomain) {
+ ret = -EINVAL;
+ goto out;
+ }
+ }
+
+ mutex_lock(&new_subdomain->lock);
+
+ list_add_tail(&new_subdomain->sd_node, &genpd->sd_list);
+ new_subdomain->parent = genpd;
+ if (!subdomain->power_is_off)
+ genpd->sd_count++;
+
+ mutex_unlock(&new_subdomain->lock);
+
+ out:
+ mutex_unlock(&genpd->lock);
+
+ return ret;
+}
+
+/**
+ * pm_genpd_remove_subdomain - Remove a subdomain from an I/O PM domain.
+ * @genpd: Master PM domain to remove the subdomain from.
+ * @target: Subdomain to be removed.
+ */
+int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
+ struct generic_pm_domain *target)
+{
+ struct generic_pm_domain *subdomain;
+ int ret = -EINVAL;
+
+ if (IS_ERR_OR_NULL(genpd) || IS_ERR_OR_NULL(target))
+ return -EINVAL;
+
+ mutex_lock(&genpd->lock);
+
+ list_for_each_entry(subdomain, &genpd->sd_list, sd_node) {
+ if (subdomain != target)
+ continue;
+
+ mutex_lock(&subdomain->lock);
+
+ list_del(&subdomain->sd_node);
+ subdomain->parent = NULL;
+ if (!subdomain->power_is_off)
+ genpd_sd_counter_dec(genpd);
+
+ mutex_unlock(&subdomain->lock);
+
+ ret = 0;
+ break;
+ }
+
+ mutex_unlock(&genpd->lock);
+
+ return ret;
+}
+
+/**
+ * pm_genpd_init - Initialize a generic I/O PM domain object.
+ * @genpd: PM domain object to initialize.
+ * @gov: PM domain governor to associate with the domain (may be NULL).
+ * @is_off: Initial value of the domain's power_is_off field.
+ */
+void pm_genpd_init(struct generic_pm_domain *genpd,
+ struct dev_power_governor *gov, bool is_off)
+{
+ if (IS_ERR_OR_NULL(genpd))
+ return;
+
+ INIT_LIST_HEAD(&genpd->sd_node);
+ genpd->parent = NULL;
+ INIT_LIST_HEAD(&genpd->dev_list);
+ INIT_LIST_HEAD(&genpd->sd_list);
+ mutex_init(&genpd->lock);
+ genpd->gov = gov;
+ INIT_WORK(&genpd->power_off_work, genpd_power_off_work_fn);
+ genpd->in_progress = 0;
+ genpd->sd_count = 0;
+ genpd->power_is_off = is_off;
+ genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
+ genpd->domain.ops.runtime_resume = pm_genpd_runtime_resume;
+ genpd->domain.ops.runtime_idle = pm_generic_runtime_idle;
+}
Index: linux-2.6/kernel/power/Kconfig
===================================================================
--- linux-2.6.orig/kernel/power/Kconfig
+++ linux-2.6/kernel/power/Kconfig
@@ -227,3 +227,7 @@ config PM_OPP
config PM_RUNTIME_CLK
def_bool y
depends on PM_RUNTIME && HAVE_CLK
+
+config PM_GENERIC_DOMAINS
+ bool
+ depends on PM

2011-06-19 22:05:21

by Rafael J. Wysocki

Subject: [Update][PATCH 7/8] PM / Domains: System-wide transitions support for generic domains (v3)

From: Rafael J. Wysocki <[email protected]>

Make generic PM domains support system-wide power transitions
(system suspend and hibernation). Add suspend, resume, freeze, thaw,
poweroff and restore callbacks to be associated with struct
generic_pm_domain objects and make pm_genpd_init() use them as
appropriate.

The new callbacks do nothing for devices belonging to power domains
that were powered down at run time (before the transition). For the
other devices the action carried out depends on the type of the
transition. During system suspend the power domain .suspend()
callback executes pm_generic_suspend() for the device, while the
PM domain .suspend_noirq() callback runs pm_generic_suspend_noirq()
for it, stops it and eventually removes power from the PM domain it
belongs to (after all devices in the domain have been stopped and its
subdomains have been powered off).

During system resume the PM domain .resume_noirq() callback
restores power to the PM domain (when executed for it first time),
starts the device and executes pm_generic_resume_noirq() for it,
while the .resume() callback executes pm_generic_resume() for the
device. Finally, the .complete() callback executes pm_runtime_idle()
for the device which should put it back into the suspended state if
its runtime PM usage count is equal to zero at that time.

The actions carried out during hibernation and resume from it are
analogous to the ones described above.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---

Hi,

In this version of the patch I made some changes following from the
modifications of [4/8] and I introduced pd_to_genpd() as a wrapper
around container_of(pm_domain, struct generic_pm_domain, domain) to
convert between struct dev_pm_domain and struct generic_pm_domain objects.

Thanks,
Rafael

---
drivers/base/power/domain.c | 539 ++++++++++++++++++++++++++++++++++++++++++--
include/linux/pm_domain.h | 12
2 files changed, 538 insertions(+), 13 deletions(-)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -21,7 +21,7 @@ static struct generic_pm_domain *dev_to_
if (IS_ERR_OR_NULL(dev->pm_domain))
return ERR_PTR(-EINVAL);

- return container_of(dev->pm_domain, struct generic_pm_domain, domain);
+ return pd_to_genpd(dev->pm_domain);
}

/**
@@ -40,7 +40,8 @@ static int pm_genpd_poweron(struct gener
mutex_lock(&genpd->parent->lock);
mutex_lock(&genpd->lock);

- if (!genpd->power_is_off)
+ if (!genpd->power_is_off
+ || (genpd->prepared_count > 0 && genpd->suspend_power_off))
goto out;

if (genpd->parent && genpd->parent->power_is_off) {
@@ -153,7 +154,7 @@ static int pm_genpd_poweroff(struct gene
unsigned int not_suspended;
int ret;

- if (genpd->power_is_off)
+ if (genpd->power_is_off || genpd->prepared_count > 0)
return 0;

if (genpd->sd_count > 0)
@@ -258,6 +259,27 @@ static int pm_genpd_runtime_suspend(stru
}

/**
+ * __pm_genpd_runtime_resume - Resume a device belonging to I/O PM domain.
+ * @dev: Device to resume.
+ * @genpd: PM domain the device belongs to.
+ */
+static void __pm_genpd_runtime_resume(struct device *dev,
+ struct generic_pm_domain *genpd)
+{
+ struct dev_list_entry *dle;
+
+ list_for_each_entry(dle, &genpd->dev_list, node) {
+ if (dle->dev == dev) {
+ __pm_genpd_restore_device(dle, genpd);
+ break;
+ }
+ }
+
+ if (genpd->start_device)
+ genpd->start_device(dev);
+}
+
+/**
* pm_genpd_runtime_resume - Resume a device belonging to I/O PM domain.
* @dev: Device to resume.
*
@@ -268,7 +290,6 @@ static int pm_genpd_runtime_suspend(stru
static int pm_genpd_runtime_resume(struct device *dev)
{
struct generic_pm_domain *genpd;
- struct dev_list_entry *dle;
int ret;

dev_dbg(dev, "%s()\n", __func__);
@@ -282,28 +303,491 @@ static int pm_genpd_runtime_resume(struc
return ret;

mutex_lock(&genpd->lock);
+ __pm_genpd_runtime_resume(dev, genpd);
+ mutex_unlock(&genpd->lock);

- list_for_each_entry(dle, &genpd->dev_list, node) {
- if (dle->dev == dev) {
- __pm_genpd_restore_device(dle, genpd);
- break;
- }
+ return 0;
+}
+
+#else
+
+static inline void __pm_genpd_runtime_resume(struct device *dev,
+ struct generic_pm_domain *genpd) {}
+
+#define pm_genpd_runtime_suspend NULL
+#define pm_genpd_runtime_resume NULL
+
+#endif /* CONFIG_PM_RUNTIME */
+
+#ifdef CONFIG_PM_SLEEP
+
+/**
+ * pm_genpd_sync_poweroff - Synchronously power off a PM domain and its parents.
+ * @genpd: PM domain to power off, if possible.
+ *
+ * Check if the given PM domain can be powered off (during system suspend or
+ * hibernation) and do that if so. Also, in that case propagate to its parent.
+ *
+ * This function is only called in "noirq" stages of system power transitions,
+ * so it need not acquire locks (all of the "noirq" callbacks are executed
+ * sequentially, so it is guaranteed that it will never run twice in parallel).
+ */
+static void pm_genpd_sync_poweroff(struct generic_pm_domain *genpd)
+{
+ struct generic_pm_domain *parent = genpd->parent;
+
+ if (genpd->power_is_off)
+ return;
+
+ if (genpd->suspended_count != genpd->device_count || genpd->sd_count > 0)
+ return;
+
+ if (genpd->power_off)
+ genpd->power_off(genpd);
+
+ genpd->power_is_off = true;
+ if (parent) {
+ genpd_sd_counter_dec(parent);
+ pm_genpd_sync_poweroff(parent);
+ }
+}
+
+/**
+ * pm_genpd_prepare - Start power transition of a device in a PM domain.
+ * @dev: Device to start the transition of.
+ *
+ * Start a power transition of a device (during a system-wide power transition)
+ * under the assumption that its pm_domain field points to the domain member of
+ * an object of type struct generic_pm_domain representing a PM domain
+ * consisting of I/O devices.
+ */
+static int pm_genpd_prepare(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ mutex_lock(&genpd->lock);
+
+ if (genpd->prepared_count++ == 0)
+ genpd->suspend_power_off = genpd->power_is_off;
+
+ if (genpd->suspend_power_off) {
+ mutex_unlock(&genpd->lock);
+ return 0;
}

+ /*
+ * If the device is in the (runtime) "suspended" state, call
+ * .start_device() for it, if defined.
+ */
+ if (pm_runtime_suspended(dev))
+ __pm_genpd_runtime_resume(dev, genpd);
+
+ pm_runtime_disable(dev);
+
+ mutex_unlock(&genpd->lock);
+
+ return pm_generic_prepare(dev);
+}
+
+/**
+ * pm_genpd_suspend - Suspend a device belonging to an I/O PM domain.
+ * @dev: Device to suspend.
+ *
+ * Suspend a device under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_suspend(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_suspend(dev);
+}
+
+/**
+ * pm_genpd_suspend_noirq - Late suspend of a device from an I/O PM domain.
+ * @dev: Device to suspend.
+ *
+ * Carry out a late suspend of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_suspend_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ int ret;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ if (genpd->suspend_power_off)
+ return 0;
+
+ ret = pm_generic_suspend_noirq(dev);
+ if (ret)
+ return ret;
+
+ if (genpd->stop_device)
+ genpd->stop_device(dev);
+
+ /*
+ * Since all of the "noirq" callbacks are executed sequentially, it is
+ * guaranteed that this function will never run twice in parallel for
+ * the same PM domain, so it is not necessary to use locking here.
+ */
+ genpd->suspended_count++;
+ pm_genpd_sync_poweroff(genpd);
+
+ return 0;
+}
+
+/**
+ * pm_genpd_resume_noirq - Early resume of a device from an I/O power domain.
+ * @dev: Device to resume.
+ *
+ * Carry out an early resume of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_resume_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ if (genpd->suspend_power_off)
+ return 0;
+
+ /*
+ * Since all of the "noirq" callbacks are executed sequentially, it is
+ * guaranteed that this function will never run twice in parallel for
+ * the same PM domain, so it is not necessary to use locking here.
+ */
+ pm_genpd_poweron(genpd);
+ genpd->suspended_count--;
if (genpd->start_device)
genpd->start_device(dev);

- mutex_unlock(&genpd->lock);
+ return pm_generic_resume_noirq(dev);
+}
+
+/**
+ * pm_genpd_resume - Resume a device belonging to an I/O power domain.
+ * @dev: Device to resume.
+ *
+ * Resume a device under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_resume(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_resume(dev);
+}
+
+/**
+ * pm_genpd_freeze - Freeze a device belonging to an I/O power domain.
+ * @dev: Device to freeze.
+ *
+ * Freeze a device under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_freeze(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_freeze(dev);
+}
+
+/**
+ * pm_genpd_freeze_noirq - Late freeze of a device from an I/O power domain.
+ * @dev: Device to freeze.
+ *
+ * Carry out a late freeze of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_freeze_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ int ret;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ if (genpd->suspend_power_off)
+ return 0;
+
+ ret = pm_generic_freeze_noirq(dev);
+ if (ret)
+ return ret;
+
+ if (genpd->stop_device)
+ genpd->stop_device(dev);

return 0;
}

+/**
+ * pm_genpd_thaw_noirq - Early thaw of a device from an I/O power domain.
+ * @dev: Device to thaw.
+ *
+ * Carry out an early thaw of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_thaw_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ if (genpd->suspend_power_off)
+ return 0;
+
+ if (genpd->start_device)
+ genpd->start_device(dev);
+
+ return pm_generic_thaw_noirq(dev);
+}
+
+/**
+ * pm_genpd_thaw - Thaw a device belonging to an I/O power domain.
+ * @dev: Device to thaw.
+ *
+ * Thaw a device under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_thaw(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_thaw(dev);
+}
+
+/**
+ * pm_genpd_dev_poweroff - Power off a device belonging to an I/O PM domain.
+ * @dev: Device to suspend.
+ *
+ * Power off a device under the assumption that its pm_domain field points to
+ * the domain member of an object of type struct generic_pm_domain representing
+ * a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_dev_poweroff(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_poweroff(dev);
+}
+
+/**
+ * pm_genpd_dev_poweroff_noirq - Late power off of a device from a PM domain.
+ * @dev: Device to suspend.
+ *
+ * Carry out a late powering off of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a PM domain consisting of I/O devices.
+ */
+static int pm_genpd_dev_poweroff_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ int ret;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ if (genpd->suspend_power_off)
+ return 0;
+
+ ret = pm_generic_poweroff_noirq(dev);
+ if (ret)
+ return ret;
+
+ if (genpd->stop_device)
+ genpd->stop_device(dev);
+
+ /*
+ * Since all of the "noirq" callbacks are executed sequentially, it is
+ * guaranteed that this function will never run twice in parallel for
+ * the same PM domain, so it is not necessary to use locking here.
+ */
+ genpd->suspended_count++;
+ pm_genpd_sync_poweroff(genpd);
+
+ return 0;
+}
+
+/**
+ * pm_genpd_restore_noirq - Early restore of a device from an I/O power domain.
+ * @dev: Device to resume.
+ *
+ * Carry out an early restore of a device under the assumption that its
+ * pm_domain field points to the domain member of an object of type
+ * struct generic_pm_domain representing a power domain consisting of I/O
+ * devices.
+ */
+static int pm_genpd_restore_noirq(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ /*
+ * Since all of the "noirq" callbacks are executed sequentially, it is
+ * guaranteed that this function will never run twice in parallel for
+ * the same PM domain, so it is not necessary to use locking here.
+ */
+ genpd->power_is_off = true;
+ if (genpd->suspend_power_off) {
+ /*
+ * The boot kernel might put the domain into the power on state,
+ * so make sure it really is powered off.
+ */
+ if (genpd->power_off)
+ genpd->power_off(genpd);
+ return 0;
+ }
+
+ pm_genpd_poweron(genpd);
+ genpd->suspended_count--;
+ if (genpd->start_device)
+ genpd->start_device(dev);
+
+ return pm_generic_restore_noirq(dev);
+}
+
+/**
+ * pm_genpd_restore - Restore a device belonging to an I/O power domain.
+ * @dev: Device to resume.
+ *
+ * Restore a device under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static int pm_genpd_restore(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return -EINVAL;
+
+ return genpd->suspend_power_off ? 0 : pm_generic_restore(dev);
+}
+
+/**
+ * pm_genpd_complete - Complete power transition of a device in a power domain.
+ * @dev: Device to complete the transition of.
+ *
+ * Complete a power transition of a device (during a system-wide power
+ * transition) under the assumption that its pm_domain field points to the
+ * domain member of an object of type struct generic_pm_domain representing
+ * a power domain consisting of I/O devices.
+ */
+static void pm_genpd_complete(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+ bool run_complete;
+
+ dev_dbg(dev, "%s()\n", __func__);
+
+ genpd = dev_to_genpd(dev);
+ if (IS_ERR(genpd))
+ return;
+
+ mutex_lock(&genpd->lock);
+
+ run_complete = !genpd->suspend_power_off;
+ if (--genpd->prepared_count == 0)
+ genpd->suspend_power_off = false;
+
+ mutex_unlock(&genpd->lock);
+
+ if (run_complete)
+ pm_generic_complete(dev);
+
+ pm_runtime_enable(dev);
+}
+
#else

-#define pm_genpd_runtime_suspend NULL
-#define pm_genpd_runtime_resume NULL
+#define pm_genpd_prepare NULL
+#define pm_genpd_suspend NULL
+#define pm_genpd_suspend_noirq NULL
+#define pm_genpd_resume_noirq NULL
+#define pm_genpd_resume NULL
+#define pm_genpd_freeze NULL
+#define pm_genpd_freeze_noirq NULL
+#define pm_genpd_thaw_noirq NULL
+#define pm_genpd_thaw NULL
+#define pm_genpd_complete NULL

-#endif /* CONFIG_PM_RUNTIME */
+#endif /* CONFIG_PM_SLEEP */

/**
* pm_genpd_add_device - Add a device to an I/O PM domain.
@@ -327,6 +811,11 @@ int pm_genpd_add_device(struct generic_p
goto out;
}

+ if (genpd->prepared_count > 0) {
+ ret = -EAGAIN;
+ goto out;
+ }
+
list_for_each_entry(dle, &genpd->dev_list, node)
if (dle->dev == dev) {
ret = -EINVAL;
@@ -342,6 +831,7 @@ int pm_genpd_add_device(struct generic_p
dle->dev = dev;
dle->need_restore = false;
list_add_tail(&dle->node, &genpd->dev_list);
+ genpd->device_count++;

spin_lock_irq(&dev->power.lock);
dev->pm_domain = &genpd->domain;
@@ -371,6 +861,11 @@ int pm_genpd_remove_device(struct generi

mutex_lock(&genpd->lock);

+ if (genpd->prepared_count > 0) {
+ ret = -EAGAIN;
+ goto out;
+ }
+
list_for_each_entry(dle, &genpd->dev_list, node) {
if (dle->dev != dev)
continue;
@@ -379,6 +874,7 @@ int pm_genpd_remove_device(struct generi
dev->pm_domain = NULL;
spin_unlock_irq(&dev->power.lock);

+ genpd->device_count--;
list_del(&dle->node);
kfree(dle);

@@ -386,6 +882,7 @@ int pm_genpd_remove_device(struct generi
break;
}

+ out:
mutex_unlock(&genpd->lock);

return ret;
@@ -494,7 +991,23 @@ void pm_genpd_init(struct generic_pm_dom
genpd->in_progress = 0;
genpd->sd_count = 0;
genpd->power_is_off = is_off;
+ genpd->device_count = 0;
+ genpd->suspended_count = 0;
genpd->domain.ops.runtime_suspend = pm_genpd_runtime_suspend;
genpd->domain.ops.runtime_resume = pm_genpd_runtime_resume;
genpd->domain.ops.runtime_idle = pm_generic_runtime_idle;
+ genpd->domain.ops.prepare = pm_genpd_prepare;
+ genpd->domain.ops.suspend = pm_genpd_suspend;
+ genpd->domain.ops.suspend_noirq = pm_genpd_suspend_noirq;
+ genpd->domain.ops.resume_noirq = pm_genpd_resume_noirq;
+ genpd->domain.ops.resume = pm_genpd_resume;
+ genpd->domain.ops.freeze = pm_genpd_freeze;
+ genpd->domain.ops.freeze_noirq = pm_genpd_freeze_noirq;
+ genpd->domain.ops.thaw_noirq = pm_genpd_thaw_noirq;
+ genpd->domain.ops.thaw = pm_genpd_thaw;
+ genpd->domain.ops.poweroff = pm_genpd_dev_poweroff;
+ genpd->domain.ops.poweroff_noirq = pm_genpd_dev_poweroff_noirq;
+ genpd->domain.ops.restore_noirq = pm_genpd_restore_noirq;
+ genpd->domain.ops.restore = pm_genpd_restore;
+ genpd->domain.ops.complete = pm_genpd_complete;
}
Index: linux-2.6/include/linux/pm_domain.h
===================================================================
--- linux-2.6.orig/include/linux/pm_domain.h
+++ linux-2.6/include/linux/pm_domain.h
@@ -11,6 +11,9 @@

#include <linux/device.h>

+#define GPD_IN_SUSPEND 1
+#define GPD_POWER_OFF 2
+
struct dev_power_governor {
bool (*power_down_ok)(struct dev_pm_domain *domain);
};
@@ -27,12 +30,21 @@ struct generic_pm_domain {
unsigned int in_progress; /* Number of devices being suspended now */
unsigned int sd_count; /* Number of subdomains with power "on" */
bool power_is_off; /* Whether or not power has been removed */
+ unsigned int device_count; /* Number of devices */
+ unsigned int suspended_count; /* System suspend device counter */
+ unsigned int prepared_count; /* Suspend counter of prepared devices */
+ bool suspend_power_off; /* Power status before system suspend */
int (*power_off)(struct generic_pm_domain *domain);
int (*power_on)(struct generic_pm_domain *domain);
int (*start_device)(struct device *dev);
int (*stop_device)(struct device *dev);
};

+static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
+{
+ return container_of(pd, struct generic_pm_domain, domain);
+}
+
struct dev_list_entry {
struct list_head node;
struct device *dev;

2011-06-19 22:06:48

by Rafael J. Wysocki

Subject: [Update][PATCH 8/8] ARM / shmobile: Support for I/O power domains for SH7372 (v6)

From: Rafael J. Wysocki <[email protected]>

Use the generic power domains support introduced by the previous
patch to implement support for power domains on SH7372.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---

Hi,

I think that all of the previous comments have been addressed in this
version of the patch.

Thanks,
Rafael

---
arch/arm/Kconfig | 1
arch/arm/mach-shmobile/Makefile | 4 +
arch/arm/mach-shmobile/board-mackerel.c | 4 +
arch/arm/mach-shmobile/include/mach/sh7372.h | 28 ++++++++++
arch/arm/mach-shmobile/pm-sh7372.c | 71 +++++++++++++++++++++++++++
5 files changed, 108 insertions(+)

Index: linux-2.6/arch/arm/mach-shmobile/board-mackerel.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/board-mackerel.c
+++ linux-2.6/arch/arm/mach-shmobile/board-mackerel.c
@@ -1582,6 +1582,10 @@ static void __init mackerel_init(void)

platform_add_devices(mackerel_devices, ARRAY_SIZE(mackerel_devices));

+ sh7372_init_pm_domain(&sh7372_a4lc_domain);
+ sh7372_add_device_to_domain(SH7372_A4LC, &lcdc_device);
+ sh7372_add_device_to_domain(SH7372_A4LC, &hdmi_lcdc_device);
+
hdmi_init_pm_clock();
sh7372_pm_init();
}
Index: linux-2.6/arch/arm/mach-shmobile/include/mach/sh7372.h
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/include/mach/sh7372.h
+++ linux-2.6/arch/arm/mach-shmobile/include/mach/sh7372.h
@@ -12,6 +12,7 @@
#define __ASM_SH7372_H__

#include <linux/sh_clk.h>
+#include <linux/pm_domain.h>

/*
* Pin Function Controller:
@@ -470,4 +471,31 @@ extern struct clk sh7372_fsibck_clk;
extern struct clk sh7372_fsidiva_clk;
extern struct clk sh7372_fsidivb_clk;

+struct platform_device;
+
+struct sh7372_pm_domain {
+ struct generic_pm_domain genpd;
+ unsigned int bit_shift;
+};
+
+static inline struct sh7372_pm_domain *to_sh7372_pd(struct generic_pm_domain *d)
+{
+ return container_of(d, struct sh7372_pm_domain, genpd);
+}
+
+#ifdef CONFIG_PM
+extern struct sh7372_pm_domain sh7372_a4lc_domain;
+#define SH7372_A4LC (&sh7372_a4lc_domain)
+
+extern void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd);
+extern void sh7372_add_device_to_domain(struct sh7372_pm_domain *sh7372_pd,
+ struct platform_device *pdev);
+#else
+#define SH7372_A4LC NULL
+
+static inline void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd) {}
+static inline void sh7372_add_device_to_domain(struct sh7372_pm_domain *pd,
+ struct platform_device *pdev) {}
+#endif /* CONFIG_PM */
+
#endif /* __ASM_SH7372_H__ */
Index: linux-2.6/arch/arm/mach-shmobile/Makefile
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/Makefile
+++ linux-2.6/arch/arm/mach-shmobile/Makefile
@@ -42,6 +42,10 @@ obj-$(CONFIG_MACH_AP4EVB) += board-ap4ev
obj-$(CONFIG_MACH_AG5EVM) += board-ag5evm.o
obj-$(CONFIG_MACH_MACKEREL) += board-mackerel.o

+# PM objects
+pm-$(CONFIG_ARCH_SH7372) += pm-sh7372.o
+
# Framework support
obj-$(CONFIG_SMP) += $(smp-y)
obj-$(CONFIG_GENERIC_GPIO) += $(pfc-y)
+obj-$(CONFIG_PM) += $(pm-y)
Index: linux-2.6/arch/arm/mach-shmobile/pm-sh7372.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux-2.6/arch/arm/mach-shmobile/pm-sh7372.c
@@ -15,16 +15,87 @@
#include <linux/list.h>
#include <linux/err.h>
#include <linux/slab.h>
+#include <linux/pm_runtime.h>
+#include <linux/platform_device.h>
#include <asm/system.h>
#include <asm/io.h>
#include <asm/tlbflush.h>
#include <mach/common.h>
+#include <mach/sh7372.h>

#define SMFRAM 0xe6a70000
#define SYSTBCR 0xe6150024
#define SBAR 0xe6180020
#define APARMBAREA 0xe6f10020

+#define SPDCR 0xe6180008
+#define SWUCR 0xe6180014
+#define PSTR 0xe6180080
+
+static int pd_power_down(struct generic_pm_domain *genpd)
+{
+ struct sh7372_pm_domain *sh7372_pd = to_sh7372_pd(genpd);
+ unsigned int mask = 1 << sh7372_pd->bit_shift;
+
+ if (__raw_readl(PSTR) & mask) {
+ __raw_writel(mask, SPDCR);
+
+ while (__raw_readl(SPDCR) & mask)
+ cpu_relax();
+
+ pr_debug("sh7372 power domain down 0x%08x -> PSTR = 0x%08x\n",
+ mask, __raw_readl(PSTR));
+ }
+
+ return 0;
+}
+
+static int pd_power_up(struct generic_pm_domain *genpd)
+{
+ struct sh7372_pm_domain *sh7372_pd = to_sh7372_pd(genpd);
+ unsigned int mask = 1 << sh7372_pd->bit_shift;
+
+ if (!(__raw_readl(PSTR) & mask)) {
+ __raw_writel(mask, SWUCR);
+
+ while (__raw_readl(SWUCR) & mask)
+ cpu_relax();
+
+ pr_debug("sh7372 power domain up 0x%08x -> PSTR = 0x%08x\n",
+ mask, __raw_readl(PSTR));
+ }
+
+ return 0;
+}
+
+void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd)
+{
+ struct generic_pm_domain *genpd = &sh7372_pd->genpd;
+
+ pm_genpd_init(genpd, NULL, false);
+ genpd->stop_device = pm_runtime_clk_suspend;
+ genpd->start_device = pm_runtime_clk_resume;
+ genpd->power_off = pd_power_down;
+ genpd->power_on = pd_power_up;
+ pd_power_up(&sh7372_pd->genpd);
+}
+
+void sh7372_add_device_to_domain(struct sh7372_pm_domain *sh7372_pd,
+ struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+
+ if (!dev->power.subsys_data) {
+ pm_runtime_clk_init(dev);
+ pm_runtime_clk_add(dev, NULL);
+ }
+ pm_genpd_add_device(&sh7372_pd->genpd, dev);
+}
+
+struct sh7372_pm_domain sh7372_a4lc_domain = {
+ .bit_shift = 1,
+};
+
static void sh7372_enter_core_standby(void)
{
void __iomem *smfram = (void __iomem *)SMFRAM;
Index: linux-2.6/arch/arm/Kconfig
===================================================================
--- linux-2.6.orig/arch/arm/Kconfig
+++ linux-2.6/arch/arm/Kconfig
@@ -642,6 +642,7 @@ config ARCH_SHMOBILE
select NO_IOPORT
select SPARSE_IRQ
select MULTI_IRQ_HANDLER
+ select PM_GENERIC_DOMAINS
help
Support for Renesas's SH-Mobile and R-Mobile ARM platforms.

2011-06-20 02:02:40

by Paul Mundt

Subject: Re: [Update][PATCH 8/8] ARM / shmobile: Support for I/O power domains for SH7372 (v6)

On Mon, Jun 20, 2011 at 12:07:47AM +0200, Rafael J. Wysocki wrote:
> +static int pd_power_down(struct generic_pm_domain *genpd)
> +{
> + struct sh7372_pm_domain *sh7372_pd = to_sh7372_pd(genpd);
> + unsigned int mask = 1 << sh7372_pd->bit_shift;
> +
> + if (__raw_readl(PSTR) & mask) {
> + __raw_writel(mask, SPDCR);
> +
> + while (__raw_readl(SPDCR) & mask)
> + cpu_relax();
> +
> + pr_debug("sh7372 power domain down 0x%08x -> PSTR = 0x%08x\n",
> + mask, __raw_readl(PSTR));
> + }
> +
> + return 0;
> +}
> +
> +static int pd_power_up(struct generic_pm_domain *genpd)
> +{
> + struct sh7372_pm_domain *sh7372_pd = to_sh7372_pd(genpd);
> + unsigned int mask = 1 << sh7372_pd->bit_shift;
> +
> + if (!(__raw_readl(PSTR) & mask)) {
> + __raw_writel(mask, SWUCR);
> +
> + while (__raw_readl(SWUCR) & mask)
> + cpu_relax();
> +
> + pr_debug("sh7372 power domain up 0x%08x -> PSTR = 0x%08x\n",
> + mask, __raw_readl(PSTR));
> + }
> +
> + return 0;
> +}
> +
Given that these functions can return errors, it's probably more prudent
to implement some timeout logic on top of the busy loop. Hardware does
get stuck, after all.

2011-06-20 22:29:25

by Rafael J. Wysocki

Subject: Re: [Update][PATCH 8/8] ARM / shmobile: Support for I/O power domains for SH7372 (v6)

On Monday, June 20, 2011, Paul Mundt wrote:
> On Mon, Jun 20, 2011 at 12:07:47AM +0200, Rafael J. Wysocki wrote:
> > +static int pd_power_down(struct generic_pm_domain *genpd)
> > +{
> > + struct sh7372_pm_domain *sh7372_pd = to_sh7372_pd(genpd);
> > + unsigned int mask = 1 << sh7372_pd->bit_shift;
> > +
> > + if (__raw_readl(PSTR) & mask) {
> > + __raw_writel(mask, SPDCR);
> > +
> > + while (__raw_readl(SPDCR) & mask)
> > + cpu_relax();
> > +
> > + pr_debug("sh7372 power domain down 0x%08x -> PSTR = 0x%08x\n",
> > + mask, __raw_readl(PSTR));
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int pd_power_up(struct generic_pm_domain *genpd)
> > +{
> > + struct sh7372_pm_domain *sh7372_pd = to_sh7372_pd(genpd);
> > + unsigned int mask = 1 << sh7372_pd->bit_shift;
> > +
> > + if (!(__raw_readl(PSTR) & mask)) {
> > + __raw_writel(mask, SWUCR);
> > +
> > + while (__raw_readl(SWUCR) & mask)
> > + cpu_relax();
> > +
> > + pr_debug("sh7372 power domain up 0x%08x -> PSTR = 0x%08x\n",
> > + mask, __raw_readl(PSTR));
> > + }
> > +
> > + return 0;
> > +}
> > +
> Given that these functions can return errors, it's probably more prudent
> to implement some timeout logic on top of the busy loop. Hardware does
> get stuck, after all.

Yes, it does, but we don't really know what timeout value to use in there.

In fact, failures of pd_power_down() don't really matter (at worst the power
domain won't be powered off really) and pd_power_up() should only return error
code if the error is known to be unrecoverable.

So, what about this: repeat certain number of times (I verified that it took
several iterations of the loop until the new value settled on my hardware)
and then return from pd_power_down() or go to sleep for a short time and repeat
in pd_power_up()?

Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: ARM / shmobile: Support for I/O power domains for SH7372 (v7)

Use the generic power domains support introduced by the previous
patch to implement support for power domains on SH7372.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
arch/arm/Kconfig | 1
arch/arm/mach-shmobile/Makefile | 4 +
arch/arm/mach-shmobile/board-mackerel.c | 4 +
arch/arm/mach-shmobile/include/mach/sh7372.h | 28 +++++++++
arch/arm/mach-shmobile/pm-sh7372.c | 84 +++++++++++++++++++++++++++
5 files changed, 121 insertions(+)

Index: linux-2.6/arch/arm/mach-shmobile/board-mackerel.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/board-mackerel.c
+++ linux-2.6/arch/arm/mach-shmobile/board-mackerel.c
@@ -1582,6 +1582,10 @@ static void __init mackerel_init(void)

platform_add_devices(mackerel_devices, ARRAY_SIZE(mackerel_devices));

+ sh7372_init_pm_domain(&sh7372_a4lc_domain);
+ sh7372_add_device_to_domain(SH7372_A4LC, &lcdc_device);
+ sh7372_add_device_to_domain(SH7372_A4LC, &hdmi_lcdc_device);
+
hdmi_init_pm_clock();
sh7372_pm_init();
}
Index: linux-2.6/arch/arm/mach-shmobile/include/mach/sh7372.h
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/include/mach/sh7372.h
+++ linux-2.6/arch/arm/mach-shmobile/include/mach/sh7372.h
@@ -12,6 +12,7 @@
#define __ASM_SH7372_H__

#include <linux/sh_clk.h>
+#include <linux/pm_domain.h>

/*
* Pin Function Controller:
@@ -470,4 +471,31 @@ extern struct clk sh7372_fsibck_clk;
extern struct clk sh7372_fsidiva_clk;
extern struct clk sh7372_fsidivb_clk;

+struct platform_device;
+
+struct sh7372_pm_domain {
+ struct generic_pm_domain genpd;
+ unsigned int bit_shift;
+};
+
+static inline struct sh7372_pm_domain *to_sh7372_pd(struct generic_pm_domain *d)
+{
+ return container_of(d, struct sh7372_pm_domain, genpd);
+}
+
+#ifdef CONFIG_PM
+extern struct sh7372_pm_domain sh7372_a4lc_domain;
+#define SH7372_A4LC (&sh7372_a4lc_domain)
+
+extern void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd);
+extern void sh7372_add_device_to_domain(struct sh7372_pm_domain *sh7372_pd,
+ struct platform_device *pdev);
+#else
+#define SH7372_A4LC NULL
+
+static inline void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd) {}
+static inline void sh7372_add_device_to_domain(struct sh7372_pm_domain *pd,
+ struct platform_device *pdev) {}
+#endif /* CONFIG_PM */
+
#endif /* __ASM_SH7372_H__ */
Index: linux-2.6/arch/arm/mach-shmobile/Makefile
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/Makefile
+++ linux-2.6/arch/arm/mach-shmobile/Makefile
@@ -42,6 +42,10 @@ obj-$(CONFIG_MACH_AP4EVB) += board-ap4ev
obj-$(CONFIG_MACH_AG5EVM) += board-ag5evm.o
obj-$(CONFIG_MACH_MACKEREL) += board-mackerel.o

+# PM objects
+pm-$(CONFIG_ARCH_SH7372) += pm-sh7372.o
+
# Framework support
obj-$(CONFIG_SMP) += $(smp-y)
obj-$(CONFIG_GENERIC_GPIO) += $(pfc-y)
+obj-$(CONFIG_PM) += $(pm-y)
Index: linux-2.6/arch/arm/mach-shmobile/pm-sh7372.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux-2.6/arch/arm/mach-shmobile/pm-sh7372.c
@@ -15,16 +15,100 @@
#include <linux/list.h>
#include <linux/err.h>
#include <linux/slab.h>
+#include <linux/pm_runtime.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
#include <asm/system.h>
#include <asm/io.h>
#include <asm/tlbflush.h>
#include <mach/common.h>
+#include <mach/sh7372.h>

#define SMFRAM 0xe6a70000
#define SYSTBCR 0xe6150024
#define SBAR 0xe6180020
#define APARMBAREA 0xe6f10020

+#define SPDCR 0xe6180008
+#define SWUCR 0xe6180014
+#define PSTR 0xe6180080
+
+#define PSTR_RETRIES 100
+#define PSTR_DELAY_MS 10
+
+static int pd_power_down(struct generic_pm_domain *genpd)
+{
+ struct sh7372_pm_domain *sh7372_pd = to_sh7372_pd(genpd);
+ unsigned int mask = 1 << sh7372_pd->bit_shift;
+
+ if (__raw_readl(PSTR) & mask) {
+ unsigned int retry_count = PSTR_RETRIES;
+
+ __raw_writel(mask, SPDCR);
+ while ((__raw_readl(SPDCR) & mask) && retry_count--)
+ cpu_relax();
+ }
+
+ pr_debug("sh7372 power domain down 0x%08x -> PSTR = 0x%08x\n",
+ mask, __raw_readl(PSTR));
+
+ return 0;
+}
+
+static int pd_power_up(struct generic_pm_domain *genpd)
+{
+ struct sh7372_pm_domain *sh7372_pd = to_sh7372_pd(genpd);
+ unsigned int mask = 1 << sh7372_pd->bit_shift;
+
+ if (!(__raw_readl(PSTR) & mask)) {
+ do {
+ unsigned int retry_count = PSTR_RETRIES;
+
+ __raw_writel(mask, SWUCR);
+ while ((__raw_readl(SWUCR) & mask) && retry_count--)
+ cpu_relax();
+
+ if (!(__raw_readl(SWUCR) & mask))
+ break;
+
+ msleep(PSTR_DELAY_MS);
+ } while (true);
+ }
+
+ pr_debug("sh7372 power domain up 0x%08x -> PSTR = 0x%08x\n",
+ mask, __raw_readl(PSTR));
+
+ return 0;
+}
+
+void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd)
+{
+ struct generic_pm_domain *genpd = &sh7372_pd->genpd;
+
+ pm_genpd_init(genpd, NULL, false);
+ genpd->stop_device = pm_runtime_clk_suspend;
+ genpd->start_device = pm_runtime_clk_resume;
+ genpd->power_off = pd_power_down;
+ genpd->power_on = pd_power_up;
+ pd_power_up(&sh7372_pd->genpd);
+}
+
+void sh7372_add_device_to_domain(struct sh7372_pm_domain *sh7372_pd,
+ struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+
+ if (!dev->power.subsys_data) {
+ pm_runtime_clk_init(dev);
+ pm_runtime_clk_add(dev, NULL);
+ }
+ pm_genpd_add_device(&sh7372_pd->genpd, dev);
+}
+
+struct sh7372_pm_domain sh7372_a4lc_domain = {
+ .bit_shift = 1,
+};
+
static void sh7372_enter_core_standby(void)
{
void __iomem *smfram = (void __iomem *)SMFRAM;
Index: linux-2.6/arch/arm/Kconfig
===================================================================
--- linux-2.6.orig/arch/arm/Kconfig
+++ linux-2.6/arch/arm/Kconfig
@@ -642,6 +642,7 @@ config ARCH_SHMOBILE
select NO_IOPORT
select SPARSE_IRQ
select MULTI_IRQ_HANDLER
+ select PM_GENERIC_DOMAINS
help
Support for Renesas's SH-Mobile and R-Mobile ARM platforms.

2011-06-20 23:05:00

by Rafael J. Wysocki

Subject: Re: [Update][PATCH 7/8] PM / Domains: System-wide transitions support for generic domains (v3)

On Monday, June 20, 2011, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Make generic PM domains support system-wide power transitions
> (system suspend and hibernation). Add suspend, resume, freeze, thaw,
> poweroff and restore callbacks to be associated with struct
> generic_pm_domain objects and make pm_genpd_init() use them as
> appropriate.
>
> The new callbacks do nothing for devices belonging to power domains
> that were powered down at run time (before the transition). For the
> other devices the action carried out depends on the type of the
> transition. During system suspend the power domain .suspend()
> callback executes pm_generic_suspend() for the device, while the
> PM domain .suspend_noirq() callback runs pm_generic_suspend_noirq()
> for it, stops it and eventually removes power from the PM domain it
> belongs to (after all devices in the domain have been stopped and its
> subdomains have been powered off).
>
> During system resume the PM domain .resume_noirq() callback
> restores power to the PM domain (when executed for it first time),
> starts the device and executes pm_generic_resume_noirq() for it,
> while the .resume() callback executes pm_generic_resume() for the
> device. Finally, the .complete() callback executes pm_runtime_idle()
> for the device which should put it back into the suspended state if
> its runtime PM usage count is equal to zero at that time.
>
> The actions carried out during hibernation and resume from it are
> analogous to the ones described above.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---

The patch below adds wakeup devices support on top of this one.

Thanks,
Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: PM / Domains: Don't stop wakeup devices during system sleep transitions

Devices that are set up to wake up the system from sleep states
should not be stopped and power should not be removed from them
when the system goes into a sleep state. Make the generic PM domain
code respect that limitation.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/base/power/domain.c | 6 ++++++
1 file changed, 6 insertions(+)

Index: linux-2.6/drivers/base/power/domain.c
===================================================================
--- linux-2.6.orig/drivers/base/power/domain.c
+++ linux-2.6/drivers/base/power/domain.c
@@ -446,6 +446,9 @@ static int pm_genpd_suspend_noirq(struct
if (ret)
return ret;

+ if (device_may_wakeup(dev))
+ return 0;
+
if (genpd->stop_device)
genpd->stop_device(dev);

@@ -666,6 +669,9 @@ static int pm_genpd_dev_poweroff_noirq(s
if (ret)
return ret;

+ if (device_may_wakeup(dev))
+ return 0;
+
if (genpd->stop_device)
genpd->stop_device(dev);

2011-06-20 23:37:44

by Kevin Hilman

Subject: Re: [PATCH 2/8] PM / Domains: Rename struct dev_power_domain to struct dev_pm_domain

"Rafael J. Wysocki" <[email protected]> writes:

> From: Rafael J. Wysocki <[email protected]>
>
> The naming convention used by commit 7538e3db6e015e890825fbd9f86599b
> (PM: Add support for device power domains), which introduced the
> struct dev_power_domain type for representing device power domains,
> evidently confuses some developers who tend to think that objects
> of this type must correspond to "power domains" as defined by
> hardware, which is not the case. Namely, at the kernel level, a
> struct dev_power_domain object can represent arbitrary set of devices
> that are mutually dependent power management-wise and need not belong
> to one hardware power domain. To avoid that confusion, rename struct
> dev_power_domain to struct dev_pm_domain and rename the related
> pointers in struct device and struct pm_clk_notifier_block from
> pwr_domain to pm_domain.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>

Acked-by: Kevin Hilman <[email protected]>

Thanks!

Kevin

2011-06-21 00:02:07

by Kevin Hilman

Subject: Re: [PATCH 0/8] PM / Domains: Support for generic I/O PM domains (v5)

"Rafael J. Wysocki" <[email protected]> writes:

> Hi,
>
> This is the 4th update of the patchset adding support for generic I/O PM
> domains.
>
> The patches have been reworked quite a bit to take feedback into
> account, but I left the Greg's ACK in [4/8] in the hope it still applies
> (Greg, please let me know in case it doesn't :-)).
>
> The model here is that a bunch of devices share a common power resource
> that can be turned on and off by software. In addition to that, there
> are means to start and stop the activity of each device, for example
> by manipulating their clocks. Moreover, there may be hierarchy of
> such things, for example power resource A may be necessary for devices
> a, b, c, which don't rely on any other power resources, and for devices
> x, y, z that also rely on power resource X. In that case there one PM
> domain object representing devices a, b, c and power resource A, and
> another PM domain object will represent devices x, y, z with power
> resource X, plus the first object will be the second one's parent.
>
> Note to Kevin: I know you'd like each PM domain to be able to go into several
> different states, but the situation will always be that in some of those
> states the devices' registers will remain intact, while in the rest of those
> states they will be reset. Say, there are states 1, 2, 3, 4 and states
> 1-3 preserve device registers. Then it is not necessary to save device
> registers for "domain" states 1-3 and it only is necessary to save them
> when going to state 4. In that case, .power_off() may map to the "go to
> state 4" operation (and analogously .power_on()), while the rest may be
> done by .stop_device() and .start_device(). IOW, .power_is_off == true
> means "the devices' registers have to be restored", so it need not map to
> any particular physical state of a (hardware) power domain.

Sure, but it's not only about register context save/restore. It's about
the the governor hook and how you decide which state to enter. IOW, the
governor decision is not only about whether or not you will lose
register context but also about the latency involved in entering &
exiting those states.

So from my perspective, having only 2-states at this level makes the
governor rather pointless since any decision making will have to be done
where ever the knowledge of the mulitple power states lives.

> Note to Magnus and Paul: I didn't use a global lock as suggested, because
> I think it may lead to completely unnecessary congestion in situations in
> which there are no hierarchies of PM domains. It is quite easy to show that
> the code doesn't deadlock, because (1) no more than 2 locks are held by the
> same thread at a time (parent lock and child lock) and (2) they are always
> acquired in the same order (parent before the child).
>
> Overall, I think I've taken all of the important dependencies into
> consideration, but if you spot something suspicious, please let me know. :-)
> Wakeup is not covered at this point, because it's not necessary for the
> SH7372's A4LC power domain that's the first user of the new code, but it
> is quite clear how add the support for it. Also, for more complicated
> cases it is necessary to take QoS requirements (latencies) into account,
> which is in the works (kind of).
>
> [1/8] - Update documentation to reflect the fact that struct dev_power_domain
> callbacks take precedence over subsystem PM callbacks.
>
> [2/8] - Rename struct dev_power_domain to struct dev_pm_domain to reflect the
> fact that those objects need not correspond to hardware power domains
> directly.
>
> [3/8] - Move subsys_data in struct dev_pm_info out of #ifdef CONFIG_PM_RUNTIME
>
> [4/8] - Introduce runtime PM support for generic I/O PM domains.
>
> [5/8] - Introduce generic "noirq" callbacks for system suspend/hibernation
> (that's necessary for the next patches).
>
> [6/8] - Move some PM domains support code fro under #ifdef CONFIG_PM_RUNTIME
>
> [7/8] - Add system-wide PM support for generic I/O PM domains.
>
> [8/8] - Use the new code to represent the SH7372's A4MP power domain.
>
> The patchset has been tested on SH7372 Mackerel board and appears to work
> correctly.
>
> I'd like to push [1/8] for 3.0 (it may be regarded as a fix), but I _think_
> that it may be a good idea to push [2/8] for 3.0 too, to limit the time in
> which people may possibly use the naming that's going to change in their new
> code. If you agree with that, please let me know, I'll need some serious
> ACKs below that patch if it's to be pushed for 3.0. ;-)

Just gave you my ack, but [2/8] will need a minor update to apply on
Linus' master branch since another fix to mach-omap1/pm_bus.c just got
merged[1] via the OMAP tree.

I don't have any other fixes touching those files queued for v3.0 so I
don't expect any other conflicts there.

Kevin

[1]
commit e9e35c5a2b2c803b5e2f25906d8ffe110670ceb6
Author: Kevin Hilman <[email protected]>
Date: Tue Jun 14 05:53:18 2011 -0700

OMAP1: PM: register notifiers with generic clock ops even when !PM_RUNTIME

When runtime PM is disabled, device clocks need to be enabled on
device add and disabled on device remove. This currently is not
happening because in the !PM_RUNTIME case, no notifiers are registered
for OMAP1 devices.

Fix this by ensuring notifiers are registered, even in the !PM_RUNTIME case.

Reported-by: Janusz Krzysztofik <[email protected]>
Tested-by: Janusz Krzysztofik <[email protected]>
Signed-off-by: Kevin Hilman <[email protected]>
Signed-off-by: Tony Lindgren <[email protected]>

2011-06-21 11:05:48

by Rafael J. Wysocki

Subject: Re: [PATCH 0/8] PM / Domains: Support for generic I/O PM domains (v5)

On Tuesday, June 21, 2011, Kevin Hilman wrote:
> "Rafael J. Wysocki" <[email protected]> writes:
>
> > Hi,
> >
> > This is the 4th update of the patchset adding support for generic I/O PM
> > domains.
> >
> > The patches have been reworked quite a bit to take feedback into
> > account, but I left the Greg's ACK in [4/8] in the hope it still applies
> > (Greg, please let me know in case it doesn't :-)).
> >
> > The model here is that a bunch of devices share a common power resource
> > that can be turned on and off by software. In addition to that, there
> > are means to start and stop the activity of each device, for example
> > by manipulating their clocks. Moreover, there may be hierarchy of
> > such things, for example power resource A may be necessary for devices
> > a, b, c, which don't rely on any other power resources, and for devices
> > x, y, z that also rely on power resource X. In that case there one PM
> > domain object representing devices a, b, c and power resource A, and
> > another PM domain object will represent devices x, y, z with power
> > resource X, plus the first object will be the second one's parent.
> >
> > Note to Kevin: I know you'd like each PM domain to be able to go into several
> > different states, but the situation will always be that in some of those
> > states the devices' registers will remain intact, while in the rest of those
> > states they will be reset. Say, there are states 1, 2, 3, 4 and states
> > 1-3 preserve device registers. Then it is not necessary to save device
> > registers for "domain" states 1-3 and it only is necessary to save them
> > when going to state 4. In that case, .power_off() may map to the "go to
> > state 4" operation (and analogously .power_on()), while the rest may be
> > done by .stop_device() and .start_device(). IOW, .power_is_off == true
> > means "the devices' registers have to be restored", so it need not map to
> > any particular physical state of a (hardware) power domain.
>
> Sure, but it's not only about register context save/restore. It's about
> the the governor hook and how you decide which state to enter. IOW, the
> governor decision is not only about whether or not you will lose
> register context but also about the latency involved in entering &
> exiting those states.
>
> So from my perspective, having only 2-states at this level makes the
> governor rather pointless since any decision making will have to be done
> where ever the knowledge of the mulitple power states lives.

Well, in principle you can make the governor whatever you want, so it may
as well know of multiple states.

Anyway, if using multiple domain states turns out to be useful at the core
level, it may be added later with a separate patch.

> > Note to Magnus and Paul: I didn't use a global lock as suggested, because
> > I think it may lead to completely unnecessary congestion in situations in
> > which there are no hierarchies of PM domains. It is quite easy to show that
> > the code doesn't deadlock, because (1) no more than 2 locks are held by the
> > same thread at a time (parent lock and child lock) and (2) they are always
> > acquired in the same order (parent before the child).
> >
> > Overall, I think I've taken all of the important dependencies into
> > consideration, but if you spot something suspicious, please let me know. :-)
> > Wakeup is not covered at this point, because it's not necessary for the
> > SH7372's A4LC power domain that's the first user of the new code, but it
> > is quite clear how add the support for it. Also, for more complicated
> > cases it is necessary to take QoS requirements (latencies) into account,
> > which is in the works (kind of).
> >
> > [1/8] - Update documentation to reflect the fact that struct dev_power_domain
> > callbacks take precedence over subsystem PM callbacks.
> >
> > [2/8] - Rename struct dev_power_domain to struct dev_pm_domain to reflect the
> > fact that those objects need not correspond to hardware power domains
> > directly.
> >
> > [3/8] - Move subsys_data in struct dev_pm_info out of #ifdef CONFIG_PM_RUNTIME
> >
> > [4/8] - Introduce runtime PM support for generic I/O PM domains.
> >
> > [5/8] - Introduce generic "noirq" callbacks for system suspend/hibernation
> > (that's necessary for the next patches).
> >
> > [6/8] - Move some PM domains support code fro under #ifdef CONFIG_PM_RUNTIME
> >
> > [7/8] - Add system-wide PM support for generic I/O PM domains.
> >
> > [8/8] - Use the new code to represent the SH7372's A4MP power domain.
> >
> > The patchset has been tested on SH7372 Mackerel board and appears to work
> > correctly.
> >
> > I'd like to push [1/8] for 3.0 (it may be regarded as a fix), but I _think_
> > that it may be a good idea to push [2/8] for 3.0 too, to limit the time in
> > which people may possibly use the naming that's going to change in their new
> > code. If you agree with that, please let me know, I'll need some serious
> > ACKs below that patch if it's to be pushed for 3.0. ;-)
>
> Just gave you my ack,

I thought the ACK was for [2/8] only, so do I understand correctly that it's
for the entire series? :-)

> but [2/8] will need a minor update to apply on
> Linus' master branch since another fix to mach-omap1/pm_bus.c just got
> merged[1] via the OMAP tree.

Yes, I already rebased my patches on top of 3.0-rc4.

> I don't have any other fixes touching those files queued for v3.0 so I
> don't expect any other conflicts there.

Thanks,
Rafael

2011-06-21 11:56:29

by Rafael J. Wysocki

Subject: Re: [Update][PATCH 8/8] ARM / shmobile: Support for I/O power domains for SH7372 (v6)

On Tuesday, June 21, 2011, Rafael J. Wysocki wrote:
> On Monday, June 20, 2011, Paul Mundt wrote:
> > On Mon, Jun 20, 2011 at 12:07:47AM +0200, Rafael J. Wysocki wrote:
> > > +static int pd_power_down(struct generic_pm_domain *genpd)
> > > +{
> > > + struct sh7372_pm_domain *sh7372_pd = to_sh7372_pd(genpd);
> > > + unsigned int mask = 1 << sh7372_pd->bit_shift;
> > > +
> > > + if (__raw_readl(PSTR) & mask) {
> > > + __raw_writel(mask, SPDCR);
> > > +
> > > + while (__raw_readl(SPDCR) & mask)
> > > + cpu_relax();
> > > +
> > > + pr_debug("sh7372 power domain down 0x%08x -> PSTR = 0x%08x\n",
> > > + mask, __raw_readl(PSTR));
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int pd_power_up(struct generic_pm_domain *genpd)
> > > +{
> > > + struct sh7372_pm_domain *sh7372_pd = to_sh7372_pd(genpd);
> > > + unsigned int mask = 1 << sh7372_pd->bit_shift;
> > > +
> > > + if (!(__raw_readl(PSTR) & mask)) {
> > > + __raw_writel(mask, SWUCR);
> > > +
> > > + while (__raw_readl(SWUCR) & mask)
> > > + cpu_relax();
> > > +
> > > + pr_debug("sh7372 power domain up 0x%08x -> PSTR = 0x%08x\n",
> > > + mask, __raw_readl(PSTR));
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > Given that these functions can return errors, it's probably more prudent
> > to implement some timeout logic on top of the busy loop. Hardware does
> > get stuck, after all.
>
> Yes, it does, but we don't really know what timeout value to use in there.
>
> In fact, failures of pd_power_down() don't really matter (at worst the power
> domain won't be powered off really) and pd_power_up() should only return error
> code if the error is known to be unrecoverable.
>
> So, what about this: repeat certain number of times (I verified that it took
> several iterations of the loop until the new value settled on my hardware)
> and then return from pd_power_down() or go to sleep for a short time and repeat
> in pd_power_up()?

Having discussed that with Magnus I changed pd_power_down() to use a timeout.
Specifically, it will bail out if the state doesn't change after approximately
1 ms.

Updated patch is appended.

Thanks,
Rafael

---
From: Rafael J. Wysocki <[email protected]>
Subject: ARM / shmobile: Support for I/O power domains for SH7372 (v7)

Use the generic power domains support introduced by the previous
patch to implement support for power domains on SH7372.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
arch/arm/Kconfig | 1
arch/arm/mach-shmobile/Makefile | 4 +
arch/arm/mach-shmobile/board-mackerel.c | 4 +
arch/arm/mach-shmobile/include/mach/sh7372.h | 28 ++++++++
arch/arm/mach-shmobile/pm-sh7372.c | 92 +++++++++++++++++++++++++++
5 files changed, 129 insertions(+)

Index: linux-2.6/arch/arm/mach-shmobile/board-mackerel.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/board-mackerel.c
+++ linux-2.6/arch/arm/mach-shmobile/board-mackerel.c
@@ -1582,6 +1582,10 @@ static void __init mackerel_init(void)

platform_add_devices(mackerel_devices, ARRAY_SIZE(mackerel_devices));

+ sh7372_init_pm_domain(&sh7372_a4lc_domain);
+ sh7372_add_device_to_domain(SH7372_A4LC, &lcdc_device);
+ sh7372_add_device_to_domain(SH7372_A4LC, &hdmi_lcdc_device);
+
hdmi_init_pm_clock();
sh7372_pm_init();
}
Index: linux-2.6/arch/arm/mach-shmobile/include/mach/sh7372.h
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/include/mach/sh7372.h
+++ linux-2.6/arch/arm/mach-shmobile/include/mach/sh7372.h
@@ -12,6 +12,7 @@
#define __ASM_SH7372_H__

#include <linux/sh_clk.h>
+#include <linux/pm_domain.h>

/*
* Pin Function Controller:
@@ -470,4 +471,31 @@ extern struct clk sh7372_fsibck_clk;
extern struct clk sh7372_fsidiva_clk;
extern struct clk sh7372_fsidivb_clk;

+struct platform_device;
+
+struct sh7372_pm_domain {
+ struct generic_pm_domain genpd;
+ unsigned int bit_shift;
+};
+
+static inline struct sh7372_pm_domain *to_sh7372_pd(struct generic_pm_domain *d)
+{
+ return container_of(d, struct sh7372_pm_domain, genpd);
+}
+
+#ifdef CONFIG_PM
+extern struct sh7372_pm_domain sh7372_a4lc_domain;
+#define SH7372_A4LC (&sh7372_a4lc_domain)
+
+extern void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd);
+extern void sh7372_add_device_to_domain(struct sh7372_pm_domain *sh7372_pd,
+ struct platform_device *pdev);
+#else
+#define SH7372_A4LC NULL
+
+static inline void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd) {}
+static inline void sh7372_add_device_to_domain(struct sh7372_pm_domain *pd,
+ struct platform_device *pdev) {}
+#endif /* CONFIG_PM */
+
#endif /* __ASM_SH7372_H__ */
Index: linux-2.6/arch/arm/mach-shmobile/Makefile
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/Makefile
+++ linux-2.6/arch/arm/mach-shmobile/Makefile
@@ -42,6 +42,10 @@ obj-$(CONFIG_MACH_AP4EVB) += board-ap4ev
obj-$(CONFIG_MACH_AG5EVM) += board-ag5evm.o
obj-$(CONFIG_MACH_MACKEREL) += board-mackerel.o

+# PM objects
+pm-$(CONFIG_ARCH_SH7372) += pm-sh7372.o
+
# Framework support
obj-$(CONFIG_SMP) += $(smp-y)
obj-$(CONFIG_GENERIC_GPIO) += $(pfc-y)
+obj-$(CONFIG_PM) += $(pm-y)
Index: linux-2.6/arch/arm/mach-shmobile/pm-sh7372.c
===================================================================
--- linux-2.6.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux-2.6/arch/arm/mach-shmobile/pm-sh7372.c
@@ -15,16 +15,108 @@
#include <linux/list.h>
#include <linux/err.h>
#include <linux/slab.h>
+#include <linux/pm_runtime.h>
+#include <linux/platform_device.h>
+#include <linux/delay.h>
#include <asm/system.h>
#include <asm/io.h>
#include <asm/tlbflush.h>
#include <mach/common.h>
+#include <mach/sh7372.h>

#define SMFRAM 0xe6a70000
#define SYSTBCR 0xe6150024
#define SBAR 0xe6180020
#define APARMBAREA 0xe6f10020

+#define SPDCR 0xe6180008
+#define SWUCR 0xe6180014
+#define PSTR 0xe6180080
+
+#define PSTR_RETRIES 100
+#define PSTR_DELAY_US 10
+
+static int pd_power_down(struct generic_pm_domain *genpd)
+{
+ struct sh7372_pm_domain *sh7372_pd = to_sh7372_pd(genpd);
+ unsigned int mask = 1 << sh7372_pd->bit_shift;
+
+ if (__raw_readl(PSTR) & mask) {
+ unsigned int retry_count;
+
+ __raw_writel(mask, SPDCR);
+
+ for (retry_count = PSTR_RETRIES; retry_count; retry_count--) {
+ if (!(__raw_readl(SPDCR) & mask))
+ break;
+ cpu_relax();
+ }
+ }
+
+ pr_debug("sh7372 power domain down 0x%08x -> PSTR = 0x%08x\n",
+ mask, __raw_readl(PSTR));
+
+ return 0;
+}
+
+static int pd_power_up(struct generic_pm_domain *genpd)
+{
+ struct sh7372_pm_domain *sh7372_pd = to_sh7372_pd(genpd);
+ unsigned int mask = 1 << sh7372_pd->bit_shift;
+ unsigned int retry_count;
+ int ret = 0;
+
+ if (__raw_readl(PSTR) & mask)
+ goto out;
+
+ __raw_writel(mask, SWUCR);
+
+ for (retry_count = 2 * PSTR_RETRIES; retry_count; retry_count--) {
+ if (!(__raw_readl(SWUCR) & mask))
+ goto out;
+ if (retry_count > PSTR_RETRIES)
+ udelay(PSTR_DELAY_US);
+ else
+ cpu_relax();
+ }
+ if (__raw_readl(SWUCR) & mask)
+ ret = -EIO;
+
+ out:
+ pr_debug("sh7372 power domain up 0x%08x -> PSTR = 0x%08x\n",
+ mask, __raw_readl(PSTR));
+
+ return ret;
+}
+
+void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd)
+{
+ struct generic_pm_domain *genpd = &sh7372_pd->genpd;
+
+ pm_genpd_init(genpd, NULL, false);
+ genpd->stop_device = pm_runtime_clk_suspend;
+ genpd->start_device = pm_runtime_clk_resume;
+ genpd->power_off = pd_power_down;
+ genpd->power_on = pd_power_up;
+ pd_power_up(&sh7372_pd->genpd);
+}
+
+void sh7372_add_device_to_domain(struct sh7372_pm_domain *sh7372_pd,
+ struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+
+ if (!dev->power.subsys_data) {
+ pm_runtime_clk_init(dev);
+ pm_runtime_clk_add(dev, NULL);
+ }
+ pm_genpd_add_device(&sh7372_pd->genpd, dev);
+}
+
+struct sh7372_pm_domain sh7372_a4lc_domain = {
+ .bit_shift = 1,
+};
+
static void sh7372_enter_core_standby(void)
{
void __iomem *smfram = (void __iomem *)SMFRAM;
Index: linux-2.6/arch/arm/Kconfig
===================================================================
--- linux-2.6.orig/arch/arm/Kconfig
+++ linux-2.6/arch/arm/Kconfig
@@ -642,6 +642,7 @@ config ARCH_SHMOBILE
select NO_IOPORT
select SPARSE_IRQ
select MULTI_IRQ_HANDLER
+ select PM_GENERIC_DOMAINS
help
Support for Renesas's SH-Mobile and R-Mobile ARM platforms.

2011-06-21 12:48:54

by Paul Mundt

Subject: Re: [Update][PATCH 8/8] ARM / shmobile: Support for I/O power domains for SH7372 (v6)

On Tue, Jun 21, 2011 at 01:57:08PM +0200, Rafael J. Wysocki wrote:
> On Tuesday, June 21, 2011, Rafael J. Wysocki wrote:
> > On Monday, June 20, 2011, Paul Mundt wrote:
> > > Given that these functions can return errors, it's probably more prudent
> > > to implement some timeout logic on top of the busy loop. Hardware does
> > > get stuck, after all.
> >
> > Yes, it does, but we don't really know what timeout value to use in there.
> >
> > In fact, failures of pd_power_down() don't really matter (at worst the power
> > domain won't be powered off really) and pd_power_up() should only return error
> > code if the error is known to be unrecoverable.
> >
> > So, what about this: repeat certain number of times (I verified that it took
> > several iterations of the loop until the new value settled on my hardware)
> > and then return from pd_power_down() or go to sleep for a short time and repeat
> > in pd_power_up()?
>
> Having discussed that with Magnus I changed pd_power_down() to use a timeout.
> Specifically, it will bail out if the state doesn't change after approximately
> 1 ms.
>
Looks good to me, thanks for fixing it up.

Acked-by: Paul Mundt <[email protected]>

2011-06-21 14:47:19

by Kevin Hilman

Subject: Re: [PATCH 0/8] PM / Domains: Support for generic I/O PM domains (v5)

On Tue, 2011-06-21 at 13:06 +0200, Rafael J. Wysocki wrote:
> On Tuesday, June 21, 2011, Kevin Hilman wrote:
> > "Rafael J. Wysocki" <[email protected]> writes:
> >
> > > Hi,
> > >
> > > This is the 4th update of the patchset adding support for generic I/O PM
> > > domains.
> > >
> > > The patches have been reworked quite a bit to take feedback into
> > > account, but I left the Greg's ACK in [4/8] in the hope it still applies
> > > (Greg, please let me know in case it doesn't :-)).
> > >
> > > The model here is that a bunch of devices share a common power resource
> > > that can be turned on and off by software. In addition to that, there
> > > are means to start and stop the activity of each device, for example
> > > by manipulating their clocks. Moreover, there may be hierarchy of
> > > such things, for example power resource A may be necessary for devices
> > > a, b, c, which don't rely on any other power resources, and for devices
> > > x, y, z that also rely on power resource X. In that case there one PM
> > > domain object representing devices a, b, c and power resource A, and
> > > another PM domain object will represent devices x, y, z with power
> > > resource X, plus the first object will be the second one's parent.
> > >
> > > Note to Kevin: I know you'd like each PM domain to be able to go into several
> > > different states, but the situation will always be that in some of those
> > > states the devices' registers will remain intact, while in the rest of those
> > > states they will be reset. Say, there are states 1, 2, 3, 4 and states
> > > 1-3 preserve device registers. Then it is not necessary to save device
> > > registers for "domain" states 1-3 and it only is necessary to save them
> > > when going to state 4. In that case, .power_off() may map to the "go to
> > > state 4" operation (and analogously .power_on()), while the rest may be
> > > done by .stop_device() and .start_device(). IOW, .power_is_off == true
> > > means "the devices' registers have to be restored", so it need not map to
> > > any particular physical state of a (hardware) power domain.
> >
> > Sure, but it's not only about register context save/restore. It's about
> > the the governor hook and how you decide which state to enter. IOW, the
> > governor decision is not only about whether or not you will lose
> > register context but also about the latency involved in entering &
> > exiting those states.
> >
> > So from my perspective, having only 2-states at this level makes the
> > governor rather pointless since any decision making will have to be done
> > where ever the knowledge of the mulitple power states lives.
>
> Well, in principle you can make the governor whatever you want, so it may
> as well know of multiple states.
>
> Anyway, if using multiple domain states turns out to be useful at the core
> level, it may be added later with a separate patch.

OK

> > > Note to Magnus and Paul: I didn't use a global lock as suggested, because
> > > I think it may lead to completely unnecessary congestion in situations in
> > > which there are no hierarchies of PM domains. It is quite easy to show that
> > > the code doesn't deadlock, because (1) no more than 2 locks are held by the
> > > same thread at a time (parent lock and child lock) and (2) they are always
> > > acquired in the same order (parent before the child).
> > >
> > > Overall, I think I've taken all of the important dependencies into
> > > consideration, but if you spot something suspicious, please let me know. :-)
> > > Wakeup is not covered at this point, because it's not necessary for the
> > > SH7372's A4LC power domain that's the first user of the new code, but it
> > > is quite clear how add the support for it. Also, for more complicated
> > > cases it is necessary to take QoS requirements (latencies) into account,
> > > which is in the works (kind of).
> > >
> > > [1/8] - Update documentation to reflect the fact that struct dev_power_domain
> > > callbacks take precedence over subsystem PM callbacks.
> > >
> > > [2/8] - Rename struct dev_power_domain to struct dev_pm_domain to reflect the
> > > fact that those objects need not correspond to hardware power domains
> > > directly.
> > >
> > > [3/8] - Move subsys_data in struct dev_pm_info out of #ifdef CONFIG_PM_RUNTIME
> > >
> > > [4/8] - Introduce runtime PM support for generic I/O PM domains.
> > >
> > > [5/8] - Introduce generic "noirq" callbacks for system suspend/hibernation
> > > (that's necessary for the next patches).
> > >
> > > [6/8] - Move some PM domains support code fro under #ifdef CONFIG_PM_RUNTIME
> > >
> > > [7/8] - Add system-wide PM support for generic I/O PM domains.
> > >
> > > [8/8] - Use the new code to represent the SH7372's A4MP power domain.
> > >
> > > The patchset has been tested on SH7372 Mackerel board and appears to work
> > > correctly.
> > >
> > > I'd like to push [1/8] for 3.0 (it may be regarded as a fix), but I _think_
> > > that it may be a good idea to push [2/8] for 3.0 too, to limit the time in
> > > which people may possibly use the naming that's going to change in their new
> > > code. If you agree with that, please let me know, I'll need some serious
> > > ACKs below that patch if it's to be pushed for 3.0. ;-)
> >
> > Just gave you my ack,
>
> I thought the ACK was for [2/8] only, so do I understand correctly that it's
> for the entire series? :-)

So far, only for 2/8. I'm planning to spend some time looking at the
rest of the series today.

Kevin

> > but [2/8] will need a minor update to apply on
> > Linus' master branch since another fix to mach-omap1/pm_bus.c just got
> > merged[1] via the OMAP tree.
>
> Yes, I already rebased my patches on top of 3.0-rc4.
>
> > I don't have any other fixes touching those files queued for v3.0 so I
> > don't expect any other conflicts there.
>
> Thanks,
> Rafael

2011-06-21 17:42:27

by Kevin Hilman

Subject: Re: [Update][PATCH 4/8] PM / Domains: Support for generic I/O PM domains (v6)

"Rafael J. Wysocki" <[email protected]> writes:

> From: Rafael J. Wysocki <[email protected]>
>
> Introduce common headers, helper functions and callbacks allowing
> platforms to use simple generic power domains for runtime power
> management.
>
> Introduce struct generic_pm_domain to be used for representing
> power domains that each contain a number of devices and may be
> parent domains or subdomains with respect to other power domains.
> Among other things, this structure includes callbacks to be
> provided by platforms for performing specific tasks related to
> power management (i.e. ->stop_device() may disable a device's
> clocks, while ->start_device() may enable them, ->power_off() is
> supposed to remove power from the entire power domain
> and ->power_on() is supposed to restore it).
>
> Introduce functions that can be used as power domain runtime PM
> callbacks, pm_genpd_runtime_suspend() and pm_genpd_runtime_resume(),
> as well as helper functions for the initialization of a power
> domain represented by a struct generic_power_domain object,
> adding a device to or removing a device from it and adding or
> removing subdomains.
>
> Introduce configuration option CONFIG_PM_GENERIC_DOMAINS to be
> selected by the platforms that want to use the new code.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> Acked-by: Greg Kroah-Hartman <[email protected]>
> ---
>
> Hi,
>
> I this version of the patch I made the following modifications:
>
> * Removed the change adding platform_data to struct dev_pm_domain,
> because that field is not going to be necessary in the near future.
>
> * Moved the code calling drv->pm->runtime_suspend(dev) to a separate
> function that returns immediately if dle->need_restore is set for the
> given device (meaning that drv->pm->runtime_suspend(dev) has already
> been called for it and the corresponding ->runtime_resume() hasn't).
> This fixes a bug where drv->pm->runtime_suspend() could be called for
> a device whose state hasn't been restored after power cycling its
> PM domain.
>
> * Made pm_genpd_add_device() return error code on an attempt to add a device
> do a PM domain whose power_is_off is set (that complemets the previous
> modification).
>
> * Makde the .power_on() and .power_off() generic PM domain callbacks take
> (struct generic_pm_domain *) arguments instead of (struct dev_pm_domain *).
>
> Thanks,
> Rafael

There's a guiding assumption in this generic PM domain layer that the
runtime PM callbacks need only be called if power to the underlying PM
domain is actually being cut. As a result, devices no longer have a
callback called for other low-power states where the power may not
actually be cut (a.k.a low-power with memory & logic retention.)

However, there are devices (at least on OMAP, but I presume on all SoCs)
where the driver will need to do other "stuff" for *all* low-power
transitions, not just the power-off ones (e.g. device specific idle mode
registers, clearing device-specific events/state that prevent low power
transitions, etc.)

Because of this, I don't currently see how to use these generic PM
domains on devices with multiple power states since the runtime PM
callbacks are only called for a subset of the power states.

I haven't given this too much thought yet (especially the system PM
aspects), but in order for generic PM domains to be more broadly useful
for runtime PM, I'm starting to think that this series should add
another set of callbacks: .power_off, .power_on or something similar.
The .runtime_suspend/.runtime_resume callbacks would then be used for
all power states and the .power_off/.power_on callbacks used only when
power is actually cut.

[...]

> Index: linux-2.6/drivers/base/power/domain.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6/drivers/base/power/domain.c
> @@ -0,0 +1,490 @@
> +/*
> + * drivers/base/power/domain.c - Common code related to device power domains.
> + *
> + * Copyright (C) 2011 Rafael J. Wysocki <[email protected]>, Renesas Electronics Corp.
> + *
> + * This file is released under the GPLv2.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/io.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pm_domain.h>
> +#include <linux/slab.h>
> +#include <linux/err.h>
> +
> +#ifdef CONFIG_PM_RUNTIME
> +
> +static void genpd_sd_counter_dec(struct generic_pm_domain *genpd)
> +{
> + if (!WARN_ON(genpd->sd_count == 0))
> + genpd->sd_count--;
> +}
> +
> +/**
> + * __pm_genpd_save_device - Save the pre-suspend state of a device.
> + * @dle: Device list entry of the device to save the state of.
> + * @genpd: PM domain the device belongs to.
> + */
> +static int __pm_genpd_save_device(struct dev_list_entry *dle,
> + struct generic_pm_domain *genpd)
> +{
> + struct device *dev = dle->dev;
> + struct device_driver *drv = dev->driver;
> + int ret = 0;
> +
> + if (dle->need_restore)
> + return 0;
> +
> + if (genpd->start_device)
> + genpd->start_device(dev);
> +
> + if (drv && drv->pm && drv->pm->runtime_suspend)

The start/stop device calls should probably be included inside this 'if'
since there's no reason to restart and re-stop the device if there is no
callback to be run.

Some drivers have alterntive ways of saving context (shadow registers,
manual save/restore per-xfer, etc.) and thus have no callbacks for
context save/restore.

> + ret = drv->pm->runtime_suspend(dev);
> +
> + if (genpd->stop_device)
> + genpd->stop_device(dev);
> +
> + if (!ret)
> + dle->need_restore = true;
> +
> + return ret;
> +}
> +
> +/**
> + * __pm_genpd_restore_device - Restore the pre-suspend state of a device.
> + * @dle: Device list entry of the device to restore the state of.
> + * @genpd: PM domain the device belongs to.
> + */
> +static void __pm_genpd_restore_device(struct dev_list_entry *dle,
> + struct generic_pm_domain *genpd)
> +{
> + struct device *dev = dle->dev;
> + struct device_driver *drv = dev->driver;
> +
> + if (!dle->need_restore)
> + return;
> +
> + if (genpd->start_device)
> + genpd->start_device(dev);
> +
> + if (drv && drv->pm && drv->pm->runtime_resume)

Similar to the 'save' case, the start/stop device calls should also be
included inside this 'if'.

> + drv->pm->runtime_resume(dev);
> +
> + if (genpd->stop_device)
> + genpd->stop_device(dev);
> +
> + dle->need_restore = false;
> +}

Kevin

2011-06-22 00:06:21

by Rafael J. Wysocki

Subject: Re: [Update][PATCH 4/8] PM / Domains: Support for generic I/O PM domains (v6)

On Tuesday, June 21, 2011, Kevin Hilman wrote:
> "Rafael J. Wysocki" <[email protected]> writes:
>
> > From: Rafael J. Wysocki <[email protected]>
> >
> > Introduce common headers, helper functions and callbacks allowing
> > platforms to use simple generic power domains for runtime power
> > management.
> >
> > Introduce struct generic_pm_domain to be used for representing
> > power domains that each contain a number of devices and may be
> > parent domains or subdomains with respect to other power domains.
> > Among other things, this structure includes callbacks to be
> > provided by platforms for performing specific tasks related to
> > power management (i.e. ->stop_device() may disable a device's
> > clocks, while ->start_device() may enable them, ->power_off() is
> > supposed to remove power from the entire power domain
> > and ->power_on() is supposed to restore it).
> >
> > Introduce functions that can be used as power domain runtime PM
> > callbacks, pm_genpd_runtime_suspend() and pm_genpd_runtime_resume(),
> > as well as helper functions for the initialization of a power
> > domain represented by a struct generic_power_domain object,
> > adding a device to or removing a device from it and adding or
> > removing subdomains.
> >
> > Introduce configuration option CONFIG_PM_GENERIC_DOMAINS to be
> > selected by the platforms that want to use the new code.
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > Acked-by: Greg Kroah-Hartman <[email protected]>
> > ---
> >
> > Hi,
> >
> > I this version of the patch I made the following modifications:
> >
> > * Removed the change adding platform_data to struct dev_pm_domain,
> > because that field is not going to be necessary in the near future.
> >
> > * Moved the code calling drv->pm->runtime_suspend(dev) to a separate
> > function that returns immediately if dle->need_restore is set for the
> > given device (meaning that drv->pm->runtime_suspend(dev) has already
> > been called for it and the corresponding ->runtime_resume() hasn't).
> > This fixes a bug where drv->pm->runtime_suspend() could be called for
> > a device whose state hasn't been restored after power cycling its
> > PM domain.
> >
> > * Made pm_genpd_add_device() return error code on an attempt to add a device
> > do a PM domain whose power_is_off is set (that complemets the previous
> > modification).
> >
> > * Makde the .power_on() and .power_off() generic PM domain callbacks take
> > (struct generic_pm_domain *) arguments instead of (struct dev_pm_domain *).
> >
> > Thanks,
> > Rafael
>
> There's a guiding assumption in this generic PM domain layer that the
> runtime PM callbacks need only be called if power to the underlying PM
> domain is actually being cut. As a result, devices no longer have a
> callback called for other low-power states where the power may not
> actually be cut (a.k.a low-power with memory & logic retention.)
>
> However, there are devices (at least on OMAP, but I presume on all SoCs)
> where the driver will need to do other "stuff" for *all* low-power
> transitions, not just the power-off ones (e.g. device specific idle mode
> registers, clearing device-specific events/state that prevent low power
> transitions, etc.)
>
> Because of this, I don't currently see how to use these generic PM
> domains on devices with multiple power states since the runtime PM
> callbacks are only called for a subset of the power states.
>
> I haven't given this too much thought yet (especially the system PM
> aspects), but in order for generic PM domains to be more broadly useful
> for runtime PM, I'm starting to think that this series should add
> another set of callbacks: .power_off, .power_on or something similar.
> The .runtime_suspend/.runtime_resume callbacks would then be used for
> all power states and the .power_off/.power_on callbacks used only when
> power is actually cut.

Well, I _really_ would like to avoid adding more callbacks to struct
dev_pm_ops, if that's what you mean, because we already seem to have
problems with managing the existing ones.

Also, IMO, you can always map every system with more power states to the
model where there are only "device active" (runtime PM RPM_ACTIVE) "device
stopped" (runtime PM RPM_SUSPENDED, need not save state) and "device
power off" (runtime PM RPM_SUSPENDED, must save state) "software" states
represented here.

If anything more fine grained is necessary or useful, I'd say you need a
more complicated model, but I'd prefer to avoid further complications in this
patchset.

> [...]
>
> > Index: linux-2.6/drivers/base/power/domain.c
> > ===================================================================
> > --- /dev/null
> > +++ linux-2.6/drivers/base/power/domain.c
> > @@ -0,0 +1,490 @@
> > +/*
> > + * drivers/base/power/domain.c - Common code related to device power domains.
> > + *
> > + * Copyright (C) 2011 Rafael J. Wysocki <[email protected]>, Renesas Electronics Corp.
> > + *
> > + * This file is released under the GPLv2.
> > + */
> > +
> > +#include <linux/init.h>
> > +#include <linux/kernel.h>
> > +#include <linux/io.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/pm_domain.h>
> > +#include <linux/slab.h>
> > +#include <linux/err.h>
> > +
> > +#ifdef CONFIG_PM_RUNTIME
> > +
> > +static void genpd_sd_counter_dec(struct generic_pm_domain *genpd)
> > +{
> > + if (!WARN_ON(genpd->sd_count == 0))
> > + genpd->sd_count--;
> > +}
> > +
> > +/**
> > + * __pm_genpd_save_device - Save the pre-suspend state of a device.
> > + * @dle: Device list entry of the device to save the state of.
> > + * @genpd: PM domain the device belongs to.
> > + */
> > +static int __pm_genpd_save_device(struct dev_list_entry *dle,
> > + struct generic_pm_domain *genpd)
> > +{
> > + struct device *dev = dle->dev;
> > + struct device_driver *drv = dev->driver;
> > + int ret = 0;
> > +
> > + if (dle->need_restore)
> > + return 0;
> > +
> > + if (genpd->start_device)
> > + genpd->start_device(dev);
> > +
> > + if (drv && drv->pm && drv->pm->runtime_suspend)
>
> The start/stop device calls should probably be included inside this 'if'
> since there's no reason to restart and re-stop the device if there is no
> callback to be run.

That's a good point, I'll do that.

> Some drivers have alterntive ways of saving context (shadow registers,
> manual save/restore per-xfer, etc.) and thus have no callbacks for
> context save/restore.
>
> > + ret = drv->pm->runtime_suspend(dev);
> > +
> > + if (genpd->stop_device)
> > + genpd->stop_device(dev);
> > +
> > + if (!ret)
> > + dle->need_restore = true;
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * __pm_genpd_restore_device - Restore the pre-suspend state of a device.
> > + * @dle: Device list entry of the device to restore the state of.
> > + * @genpd: PM domain the device belongs to.
> > + */
> > +static void __pm_genpd_restore_device(struct dev_list_entry *dle,
> > + struct generic_pm_domain *genpd)
> > +{
> > + struct device *dev = dle->dev;
> > + struct device_driver *drv = dev->driver;
> > +
> > + if (!dle->need_restore)
> > + return;
> > +
> > + if (genpd->start_device)
> > + genpd->start_device(dev);
> > +
> > + if (drv && drv->pm && drv->pm->runtime_resume)
>
> Similar to the 'save' case, the start/stop device calls should also be
> included inside this 'if'.

Agreed.

> > + drv->pm->runtime_resume(dev);
> > +
> > + if (genpd->stop_device)
> > + genpd->stop_device(dev);
> > +
> > + dle->need_restore = false;
> > +}

Thanks for the comments, I'm going to implement your suggestions.

Take care,
Rafael

2011-06-22 19:51:08

by Kevin Hilman

Subject: Re: [Update][PATCH 4/8] PM / Domains: Support for generic I/O PM domains (v6)

"Rafael J. Wysocki" <[email protected]> writes:

> On Tuesday, June 21, 2011, Kevin Hilman wrote:
>> "Rafael J. Wysocki" <[email protected]> writes:

[...]

>>
>> There's a guiding assumption in this generic PM domain layer that the
>> runtime PM callbacks need only be called if power to the underlying PM
>> domain is actually being cut. As a result, devices no longer have a
>> callback called for other low-power states where the power may not
>> actually be cut (a.k.a low-power with memory & logic retention.)
>>
>> However, there are devices (at least on OMAP, but I presume on all SoCs)
>> where the driver will need to do other "stuff" for *all* low-power
>> transitions, not just the power-off ones (e.g. device specific idle mode
>> registers, clearing device-specific events/state that prevent low power
>> transitions, etc.)
>>
>> Because of this, I don't currently see how to use these generic PM
>> domains on devices with multiple power states since the runtime PM
>> callbacks are only called for a subset of the power states.
>>
>> I haven't given this too much thought yet (especially the system PM
>> aspects), but in order for generic PM domains to be more broadly useful
>> for runtime PM, I'm starting to think that this series should add
>> another set of callbacks: .power_off, .power_on or something similar.
>> The .runtime_suspend/.runtime_resume callbacks would then be used for
>> all power states and the .power_off/.power_on callbacks used only when
>> power is actually cut.
>
> Well, I _really_ would like to avoid adding more callbacks to struct
> dev_pm_ops, if that's what you mean, because we already seem to have
> problems with managing the existing ones.

I agree, I don't really want to see more callbacks either.

> Also, IMO, you can always map every system with more power states to the
> model where there are only "device active" (runtime PM RPM_ACTIVE) "device
> stopped" (runtime PM RPM_SUSPENDED, need not save state) and "device
> power off" (runtime PM RPM_SUSPENDED, must save state) "software" states
> represented here.

Yes, but note that you list 2 RPM_SUSPENDED states, but there is only
one .runtime_suspend callback, so the driver is only be notified of one
of the them.

More specifically, the problem is that using generic PM domains, there
are no callbacks to the driver for "device stopped", since the driver's
.runtime_suspend() is not called until "device power off."

What I tried to say in my initial reply is that many devices actually
have device specific stuff to do in preparation for "device stopped", or
the hardware will not actually reach the targetted power state. Without
a callback, no device-specific preparation for "device stopped" can be
done.

thinking out loud: hmm..., the more I think of this, maybe we should
actually be using RPM_IDLE to represent your definition of "device
stopped."

> If anything more fine grained is necessary or useful, I'd say you need
> a more complicated model, but I'd prefer to avoid further
> complications in this patchset.

Unfortunately, PM on embedded devices is very fine grained and very
complicated.

Kevin

2011-06-22 21:29:31

by Rafael J. Wysocki

Subject: Re: [Update][PATCH 4/8] PM / Domains: Support for generic I/O PM domains (v6)

On Wednesday, June 22, 2011, Kevin Hilman wrote:
> "Rafael J. Wysocki" <[email protected]> writes:
>
> > On Tuesday, June 21, 2011, Kevin Hilman wrote:
> >> "Rafael J. Wysocki" <[email protected]> writes:
>
> [...]
>
> >>
> >> There's a guiding assumption in this generic PM domain layer that the
> >> runtime PM callbacks need only be called if power to the underlying PM
> >> domain is actually being cut. As a result, devices no longer have a
> >> callback called for other low-power states where the power may not
> >> actually be cut (a.k.a low-power with memory & logic retention.)
> >>
> >> However, there are devices (at least on OMAP, but I presume on all SoCs)
> >> where the driver will need to do other "stuff" for *all* low-power
> >> transitions, not just the power-off ones (e.g. device specific idle mode
> >> registers, clearing device-specific events/state that prevent low power
> >> transitions, etc.)
> >>
> >> Because of this, I don't currently see how to use these generic PM
> >> domains on devices with multiple power states since the runtime PM
> >> callbacks are only called for a subset of the power states.
> >>
> >> I haven't given this too much thought yet (especially the system PM
> >> aspects), but in order for generic PM domains to be more broadly useful
> >> for runtime PM, I'm starting to think that this series should add
> >> another set of callbacks: .power_off, .power_on or something similar.
> >> The .runtime_suspend/.runtime_resume callbacks would then be used for
> >> all power states and the .power_off/.power_on callbacks used only when
> >> power is actually cut.
> >
> > Well, I _really_ would like to avoid adding more callbacks to struct
> > dev_pm_ops, if that's what you mean, because we already seem to have
> > problems with managing the existing ones.
>
> I agree, I don't really want to see more callbacks either.
>
> > Also, IMO, you can always map every system with more power states to the
> > model where there are only "device active" (runtime PM RPM_ACTIVE) "device
> > stopped" (runtime PM RPM_SUSPENDED, need not save state) and "device
> > power off" (runtime PM RPM_SUSPENDED, must save state) "software" states
> > represented here.
>
> Yes, but note that you list 2 RPM_SUSPENDED states, but there is only
> one .runtime_suspend callback, so the driver is only be notified of one
> of the them.

That's correct.

> More specifically, the problem is that using generic PM domains, there
> are no callbacks to the driver for "device stopped", since the driver's
> .runtime_suspend() is not called until "device power off."

Yes, it is, because that is not necessary for the first user (the shmobile
domain added by [8/8]). However, I have a plan to add such a mechanism
using the subsys_data field from struct dev_pm_info (it's only used for
the clocks management right now, but it's going to see more usage anyway :-)).

> What I tried to say in my initial reply is that many devices actually
> have device specific stuff to do in preparation for "device stopped", or
> the hardware will not actually reach the targetted power state. Without
> a callback, no device-specific preparation for "device stopped" can be
> done.
>
> thinking out loud: hmm..., the more I think of this, maybe we should
> actually be using RPM_IDLE to represent your definition of "device
> stopped."

I don't think that will work as expected.

> > If anything more fine grained is necessary or useful, I'd say you need
> > a more complicated model, but I'd prefer to avoid further
> > complications in this patchset.
>
> Unfortunately, PM on embedded devices is very fine grained and very
> complicated.

Which hopefully doesn't mean the code has to cover all of the possible
complications from the start. :-)

Thanks,
Rafael

2011-06-22 21:50:56

by Kevin Hilman

Subject: Re: [Update][PATCH 7/8] PM / Domains: System-wide transitions support for generic domains (v3)

"Rafael J. Wysocki" <[email protected]> writes:

> From: Rafael J. Wysocki <[email protected]>
>
> Make generic PM domains support system-wide power transitions
> (system suspend and hibernation). Add suspend, resume, freeze, thaw,
> poweroff and restore callbacks to be associated with struct
> generic_pm_domain objects and make pm_genpd_init() use them as
> appropriate.
>
> The new callbacks do nothing for devices belonging to power domains
> that were powered down at run time (before the transition).

Great, this is the approach I prefer too, but...

Now I'm confused. Leaving runtime suspended devices alone is what I was
doing in my subsystem but was told not to. According to

http://www.mail-archive.com/[email protected]/msg50690.html

"it's generally agreed that _all_ devices should return to full
power during system resume -- even if they were runtime suspended
before the system sleep."

Kevin

2011-06-22 22:16:09

by Rafael J. Wysocki

Subject: Re: [Update][PATCH 7/8] PM / Domains: System-wide transitions support for generic domains (v3)

On Wednesday, June 22, 2011, Kevin Hilman wrote:
> "Rafael J. Wysocki" <[email protected]> writes:
>
> > From: Rafael J. Wysocki <[email protected]>
> >
> > Make generic PM domains support system-wide power transitions
> > (system suspend and hibernation). Add suspend, resume, freeze, thaw,
> > poweroff and restore callbacks to be associated with struct
> > generic_pm_domain objects and make pm_genpd_init() use them as
> > appropriate.
> >
> > The new callbacks do nothing for devices belonging to power domains
> > that were powered down at run time (before the transition).
>
> Great, this is the approach I prefer too, but...
>
> Now I'm confused. Leaving runtime suspended devices alone is what I was
> doing in my subsystem but was told not to. According to
>
> http://www.mail-archive.com/[email protected]/msg50690.html
>
> "it's generally agreed that _all_ devices should return to full
> power during system resume -- even if they were runtime suspended
> before the system sleep."

Well, let's say this part of the documentation is slightly outdated.

It basically refers to the model in which system suspend is a separate global
hardware or firmware operation, so the state of devices may be changed by the
BIOS or whatever takes over control in the meantime. In that case the kernel
has to ensure that the states of devices are consistent with what it thinks
about them and the simplest way to achieve that is to put the devices to
full power during resume (and back to low power if that's desirable).

However, in the case of the systems this patchset is intended for system
suspend is achieved by putting various hardware components into low-power
states directly in a coordinated way and the system sleep state effectively
follows from the low-power states the hardware components end up in. The
system is woken up from this state by an interrupt or another mechanism under
the kernel's control. As a result, the kernel never gives control away, so
the state of devices after the resume is precisely known to it.
In consequence, it need not ensure that the state of devices is consistent with
its view, because it knows that this is the case. :-)

So the documentation should be updated to say what hardware model it is
referring to.

Thanks,
Rafael

2011-06-22 22:18:22

by Kevin Hilman

Subject: Re: [Update][PATCH 7/8] PM / Domains: System-wide transitions support for generic domains (v3)

"Rafael J. Wysocki" <[email protected]> writes:

> On Wednesday, June 22, 2011, Kevin Hilman wrote:
>> "Rafael J. Wysocki" <[email protected]> writes:
>>
>> > From: Rafael J. Wysocki <[email protected]>
>> >
>> > Make generic PM domains support system-wide power transitions
>> > (system suspend and hibernation). Add suspend, resume, freeze, thaw,
>> > poweroff and restore callbacks to be associated with struct
>> > generic_pm_domain objects and make pm_genpd_init() use them as
>> > appropriate.
>> >
>> > The new callbacks do nothing for devices belonging to power domains
>> > that were powered down at run time (before the transition).
>>
>> Great, this is the approach I prefer too, but...
>>
>> Now I'm confused. Leaving runtime suspended devices alone is what I was
>> doing in my subsystem but was told not to. According to
>>
>> http://www.mail-archive.com/[email protected]/msg50690.html
>>
>> "it's generally agreed that _all_ devices should return to full
>> power during system resume -- even if they were runtime suspended
>> before the system sleep."
>
> Well, let's say this part of the documentation is slightly outdated.
>
> It basically refers to the model in which system suspend is a separate global
> hardware or firmware operation, so the state of devices may be changed by the
> BIOS or whatever takes over control in the meantime. In that case the kernel
> has to ensure that the states of devices are consistent with what it thinks
> about them and the simplest way to achieve that is to put the devices to
> full power during resume (and back to low power if that's desirable).
>
> However, in the case of the systems this patchset is intended for system
> suspend is achieved by putting various hardware components into low-power
> states directly in a coordinated way and the system sleep state effectively
> follows from the low-power states the hardware components end up in. The
> system is woken up from this state by an interrupt or another mechanism under
> the kernel's control. As a result, the kernel never gives control away, so
> the state of devices after the resume is precisely known to it.
> In consequence, it need not ensure that the state of devices is consistent with
> its view, because it knows that this is the case. :-)
>
> So the documentation should be updated to say what hardware model it is
> referring to.

Great! Thanks for the clarification.

Kevin

2011-06-22 22:22:20

by Rafael J. Wysocki

Subject: Re: [Update][PATCH 7/8] PM / Domains: System-wide transitions support for generic domains (v3)

On Thursday, June 23, 2011, Kevin Hilman wrote:
> "Rafael J. Wysocki" <[email protected]> writes:
>
> > On Wednesday, June 22, 2011, Kevin Hilman wrote:
> >> "Rafael J. Wysocki" <[email protected]> writes:
> >>
> >> > From: Rafael J. Wysocki <[email protected]>
> >> >
> >> > Make generic PM domains support system-wide power transitions
> >> > (system suspend and hibernation). Add suspend, resume, freeze, thaw,
> >> > poweroff and restore callbacks to be associated with struct
> >> > generic_pm_domain objects and make pm_genpd_init() use them as
> >> > appropriate.
> >> >
> >> > The new callbacks do nothing for devices belonging to power domains
> >> > that were powered down at run time (before the transition).
> >>
> >> Great, this is the approach I prefer too, but...
> >>
> >> Now I'm confused. Leaving runtime suspended devices alone is what I was
> >> doing in my subsystem but was told not to. According to
> >>
> >> http://www.mail-archive.com/[email protected]/msg50690.html
> >>
> >> "it's generally agreed that _all_ devices should return to full
> >> power during system resume -- even if they were runtime suspended
> >> before the system sleep."
> >
> > Well, let's say this part of the documentation is slightly outdated.
> >
> > It basically refers to the model in which system suspend is a separate global
> > hardware or firmware operation, so the state of devices may be changed by the
> > BIOS or whatever takes over control in the meantime. In that case the kernel
> > has to ensure that the states of devices are consistent with what it thinks
> > about them and the simplest way to achieve that is to put the devices to
> > full power during resume (and back to low power if that's desirable).
> >
> > However, in the case of the systems this patchset is intended for system
> > suspend is achieved by putting various hardware components into low-power
> > states directly in a coordinated way and the system sleep state effectively
> > follows from the low-power states the hardware components end up in. The
> > system is woken up from this state by an interrupt or another mechanism under
> > the kernel's control. As a result, the kernel never gives control away, so
> > the state of devices after the resume is precisely known to it.
> > In consequence, it need not ensure that the state of devices is consistent with
> > its view, because it knows that this is the case. :-)
> >
> > So the documentation should be updated to say what hardware model it is
> > referring to.
>
> Great! Thanks for the clarification.

No problem, I guess I should update the documentation eventually.

Thanks,
Rafael

2011-06-23 13:57:04

by Rafael J. Wysocki

Subject: [PATCH] PM / Runtime: Update documentation of interactions with system sleep

From: Rafael J. Wysocki <[email protected]>

The documents describing the interactions between runtime PM and
system sleep generally refer to the model in which the system sleep
state is entered through a global firmware or hardware operation.
As a result, some recommendations given in there are not entirely
suitable for systems in which this is not the case. Update the
documentation take the existence of those systems into accout.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
Documentation/power/devices.txt | 6 +++---
Documentation/power/runtime_pm.txt | 25 +++++++++++++++++--------
2 files changed, 20 insertions(+), 11 deletions(-)

Index: linux-2.6/Documentation/power/runtime_pm.txt
===================================================================
--- linux-2.6.orig/Documentation/power/runtime_pm.txt
+++ linux-2.6/Documentation/power/runtime_pm.txt
@@ -537,9 +537,9 @@ suspend routine). It may be necessary t
in order to do so. The same is true if the driver uses different power levels
or other settings for run-time suspend and system sleep.

-During system resume, devices generally should be brought back to full power,
-even if they were suspended before the system sleep began. There are several
-reasons for this, including:
+During system resume, the simplest approach is to bring all devices back to full
+power, even if they were suspended before the system sleep began. There are
+several reasons for this, including:

* The device might need to switch power levels, wake-up settings, etc.

@@ -556,16 +556,25 @@ reasons for this, including:
* Even though the device was suspended, if its usage counter was > 0 then most
likely it would need a run-time resume in the near future anyway.

- * Always going back to full power is simplest.
-
-If the device was suspended before the sleep began, then its run-time PM status
-will have to be updated to reflect the actual post-system sleep status. The way
-to do this is:
+If the device had been suspended before the sleep began and it's brought back to
+full power during resume, then its run-time PM status will have to be updated to
+reflect the actual post-system sleep status. The way to do this is:

pm_runtime_disable(dev);
pm_runtime_set_active(dev);
pm_runtime_enable(dev);

+On some systems, however, system sleep is not entered through a global firmware
+or hardware operation. Instead, all hardware components are put into low-power
+states directly by the kernel in a coordinated way. Then, the system sleep
+state effectively follows from the states the hardware components end up in
+and the system is woken up from that state by a hardware interrupt or a similar
+mechanism entirely under the kernel's control. As a result, the kernel never
+gives control away and the states of all devices during resume are precisely
+known to it. If that is the case and none of the situations listed above takes
+place, it may be more efficient to leave the devices that had been suspended
+before the system sleep began in the suspended state.
+
7. Generic subsystem callbacks

Subsystems may wish to conserve code space by using the set of generic power
Index: linux-2.6/Documentation/power/devices.txt
===================================================================
--- linux-2.6.orig/Documentation/power/devices.txt
+++ linux-2.6/Documentation/power/devices.txt
@@ -604,7 +604,7 @@ state temporarily, for example so that i
disabled. This all depends on the hardware and the design of the subsystem and
device driver in question.

-During system-wide resume from a sleep state it's best to put devices into the
-full-power state, as explained in Documentation/power/runtime_pm.txt. Refer to
-that document for more information regarding this particular issue as well as
+During system-wide resume from a sleep state it's easiest to put devices into
+the full-power state, as explained in Documentation/power/runtime_pm.txt. Refer
+to that document for more information regarding this particular issue as well as
for information on the device runtime power management framework in general.

2011-06-23 14:20:02

by Alan Stern

Subject: Re: [Update][PATCH 7/8] PM / Domains: System-wide transitions support for generic domains (v3)

On Thu, 23 Jun 2011, Rafael J. Wysocki wrote:

> Well, let's say this part of the documentation is slightly outdated.
>
> It basically refers to the model in which system suspend is a separate global
> hardware or firmware operation, so the state of devices may be changed by the
> BIOS or whatever takes over control in the meantime. In that case the kernel
> has to ensure that the states of devices are consistent with what it thinks
> about them and the simplest way to achieve that is to put the devices to
> full power during resume (and back to low power if that's desirable).
>
> However, in the case of the systems this patchset is intended for system
> suspend is achieved by putting various hardware components into low-power
> states directly in a coordinated way and the system sleep state effectively
> follows from the low-power states the hardware components end up in. The
> system is woken up from this state by an interrupt or another mechanism under
> the kernel's control. As a result, the kernel never gives control away, so
> the state of devices after the resume is precisely known to it.
> In consequence, it need not ensure that the state of devices is consistent with
> its view, because it knows that this is the case. :-)

That's true for system suspend, but it's probably not true for
hibernation, even in embedded systems. Of course, many embedded
systems don't use hibernation at all -- but those that do should be
aware of this issue.

> So the documentation should be updated to say what hardware model it is
> referring to.

It might be worthwhile to include a little warning about the difference
between suspend and hibernate.

Alan Stern

2011-06-23 14:44:18

by Rafael J. Wysocki

Subject: Re: [Update][PATCH 7/8] PM / Domains: System-wide transitions support for generic domains (v3)

On Thursday, June 23, 2011, Alan Stern wrote:
> On Thu, 23 Jun 2011, Rafael J. Wysocki wrote:
>
> > Well, let's say this part of the documentation is slightly outdated.
> >
> > It basically refers to the model in which system suspend is a separate global
> > hardware or firmware operation, so the state of devices may be changed by the
> > BIOS or whatever takes over control in the meantime. In that case the kernel
> > has to ensure that the states of devices are consistent with what it thinks
> > about them and the simplest way to achieve that is to put the devices to
> > full power during resume (and back to low power if that's desirable).
> >
> > However, in the case of the systems this patchset is intended for system
> > suspend is achieved by putting various hardware components into low-power
> > states directly in a coordinated way and the system sleep state effectively
> > follows from the low-power states the hardware components end up in. The
> > system is woken up from this state by an interrupt or another mechanism under
> > the kernel's control. As a result, the kernel never gives control away, so
> > the state of devices after the resume is precisely known to it.
> > In consequence, it need not ensure that the state of devices is consistent with
> > its view, because it knows that this is the case. :-)
>
> That's true for system suspend, but it's probably not true for
> hibernation, even in embedded systems. Of course, many embedded
> systems don't use hibernation at all -- but those that do should be
> aware of this issue.
>
> > So the documentation should be updated to say what hardware model it is
> > referring to.
>
> It might be worthwhile to include a little warning about the difference
> between suspend and hibernate.

Well, there already is one:

"* The driver's idea of the device state may not agree with the device's
physical state. This can happen during resume from hibernation."

(in Documentation/power/runtime_pm.txt). Also, the new text in the patch
https://lkml.org/lkml/2011/6/23/200 I've just sent says literally:

"If that is the case and none of the situations listed above takes place, it
may be more efficient to leave the devices that had been suspended before
the system sleep began in the suspended state."

where the "situations listed above" include it.

Do you think that's not sufficient?

Rafael

2011-06-23 15:11:49

by Alan Stern

Subject: Re: [Update][PATCH 7/8] PM / Domains: System-wide transitions support for generic domains (v3)

On Thu, 23 Jun 2011, Rafael J. Wysocki wrote:

> > It might be worthwhile to include a little warning about the difference
> > between suspend and hibernate.
>
> Well, there already is one:
>
> "* The driver's idea of the device state may not agree with the device's
> physical state. This can happen during resume from hibernation."
>
> (in Documentation/power/runtime_pm.txt). Also, the new text in the patch
> https://lkml.org/lkml/2011/6/23/200 I've just sent says literally:
>
> "If that is the case and none of the situations listed above takes place, it
> may be more efficient to leave the devices that had been suspended before
> the system sleep began in the suspended state."
>
> where the "situations listed above" include it.
>
> Do you think that's not sufficient?

The preceding text in your new patch says:

+On some systems, however, system sleep is not entered through a global firmware
+or hardware operation. Instead, all hardware components are put into low-power
+states directly by the kernel in a coordinated way. Then, the system sleep
+state effectively follows from the states the hardware components end up in
+and the system is woken up from that state by a hardware interrupt or a similar
+mechanism entirely under the kernel's control. As a result, the kernel never
+gives control away and the states of all devices during resume are precisely
+known to it.

It should say "system suspend" rather than "system sleep".

Then to drive the point home, the following sentence chould say
something like this:

If that is the case and none of the situations listed above takes place
(in particular, if the system is waking up from suspend and not from
hibernation), it may be more efficient to leave the devices that had
been suspended before the system suspend began in the suspended state.

Alan Stern

2011-06-23 17:40:42

by Rafael J. Wysocki

Subject: Re: [Update][PATCH 7/8] PM / Domains: System-wide transitions support for generic domains (v3)

On Thursday, June 23, 2011, Alan Stern wrote:
> On Thu, 23 Jun 2011, Rafael J. Wysocki wrote:
>
> > > It might be worthwhile to include a little warning about the difference
> > > between suspend and hibernate.
> >
> > Well, there already is one:
> >
> > "* The driver's idea of the device state may not agree with the device's
> > physical state. This can happen during resume from hibernation."
> >
> > (in Documentation/power/runtime_pm.txt). Also, the new text in the patch
> > https://lkml.org/lkml/2011/6/23/200 I've just sent says literally:
> >
> > "If that is the case and none of the situations listed above takes place, it
> > may be more efficient to leave the devices that had been suspended before
> > the system sleep began in the suspended state."
> >
> > where the "situations listed above" include it.
> >
> > Do you think that's not sufficient?
>
> The preceding text in your new patch says:
>
> +On some systems, however, system sleep is not entered through a global firmware
> +or hardware operation. Instead, all hardware components are put into low-power
> +states directly by the kernel in a coordinated way. Then, the system sleep
> +state effectively follows from the states the hardware components end up in
> +and the system is woken up from that state by a hardware interrupt or a similar
> +mechanism entirely under the kernel's control. As a result, the kernel never
> +gives control away and the states of all devices during resume are precisely
> +known to it.
>
> It should say "system suspend" rather than "system sleep".

It says "system sleep" to distinguish between the state of the system
("system sleep") and the operation leading to that state ("system suspend").
That terminology is used all over the document, so I don't think it's a good
idea to change it just for this specific paragraph.

I agree that "suspend" should be used where it talks about starting, stopping
etc.

> Then to drive the point home, the following sentence chould say
> something like this:
>
> If that is the case and none of the situations listed above takes place
> (in particular, if the system is waking up from suspend and not from
> hibernation), it may be more efficient to leave the devices that had
> been suspended before the system suspend began in the suspended state.

That's fine by me, except that I'd simply say "(in particular, if the system
is not waking up from hibernation)".

Thanks,
Rafael

2011-06-23 18:22:56

by Alan Stern

Subject: Re: [Update][PATCH 7/8] PM / Domains: System-wide transitions support for generic domains (v3)

On Thu, 23 Jun 2011, Rafael J. Wysocki wrote:

> > It should say "system suspend" rather than "system sleep".
>
> It says "system sleep" to distinguish between the state of the system
> ("system sleep") and the operation leading to that state ("system suspend").
> That terminology is used all over the document, so I don't think it's a good
> idea to change it just for this specific paragraph.
>
> I agree that "suspend" should be used where it talks about starting, stopping
> etc.
>
> > Then to drive the point home, the following sentence chould say
> > something like this:
> >
> > If that is the case and none of the situations listed above takes place
> > (in particular, if the system is waking up from suspend and not from
> > hibernation), it may be more efficient to leave the devices that had
> > been suspended before the system suspend began in the suspended state.
>
> That's fine by me, except that I'd simply say "(in particular, if the system
> is not waking up from hibernation)".

Okay.

Alan Stern