linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS
@ 2011-11-07  0:01 Rafael J. Wysocki
  2011-11-07  0:06 ` [PATCH 1/7] PM / Domains: Make it possible to use per-device start/stop routines Rafael J. Wysocki
                   ` (7 more replies)
  0 siblings, 8 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-07  0:01 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

Hi,

The following patchset makes it possible to define device-specific PM domain
callbacks, allowing device drivers to be designed to support generic PM domains
and work without them as well, and introduces governor functions deciding
whether or not it is beneficial to put devices into low-power states.  The new
code is designed to be backwards compatible, as far as necessary.

[1/7] - Make it possible to defing per-device start/stop routines.
[2/7] - Make it possible to defing per-device .active_wakeup() callbacks.
[3/7] - Introduce "save/restore state" device callbacks.
[4/7] - Introduce per-device PM domain callbacks for system suspend.
[5/7] - Add device "stop governor".
[6/7] - Add domain "power off governor".
[7/7] - Automatically update overoptimistic latency information.

Please refer to the changelogs for more information.

The patches haven't been fully tested yet, so most likely there still are some
rough edges here and there.

Thanks,
Rafael


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 1/7] PM / Domains: Make it possible to use per-device start/stop routines
  2011-11-07  0:01 [PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
@ 2011-11-07  0:06 ` Rafael J. Wysocki
  2011-11-08  9:30   ` Guennadi Liakhovetski
  2011-11-07  0:06 ` [PATCH 2/7] PM / Domains: Make it possible to use per-device .active_wakeup() Rafael J. Wysocki
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-07  0:06 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

The current generic PM domains code requires that the same .stop()
and .start() device callback routines be used for all devices in the
given domain, which is inflexible and may not cover some specific use
cases.  For this reason, make it possible to use device specific
.start() and .stop() callback routines by adding corresponding
callback pointers to struct generic_pm_domain_data.  Add a new helper
routine, pm_genpd_register_callbacks(), that can be used to populate
the new per-device callback pointers.

Modify the shmobile's power domains code to allow drivers to add
their own code to be run during the device stop and start operations
with the help of the new callback pointers.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/mach-shmobile/pm-sh7372.c |   33 ++++++++-
 drivers/base/power/domain.c        |  135 ++++++++++++++++++++++++++++---------
 include/linux/pm_domain.h          |   22 ++++++
 3 files changed, 156 insertions(+), 34 deletions(-)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -62,8 +62,14 @@ struct gpd_link {
 	struct list_head slave_node;
 };
 
+struct gpd_dev_ops {
+	int (*start)(struct device *dev);
+	int (*stop)(struct device *dev);
+};
+
 struct generic_pm_domain_data {
 	struct pm_domain_data base;
+	struct gpd_dev_ops ops;
 	bool need_restore;
 };
 
@@ -72,6 +78,11 @@ static inline struct generic_pm_domain_d
 	return container_of(pdd, struct generic_pm_domain_data, base);
 }
 
+static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
+{
+	return to_gpd_data(dev->power.subsys_data->domain_data);
+}
+
 #ifdef CONFIG_PM_GENERIC_DOMAINS
 extern int pm_genpd_add_device(struct generic_pm_domain *genpd,
 			       struct device *dev);
@@ -81,6 +92,8 @@ extern int pm_genpd_add_subdomain(struct
 				  struct generic_pm_domain *new_subdomain);
 extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
 				     struct generic_pm_domain *target);
+extern int pm_genpd_add_callbacks(struct device *dev, struct gpd_dev_ops *ops);
+extern int pm_genpd_remove_callbacks(struct device *dev);
 extern void pm_genpd_init(struct generic_pm_domain *genpd,
 			  struct dev_power_governor *gov, bool is_off);
 extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
@@ -105,6 +118,15 @@ static inline int pm_genpd_remove_subdom
 {
 	return -ENOSYS;
 }
+static inline int pm_genpd_add_callbacks(struct device *dev,
+					 struct gpd_dev_ops *ops)
+{
+	return -ENOSYS;
+}
+static inline int pm_genpd_remove_callbacks(struct device *dev)
+{
+	return -ENOSYS;
+}
 static inline void pm_genpd_init(struct generic_pm_domain *genpd,
 				 struct dev_power_governor *gov, bool is_off) {}
 static inline int pm_genpd_poweron(struct generic_pm_domain *genpd)
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -29,6 +29,36 @@ static struct generic_pm_domain *dev_to_
 	return pd_to_genpd(dev->pm_domain);
 }
 
+static int genpd_stop_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	int (*stop)(struct device *dev);
+
+	stop = genpd->stop_device;
+	if (stop)
+		return stop(dev);
+
+	stop = dev_gpd_data(dev)->ops.stop;
+	if (stop)
+		return stop(dev);
+
+	return 0;
+}
+
+static int genpd_start_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	int (*start)(struct device *dev);
+
+	start = genpd->start_device;
+	if (start)
+		return start(dev);
+
+	start = dev_gpd_data(dev)->ops.start;
+	if (start)
+		return start(dev);
+
+	return 0;
+}
+
 static bool genpd_sd_counter_dec(struct generic_pm_domain *genpd)
 {
 	bool ret = false;
@@ -199,13 +229,9 @@ static int __pm_genpd_save_device(struct
 	mutex_unlock(&genpd->lock);
 
 	if (drv && drv->pm && drv->pm->runtime_suspend) {
-		if (genpd->start_device)
-			genpd->start_device(dev);
-
+		genpd_start_dev(genpd, dev);
 		ret = drv->pm->runtime_suspend(dev);
-
-		if (genpd->stop_device)
-			genpd->stop_device(dev);
+		genpd_stop_dev(genpd, dev);
 	}
 
 	mutex_lock(&genpd->lock);
@@ -235,13 +261,9 @@ static void __pm_genpd_restore_device(st
 	mutex_unlock(&genpd->lock);
 
 	if (drv && drv->pm && drv->pm->runtime_resume) {
-		if (genpd->start_device)
-			genpd->start_device(dev);
-
+		genpd_start_dev(genpd, dev);
 		drv->pm->runtime_resume(dev);
-
-		if (genpd->stop_device)
-			genpd->stop_device(dev);
+		genpd_stop_dev(genpd, dev);
 	}
 
 	mutex_lock(&genpd->lock);
@@ -413,6 +435,7 @@ static void genpd_power_off_work_fn(stru
 static int pm_genpd_runtime_suspend(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
+	int ret;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
@@ -422,11 +445,9 @@ static int pm_genpd_runtime_suspend(stru
 
 	might_sleep_if(!genpd->dev_irq_safe);
 
-	if (genpd->stop_device) {
-		int ret = genpd->stop_device(dev);
-		if (ret)
-			return ret;
-	}
+	ret = genpd_stop_dev(genpd, dev);
+	if (ret)
+		return ret;
 
 	/*
 	 * If power.irq_safe is set, this routine will be run with interrupts
@@ -502,8 +523,7 @@ static int pm_genpd_runtime_resume(struc
 	mutex_unlock(&genpd->lock);
 
  out:
-	if (genpd->start_device)
-		genpd->start_device(dev);
+	genpd_start_dev(genpd, dev);
 
 	return 0;
 }
@@ -646,7 +666,7 @@ static int pm_genpd_prepare(struct devic
 	/*
 	 * The PM domain must be in the GPD_STATE_ACTIVE state at this point,
 	 * so pm_genpd_poweron() will return immediately, but if the device
-	 * is suspended (e.g. it's been stopped by .stop_device()), we need
+	 * is suspended (e.g. it's been stopped by genpd_stop_dev()), we need
 	 * to make it operational.
 	 */
 	pm_runtime_resume(dev);
@@ -718,8 +738,7 @@ static int pm_genpd_suspend_noirq(struct
 	    && genpd->active_wakeup && genpd->active_wakeup(dev))
 		return 0;
 
-	if (genpd->stop_device)
-		genpd->stop_device(dev);
+	genpd_stop_dev(genpd, dev);
 
 	/*
 	 * Since all of the "noirq" callbacks are executed sequentially, it is
@@ -761,8 +780,7 @@ static int pm_genpd_resume_noirq(struct
 	 */
 	pm_genpd_poweron(genpd);
 	genpd->suspended_count--;
-	if (genpd->start_device)
-		genpd->start_device(dev);
+	genpd_start_dev(genpd, dev);
 
 	return pm_generic_resume_noirq(dev);
 }
@@ -836,8 +854,7 @@ static int pm_genpd_freeze_noirq(struct
 	if (ret)
 		return ret;
 
-	if (genpd->stop_device)
-		genpd->stop_device(dev);
+	genpd_stop_dev(genpd, dev);
 
 	return 0;
 }
@@ -864,8 +881,7 @@ static int pm_genpd_thaw_noirq(struct de
 	if (genpd->suspend_power_off)
 		return 0;
 
-	if (genpd->start_device)
-		genpd->start_device(dev);
+	genpd_start_dev(genpd, dev);
 
 	return pm_generic_thaw_noirq(dev);
 }
@@ -942,8 +958,7 @@ static int pm_genpd_dev_poweroff_noirq(s
 	    && genpd->active_wakeup && genpd->active_wakeup(dev))
 		return 0;
 
-	if (genpd->stop_device)
-		genpd->stop_device(dev);
+	genpd_stop_dev(genpd, dev);
 
 	/*
 	 * Since all of the "noirq" callbacks are executed sequentially, it is
@@ -993,8 +1008,7 @@ static int pm_genpd_restore_noirq(struct
 
 	pm_genpd_poweron(genpd);
 	genpd->suspended_count--;
-	if (genpd->start_device)
-		genpd->start_device(dev);
+	genpd_start_dev(genpd, dev);
 
 	return pm_generic_restore_noirq(dev);
 }
@@ -1280,6 +1294,63 @@ int pm_genpd_remove_subdomain(struct gen
 }
 
 /**
+ * pm_genpd_add_callbacks - Add PM domain callbacks to a given device.
+ * @dev: Device to add the callbacks to.
+ * @ops: Set of callbacks to add.
+ */
+int pm_genpd_add_callbacks(struct device *dev, struct gpd_dev_ops *ops)
+{
+	struct generic_pm_domain_data *gpd_data;
+	int ret = 0;
+
+	if (!(dev && dev->power.subsys_data && ops))
+		return -EINVAL;
+
+	pm_runtime_disable(dev);
+	device_pm_lock();
+
+	gpd_data = dev_gpd_data(dev);
+	if (gpd_data)
+		gpd_data->ops = *ops;
+	else
+		ret = -EINVAL;
+
+	device_pm_unlock();
+	pm_runtime_enable(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pm_genpd_add_callbacks);
+
+/**
+ * pm_genpd_remove_callbacks - Remove PM domain callbacks from a given device.
+ * @dev: Device to remove the callbacks from.
+ */
+int pm_genpd_remove_callbacks(struct device *dev)
+{
+	struct generic_pm_domain_data *gpd_data;
+	int ret = 0;
+
+	if (!(dev && dev->power.subsys_data))
+		return -EINVAL;
+
+	pm_runtime_disable(dev);
+	device_pm_lock();
+
+	gpd_data = dev_gpd_data(dev);
+	if (gpd_data)
+		gpd_data->ops = (struct gpd_dev_ops){ 0 };
+	else
+		ret = -EINVAL;
+
+	device_pm_unlock();
+	pm_runtime_enable(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pm_genpd_remove_callbacks);
+
+/**
  * 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).
Index: linux/arch/arm/mach-shmobile/pm-sh7372.c
===================================================================
--- linux.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux/arch/arm/mach-shmobile/pm-sh7372.c
@@ -163,13 +163,42 @@ struct dev_power_governor sh7372_always_
 	.power_down_ok = sh7372_power_down_forbidden,
 };
 
+static int sh7372_stop_dev(struct device *dev)
+{
+	int (*stop)(struct device *dev);
+
+	stop = dev_gpd_data(dev)->ops.stop;
+	if (stop) {
+		int ret = stop(dev);
+		if (ret)
+			return ret;
+	}
+	return pm_clk_suspend(dev);
+}
+
+static int sh7372_start_dev(struct device *dev)
+{
+	int (*start)(struct device *dev);
+	int ret;
+
+	ret = pm_clk_resume(dev);
+	if (ret)
+		return ret;
+
+	start = dev_gpd_data(dev)->ops.start;
+	if (start)
+		ret = start(dev);
+
+	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, sh7372_pd->gov, false);
-	genpd->stop_device = pm_clk_suspend;
-	genpd->start_device = pm_clk_resume;
+	genpd->stop_device = sh7372_stop_dev;
+	genpd->start_device = sh7372_start_dev;
 	genpd->dev_irq_safe = true;
 	genpd->active_wakeup = pd_active_wakeup;
 	genpd->power_off = pd_power_down;


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 2/7] PM / Domains: Make it possible to use per-device .active_wakeup()
  2011-11-07  0:01 [PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
  2011-11-07  0:06 ` [PATCH 1/7] PM / Domains: Make it possible to use per-device start/stop routines Rafael J. Wysocki
@ 2011-11-07  0:06 ` Rafael J. Wysocki
  2011-11-08 10:27   ` Guennadi Liakhovetski
  2011-11-07  0:07 ` [PATCH 3/7] PM / Domains: Introduce "save/restore state" device callbacks Rafael J. Wysocki
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-07  0:06 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

The current generic PM domains code requires that the same
.active_wakeup() device callback routine be used for all devices in
the given domain, which is inflexible and may not cover some specific
use cases.  For this reason, make it possible to use device specific
.active_wakeup() callback routines by adding a corresponding callback
pointer to struct generic_pm_domain_data.  To reduce code duplication
use struct gpd_dev_ops to represent PM domain device callbacks as
well as device-specific ones and add a macro for defining routines
that will execute those callbacks.

Modify the shmobile's power domains code to allow drivers to use
their own .active_wakeup() callback routines.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/mach-shmobile/pm-sh7372.c |   11 ++++---
 drivers/base/power/domain.c        |   54 ++++++++++++++++++-------------------
 include/linux/pm_domain.h          |   15 ++++------
 3 files changed, 41 insertions(+), 39 deletions(-)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -23,6 +23,12 @@ struct dev_power_governor {
 	bool (*power_down_ok)(struct dev_pm_domain *domain);
 };
 
+struct gpd_dev_ops {
+	int (*start)(struct device *dev);
+	int (*stop)(struct device *dev);
+	bool (*active_wakeup)(struct device *dev);
+};
+
 struct generic_pm_domain {
 	struct dev_pm_domain domain;	/* PM domain operations */
 	struct list_head gpd_list_node;	/* Node in the global PM domains list */
@@ -45,9 +51,7 @@ struct generic_pm_domain {
 	bool dev_irq_safe;	/* Device callbacks are IRQ-safe */
 	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);
-	bool (*active_wakeup)(struct device *dev);
+	struct gpd_dev_ops dev_ops;
 };
 
 static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
@@ -62,11 +66,6 @@ struct gpd_link {
 	struct list_head slave_node;
 };
 
-struct gpd_dev_ops {
-	int (*start)(struct device *dev);
-	int (*stop)(struct device *dev);
-};
-
 struct generic_pm_domain_data {
 	struct pm_domain_data base;
 	struct gpd_dev_ops ops;
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -16,6 +16,22 @@
 #include <linux/sched.h>
 #include <linux/suspend.h>
 
+#define GENPD_DEV_CALLBACK(genpd, type, callback, dev)		\
+({								\
+	type (*__routine)(struct device *__d); 			\
+	type __ret = (type)0;					\
+								\
+	__routine = genpd->dev_ops.callback; 			\
+	if (__routine) {					\
+		__ret = __routine(dev); 			\
+	} else {						\
+		__routine = dev_gpd_data(dev)->ops.callback;	\
+		if (__routine) 					\
+			__ret = __routine(dev);			\
+	}							\
+	__ret;							\
+})
+
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
 
@@ -31,32 +47,12 @@ static struct generic_pm_domain *dev_to_
 
 static int genpd_stop_dev(struct generic_pm_domain *genpd, struct device *dev)
 {
-	int (*stop)(struct device *dev);
-
-	stop = genpd->stop_device;
-	if (stop)
-		return stop(dev);
-
-	stop = dev_gpd_data(dev)->ops.stop;
-	if (stop)
-		return stop(dev);
-
-	return 0;
+	return GENPD_DEV_CALLBACK(genpd, int, stop, dev);
 }
 
 static int genpd_start_dev(struct generic_pm_domain *genpd, struct device *dev)
 {
-	int (*start)(struct device *dev);
-
-	start = genpd->start_device;
-	if (start)
-		return start(dev);
-
-	start = dev_gpd_data(dev)->ops.start;
-	if (start)
-		return start(dev);
-
-	return 0;
+	return GENPD_DEV_CALLBACK(genpd, int, start, dev);
 }
 
 static bool genpd_sd_counter_dec(struct generic_pm_domain *genpd)
@@ -554,6 +550,12 @@ static inline void genpd_power_off_work_
 
 #ifdef CONFIG_PM_SLEEP
 
+static bool genpd_dev_active_wakeup(struct generic_pm_domain *genpd,
+				    struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, bool, active_wakeup, dev);
+}
+
 /**
  * pm_genpd_sync_poweroff - Synchronously power off a PM domain and its masters.
  * @genpd: PM domain to power off, if possible.
@@ -610,7 +612,7 @@ static bool resume_needed(struct device
 	if (!device_can_wakeup(dev))
 		return false;
 
-	active_wakeup = genpd->active_wakeup && genpd->active_wakeup(dev);
+	active_wakeup = genpd_dev_active_wakeup(genpd, dev);
 	return device_may_wakeup(dev) ? active_wakeup : !active_wakeup;
 }
 
@@ -734,8 +736,7 @@ static int pm_genpd_suspend_noirq(struct
 	if (ret)
 		return ret;
 
-	if (dev->power.wakeup_path
-	    && genpd->active_wakeup && genpd->active_wakeup(dev))
+	if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
 		return 0;
 
 	genpd_stop_dev(genpd, dev);
@@ -954,8 +955,7 @@ static int pm_genpd_dev_poweroff_noirq(s
 	if (ret)
 		return ret;
 
-	if (dev->power.wakeup_path
-	    && genpd->active_wakeup && genpd->active_wakeup(dev))
+	if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
 		return 0;
 
 	genpd_stop_dev(genpd, dev);
Index: linux/arch/arm/mach-shmobile/pm-sh7372.c
===================================================================
--- linux.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux/arch/arm/mach-shmobile/pm-sh7372.c
@@ -151,7 +151,10 @@ static void sh7372_a4r_suspend(void)
 
 static bool pd_active_wakeup(struct device *dev)
 {
-	return true;
+	bool (*active_wakeup)(struct device *dev);
+
+	active_wakeup = dev_gpd_data(dev)->ops.active_wakeup;
+	return active_wakeup ? active_wakeup(dev) : true;
 }
 
 static bool sh7372_power_down_forbidden(struct dev_pm_domain *domain)
@@ -197,10 +200,10 @@ void sh7372_init_pm_domain(struct sh7372
 	struct generic_pm_domain *genpd = &sh7372_pd->genpd;
 
 	pm_genpd_init(genpd, sh7372_pd->gov, false);
-	genpd->stop_device = sh7372_stop_dev;
-	genpd->start_device = sh7372_start_dev;
+	genpd->dev_ops.stop = sh7372_stop_dev;
+	genpd->dev_ops.start = sh7372_start_dev;
+	genpd->dev_ops.active_wakeup = pd_active_wakeup;
 	genpd->dev_irq_safe = true;
-	genpd->active_wakeup = pd_active_wakeup;
 	genpd->power_off = pd_power_down;
 	genpd->power_on = pd_power_up;
 	genpd->power_on(&sh7372_pd->genpd);


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 3/7] PM / Domains: Introduce "save/restore state" device callbacks
  2011-11-07  0:01 [PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
  2011-11-07  0:06 ` [PATCH 1/7] PM / Domains: Make it possible to use per-device start/stop routines Rafael J. Wysocki
  2011-11-07  0:06 ` [PATCH 2/7] PM / Domains: Make it possible to use per-device .active_wakeup() Rafael J. Wysocki
@ 2011-11-07  0:07 ` Rafael J. Wysocki
  2011-11-07  0:08 ` [PATCH 4/7] PM / Domains: Rework system suspend callback routines Rafael J. Wysocki
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-07  0:07 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

The current PM domains code uses device drivers' .runtime_suspend()
and .runtime_resume() callbacks as the "save device state" and
"restore device state" operations, which may not be appropriate in
general, because it forces drivers to assume that they always will
be used with generic PM domains.  However, in theory, the same
hardware may be used in devices that don't belong to any PM
domain, in which case it would be necessary to add "fake" PM
domains to satisfy the above assumption.  It also may be located in
a PM domain that's not handled with the help of the generic code.

To allow device drivers that may be used along with the generic PM
domains code of more flexibility, introduce new device callbacks,
.save_state() and .restore_state(), that can be supplied by the
drivers in addition to their "standard" runtime PM callbacks.  This
will allow the drivers to be designed to work with generic PM domains
as well as without them.

For backwards compatibility, introduce default .save_state() and
.restore_state() callback routines for PM domains that will execute
a device driver's .runtime_suspend() and .runtime_resume() callbacks,
respectively, for the given device if the driver doesn't provide its
own implementations of .save_state() and .restore_state().

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |   68 ++++++++++++++++++++++++++++++++++++--------
 include/linux/pm_domain.h   |    2 +
 2 files changed, 58 insertions(+), 12 deletions(-)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -26,6 +26,8 @@ struct dev_power_governor {
 struct gpd_dev_ops {
 	int (*start)(struct device *dev);
 	int (*stop)(struct device *dev);
+	int (*save_state)(struct device *dev);
+	int (*restore_state)(struct device *dev);
 	bool (*active_wakeup)(struct device *dev);
 };
 
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -55,6 +55,16 @@ static int genpd_start_dev(struct generi
 	return GENPD_DEV_CALLBACK(genpd, int, start, dev);
 }
 
+static int genpd_save_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, save_state, dev);
+}
+
+static int genpd_restore_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, restore_state, dev);
+}
+
 static bool genpd_sd_counter_dec(struct generic_pm_domain *genpd)
 {
 	bool ret = false;
@@ -216,7 +226,6 @@ static int __pm_genpd_save_device(struct
 {
 	struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
 	struct device *dev = pdd->dev;
-	struct device_driver *drv = dev->driver;
 	int ret = 0;
 
 	if (gpd_data->need_restore)
@@ -224,11 +233,9 @@ static int __pm_genpd_save_device(struct
 
 	mutex_unlock(&genpd->lock);
 
-	if (drv && drv->pm && drv->pm->runtime_suspend) {
-		genpd_start_dev(genpd, dev);
-		ret = drv->pm->runtime_suspend(dev);
-		genpd_stop_dev(genpd, dev);
-	}
+	genpd_start_dev(genpd, dev);
+	ret = genpd_save_dev(genpd, dev);
+	genpd_stop_dev(genpd, dev);
 
 	mutex_lock(&genpd->lock);
 
@@ -249,18 +256,15 @@ static void __pm_genpd_restore_device(st
 {
 	struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
 	struct device *dev = pdd->dev;
-	struct device_driver *drv = dev->driver;
 
 	if (!gpd_data->need_restore)
 		return;
 
 	mutex_unlock(&genpd->lock);
 
-	if (drv && drv->pm && drv->pm->runtime_resume) {
-		genpd_start_dev(genpd, dev);
-		drv->pm->runtime_resume(dev);
-		genpd_stop_dev(genpd, dev);
-	}
+	genpd_start_dev(genpd, dev);
+	genpd_restore_dev(genpd, dev);
+	genpd_stop_dev(genpd, dev);
 
 	mutex_lock(&genpd->lock);
 
@@ -1351,6 +1355,44 @@ int pm_genpd_remove_callbacks(struct dev
 EXPORT_SYMBOL_GPL(pm_genpd_remove_callbacks);
 
 /**
+ * pm_genpd_default_save_state - Default "save device state" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_save_state(struct device *dev)
+{
+	int (*cb)(struct device *__dev);
+	struct device_driver *drv = dev->driver;
+
+	cb = dev_gpd_data(dev)->ops.save_state;
+	if (cb)
+		return cb(dev);
+
+	if (drv && drv->pm && drv->pm->runtime_suspend)
+		return drv->pm->runtime_suspend(dev);
+
+	return 0;
+}
+
+/**
+ * pm_genpd_default_restore_state - Default PM domians "restore device state".
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_restore_state(struct device *dev)
+{
+	int (*cb)(struct device *__dev);
+	struct device_driver *drv = dev->driver;
+
+	cb = dev_gpd_data(dev)->ops.restore_state;
+	if (cb)
+		return cb(dev);
+
+	if (drv && drv->pm && drv->pm->runtime_resume)
+		return drv->pm->runtime_resume(dev);
+
+	return 0;
+}
+
+/**
  * 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).
@@ -1393,6 +1435,8 @@ void pm_genpd_init(struct generic_pm_dom
 	genpd->domain.ops.restore_noirq = pm_genpd_restore_noirq;
 	genpd->domain.ops.restore = pm_genpd_restore;
 	genpd->domain.ops.complete = pm_genpd_complete;
+	genpd->dev_ops.save_state = pm_genpd_default_save_state;
+	genpd->dev_ops.restore_state = pm_genpd_default_restore_state;
 	mutex_lock(&gpd_list_lock);
 	list_add(&genpd->gpd_list_node, &gpd_list);
 	mutex_unlock(&gpd_list_lock);


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 4/7] PM / Domains: Rework system suspend callback routines
  2011-11-07  0:01 [PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2011-11-07  0:07 ` [PATCH 3/7] PM / Domains: Introduce "save/restore state" device callbacks Rafael J. Wysocki
@ 2011-11-07  0:08 ` Rafael J. Wysocki
  2012-02-17 19:29   ` Pavel Machek
  2011-11-07  0:08 ` [PATCH 5/7] PM / Domains: Add device stop governor function (v3) Rafael J. Wysocki
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-07  0:08 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

The current generic PM domains code attempts to use the generic
system suspend operations along with the domains' device stop/start
routines, which requires device drivers to assume that their
system suspend/resume (and hibernation/restore) callbacks will always
be used with generic PM domains.  However, in theory, the same
hardware may be used in devices that don't belong to any PM domain,
in which case it would be necessary to add "fake" PM domains to
satisfy the above assumption.  Also, the domain the hardware belongs
to may not be handled with the help of the generic code.

To allow device drivers that may be used along with the generic PM
domains code of more flexibility, add new device callbacks, .freeze(),
.freeze_late(), .thaw_early() and .thaw(), that can be supplied by
the drivers in addition to their "standard" system suspend and
hibernation callbacks.  These new callbacks, if defined, will be used
by the generic PM domains code for the handling of system suspend and
hibernation instead of the "standard" ones.  This will allow drivers
to be designed to work with generic PM domains as well as without
them.

For backwards compatibility, introduce default .freeze(), .thaw(),
.freeze_late() and .thaw_early() callback routines for PM domains that
will execute pm_generic_suspend(), pm_generic_resume(),
pm_generic_suspend_noirq() and pm_generic_resume_noirq(),
respectively, for the given device if its driver doesn't provide its
own implementations of .freeze(), .thaw(), .freeze_late() and
.thaw_early() (this works slightly differently from the current code,
but its existing users don't care anyway).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |  189 +++++++++++++++++++-------------------------
 include/linux/pm_domain.h   |    4 
 2 files changed, 87 insertions(+), 106 deletions(-)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -28,6 +28,10 @@ struct gpd_dev_ops {
 	int (*stop)(struct device *dev);
 	int (*save_state)(struct device *dev);
 	int (*restore_state)(struct device *dev);
+	int (*freeze)(struct device *dev);
+	int (*freeze_late)(struct device *dev);
+	int (*thaw_early)(struct device *dev);
+	int (*thaw)(struct device *dev);
 	bool (*active_wakeup)(struct device *dev);
 };
 
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -560,6 +560,26 @@ static bool genpd_dev_active_wakeup(stru
 	return GENPD_DEV_CALLBACK(genpd, bool, active_wakeup, dev);
 }
 
+static int genpd_freeze_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, freeze, dev);
+}
+
+static int genpd_freeze_late(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, freeze_late, dev);
+}
+
+static int genpd_thaw_early(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, thaw_early, dev);
+}
+
+static int genpd_thaw_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, thaw, dev);
+}
+
 /**
  * pm_genpd_sync_poweroff - Synchronously power off a PM domain and its masters.
  * @genpd: PM domain to power off, if possible.
@@ -694,27 +714,6 @@ static int pm_genpd_prepare(struct devic
 }
 
 /**
- * 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.
  *
@@ -736,7 +735,7 @@ static int pm_genpd_suspend_noirq(struct
 	if (genpd->suspend_power_off)
 		return 0;
 
-	ret = pm_generic_suspend_noirq(dev);
+	ret = genpd_freeze_late(genpd, dev);
 	if (ret)
 		return ret;
 
@@ -787,28 +786,7 @@ static int pm_genpd_resume_noirq(struct
 	genpd->suspended_count--;
 	genpd_start_dev(genpd, dev);
 
-	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);
+	return genpd_thaw_early(genpd, dev);
 }
 
 /**
@@ -829,14 +807,14 @@ static int pm_genpd_freeze(struct device
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	return genpd->suspend_power_off ? 0 : pm_generic_freeze(dev);
+	return genpd->suspend_power_off ? 0 : genpd_freeze_dev(genpd, 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
+ * 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.
@@ -855,7 +833,7 @@ static int pm_genpd_freeze_noirq(struct
 	if (genpd->suspend_power_off)
 		return 0;
 
-	ret = pm_generic_freeze_noirq(dev);
+	ret = genpd_freeze_late(genpd, dev);
 	if (ret)
 		return ret;
 
@@ -868,7 +846,7 @@ static int pm_genpd_freeze_noirq(struct
  * 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
+ * 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.
@@ -888,7 +866,7 @@ static int pm_genpd_thaw_noirq(struct de
 
 	genpd_start_dev(genpd, dev);
 
-	return pm_generic_thaw_noirq(dev);
+	return genpd_thaw_early(genpd, dev);
 }
 
 /**
@@ -909,28 +887,7 @@ static int pm_genpd_thaw(struct device *
 	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);
+	return genpd->suspend_power_off ? 0 : genpd_thaw_dev(genpd, dev);
 }
 
 /**
@@ -944,7 +901,6 @@ static int pm_genpd_dev_poweroff(struct
 static int pm_genpd_dev_poweroff_noirq(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
-	int ret;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
@@ -955,10 +911,6 @@ static int pm_genpd_dev_poweroff_noirq(s
 	if (genpd->suspend_power_off)
 		return 0;
 
-	ret = pm_generic_poweroff_noirq(dev);
-	if (ret)
-		return ret;
-
 	if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
 		return 0;
 
@@ -1014,28 +966,7 @@ static int pm_genpd_restore_noirq(struct
 	genpd->suspended_count--;
 	genpd_start_dev(genpd, 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);
+	return genpd_thaw_early(genpd, dev);
 }
 
 /**
@@ -1077,18 +1008,14 @@ static void pm_genpd_complete(struct dev
 #else
 
 #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_dev_poweroff_noirq	NULL
-#define pm_genpd_dev_poweroff		NULL
 #define pm_genpd_restore_noirq		NULL
-#define pm_genpd_restore		NULL
 #define pm_genpd_complete		NULL
 
 #endif /* CONFIG_PM_SLEEP */
@@ -1354,6 +1281,8 @@ int pm_genpd_remove_callbacks(struct dev
 }
 EXPORT_SYMBOL_GPL(pm_genpd_remove_callbacks);
 
+/* Default device callbacks for generic PM domains. */
+
 /**
  * pm_genpd_default_save_state - Default "save device state" for PM domians.
  * @dev: Device to handle.
@@ -1393,6 +1322,50 @@ static int pm_genpd_default_restore_stat
 }
 
 /**
+ * pm_genpd_default_freeze - Default "device freeze" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_freeze(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.freeze;
+
+	return cb ? cb(dev) : pm_generic_suspend(dev);
+}
+
+/**
+ * pm_genpd_default_freeze_late - Default "late device freeze" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_freeze_late(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.freeze_late;
+
+	return cb ? cb(dev) : pm_generic_suspend_noirq(dev);
+}
+
+/**
+ * pm_genpd_default_thaw_early - Default "early device thaw" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_thaw_early(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.thaw_early;
+
+	return cb ? cb(dev) : pm_generic_resume_noirq(dev);
+}
+
+/**
+ * pm_genpd_default_thaw - Default "device thaw" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_thaw(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.thaw;
+
+	return cb ? cb(dev) : pm_generic_resume(dev);
+}
+
+/**
  * 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).
@@ -1422,21 +1395,25 @@ void pm_genpd_init(struct generic_pm_dom
 	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 = pm_genpd_freeze;
 	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.resume = pm_genpd_thaw;
 	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 = pm_genpd_freeze;
 	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.restore = pm_genpd_thaw;
 	genpd->domain.ops.complete = pm_genpd_complete;
 	genpd->dev_ops.save_state = pm_genpd_default_save_state;
 	genpd->dev_ops.restore_state = pm_genpd_default_restore_state;
+	genpd->dev_ops.freeze = pm_genpd_default_freeze;
+	genpd->dev_ops.freeze_late = pm_genpd_default_freeze_late;
+	genpd->dev_ops.thaw_early = pm_genpd_default_thaw_early;
+	genpd->dev_ops.thaw = pm_genpd_default_thaw;
 	mutex_lock(&gpd_list_lock);
 	list_add(&genpd->gpd_list_node, &gpd_list);
 	mutex_unlock(&gpd_list_lock);


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 5/7] PM / Domains: Add device stop governor function (v3)
  2011-11-07  0:01 [PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2011-11-07  0:08 ` [PATCH 4/7] PM / Domains: Rework system suspend callback routines Rafael J. Wysocki
@ 2011-11-07  0:08 ` Rafael J. Wysocki
  2011-11-07  0:09 ` [PATCH 6/7] PM / Domains: Add default power off " Rafael J. Wysocki
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-07  0:08 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

Add a function deciding whether or not devices should be stopped in
pm_genpd_runtime_suspend() depending on their PM QoS values.  Make
it possible to add information used by this function to device
objects.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/mach-shmobile/pm-sh7372.c   |    4 +-
 drivers/base/power/Makefile          |    2 -
 drivers/base/power/domain.c          |   40 ++++++++++++++++------
 drivers/base/power/domain_governor.c |   39 ++++++++++++++++++++++
 include/linux/pm_domain.h            |   61 ++++++++++++++++++++++++++++++-----
 5 files changed, 125 insertions(+), 21 deletions(-)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -21,6 +21,7 @@ enum gpd_status {
 
 struct dev_power_governor {
 	bool (*power_down_ok)(struct dev_pm_domain *domain);
+	bool (*stop_ok)(struct device *dev);
 };
 
 struct gpd_dev_ops {
@@ -72,9 +73,14 @@ struct gpd_link {
 	struct list_head slave_node;
 };
 
+struct gpd_timing_data {
+	s64 break_even_ns;
+};
+
 struct generic_pm_domain_data {
 	struct pm_domain_data base;
 	struct gpd_dev_ops ops;
+	struct gpd_timing_data td;
 	bool need_restore;
 };
 
@@ -89,20 +95,48 @@ static inline struct generic_pm_domain_d
 }
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS
-extern int pm_genpd_add_device(struct generic_pm_domain *genpd,
-			       struct device *dev);
+extern struct dev_power_governor simple_qos_governor;
+
+extern struct generic_pm_domain *dev_to_genpd(struct device *dev);
+extern int __pm_genpd_add_device(struct generic_pm_domain *genpd,
+				 struct device *dev,
+				 struct gpd_timing_data *td);
+
+static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
+				      struct device *dev)
+{
+	return __pm_genpd_add_device(genpd, dev, NULL);
+}
+
 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 int pm_genpd_add_callbacks(struct device *dev, struct gpd_dev_ops *ops);
-extern int pm_genpd_remove_callbacks(struct device *dev);
+extern int pm_genpd_add_callbacks(struct device *dev,
+				  struct gpd_dev_ops *ops,
+				  struct gpd_timing_data *td);
+extern int __pm_genpd_remove_callbacks(struct device *dev, bool clear_td);
 extern void pm_genpd_init(struct generic_pm_domain *genpd,
 			  struct dev_power_governor *gov, bool is_off);
+
 extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
+
+extern bool default_stop_ok(struct device *dev);
+
 #else
+
+static inline struct generic_pm_domain *dev_to_genpd(struct device *dev)
+{
+	return ERR_PTR(-ENOSYS);
+}
+static inline int __pm_genpd_add_device(struct generic_pm_domain *genpd,
+					struct device *dev,
+					struct gpd_timing_data *td)
+{
+	return -ENOSYS;
+}
 static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
 				      struct device *dev)
 {
@@ -124,22 +158,33 @@ static inline int pm_genpd_remove_subdom
 	return -ENOSYS;
 }
 static inline int pm_genpd_add_callbacks(struct device *dev,
-					 struct gpd_dev_ops *ops)
+					 struct gpd_dev_ops *ops,
+					 struct gpd_timing_data *td)
 {
 	return -ENOSYS;
 }
-static inline int pm_genpd_remove_callbacks(struct device *dev)
+static inline int __pm_genpd_remove_callbacks(struct device *dev, bool clear_td)
 {
 	return -ENOSYS;
 }
-static inline void pm_genpd_init(struct generic_pm_domain *genpd,
-				 struct dev_power_governor *gov, bool is_off) {}
+static inline void pm_genpd_init(struct generic_pm_domain *genpd, bool is_off)
+{
+}
 static inline int pm_genpd_poweron(struct generic_pm_domain *genpd)
 {
 	return -ENOSYS;
 }
+static inline bool default_stop_ok(struct device *dev)
+{
+	return false;
+}
 #endif
 
+static inline int pm_genpd_remove_callbacks(struct device *dev)
+{
+	return __pm_genpd_remove_callbacks(dev, true);
+}
+
 #ifdef CONFIG_PM_GENERIC_DOMAINS_RUNTIME
 extern void genpd_queue_power_off_work(struct generic_pm_domain *genpd);
 extern void pm_genpd_poweroff_unused(void);
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -37,7 +37,7 @@ static DEFINE_MUTEX(gpd_list_lock);
 
 #ifdef CONFIG_PM
 
-static struct generic_pm_domain *dev_to_genpd(struct device *dev)
+struct generic_pm_domain *dev_to_genpd(struct device *dev)
 {
 	if (IS_ERR_OR_NULL(dev->pm_domain))
 		return ERR_PTR(-EINVAL);
@@ -435,6 +435,7 @@ static void genpd_power_off_work_fn(stru
 static int pm_genpd_runtime_suspend(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
+	bool (*stop_ok)(struct device *__dev);
 	int ret;
 
 	dev_dbg(dev, "%s()\n", __func__);
@@ -445,6 +446,10 @@ static int pm_genpd_runtime_suspend(stru
 
 	might_sleep_if(!genpd->dev_irq_safe);
 
+	stop_ok = genpd->gov ? genpd->gov->stop_ok : NULL;
+	if (stop_ok && !stop_ok(dev))
+		return -EBUSY;
+
 	ret = genpd_stop_dev(genpd, dev);
 	if (ret)
 		return ret;
@@ -1021,11 +1026,13 @@ static void pm_genpd_complete(struct dev
 #endif /* CONFIG_PM_SLEEP */
 
 /**
- * pm_genpd_add_device - Add a device to an I/O PM domain.
+ * __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.
+ * @td: Set of PM QoS timing parameters to attach to the device.
  */
-int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
+int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
+			  struct gpd_timing_data *td)
 {
 	struct generic_pm_domain_data *gpd_data;
 	struct pm_domain_data *pdd;
@@ -1068,6 +1075,8 @@ int pm_genpd_add_device(struct generic_p
 	gpd_data->base.dev = dev;
 	gpd_data->need_restore = false;
 	list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);
+	if (td)
+		gpd_data->td = *td;
 
  out:
 	genpd_release_lock(genpd);
@@ -1228,8 +1237,10 @@ int pm_genpd_remove_subdomain(struct gen
  * pm_genpd_add_callbacks - Add PM domain callbacks to a given device.
  * @dev: Device to add the callbacks to.
  * @ops: Set of callbacks to add.
+ * @td: Timing data to add to the device along with the callbacks (optional).
  */
-int pm_genpd_add_callbacks(struct device *dev, struct gpd_dev_ops *ops)
+int pm_genpd_add_callbacks(struct device *dev, struct gpd_dev_ops *ops,
+			   struct gpd_timing_data *td)
 {
 	struct generic_pm_domain_data *gpd_data;
 	int ret = 0;
@@ -1241,10 +1252,13 @@ int pm_genpd_add_callbacks(struct device
 	device_pm_lock();
 
 	gpd_data = dev_gpd_data(dev);
-	if (gpd_data)
+	if (gpd_data) {
 		gpd_data->ops = *ops;
-	else
+		if (td)
+			gpd_data->td = *td;
+	} else {
 		ret = -EINVAL;
+	}
 
 	device_pm_unlock();
 	pm_runtime_enable(dev);
@@ -1254,10 +1268,11 @@ int pm_genpd_add_callbacks(struct device
 EXPORT_SYMBOL_GPL(pm_genpd_add_callbacks);
 
 /**
- * pm_genpd_remove_callbacks - Remove PM domain callbacks from a given device.
+ * __pm_genpd_remove_callbacks - Remove PM domain callbacks from a given device.
  * @dev: Device to remove the callbacks from.
+ * @clear_td: If set, clear the device's timing data too.
  */
-int pm_genpd_remove_callbacks(struct device *dev)
+int __pm_genpd_remove_callbacks(struct device *dev, bool clear_td)
 {
 	struct generic_pm_domain_data *gpd_data;
 	int ret = 0;
@@ -1269,17 +1284,20 @@ int pm_genpd_remove_callbacks(struct dev
 	device_pm_lock();
 
 	gpd_data = dev_gpd_data(dev);
-	if (gpd_data)
+	if (gpd_data) {
 		gpd_data->ops = (struct gpd_dev_ops){ 0 };
-	else
+		if (clear_td)
+			gpd_data->td = (struct gpd_timing_data){ 0 };
+	} else {
 		ret = -EINVAL;
+	}
 
 	device_pm_unlock();
 	pm_runtime_enable(dev);
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(pm_genpd_remove_callbacks);
+EXPORT_SYMBOL_GPL(__pm_genpd_remove_callbacks);
 
 /* Default device callbacks for generic PM domains. */
 
Index: linux/drivers/base/power/Makefile
===================================================================
--- linux.orig/drivers/base/power/Makefile
+++ linux/drivers/base/power/Makefile
@@ -3,7 +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_PM_GENERIC_DOMAINS)	+=  domain.o domain_governor.o
 obj-$(CONFIG_HAVE_CLK)	+= clock_ops.o
 
 ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
Index: linux/drivers/base/power/domain_governor.c
===================================================================
--- /dev/null
+++ linux/drivers/base/power/domain_governor.c
@@ -0,0 +1,39 @@
+/*
+ * drivers/base/power/domain_governor.c - Governors for device PM domains.
+ *
+ * Copyright (C) 2011 Rafael J. Wysocki <rjw@sisk.pl>, Renesas Electronics Corp.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_qos.h>
+
+/**
+ * default_stop_ok - Default PM domain governor routine for stopping devices.
+ * @dev: Device to check.
+ */
+bool default_stop_ok(struct device *dev)
+{
+	s64 constraint_ns;
+	s32 constraint;
+
+	dev_dbg(dev, "%s()\n", __func__);
+
+	constraint = dev_pm_qos_read_value(dev);
+	if (constraint < 0) /* negative means "never stop" */
+		return false;
+	else if (constraint == 0) /* 0 means "don't care" */
+		return true;
+
+	constraint_ns = constraint;
+	constraint_ns *= NSEC_PER_USEC;
+
+	return constraint_ns > dev_gpd_data(dev)->td.break_even_ns;
+}
+
+struct dev_power_governor simple_qos_governor = {
+	.stop_ok = default_stop_ok,
+};
Index: linux/arch/arm/mach-shmobile/pm-sh7372.c
===================================================================
--- linux.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux/arch/arm/mach-shmobile/pm-sh7372.c
@@ -164,6 +164,7 @@ static bool sh7372_power_down_forbidden(
 
 struct dev_power_governor sh7372_always_on_gov = {
 	.power_down_ok = sh7372_power_down_forbidden,
+	.stop_ok = default_stop_ok,
 };
 
 static int sh7372_stop_dev(struct device *dev)
@@ -198,8 +199,9 @@ static int sh7372_start_dev(struct devic
 void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd)
 {
 	struct generic_pm_domain *genpd = &sh7372_pd->genpd;
+	struct dev_power_governor *gov = sh7372_pd->gov;
 
-	pm_genpd_init(genpd, sh7372_pd->gov, false);
+	pm_genpd_init(genpd, gov ? : &simple_qos_governor, false);
 	genpd->dev_ops.stop = sh7372_stop_dev;
 	genpd->dev_ops.start = sh7372_start_dev;
 	genpd->dev_ops.active_wakeup = pd_active_wakeup;


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 6/7] PM / Domains: Add default power off governor function (v3)
  2011-11-07  0:01 [PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
                   ` (4 preceding siblings ...)
  2011-11-07  0:08 ` [PATCH 5/7] PM / Domains: Add device stop governor function (v3) Rafael J. Wysocki
@ 2011-11-07  0:09 ` Rafael J. Wysocki
  2011-11-07  0:10 ` [PATCH 7/7] PM / Domains: Automatically update overoptimistic latency information Rafael J. Wysocki
  2011-11-14  0:22 ` [update][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
  7 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-07  0:09 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

Add a function deciding whether or not a given PM domain should
be powered off on the basis of the PM QoS constraints of devices
belonging to it.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c          |    2 
 drivers/base/power/domain_governor.c |  125 +++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h            |    9 ++
 3 files changed, 136 insertions(+)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -57,8 +57,13 @@ struct generic_pm_domain {
 	bool suspend_power_off;	/* Power status before system suspend */
 	bool dev_irq_safe;	/* Device callbacks are IRQ-safe */
 	int (*power_off)(struct generic_pm_domain *domain);
+	s64 power_off_latency_ns;
 	int (*power_on)(struct generic_pm_domain *domain);
+	s64 power_on_latency_ns;
 	struct gpd_dev_ops dev_ops;
+	s64 break_even_ns;	/* Power break even for the entire domain. */
+	s64 max_off_time_ns;	/* Maximum allowed "suspended" time. */
+	ktime_t power_off_time;
 };
 
 static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
@@ -74,6 +79,9 @@ struct gpd_link {
 };
 
 struct gpd_timing_data {
+	s64 start_latency_ns;
+	s64 save_state_latency_ns;
+	s64 restore_state_latency_ns;
 	s64 break_even_ns;
 };
 
@@ -81,6 +89,7 @@ struct generic_pm_domain_data {
 	struct pm_domain_data base;
 	struct gpd_dev_ops ops;
 	struct gpd_timing_data td;
+	ktime_t stop_time;
 	bool need_restore;
 };
 
Index: linux/drivers/base/power/domain_governor.c
===================================================================
--- linux.orig/drivers/base/power/domain_governor.c
+++ linux/drivers/base/power/domain_governor.c
@@ -10,6 +10,7 @@
 #include <linux/kernel.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_qos.h>
+#include <linux/hrtimer.h>
 
 /**
  * default_stop_ok - Default PM domain governor routine for stopping devices.
@@ -34,6 +35,130 @@ bool default_stop_ok(struct device *dev)
 	return constraint_ns > dev_gpd_data(dev)->td.break_even_ns;
 }
 
+/**
+ * default_power_down_ok - Default generic PM domain power off governor routine.
+ * @pd: PM domain to check.
+ *
+ * This routine must be executed under the PM domain's lock.
+ */
+static bool default_power_down_ok(struct dev_pm_domain *pd)
+{
+	struct generic_pm_domain *genpd = pd_to_genpd(pd);
+	struct gpd_link *link;
+	struct pm_domain_data *pdd;
+	s64 delta_ns, min_delta_ns;
+	s64 max_save_time_ns;
+	ktime_t time_now = ktime_get();
+
+	/*
+	 * Check if subdomains can be off for enough time.
+	 *
+	 * All subdomains have been powered off already at this point.
+	 */
+	delta_ns = genpd->power_off_latency_ns + genpd->power_on_latency_ns;
+	list_for_each_entry(link, &genpd->master_links, master_node) {
+		struct generic_pm_domain *sd = link->slave;
+		s64 sd_max_off_ns = sd->max_off_time_ns;
+
+		if (sd_max_off_ns < 0)
+			continue;
+
+		sd_max_off_ns -= ktime_to_ns(ktime_sub(time_now,
+						       sd->power_off_time));
+		/*
+		 * Check if the subdomain is allowed to be off long enough for
+		 * the current domain to turn off and on (that's how much time
+		 * it will have to wait worst case).
+		 */
+		if (sd_max_off_ns <= delta_ns)
+			return false;
+	}
+
+	/*
+	 * If the "domain power off" operation is aborted, it may be necessary
+	 * to wait for the slowest "save device state operation to complete.
+	 *
+	 * All devices in this domain have been stopped already at this point.
+	 */
+	max_save_time_ns = 0;
+	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
+		s64 save_time_ns;
+
+		if (!pdd->dev->driver)
+			continue;
+
+		save_time_ns = to_gpd_data(pdd)->td.save_state_latency_ns;
+		if (save_time_ns > max_save_time_ns)
+			max_save_time_ns = save_time_ns;
+	}
+
+	/*
+	 * For each device in the domain compute the difference between the
+	 * QoS latency constraint and the time already spent in the "stopped"
+	 * state plus the total time required to bring the device back to the
+	 * full power state (after the domain has been turned on).  Compute the
+	 * minimum of these values.
+	 */
+	min_delta_ns = -1;
+	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
+		struct generic_pm_domain_data *gpd_data;
+		struct device *dev = pdd->dev;
+		s32 constraint;
+
+		if (!dev->driver)
+			continue;
+
+		constraint = dev_pm_qos_read_value(dev);
+		if (constraint < 0) /* negative means "never stop" */
+			return false;
+		else if (constraint == 0) /* 0 means "don't care" */
+			continue;
+
+		delta_ns = constraint;
+		delta_ns *= NSEC_PER_USEC;
+
+		gpd_data = to_gpd_data(pdd);
+		delta_ns -= gpd_data->td.start_latency_ns +
+			ktime_to_ns(ktime_sub(time_now, gpd_data->stop_time));
+		if (delta_ns <= max_save_time_ns)
+			return false;
+
+		delta_ns -= gpd_data->td.restore_state_latency_ns;
+		if (delta_ns <= 0)
+			return false;
+
+		if (delta_ns < min_delta_ns)
+			min_delta_ns = delta_ns;
+	}
+
+	if (min_delta_ns < 0) {
+		/*
+		 * There are no latency constraints, so the domain can spend
+		 * arbitrary time in the "off" state.
+		 */
+		genpd->max_off_time_ns = -1;
+		return true;
+	}
+
+	/*
+	 * The difference between the computed minimum delta and the time needed
+	 * to turn the domain on is the maximum theoretical time this domain can
+	 * spend in the "off" state.
+	 */
+	min_delta_ns -= genpd->power_on_latency_ns;
+	genpd->max_off_time_ns = min_delta_ns;
+
+	/*
+	 * If the difference between the computed minimum delta and the time
+	 * needed to turn the domain off and back on on is smaller than the
+	 * domain's power break even time, removing power from the domain is not
+	 * worth it.
+	 */
+	min_delta_ns -= genpd->power_off_latency_ns;
+	return min_delta_ns > genpd->break_even_ns;
+}
+
 struct dev_power_governor simple_qos_governor = {
 	.stop_ok = default_stop_ok,
+	.power_down_ok = default_power_down_ok,
 };
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -47,6 +47,7 @@ struct generic_pm_domain *dev_to_genpd(s
 
 static int genpd_stop_dev(struct generic_pm_domain *genpd, struct device *dev)
 {
+	dev_gpd_data(dev)->stop_time = ktime_get();
 	return GENPD_DEV_CALLBACK(genpd, int, stop, dev);
 }
 
@@ -397,6 +398,7 @@ static int pm_genpd_poweroff(struct gene
 	}
 
 	genpd->status = GPD_STATE_POWER_OFF;
+	genpd->power_off_time = ktime_get();
 
 	list_for_each_entry(link, &genpd->slave_links, slave_node) {
 		genpd_sd_counter_dec(link->master);


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 7/7] PM / Domains: Automatically update overoptimistic latency information
  2011-11-07  0:01 [PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
                   ` (5 preceding siblings ...)
  2011-11-07  0:09 ` [PATCH 6/7] PM / Domains: Add default power off " Rafael J. Wysocki
@ 2011-11-07  0:10 ` Rafael J. Wysocki
  2011-11-14  0:22 ` [update][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
  7 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-07  0:10 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

Measure the time of execution of the .start(), .save_state()
and .restore_state() PM domain device callbacks and if the result
is greater than the corresponding latency value stored in the
device's struct generic_pm_domain_data object, replace the inaccurate
value with the measured time.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |   20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -32,6 +32,17 @@
 	__ret;							\
 })
 
+#define GENPD_DEV_TIMED_CALLBACK(genpd, type, callback, dev, latency)	\
+({									\
+	ktime_t __start = ktime_get();					\
+	type __retval = GENPD_DEV_CALLBACK(genpd, type, callback, dev);	\
+	s64 __elapsed = ktime_to_ns(ktime_sub(ktime_get(), __start));	\
+	struct generic_pm_domain_data *__gpd_data = dev_gpd_data(dev);	\
+	if (__elapsed > __gpd_data->td.latency)				\
+		__gpd_data->td.latency = __elapsed;			\
+	__retval;							\
+})
+
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
 
@@ -53,17 +64,20 @@ static int genpd_stop_dev(struct generic
 
 static int genpd_start_dev(struct generic_pm_domain *genpd, struct device *dev)
 {
-	return GENPD_DEV_CALLBACK(genpd, int, start, dev);
+	return GENPD_DEV_TIMED_CALLBACK(genpd, int, start, dev,
+					start_latency_ns);
 }
 
 static int genpd_save_dev(struct generic_pm_domain *genpd, struct device *dev)
 {
-	return GENPD_DEV_CALLBACK(genpd, int, save_state, dev);
+	return GENPD_DEV_TIMED_CALLBACK(genpd, int, save_state, dev,
+					save_state_latency_ns);
 }
 
 static int genpd_restore_dev(struct generic_pm_domain *genpd, struct device *dev)
 {
-	return GENPD_DEV_CALLBACK(genpd, int, restore_state, dev);
+	return GENPD_DEV_TIMED_CALLBACK(genpd, int, restore_state, dev,
+					restore_state_latency_ns);
 }
 
 static bool genpd_sd_counter_dec(struct generic_pm_domain *genpd)


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/7] PM / Domains: Make it possible to use per-device start/stop routines
  2011-11-07  0:06 ` [PATCH 1/7] PM / Domains: Make it possible to use per-device start/stop routines Rafael J. Wysocki
@ 2011-11-08  9:30   ` Guennadi Liakhovetski
  2011-11-08 20:38     ` Rafael J. Wysocki
  0 siblings, 1 reply; 35+ messages in thread
From: Guennadi Liakhovetski @ 2011-11-08  9:30 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Linux-sh list, Magnus Damm, Kevin Hilman,
	jean.pihet

Hi Rafael

On Mon, 7 Nov 2011, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> The current generic PM domains code requires that the same .stop()
> and .start() device callback routines be used for all devices in the
> given domain, which is inflexible and may not cover some specific use
> cases.  For this reason, make it possible to use device specific
> .start() and .stop() callback routines by adding corresponding
> callback pointers to struct generic_pm_domain_data.  Add a new helper
> routine, pm_genpd_register_callbacks(), that can be used to populate
> the new per-device callback pointers.
> 
> Modify the shmobile's power domains code to allow drivers to add
> their own code to be run during the device stop and start operations
> with the help of the new callback pointers.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---

[snip]

> Index: linux/drivers/base/power/domain.c
> ===================================================================
> --- linux.orig/drivers/base/power/domain.c
> +++ linux/drivers/base/power/domain.c
> @@ -29,6 +29,36 @@ static struct generic_pm_domain *dev_to_
>  	return pd_to_genpd(dev->pm_domain);
>  }
>  
> +static int genpd_stop_dev(struct generic_pm_domain *genpd, struct device *dev)
> +{
> +	int (*stop)(struct device *dev);
> +
> +	stop = genpd->stop_device;
> +	if (stop)
> +		return stop(dev);
> +
> +	stop = dev_gpd_data(dev)->ops.stop;
> +	if (stop)
> +		return stop(dev);

With this implementation your approach is: in your genpd_stop_dev() and 
genpd_start_dev() you first check for the pm-domain _common_ .start() and 
.stop(), and if they exist, the specific ones will not be called. Then in 
your sh7372 pm domain implementation you do the domain-common part - 
switch the PM clock on or off, and you _again_ check for device-specific 
domains, and this time you call both of them.

Do you have specific reasons to think, that calling both of them from 
genpd_stop_dev() / genpd_start_dev() might not fit some PM-domain 
implementations?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 2/7] PM / Domains: Make it possible to use per-device .active_wakeup()
  2011-11-07  0:06 ` [PATCH 2/7] PM / Domains: Make it possible to use per-device .active_wakeup() Rafael J. Wysocki
@ 2011-11-08 10:27   ` Guennadi Liakhovetski
  2011-11-08 20:40     ` Rafael J. Wysocki
  0 siblings, 1 reply; 35+ messages in thread
From: Guennadi Liakhovetski @ 2011-11-08 10:27 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Linux-sh list, Magnus Damm, Kevin Hilman,
	jean.pihet

On Mon, 7 Nov 2011, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> The current generic PM domains code requires that the same
> .active_wakeup() device callback routine be used for all devices in
> the given domain, which is inflexible and may not cover some specific
> use cases.  For this reason, make it possible to use device specific
> .active_wakeup() callback routines by adding a corresponding callback
> pointer to struct generic_pm_domain_data.  To reduce code duplication
> use struct gpd_dev_ops to represent PM domain device callbacks as
> well as device-specific ones and add a macro for defining routines
> that will execute those callbacks.
> 
> Modify the shmobile's power domains code to allow drivers to use
> their own .active_wakeup() callback routines.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  arch/arm/mach-shmobile/pm-sh7372.c |   11 ++++---
>  drivers/base/power/domain.c        |   54 ++++++++++++++++++-------------------
>  include/linux/pm_domain.h          |   15 ++++------
>  3 files changed, 41 insertions(+), 39 deletions(-)
> 
> Index: linux/include/linux/pm_domain.h
> ===================================================================
> --- linux.orig/include/linux/pm_domain.h
> +++ linux/include/linux/pm_domain.h
> @@ -23,6 +23,12 @@ struct dev_power_governor {
>  	bool (*power_down_ok)(struct dev_pm_domain *domain);
>  };
>  
> +struct gpd_dev_ops {
> +	int (*start)(struct device *dev);
> +	int (*stop)(struct device *dev);
> +	bool (*active_wakeup)(struct device *dev);
> +};
> +
>  struct generic_pm_domain {
>  	struct dev_pm_domain domain;	/* PM domain operations */
>  	struct list_head gpd_list_node;	/* Node in the global PM domains list */
> @@ -45,9 +51,7 @@ struct generic_pm_domain {
>  	bool dev_irq_safe;	/* Device callbacks are IRQ-safe */
>  	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);
> -	bool (*active_wakeup)(struct device *dev);
> +	struct gpd_dev_ops dev_ops;

Wouldn't it be better to merge patches 1 and 2?

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/7] PM / Domains: Make it possible to use per-device start/stop routines
  2011-11-08  9:30   ` Guennadi Liakhovetski
@ 2011-11-08 20:38     ` Rafael J. Wysocki
  0 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-08 20:38 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Linux PM list, LKML, Linux-sh list, Magnus Damm, Kevin Hilman,
	jean.pihet

On Tuesday, November 08, 2011, Guennadi Liakhovetski wrote:
> Hi Rafael
> 
> On Mon, 7 Nov 2011, Rafael J. Wysocki wrote:
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > The current generic PM domains code requires that the same .stop()
> > and .start() device callback routines be used for all devices in the
> > given domain, which is inflexible and may not cover some specific use
> > cases.  For this reason, make it possible to use device specific
> > .start() and .stop() callback routines by adding corresponding
> > callback pointers to struct generic_pm_domain_data.  Add a new helper
> > routine, pm_genpd_register_callbacks(), that can be used to populate
> > the new per-device callback pointers.
> > 
> > Modify the shmobile's power domains code to allow drivers to add
> > their own code to be run during the device stop and start operations
> > with the help of the new callback pointers.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> 
> [snip]
> 
> > Index: linux/drivers/base/power/domain.c
> > ===================================================================
> > --- linux.orig/drivers/base/power/domain.c
> > +++ linux/drivers/base/power/domain.c
> > @@ -29,6 +29,36 @@ static struct generic_pm_domain *dev_to_
> >  	return pd_to_genpd(dev->pm_domain);
> >  }
> >  
> > +static int genpd_stop_dev(struct generic_pm_domain *genpd, struct device *dev)
> > +{
> > +	int (*stop)(struct device *dev);
> > +
> > +	stop = genpd->stop_device;
> > +	if (stop)
> > +		return stop(dev);
> > +
> > +	stop = dev_gpd_data(dev)->ops.stop;
> > +	if (stop)
> > +		return stop(dev);
> 
> With this implementation your approach is: in your genpd_stop_dev() and 
> genpd_start_dev() you first check for the pm-domain _common_ .start() and 
> .stop(), and if they exist, the specific ones will not be called. Then in 
> your sh7372 pm domain implementation you do the domain-common part - 
> switch the PM clock on or off, and you _again_ check for device-specific 
> domains, and this time you call both of them.
> 
> Do you have specific reasons to think, that calling both of them from 
> genpd_stop_dev() / genpd_start_dev() might not fit some PM-domain 
> implementations?

Not very specific, but the current way is more flexible, because it
allows the PM domain to do something completely different from the
default thing.

Also that follows the strategy used in dd.c for .probe().

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 2/7] PM / Domains: Make it possible to use per-device .active_wakeup()
  2011-11-08 10:27   ` Guennadi Liakhovetski
@ 2011-11-08 20:40     ` Rafael J. Wysocki
  2011-11-09  8:52       ` Guennadi Liakhovetski
  0 siblings, 1 reply; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-08 20:40 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Linux PM list, LKML, Linux-sh list, Magnus Damm, Kevin Hilman,
	jean.pihet

On Tuesday, November 08, 2011, Guennadi Liakhovetski wrote:
> On Mon, 7 Nov 2011, Rafael J. Wysocki wrote:
> 
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > The current generic PM domains code requires that the same
> > .active_wakeup() device callback routine be used for all devices in
> > the given domain, which is inflexible and may not cover some specific
> > use cases.  For this reason, make it possible to use device specific
> > .active_wakeup() callback routines by adding a corresponding callback
> > pointer to struct generic_pm_domain_data.  To reduce code duplication
> > use struct gpd_dev_ops to represent PM domain device callbacks as
> > well as device-specific ones and add a macro for defining routines
> > that will execute those callbacks.
> > 
> > Modify the shmobile's power domains code to allow drivers to use
> > their own .active_wakeup() callback routines.
> > 
> > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > ---
> >  arch/arm/mach-shmobile/pm-sh7372.c |   11 ++++---
> >  drivers/base/power/domain.c        |   54 ++++++++++++++++++-------------------
> >  include/linux/pm_domain.h          |   15 ++++------
> >  3 files changed, 41 insertions(+), 39 deletions(-)
> > 
> > Index: linux/include/linux/pm_domain.h
> > ===================================================================
> > --- linux.orig/include/linux/pm_domain.h
> > +++ linux/include/linux/pm_domain.h
> > @@ -23,6 +23,12 @@ struct dev_power_governor {
> >  	bool (*power_down_ok)(struct dev_pm_domain *domain);
> >  };
> >  
> > +struct gpd_dev_ops {
> > +	int (*start)(struct device *dev);
> > +	int (*stop)(struct device *dev);
> > +	bool (*active_wakeup)(struct device *dev);
> > +};
> > +
> >  struct generic_pm_domain {
> >  	struct dev_pm_domain domain;	/* PM domain operations */
> >  	struct list_head gpd_list_node;	/* Node in the global PM domains list */
> > @@ -45,9 +51,7 @@ struct generic_pm_domain {
> >  	bool dev_irq_safe;	/* Device callbacks are IRQ-safe */
> >  	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);
> > -	bool (*active_wakeup)(struct device *dev);
> > +	struct gpd_dev_ops dev_ops;
> 
> Wouldn't it be better to merge patches 1 and 2?

First, why would it?

Second, why does it matter?

Rafael

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 2/7] PM / Domains: Make it possible to use per-device .active_wakeup()
  2011-11-08 20:40     ` Rafael J. Wysocki
@ 2011-11-09  8:52       ` Guennadi Liakhovetski
  2011-11-09 22:40         ` Rafael J. Wysocki
  0 siblings, 1 reply; 35+ messages in thread
From: Guennadi Liakhovetski @ 2011-11-09  8:52 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Linux-sh list, Magnus Damm, Kevin Hilman,
	jean.pihet

On Tue, 8 Nov 2011, Rafael J. Wysocki wrote:

> On Tuesday, November 08, 2011, Guennadi Liakhovetski wrote:
> > On Mon, 7 Nov 2011, Rafael J. Wysocki wrote:
> > 
> > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > 
> > > The current generic PM domains code requires that the same
> > > .active_wakeup() device callback routine be used for all devices in
> > > the given domain, which is inflexible and may not cover some specific
> > > use cases.  For this reason, make it possible to use device specific
> > > .active_wakeup() callback routines by adding a corresponding callback
> > > pointer to struct generic_pm_domain_data.  To reduce code duplication
> > > use struct gpd_dev_ops to represent PM domain device callbacks as
> > > well as device-specific ones and add a macro for defining routines
> > > that will execute those callbacks.
> > > 
> > > Modify the shmobile's power domains code to allow drivers to use
> > > their own .active_wakeup() callback routines.
> > > 
> > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > > ---
> > >  arch/arm/mach-shmobile/pm-sh7372.c |   11 ++++---
> > >  drivers/base/power/domain.c        |   54 ++++++++++++++++++-------------------
> > >  include/linux/pm_domain.h          |   15 ++++------
> > >  3 files changed, 41 insertions(+), 39 deletions(-)
> > > 
> > > Index: linux/include/linux/pm_domain.h
> > > ===================================================================
> > > --- linux.orig/include/linux/pm_domain.h
> > > +++ linux/include/linux/pm_domain.h
> > > @@ -23,6 +23,12 @@ struct dev_power_governor {
> > >  	bool (*power_down_ok)(struct dev_pm_domain *domain);
> > >  };
> > >  
> > > +struct gpd_dev_ops {
> > > +	int (*start)(struct device *dev);
> > > +	int (*stop)(struct device *dev);
> > > +	bool (*active_wakeup)(struct device *dev);
> > > +};
> > > +
> > >  struct generic_pm_domain {
> > >  	struct dev_pm_domain domain;	/* PM domain operations */
> > >  	struct list_head gpd_list_node;	/* Node in the global PM domains list */
> > > @@ -45,9 +51,7 @@ struct generic_pm_domain {
> > >  	bool dev_irq_safe;	/* Device callbacks are IRQ-safe */
> > >  	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);
> > > -	bool (*active_wakeup)(struct device *dev);
> > > +	struct gpd_dev_ops dev_ops;
> > 
> > Wouldn't it be better to merge patches 1 and 2?
> 
> First, why would it?

Because (1) AFAICS both these patches add new logically rather close to 
each other methods to the same existing API, and (2) it would reduce the 
total changed lines count and simplify reading of the patch(es), because 
your second patch moves around and modifies lines of code, that the first 
patch adds.

> Second, why does it matter?

See above.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 2/7] PM / Domains: Make it possible to use per-device .active_wakeup()
  2011-11-09  8:52       ` Guennadi Liakhovetski
@ 2011-11-09 22:40         ` Rafael J. Wysocki
  2011-11-09 23:02           ` Guennadi Liakhovetski
  0 siblings, 1 reply; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-09 22:40 UTC (permalink / raw)
  To: Guennadi Liakhovetski
  Cc: Linux PM list, LKML, Linux-sh list, Magnus Damm, Kevin Hilman,
	jean.pihet

On Wednesday, November 09, 2011, Guennadi Liakhovetski wrote:
> On Tue, 8 Nov 2011, Rafael J. Wysocki wrote:
> 
> > On Tuesday, November 08, 2011, Guennadi Liakhovetski wrote:
> > > On Mon, 7 Nov 2011, Rafael J. Wysocki wrote:
> > > 
> > > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > > 
> > > > The current generic PM domains code requires that the same
> > > > .active_wakeup() device callback routine be used for all devices in
> > > > the given domain, which is inflexible and may not cover some specific
> > > > use cases.  For this reason, make it possible to use device specific
> > > > .active_wakeup() callback routines by adding a corresponding callback
> > > > pointer to struct generic_pm_domain_data.  To reduce code duplication
> > > > use struct gpd_dev_ops to represent PM domain device callbacks as
> > > > well as device-specific ones and add a macro for defining routines
> > > > that will execute those callbacks.
> > > > 
> > > > Modify the shmobile's power domains code to allow drivers to use
> > > > their own .active_wakeup() callback routines.
> > > > 
> > > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > > > ---
> > > >  arch/arm/mach-shmobile/pm-sh7372.c |   11 ++++---
> > > >  drivers/base/power/domain.c        |   54 ++++++++++++++++++-------------------
> > > >  include/linux/pm_domain.h          |   15 ++++------
> > > >  3 files changed, 41 insertions(+), 39 deletions(-)
> > > > 
> > > > Index: linux/include/linux/pm_domain.h
> > > > ===================================================================
> > > > --- linux.orig/include/linux/pm_domain.h
> > > > +++ linux/include/linux/pm_domain.h
> > > > @@ -23,6 +23,12 @@ struct dev_power_governor {
> > > >  	bool (*power_down_ok)(struct dev_pm_domain *domain);
> > > >  };
> > > >  
> > > > +struct gpd_dev_ops {
> > > > +	int (*start)(struct device *dev);
> > > > +	int (*stop)(struct device *dev);
> > > > +	bool (*active_wakeup)(struct device *dev);
> > > > +};
> > > > +
> > > >  struct generic_pm_domain {
> > > >  	struct dev_pm_domain domain;	/* PM domain operations */
> > > >  	struct list_head gpd_list_node;	/* Node in the global PM domains list */
> > > > @@ -45,9 +51,7 @@ struct generic_pm_domain {
> > > >  	bool dev_irq_safe;	/* Device callbacks are IRQ-safe */
> > > >  	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);
> > > > -	bool (*active_wakeup)(struct device *dev);
> > > > +	struct gpd_dev_ops dev_ops;
> > > 
> > > Wouldn't it be better to merge patches 1 and 2?
> > 
> > First, why would it?
> 
> Because (1) AFAICS both these patches add new logically rather close to 
> each other methods to the same existing API,

Well, in fact .active_wakeup() was added for a totally different reason,
but I agree that now it _looks_ analogous.

> and (2) it would reduce the 
> total changed lines count and simplify reading of the patch(es), because 
> your second patch moves around and modifies lines of code, that the first 
> patch adds.

Well, I can fold [2/7] into [1/7] if that helps.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 2/7] PM / Domains: Make it possible to use per-device .active_wakeup()
  2011-11-09 22:40         ` Rafael J. Wysocki
@ 2011-11-09 23:02           ` Guennadi Liakhovetski
  0 siblings, 0 replies; 35+ messages in thread
From: Guennadi Liakhovetski @ 2011-11-09 23:02 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Linux-sh list, Magnus Damm, Kevin Hilman,
	jean.pihet

On Wed, 9 Nov 2011, Rafael J. Wysocki wrote:

> On Wednesday, November 09, 2011, Guennadi Liakhovetski wrote:
> > On Tue, 8 Nov 2011, Rafael J. Wysocki wrote:
> > 
> > > On Tuesday, November 08, 2011, Guennadi Liakhovetski wrote:
> > > > On Mon, 7 Nov 2011, Rafael J. Wysocki wrote:
> > > > 
> > > > > From: Rafael J. Wysocki <rjw@sisk.pl>
> > > > > 
> > > > > The current generic PM domains code requires that the same
> > > > > .active_wakeup() device callback routine be used for all devices in
> > > > > the given domain, which is inflexible and may not cover some specific
> > > > > use cases.  For this reason, make it possible to use device specific
> > > > > .active_wakeup() callback routines by adding a corresponding callback
> > > > > pointer to struct generic_pm_domain_data.  To reduce code duplication
> > > > > use struct gpd_dev_ops to represent PM domain device callbacks as
> > > > > well as device-specific ones and add a macro for defining routines
> > > > > that will execute those callbacks.
> > > > > 
> > > > > Modify the shmobile's power domains code to allow drivers to use
> > > > > their own .active_wakeup() callback routines.
> > > > > 
> > > > > Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> > > > > ---
> > > > >  arch/arm/mach-shmobile/pm-sh7372.c |   11 ++++---
> > > > >  drivers/base/power/domain.c        |   54 ++++++++++++++++++-------------------
> > > > >  include/linux/pm_domain.h          |   15 ++++------
> > > > >  3 files changed, 41 insertions(+), 39 deletions(-)
> > > > > 
> > > > > Index: linux/include/linux/pm_domain.h
> > > > > ===================================================================
> > > > > --- linux.orig/include/linux/pm_domain.h
> > > > > +++ linux/include/linux/pm_domain.h
> > > > > @@ -23,6 +23,12 @@ struct dev_power_governor {
> > > > >  	bool (*power_down_ok)(struct dev_pm_domain *domain);
> > > > >  };
> > > > >  
> > > > > +struct gpd_dev_ops {
> > > > > +	int (*start)(struct device *dev);
> > > > > +	int (*stop)(struct device *dev);
> > > > > +	bool (*active_wakeup)(struct device *dev);
> > > > > +};
> > > > > +
> > > > >  struct generic_pm_domain {
> > > > >  	struct dev_pm_domain domain;	/* PM domain operations */
> > > > >  	struct list_head gpd_list_node;	/* Node in the global PM domains list */
> > > > > @@ -45,9 +51,7 @@ struct generic_pm_domain {
> > > > >  	bool dev_irq_safe;	/* Device callbacks are IRQ-safe */
> > > > >  	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);
> > > > > -	bool (*active_wakeup)(struct device *dev);
> > > > > +	struct gpd_dev_ops dev_ops;
> > > > 
> > > > Wouldn't it be better to merge patches 1 and 2?
> > > 
> > > First, why would it?
> > 
> > Because (1) AFAICS both these patches add new logically rather close to 
> > each other methods to the same existing API,
> 
> Well, in fact .active_wakeup() was added for a totally different reason,
> but I agree that now it _looks_ analogous.
> 
> > and (2) it would reduce the 
> > total changed lines count and simplify reading of the patch(es), because 
> > your second patch moves around and modifies lines of code, that the first 
> > patch adds.
> 
> Well, I can fold [2/7] into [1/7] if that helps.

IMHO that would look nicer, yes.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [update][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS
  2011-11-07  0:01 [PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
                   ` (6 preceding siblings ...)
  2011-11-07  0:10 ` [PATCH 7/7] PM / Domains: Automatically update overoptimistic latency information Rafael J. Wysocki
@ 2011-11-14  0:22 ` Rafael J. Wysocki
  2011-11-14  0:23   ` [update][PATCH 1/7] PM / Domains: Make it possible to use per-device domain callbacks Rafael J. Wysocki
                     ` (7 more replies)
  7 siblings, 8 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-14  0:22 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

Hi,

This is the second iteration of the patchset introducing per-device callbacks
and PM QoS support to PM domains.

All patches have been updated, the second one have been folded into the
first one and there's one more patch in  the series.  Please look into the
changelogs for more details.

[1/7] - Make it possible to define per-device start/stop and .active_wakeup()
        routines.

[2/7] - Introduce "save/restore state" device callbacks.

[3/7] - Introduce per-device PM domain callbacks for system suspend.

[4/7] - Make runtime PM core take device PM QoS constraints into account.

[5/7] - Add device "stop governor".

[6/7] - Add domain "power off governor".

[7/7] - Automatically update overoptimistic latency information.

The patchset have been tested for backwards compatibility, but it still needs
some testing for the new features it adds.

Thanks,
Rafael


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [update][PATCH 1/7] PM / Domains: Make it possible to use per-device domain callbacks
  2011-11-14  0:22 ` [update][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
@ 2011-11-14  0:23   ` Rafael J. Wysocki
  2011-11-14  0:24   ` [update][PATCH 2/7] PM / Domains: Introduce "save/restore state" device callbacks Rafael J. Wysocki
                     ` (6 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-14  0:23 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

The current generic PM domains code requires that the same .stop(),
.start() and .active_wakeup() device callback routines be used for
all devices in the given domain, which is inflexible and may not
cover some specific use cases.  For this reason, make it possible to
use device specific .start()/.stop() and .active_wakeup() callback
routines by adding corresponding callback pointers to struct
generic_pm_domain_data.  Add a new helper routine,
pm_genpd_register_callbacks(), that can be used to populate
the new per-device callback pointers.

Modify the shmobile's power domains code to allow drivers to add
their own code to be run during the device stop and start operations
with the help of the new callback pointers.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/mach-shmobile/pm-sh7372.c |   40 ++++++++-
 drivers/base/power/domain.c        |  152 +++++++++++++++++++++++++++----------
 include/linux/pm_domain.h          |   27 +++++-
 3 files changed, 175 insertions(+), 44 deletions(-)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -23,6 +23,12 @@ struct dev_power_governor {
 	bool (*power_down_ok)(struct dev_pm_domain *domain);
 };
 
+struct gpd_dev_ops {
+	int (*start)(struct device *dev);
+	int (*stop)(struct device *dev);
+	bool (*active_wakeup)(struct device *dev);
+};
+
 struct generic_pm_domain {
 	struct dev_pm_domain domain;	/* PM domain operations */
 	struct list_head gpd_list_node;	/* Node in the global PM domains list */
@@ -45,9 +51,7 @@ struct generic_pm_domain {
 	bool dev_irq_safe;	/* Device callbacks are IRQ-safe */
 	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);
-	bool (*active_wakeup)(struct device *dev);
+	struct gpd_dev_ops dev_ops;
 };
 
 static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
@@ -64,6 +68,7 @@ struct gpd_link {
 
 struct generic_pm_domain_data {
 	struct pm_domain_data base;
+	struct gpd_dev_ops ops;
 	bool need_restore;
 };
 
@@ -72,6 +77,11 @@ static inline struct generic_pm_domain_d
 	return container_of(pdd, struct generic_pm_domain_data, base);
 }
 
+static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
+{
+	return to_gpd_data(dev->power.subsys_data->domain_data);
+}
+
 #ifdef CONFIG_PM_GENERIC_DOMAINS
 extern int pm_genpd_add_device(struct generic_pm_domain *genpd,
 			       struct device *dev);
@@ -81,6 +91,8 @@ extern int pm_genpd_add_subdomain(struct
 				  struct generic_pm_domain *new_subdomain);
 extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
 				     struct generic_pm_domain *target);
+extern int pm_genpd_add_callbacks(struct device *dev, struct gpd_dev_ops *ops);
+extern int pm_genpd_remove_callbacks(struct device *dev);
 extern void pm_genpd_init(struct generic_pm_domain *genpd,
 			  struct dev_power_governor *gov, bool is_off);
 extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
@@ -105,6 +117,15 @@ static inline int pm_genpd_remove_subdom
 {
 	return -ENOSYS;
 }
+static inline int pm_genpd_add_callbacks(struct device *dev,
+					 struct gpd_dev_ops *ops)
+{
+	return -ENOSYS;
+}
+static inline int pm_genpd_remove_callbacks(struct device *dev)
+{
+	return -ENOSYS;
+}
 static inline void pm_genpd_init(struct generic_pm_domain *genpd,
 				 struct dev_power_governor *gov, bool is_off) {}
 static inline int pm_genpd_poweron(struct generic_pm_domain *genpd)
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -15,6 +15,23 @@
 #include <linux/err.h>
 #include <linux/sched.h>
 #include <linux/suspend.h>
+#include <linux/export.h>
+
+#define GENPD_DEV_CALLBACK(genpd, type, callback, dev)		\
+({								\
+	type (*__routine)(struct device *__d); 			\
+	type __ret = (type)0;					\
+								\
+	__routine = genpd->dev_ops.callback; 			\
+	if (__routine) {					\
+		__ret = __routine(dev); 			\
+	} else {						\
+		__routine = dev_gpd_data(dev)->ops.callback;	\
+		if (__routine) 					\
+			__ret = __routine(dev);			\
+	}							\
+	__ret;							\
+})
 
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
@@ -29,6 +46,16 @@ static struct generic_pm_domain *dev_to_
 	return pd_to_genpd(dev->pm_domain);
 }
 
+static int genpd_stop_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, stop, dev);
+}
+
+static int genpd_start_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, start, dev);
+}
+
 static bool genpd_sd_counter_dec(struct generic_pm_domain *genpd)
 {
 	bool ret = false;
@@ -199,13 +226,9 @@ static int __pm_genpd_save_device(struct
 	mutex_unlock(&genpd->lock);
 
 	if (drv && drv->pm && drv->pm->runtime_suspend) {
-		if (genpd->start_device)
-			genpd->start_device(dev);
-
+		genpd_start_dev(genpd, dev);
 		ret = drv->pm->runtime_suspend(dev);
-
-		if (genpd->stop_device)
-			genpd->stop_device(dev);
+		genpd_stop_dev(genpd, dev);
 	}
 
 	mutex_lock(&genpd->lock);
@@ -235,13 +258,9 @@ static void __pm_genpd_restore_device(st
 	mutex_unlock(&genpd->lock);
 
 	if (drv && drv->pm && drv->pm->runtime_resume) {
-		if (genpd->start_device)
-			genpd->start_device(dev);
-
+		genpd_start_dev(genpd, dev);
 		drv->pm->runtime_resume(dev);
-
-		if (genpd->stop_device)
-			genpd->stop_device(dev);
+		genpd_stop_dev(genpd, dev);
 	}
 
 	mutex_lock(&genpd->lock);
@@ -413,6 +432,7 @@ static void genpd_power_off_work_fn(stru
 static int pm_genpd_runtime_suspend(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
+	int ret;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
@@ -422,11 +442,9 @@ static int pm_genpd_runtime_suspend(stru
 
 	might_sleep_if(!genpd->dev_irq_safe);
 
-	if (genpd->stop_device) {
-		int ret = genpd->stop_device(dev);
-		if (ret)
-			return ret;
-	}
+	ret = genpd_stop_dev(genpd, dev);
+	if (ret)
+		return ret;
 
 	/*
 	 * If power.irq_safe is set, this routine will be run with interrupts
@@ -502,8 +520,7 @@ static int pm_genpd_runtime_resume(struc
 	mutex_unlock(&genpd->lock);
 
  out:
-	if (genpd->start_device)
-		genpd->start_device(dev);
+	genpd_start_dev(genpd, dev);
 
 	return 0;
 }
@@ -534,6 +551,12 @@ static inline void genpd_power_off_work_
 
 #ifdef CONFIG_PM_SLEEP
 
+static bool genpd_dev_active_wakeup(struct generic_pm_domain *genpd,
+				    struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, bool, active_wakeup, dev);
+}
+
 /**
  * pm_genpd_sync_poweroff - Synchronously power off a PM domain and its masters.
  * @genpd: PM domain to power off, if possible.
@@ -590,7 +613,7 @@ static bool resume_needed(struct device
 	if (!device_can_wakeup(dev))
 		return false;
 
-	active_wakeup = genpd->active_wakeup && genpd->active_wakeup(dev);
+	active_wakeup = genpd_dev_active_wakeup(genpd, dev);
 	return device_may_wakeup(dev) ? active_wakeup : !active_wakeup;
 }
 
@@ -646,7 +669,7 @@ static int pm_genpd_prepare(struct devic
 	/*
 	 * The PM domain must be in the GPD_STATE_ACTIVE state at this point,
 	 * so pm_genpd_poweron() will return immediately, but if the device
-	 * is suspended (e.g. it's been stopped by .stop_device()), we need
+	 * is suspended (e.g. it's been stopped by genpd_stop_dev()), we need
 	 * to make it operational.
 	 */
 	pm_runtime_resume(dev);
@@ -714,12 +737,10 @@ static int pm_genpd_suspend_noirq(struct
 	if (ret)
 		return ret;
 
-	if (dev->power.wakeup_path
-	    && genpd->active_wakeup && genpd->active_wakeup(dev))
+	if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
 		return 0;
 
-	if (genpd->stop_device)
-		genpd->stop_device(dev);
+	genpd_stop_dev(genpd, dev);
 
 	/*
 	 * Since all of the "noirq" callbacks are executed sequentially, it is
@@ -761,8 +782,7 @@ static int pm_genpd_resume_noirq(struct
 	 */
 	pm_genpd_poweron(genpd);
 	genpd->suspended_count--;
-	if (genpd->start_device)
-		genpd->start_device(dev);
+	genpd_start_dev(genpd, dev);
 
 	return pm_generic_resume_noirq(dev);
 }
@@ -836,8 +856,7 @@ static int pm_genpd_freeze_noirq(struct
 	if (ret)
 		return ret;
 
-	if (genpd->stop_device)
-		genpd->stop_device(dev);
+	genpd_stop_dev(genpd, dev);
 
 	return 0;
 }
@@ -864,8 +883,7 @@ static int pm_genpd_thaw_noirq(struct de
 	if (genpd->suspend_power_off)
 		return 0;
 
-	if (genpd->start_device)
-		genpd->start_device(dev);
+	genpd_start_dev(genpd, dev);
 
 	return pm_generic_thaw_noirq(dev);
 }
@@ -938,12 +956,10 @@ static int pm_genpd_dev_poweroff_noirq(s
 	if (ret)
 		return ret;
 
-	if (dev->power.wakeup_path
-	    && genpd->active_wakeup && genpd->active_wakeup(dev))
+	if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
 		return 0;
 
-	if (genpd->stop_device)
-		genpd->stop_device(dev);
+	genpd_stop_dev(genpd, dev);
 
 	/*
 	 * Since all of the "noirq" callbacks are executed sequentially, it is
@@ -993,8 +1009,7 @@ static int pm_genpd_restore_noirq(struct
 
 	pm_genpd_poweron(genpd);
 	genpd->suspended_count--;
-	if (genpd->start_device)
-		genpd->start_device(dev);
+	genpd_start_dev(genpd, dev);
 
 	return pm_generic_restore_noirq(dev);
 }
@@ -1280,6 +1295,69 @@ int pm_genpd_remove_subdomain(struct gen
 }
 
 /**
+ * pm_genpd_add_callbacks - Add PM domain callbacks to a given device.
+ * @dev: Device to add the callbacks to.
+ * @ops: Set of callbacks to add.
+ */
+int pm_genpd_add_callbacks(struct device *dev, struct gpd_dev_ops *ops)
+{
+	struct pm_domain_data *pdd;
+	int ret = 0;
+
+	if (!(dev && dev->power.subsys_data && ops))
+		return -EINVAL;
+
+	pm_runtime_disable(dev);
+	device_pm_lock();
+
+	pdd = dev->power.subsys_data->domain_data;
+	if (pdd) {
+		struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
+
+		gpd_data->ops = *ops;
+	} else {
+		ret = -EINVAL;
+	}
+
+	device_pm_unlock();
+	pm_runtime_enable(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pm_genpd_add_callbacks);
+
+/**
+ * pm_genpd_remove_callbacks - Remove PM domain callbacks from a given device.
+ * @dev: Device to remove the callbacks from.
+ */
+int pm_genpd_remove_callbacks(struct device *dev)
+{
+	struct pm_domain_data *pdd;
+	int ret = 0;
+
+	if (!(dev && dev->power.subsys_data))
+		return -EINVAL;
+
+	pm_runtime_disable(dev);
+	device_pm_lock();
+
+	pdd = dev->power.subsys_data->domain_data;
+	if (pdd) {
+		struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
+
+		gpd_data->ops = (struct gpd_dev_ops){ 0 };
+	} else {
+		ret = -EINVAL;
+	}
+
+	device_pm_unlock();
+	pm_runtime_enable(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pm_genpd_remove_callbacks);
+
+/**
  * 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).
Index: linux/arch/arm/mach-shmobile/pm-sh7372.c
===================================================================
--- linux.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux/arch/arm/mach-shmobile/pm-sh7372.c
@@ -156,7 +156,10 @@ static void sh7372_a4r_suspend(void)
 
 static bool pd_active_wakeup(struct device *dev)
 {
-	return true;
+	bool (*active_wakeup)(struct device *dev);
+
+	active_wakeup = dev_gpd_data(dev)->ops.active_wakeup;
+	return active_wakeup ? active_wakeup(dev) : true;
 }
 
 static bool sh7372_power_down_forbidden(struct dev_pm_domain *domain)
@@ -168,15 +171,44 @@ struct dev_power_governor sh7372_always_
 	.power_down_ok = sh7372_power_down_forbidden,
 };
 
+static int sh7372_stop_dev(struct device *dev)
+{
+	int (*stop)(struct device *dev);
+
+	stop = dev_gpd_data(dev)->ops.stop;
+	if (stop) {
+		int ret = stop(dev);
+		if (ret)
+			return ret;
+	}
+	return pm_clk_suspend(dev);
+}
+
+static int sh7372_start_dev(struct device *dev)
+{
+	int (*start)(struct device *dev);
+	int ret;
+
+	ret = pm_clk_resume(dev);
+	if (ret)
+		return ret;
+
+	start = dev_gpd_data(dev)->ops.start;
+	if (start)
+		ret = start(dev);
+
+	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, sh7372_pd->gov, false);
-	genpd->stop_device = pm_clk_suspend;
-	genpd->start_device = pm_clk_resume;
+	genpd->dev_ops.stop = sh7372_stop_dev;
+	genpd->dev_ops.start = sh7372_start_dev;
+	genpd->dev_ops.active_wakeup = pd_active_wakeup;
 	genpd->dev_irq_safe = true;
-	genpd->active_wakeup = pd_active_wakeup;
 	genpd->power_off = pd_power_down;
 	genpd->power_on = pd_power_up;
 	__pd_power_up(sh7372_pd, false);


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [update][PATCH 2/7] PM / Domains: Introduce "save/restore state" device callbacks
  2011-11-14  0:22 ` [update][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
  2011-11-14  0:23   ` [update][PATCH 1/7] PM / Domains: Make it possible to use per-device domain callbacks Rafael J. Wysocki
@ 2011-11-14  0:24   ` Rafael J. Wysocki
  2011-11-14  0:25   ` [update][PATCH 3/7] PM / Domains: Rework system suspend callback routines Rafael J. Wysocki
                     ` (5 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-14  0:24 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

The current PM domains code uses device drivers' .runtime_suspend()
and .runtime_resume() callbacks as the "save device state" and
"restore device state" operations, which may not be appropriate in
general, because it forces drivers to assume that they always will
be used with generic PM domains.  However, in theory, the same
hardware may be used in devices that don't belong to any PM
domain, in which case it would be necessary to add "fake" PM
domains to satisfy the above assumption.  It also may be located in
a PM domain that's not handled with the help of the generic code.

To allow device drivers that may be used along with the generic PM
domains code of more flexibility, introduce new device callbacks,
.save_state() and .restore_state(), that can be supplied by the
drivers in addition to their "standard" runtime PM callbacks.  This
will allow the drivers to be designed to work with generic PM domains
as well as without them.

For backwards compatibility, introduce default .save_state() and
.restore_state() callback routines for PM domains that will execute
a device driver's .runtime_suspend() and .runtime_resume() callbacks,
respectively, for the given device if the driver doesn't provide its
own implementations of .save_state() and .restore_state().

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |   68 ++++++++++++++++++++++++++++++++++++--------
 include/linux/pm_domain.h   |    2 +
 2 files changed, 58 insertions(+), 12 deletions(-)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -26,6 +26,8 @@ struct dev_power_governor {
 struct gpd_dev_ops {
 	int (*start)(struct device *dev);
 	int (*stop)(struct device *dev);
+	int (*save_state)(struct device *dev);
+	int (*restore_state)(struct device *dev);
 	bool (*active_wakeup)(struct device *dev);
 };
 
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -56,6 +56,16 @@ static int genpd_start_dev(struct generi
 	return GENPD_DEV_CALLBACK(genpd, int, start, dev);
 }
 
+static int genpd_save_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, save_state, dev);
+}
+
+static int genpd_restore_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, restore_state, dev);
+}
+
 static bool genpd_sd_counter_dec(struct generic_pm_domain *genpd)
 {
 	bool ret = false;
@@ -217,7 +227,6 @@ static int __pm_genpd_save_device(struct
 {
 	struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
 	struct device *dev = pdd->dev;
-	struct device_driver *drv = dev->driver;
 	int ret = 0;
 
 	if (gpd_data->need_restore)
@@ -225,11 +234,9 @@ static int __pm_genpd_save_device(struct
 
 	mutex_unlock(&genpd->lock);
 
-	if (drv && drv->pm && drv->pm->runtime_suspend) {
-		genpd_start_dev(genpd, dev);
-		ret = drv->pm->runtime_suspend(dev);
-		genpd_stop_dev(genpd, dev);
-	}
+	genpd_start_dev(genpd, dev);
+	ret = genpd_save_dev(genpd, dev);
+	genpd_stop_dev(genpd, dev);
 
 	mutex_lock(&genpd->lock);
 
@@ -250,18 +257,15 @@ static void __pm_genpd_restore_device(st
 {
 	struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
 	struct device *dev = pdd->dev;
-	struct device_driver *drv = dev->driver;
 
 	if (!gpd_data->need_restore)
 		return;
 
 	mutex_unlock(&genpd->lock);
 
-	if (drv && drv->pm && drv->pm->runtime_resume) {
-		genpd_start_dev(genpd, dev);
-		drv->pm->runtime_resume(dev);
-		genpd_stop_dev(genpd, dev);
-	}
+	genpd_start_dev(genpd, dev);
+	genpd_restore_dev(genpd, dev);
+	genpd_stop_dev(genpd, dev);
 
 	mutex_lock(&genpd->lock);
 
@@ -1358,6 +1362,44 @@ int pm_genpd_remove_callbacks(struct dev
 EXPORT_SYMBOL_GPL(pm_genpd_remove_callbacks);
 
 /**
+ * pm_genpd_default_save_state - Default "save device state" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_save_state(struct device *dev)
+{
+	int (*cb)(struct device *__dev);
+	struct device_driver *drv = dev->driver;
+
+	cb = dev_gpd_data(dev)->ops.save_state;
+	if (cb)
+		return cb(dev);
+
+	if (drv && drv->pm && drv->pm->runtime_suspend)
+		return drv->pm->runtime_suspend(dev);
+
+	return 0;
+}
+
+/**
+ * pm_genpd_default_restore_state - Default PM domians "restore device state".
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_restore_state(struct device *dev)
+{
+	int (*cb)(struct device *__dev);
+	struct device_driver *drv = dev->driver;
+
+	cb = dev_gpd_data(dev)->ops.restore_state;
+	if (cb)
+		return cb(dev);
+
+	if (drv && drv->pm && drv->pm->runtime_resume)
+		return drv->pm->runtime_resume(dev);
+
+	return 0;
+}
+
+/**
  * 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).
@@ -1400,6 +1442,8 @@ void pm_genpd_init(struct generic_pm_dom
 	genpd->domain.ops.restore_noirq = pm_genpd_restore_noirq;
 	genpd->domain.ops.restore = pm_genpd_restore;
 	genpd->domain.ops.complete = pm_genpd_complete;
+	genpd->dev_ops.save_state = pm_genpd_default_save_state;
+	genpd->dev_ops.restore_state = pm_genpd_default_restore_state;
 	mutex_lock(&gpd_list_lock);
 	list_add(&genpd->gpd_list_node, &gpd_list);
 	mutex_unlock(&gpd_list_lock);


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [update][PATCH 3/7] PM / Domains: Rework system suspend callback routines
  2011-11-14  0:22 ` [update][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
  2011-11-14  0:23   ` [update][PATCH 1/7] PM / Domains: Make it possible to use per-device domain callbacks Rafael J. Wysocki
  2011-11-14  0:24   ` [update][PATCH 2/7] PM / Domains: Introduce "save/restore state" device callbacks Rafael J. Wysocki
@ 2011-11-14  0:25   ` Rafael J. Wysocki
  2011-11-14  0:26   ` [update][PATCH 4/7] PM / Runtime: Use device PM QoS constraints Rafael J. Wysocki
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-14  0:25 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

The current generic PM domains code attempts to use the generic
system suspend operations along with the domains' device stop/start
routines, which requires device drivers to assume that their
system suspend/resume (and hibernation/restore) callbacks will always
be used with generic PM domains.  However, in theory, the same
hardware may be used in devices that don't belong to any PM domain,
in which case it would be necessary to add "fake" PM domains to
satisfy the above assumption.  Also, the domain the hardware belongs
to may not be handled with the help of the generic code.

To allow device drivers that may be used along with the generic PM
domains code of more flexibility, add new device callbacks, .freeze(),
.freeze_late(), .thaw_early() and .thaw(), that can be supplied by
the drivers in addition to their "standard" system suspend and
hibernation callbacks.  These new callbacks, if defined, will be used
by the generic PM domains code for the handling of system suspend and
hibernation instead of the "standard" ones.  This will allow drivers
to be designed to work with generic PM domains as well as without
them.

For backwards compatibility, introduce default .freeze(), .thaw(),
.freeze_late() and .thaw_early() callback routines for PM domains that
will execute pm_generic_suspend(), pm_generic_resume(),
pm_generic_suspend_noirq() and pm_generic_resume_noirq(),
respectively, for the given device if its driver doesn't provide its
own implementations of .freeze(), .thaw(), .freeze_late() and
.thaw_early() (this works slightly differently from the current code,
but its existing users don't care anyway).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |  189 +++++++++++++++++++-------------------------
 include/linux/pm_domain.h   |    4 
 2 files changed, 87 insertions(+), 106 deletions(-)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -28,6 +28,10 @@ struct gpd_dev_ops {
 	int (*stop)(struct device *dev);
 	int (*save_state)(struct device *dev);
 	int (*restore_state)(struct device *dev);
+	int (*freeze)(struct device *dev);
+	int (*freeze_late)(struct device *dev);
+	int (*thaw_early)(struct device *dev);
+	int (*thaw)(struct device *dev);
 	bool (*active_wakeup)(struct device *dev);
 };
 
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -561,6 +561,26 @@ static bool genpd_dev_active_wakeup(stru
 	return GENPD_DEV_CALLBACK(genpd, bool, active_wakeup, dev);
 }
 
+static int genpd_freeze_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, freeze, dev);
+}
+
+static int genpd_freeze_late(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, freeze_late, dev);
+}
+
+static int genpd_thaw_early(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, thaw_early, dev);
+}
+
+static int genpd_thaw_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, thaw, dev);
+}
+
 /**
  * pm_genpd_sync_poweroff - Synchronously power off a PM domain and its masters.
  * @genpd: PM domain to power off, if possible.
@@ -695,27 +715,6 @@ static int pm_genpd_prepare(struct devic
 }
 
 /**
- * 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.
  *
@@ -737,7 +736,7 @@ static int pm_genpd_suspend_noirq(struct
 	if (genpd->suspend_power_off)
 		return 0;
 
-	ret = pm_generic_suspend_noirq(dev);
+	ret = genpd_freeze_late(genpd, dev);
 	if (ret)
 		return ret;
 
@@ -788,28 +787,7 @@ static int pm_genpd_resume_noirq(struct
 	genpd->suspended_count--;
 	genpd_start_dev(genpd, dev);
 
-	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);
+	return genpd_thaw_early(genpd, dev);
 }
 
 /**
@@ -830,14 +808,14 @@ static int pm_genpd_freeze(struct device
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	return genpd->suspend_power_off ? 0 : pm_generic_freeze(dev);
+	return genpd->suspend_power_off ? 0 : genpd_freeze_dev(genpd, 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
+ * 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.
@@ -856,7 +834,7 @@ static int pm_genpd_freeze_noirq(struct
 	if (genpd->suspend_power_off)
 		return 0;
 
-	ret = pm_generic_freeze_noirq(dev);
+	ret = genpd_freeze_late(genpd, dev);
 	if (ret)
 		return ret;
 
@@ -869,7 +847,7 @@ static int pm_genpd_freeze_noirq(struct
  * 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
+ * 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.
@@ -889,7 +867,7 @@ static int pm_genpd_thaw_noirq(struct de
 
 	genpd_start_dev(genpd, dev);
 
-	return pm_generic_thaw_noirq(dev);
+	return genpd_thaw_early(genpd, dev);
 }
 
 /**
@@ -910,28 +888,7 @@ static int pm_genpd_thaw(struct device *
 	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);
+	return genpd->suspend_power_off ? 0 : genpd_thaw_dev(genpd, dev);
 }
 
 /**
@@ -945,7 +902,6 @@ static int pm_genpd_dev_poweroff(struct
 static int pm_genpd_dev_poweroff_noirq(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
-	int ret;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
@@ -956,10 +912,6 @@ static int pm_genpd_dev_poweroff_noirq(s
 	if (genpd->suspend_power_off)
 		return 0;
 
-	ret = pm_generic_poweroff_noirq(dev);
-	if (ret)
-		return ret;
-
 	if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
 		return 0;
 
@@ -1015,28 +967,7 @@ static int pm_genpd_restore_noirq(struct
 	genpd->suspended_count--;
 	genpd_start_dev(genpd, 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);
+	return genpd_thaw_early(genpd, dev);
 }
 
 /**
@@ -1078,18 +1009,14 @@ static void pm_genpd_complete(struct dev
 #else
 
 #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_dev_poweroff_noirq	NULL
-#define pm_genpd_dev_poweroff		NULL
 #define pm_genpd_restore_noirq		NULL
-#define pm_genpd_restore		NULL
 #define pm_genpd_complete		NULL
 
 #endif /* CONFIG_PM_SLEEP */
@@ -1361,6 +1288,8 @@ int pm_genpd_remove_callbacks(struct dev
 }
 EXPORT_SYMBOL_GPL(pm_genpd_remove_callbacks);
 
+/* Default device callbacks for generic PM domains. */
+
 /**
  * pm_genpd_default_save_state - Default "save device state" for PM domians.
  * @dev: Device to handle.
@@ -1400,6 +1329,50 @@ static int pm_genpd_default_restore_stat
 }
 
 /**
+ * pm_genpd_default_freeze - Default "device freeze" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_freeze(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.freeze;
+
+	return cb ? cb(dev) : pm_generic_suspend(dev);
+}
+
+/**
+ * pm_genpd_default_freeze_late - Default "late device freeze" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_freeze_late(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.freeze_late;
+
+	return cb ? cb(dev) : pm_generic_suspend_noirq(dev);
+}
+
+/**
+ * pm_genpd_default_thaw_early - Default "early device thaw" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_thaw_early(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.thaw_early;
+
+	return cb ? cb(dev) : pm_generic_resume_noirq(dev);
+}
+
+/**
+ * pm_genpd_default_thaw - Default "device thaw" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_thaw(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.thaw;
+
+	return cb ? cb(dev) : pm_generic_resume(dev);
+}
+
+/**
  * 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).
@@ -1429,21 +1402,25 @@ void pm_genpd_init(struct generic_pm_dom
 	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 = pm_genpd_freeze;
 	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.resume = pm_genpd_thaw;
 	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 = pm_genpd_freeze;
 	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.restore = pm_genpd_thaw;
 	genpd->domain.ops.complete = pm_genpd_complete;
 	genpd->dev_ops.save_state = pm_genpd_default_save_state;
 	genpd->dev_ops.restore_state = pm_genpd_default_restore_state;
+	genpd->dev_ops.freeze = pm_genpd_default_freeze;
+	genpd->dev_ops.freeze_late = pm_genpd_default_freeze_late;
+	genpd->dev_ops.thaw_early = pm_genpd_default_thaw_early;
+	genpd->dev_ops.thaw = pm_genpd_default_thaw;
 	mutex_lock(&gpd_list_lock);
 	list_add(&genpd->gpd_list_node, &gpd_list);
 	mutex_unlock(&gpd_list_lock);


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [update][PATCH 4/7] PM / Runtime: Use device PM QoS constraints
  2011-11-14  0:22 ` [update][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
                     ` (2 preceding siblings ...)
  2011-11-14  0:25   ` [update][PATCH 3/7] PM / Domains: Rework system suspend callback routines Rafael J. Wysocki
@ 2011-11-14  0:26   ` Rafael J. Wysocki
  2011-11-14  0:27   ` [update][PATCH 5/7] PM / Domains: Add device stop governor function (v4) Rafael J. Wysocki
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-14  0:26 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

Make the runtime PM core use device PM QoS constraints to check if
it is allowed to suspend a given device, so that an error code is
returned if the device's own PM QoS constraint is negative or one of
its children has already been suspended for too long.  If this is
not the case, the maximum estimated time the device is allowed to be
suspended, computed as the minimum of the device's PM QoS constraint
and the PM QoS constraints of its children (reduced by the difference
between the current time and their suspend times) is stored in a new
device's PM field power.max_time_suspended_ns that can be used by
the device's subsystem or PM domain to decide whether or not to put
the device into lower-power (and presumably higher-latency) states
later (if the constraint is 0, which means "no constraint", the
power.max_time_suspended_ns is set to -1).

Additionally, the time of execution of the subsystem-level
.runtime_suspend() callback for the device is recorded in the new
power.suspend_time field for later use by the device's subsystem or
PM domain along with power.max_time_suspended_ns (it also is used
by the core code when the device's parent is suspended).

Introduce a new helper function,
pm_runtime_update_max_time_suspended(), allowing subsystems and PM
domains (or device drivers) to update the power.max_time_suspended_ns
field, for example after changing the power state of a suspended
device.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/qos.c     |   24 ++++++++----
 drivers/base/power/runtime.c |   85 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm.h           |    2 +
 include/linux/pm_runtime.h   |    5 ++
 4 files changed, 108 insertions(+), 8 deletions(-)

Index: linux/include/linux/pm.h
===================================================================
--- linux.orig/include/linux/pm.h
+++ linux/include/linux/pm.h
@@ -482,6 +482,8 @@ struct dev_pm_info {
 	unsigned long		active_jiffies;
 	unsigned long		suspended_jiffies;
 	unsigned long		accounting_timestamp;
+	ktime_t			suspend_time;
+	s64			max_time_suspended_ns;
 #endif
 	struct pm_subsys_data	*subsys_data;  /* Owned by the subsystem. */
 	struct pm_qos_constraints *constraints;
Index: linux/drivers/base/power/runtime.c
===================================================================
--- linux.orig/drivers/base/power/runtime.c
+++ linux/drivers/base/power/runtime.c
@@ -279,6 +279,38 @@ static int rpm_callback(int (*cb)(struct
 	return retval != -EACCES ? retval : -EIO;
 }
 
+struct rpm_qos_data {
+	ktime_t time_now;
+	s64 constraint_ns;
+};
+
+/**
+ * rpm_update_qos_constraint - Update a given PM QoS constraint data.
+ * @dev: Device whose timing data to use.
+ * @data: PM QoS constraint data to update.
+ *
+ * Use the suspend timing data of @dev to update PM QoS constraint data pointed
+ * to by @data.
+ */
+static int rpm_update_qos_constraint(struct device *dev, void *data)
+{
+	struct rpm_qos_data *qos = data;
+	s64 delta_ns;
+
+	if (dev->power.max_time_suspended_ns < 0)
+		return 0;
+
+	delta_ns = dev->power.max_time_suspended_ns -
+		ktime_to_ns(ktime_sub(qos->time_now, dev->power.suspend_time));
+	if (delta_ns <= 0)
+		return -EBUSY;
+
+	if (qos->constraint_ns > delta_ns || qos->constraint_ns == 0)
+		qos->constraint_ns = delta_ns;
+
+	return 0;
+}
+
 /**
  * rpm_suspend - Carry out runtime suspend of given device.
  * @dev: Device to suspend.
@@ -305,6 +337,7 @@ static int rpm_suspend(struct device *de
 {
 	int (*callback)(struct device *);
 	struct device *parent = NULL;
+	struct rpm_qos_data qos;
 	int retval;
 
 	trace_rpm_suspend(dev, rpmflags);
@@ -400,6 +433,25 @@ static int rpm_suspend(struct device *de
 		goto out;
 	}
 
+	qos.constraint_ns = __dev_pm_qos_read_value(dev);
+	if (qos.constraint_ns < 0) {
+		/* Negative constraint means "never suspend". */
+		retval = -EPERM;
+		goto out;
+	}
+	qos.constraint_ns *= NSEC_PER_USEC;
+	qos.time_now = ktime_get();
+
+	if (!dev->power.ignore_children) {
+		retval = device_for_each_child(dev, &qos,
+					       rpm_update_qos_constraint);
+		if (retval)
+			goto out;
+	}
+
+	dev->power.suspend_time = qos.time_now;
+	dev->power.max_time_suspended_ns = qos.constraint_ns ? : -1;
+
 	__update_runtime_status(dev, RPM_SUSPENDING);
 
 	if (dev->pm_domain)
@@ -416,6 +468,8 @@ static int rpm_suspend(struct device *de
 	retval = rpm_callback(callback, dev);
 	if (retval) {
 		__update_runtime_status(dev, RPM_ACTIVE);
+		dev->power.suspend_time = ktime_set(0, 0);
+		dev->power.max_time_suspended_ns = -1;
 		dev->power.deferred_resume = false;
 		if (retval == -EAGAIN || retval == -EBUSY) {
 			dev->power.runtime_error = 0;
@@ -620,6 +674,9 @@ static int rpm_resume(struct device *dev
 	if (dev->power.no_callbacks)
 		goto no_callback;	/* Assume success. */
 
+	dev->power.suspend_time = ktime_set(0, 0);
+	dev->power.max_time_suspended_ns = -1;
+
 	__update_runtime_status(dev, RPM_RESUMING);
 
 	if (dev->pm_domain)
@@ -1279,6 +1336,9 @@ void pm_runtime_init(struct device *dev)
 	setup_timer(&dev->power.suspend_timer, pm_suspend_timer_fn,
 			(unsigned long)dev);
 
+	dev->power.suspend_time = ktime_set(0, 0);
+	dev->power.max_time_suspended_ns = -1;
+
 	init_waitqueue_head(&dev->power.wait_queue);
 }
 
@@ -1296,3 +1356,28 @@ void pm_runtime_remove(struct device *de
 	if (dev->power.irq_safe && dev->parent)
 		pm_runtime_put_sync(dev->parent);
 }
+
+/**
+ * pm_runtime_update_max_time_suspended - Update device's suspend time data.
+ * @dev: Device to handle.
+ * @delta_ns: Value to subtract from the device's max_time_suspended_ns field.
+ *
+ * Update the device's power.max_time_suspended_ns field by subtracting
+ * @delta_ns from it.  The resulting value of power.max_time_suspended_ns is
+ * never negative.
+ */
+void pm_runtime_update_max_time_suspended(struct device *dev, s64 delta_ns)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->power.lock, flags);
+
+	if (delta_ns > 0 && dev->power.max_time_suspended_ns > 0) {
+		if (dev->power.max_time_suspended_ns > delta_ns)
+			dev->power.max_time_suspended_ns -= delta_ns;
+		else
+			dev->power.max_time_suspended_ns = 0;
+	}
+
+	spin_unlock_irqrestore(&dev->power.lock, flags);
+}
Index: linux/include/linux/pm_runtime.h
===================================================================
--- linux.orig/include/linux/pm_runtime.h
+++ linux/include/linux/pm_runtime.h
@@ -45,6 +45,8 @@ extern void pm_runtime_irq_safe(struct d
 extern void __pm_runtime_use_autosuspend(struct device *dev, bool use);
 extern void pm_runtime_set_autosuspend_delay(struct device *dev, int delay);
 extern unsigned long pm_runtime_autosuspend_expiration(struct device *dev);
+extern void pm_runtime_update_max_time_suspended(struct device *dev,
+						 s64 delta_ns);
 
 static inline bool pm_children_suspended(struct device *dev)
 {
@@ -154,6 +156,9 @@ static inline void pm_runtime_set_autosu
 static inline unsigned long pm_runtime_autosuspend_expiration(
 				struct device *dev) { return 0; }
 
+static inline void pm_runtime_update_max_time_suspended(struct device *dev,
+							s64 delta_ns) {}
+
 #endif /* !CONFIG_PM_RUNTIME */
 
 static inline int pm_runtime_idle(struct device *dev)
Index: linux/drivers/base/power/qos.c
===================================================================
--- linux.orig/drivers/base/power/qos.c
+++ linux/drivers/base/power/qos.c
@@ -47,21 +47,29 @@ static DEFINE_MUTEX(dev_pm_qos_mtx);
 static BLOCKING_NOTIFIER_HEAD(dev_pm_notifiers);
 
 /**
- * dev_pm_qos_read_value - Get PM QoS constraint for a given device.
+ * __dev_pm_qos_read_value - Get PM QoS constraint for a given device.
+ * @dev: Device to get the PM QoS constraint value for.
+ *
+ * This routine must be called with dev->power.lock held.
+ */
+s32 __dev_pm_qos_read_value(struct device *dev)
+{
+	struct pm_qos_constraints *c = dev->power.constraints;
+
+	return c ? pm_qos_read_value(c) : 0;
+}
+
+/**
+ * dev_pm_qos_read_value - Get PM QoS constraint for a given device (locked).
  * @dev: Device to get the PM QoS constraint value for.
  */
 s32 dev_pm_qos_read_value(struct device *dev)
 {
-	struct pm_qos_constraints *c;
 	unsigned long flags;
-	s32 ret = 0;
+	s32 ret;
 
 	spin_lock_irqsave(&dev->power.lock, flags);
-
-	c = dev->power.constraints;
-	if (c)
-		ret = pm_qos_read_value(c);
-
+	ret = __dev_pm_qos_read_value(dev);
 	spin_unlock_irqrestore(&dev->power.lock, flags);
 
 	return ret;


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [update][PATCH 5/7] PM / Domains: Add device stop governor function (v4)
  2011-11-14  0:22 ` [update][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
                     ` (3 preceding siblings ...)
  2011-11-14  0:26   ` [update][PATCH 4/7] PM / Runtime: Use device PM QoS constraints Rafael J. Wysocki
@ 2011-11-14  0:27   ` Rafael J. Wysocki
  2011-11-14  0:27   ` [update][PATCH 6/7] PM / Domains: Add default power off " Rafael J. Wysocki
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-14  0:27 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

Add a function deciding whether or not devices should be stopped in
pm_genpd_runtime_suspend() depending on their PM QoS constraints
and stop/start timing values.  Make it possible to add information
used by this function to device objects.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/mach-shmobile/pm-sh7372.c   |    4 +-
 drivers/base/power/Makefile          |    2 -
 drivers/base/power/domain.c          |   33 ++++++++++++++----
 drivers/base/power/domain_governor.c |   33 ++++++++++++++++++
 include/linux/pm_domain.h            |   63 ++++++++++++++++++++++++++++++-----
 5 files changed, 118 insertions(+), 17 deletions(-)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -21,6 +21,7 @@ enum gpd_status {
 
 struct dev_power_governor {
 	bool (*power_down_ok)(struct dev_pm_domain *domain);
+	bool (*stop_ok)(struct device *dev);
 };
 
 struct gpd_dev_ops {
@@ -72,9 +73,16 @@ struct gpd_link {
 	struct list_head slave_node;
 };
 
+struct gpd_timing_data {
+	s64 stop_latency_ns;
+	s64 start_latency_ns;
+	s64 break_even_ns;
+};
+
 struct generic_pm_domain_data {
 	struct pm_domain_data base;
 	struct gpd_dev_ops ops;
+	struct gpd_timing_data td;
 	bool need_restore;
 };
 
@@ -89,20 +97,48 @@ static inline struct generic_pm_domain_d
 }
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS
-extern int pm_genpd_add_device(struct generic_pm_domain *genpd,
-			       struct device *dev);
+extern struct dev_power_governor simple_qos_governor;
+
+extern struct generic_pm_domain *dev_to_genpd(struct device *dev);
+extern int __pm_genpd_add_device(struct generic_pm_domain *genpd,
+				 struct device *dev,
+				 struct gpd_timing_data *td);
+
+static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
+				      struct device *dev)
+{
+	return __pm_genpd_add_device(genpd, dev, NULL);
+}
+
 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 int pm_genpd_add_callbacks(struct device *dev, struct gpd_dev_ops *ops);
-extern int pm_genpd_remove_callbacks(struct device *dev);
+extern int pm_genpd_add_callbacks(struct device *dev,
+				  struct gpd_dev_ops *ops,
+				  struct gpd_timing_data *td);
+extern int __pm_genpd_remove_callbacks(struct device *dev, bool clear_td);
 extern void pm_genpd_init(struct generic_pm_domain *genpd,
 			  struct dev_power_governor *gov, bool is_off);
+
 extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
+
+extern bool default_stop_ok(struct device *dev);
+
 #else
+
+static inline struct generic_pm_domain *dev_to_genpd(struct device *dev)
+{
+	return ERR_PTR(-ENOSYS);
+}
+static inline int __pm_genpd_add_device(struct generic_pm_domain *genpd,
+					struct device *dev,
+					struct gpd_timing_data *td)
+{
+	return -ENOSYS;
+}
 static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
 				      struct device *dev)
 {
@@ -124,22 +160,33 @@ static inline int pm_genpd_remove_subdom
 	return -ENOSYS;
 }
 static inline int pm_genpd_add_callbacks(struct device *dev,
-					 struct gpd_dev_ops *ops)
+					 struct gpd_dev_ops *ops,
+					 struct gpd_timing_data *td)
 {
 	return -ENOSYS;
 }
-static inline int pm_genpd_remove_callbacks(struct device *dev)
+static inline int __pm_genpd_remove_callbacks(struct device *dev, bool clear_td)
 {
 	return -ENOSYS;
 }
-static inline void pm_genpd_init(struct generic_pm_domain *genpd,
-				 struct dev_power_governor *gov, bool is_off) {}
+static inline void pm_genpd_init(struct generic_pm_domain *genpd, bool is_off)
+{
+}
 static inline int pm_genpd_poweron(struct generic_pm_domain *genpd)
 {
 	return -ENOSYS;
 }
+static inline bool default_stop_ok(struct device *dev)
+{
+	return false;
+}
 #endif
 
+static inline int pm_genpd_remove_callbacks(struct device *dev)
+{
+	return __pm_genpd_remove_callbacks(dev, true);
+}
+
 #ifdef CONFIG_PM_GENERIC_DOMAINS_RUNTIME
 extern void genpd_queue_power_off_work(struct generic_pm_domain *genpd);
 extern void pm_genpd_poweroff_unused(void);
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -38,7 +38,7 @@ static DEFINE_MUTEX(gpd_list_lock);
 
 #ifdef CONFIG_PM
 
-static struct generic_pm_domain *dev_to_genpd(struct device *dev)
+struct generic_pm_domain *dev_to_genpd(struct device *dev)
 {
 	if (IS_ERR_OR_NULL(dev->pm_domain))
 		return ERR_PTR(-EINVAL);
@@ -436,6 +436,7 @@ static void genpd_power_off_work_fn(stru
 static int pm_genpd_runtime_suspend(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
+	bool (*stop_ok)(struct device *__dev);
 	int ret;
 
 	dev_dbg(dev, "%s()\n", __func__);
@@ -446,10 +447,17 @@ static int pm_genpd_runtime_suspend(stru
 
 	might_sleep_if(!genpd->dev_irq_safe);
 
+	stop_ok = genpd->gov ? genpd->gov->stop_ok : NULL;
+	if (stop_ok && !stop_ok(dev))
+		return -EBUSY;
+
 	ret = genpd_stop_dev(genpd, dev);
 	if (ret)
 		return ret;
 
+	pm_runtime_update_max_time_suspended(dev,
+				dev_gpd_data(dev)->td.start_latency_ns);
+
 	/*
 	 * If power.irq_safe is set, this routine will be run with interrupts
 	 * off, so it can't use mutexes.
@@ -1022,11 +1030,13 @@ static void pm_genpd_complete(struct dev
 #endif /* CONFIG_PM_SLEEP */
 
 /**
- * pm_genpd_add_device - Add a device to an I/O PM domain.
+ * __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.
+ * @td: Set of PM QoS timing parameters to attach to the device.
  */
-int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
+int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
+			  struct gpd_timing_data *td)
 {
 	struct generic_pm_domain_data *gpd_data;
 	struct pm_domain_data *pdd;
@@ -1069,6 +1079,8 @@ int pm_genpd_add_device(struct generic_p
 	gpd_data->base.dev = dev;
 	gpd_data->need_restore = false;
 	list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);
+	if (td)
+		gpd_data->td = *td;
 
  out:
 	genpd_release_lock(genpd);
@@ -1229,8 +1241,10 @@ int pm_genpd_remove_subdomain(struct gen
  * pm_genpd_add_callbacks - Add PM domain callbacks to a given device.
  * @dev: Device to add the callbacks to.
  * @ops: Set of callbacks to add.
+ * @td: Timing data to add to the device along with the callbacks (optional).
  */
-int pm_genpd_add_callbacks(struct device *dev, struct gpd_dev_ops *ops)
+int pm_genpd_add_callbacks(struct device *dev, struct gpd_dev_ops *ops,
+			   struct gpd_timing_data *td)
 {
 	struct pm_domain_data *pdd;
 	int ret = 0;
@@ -1246,6 +1260,8 @@ int pm_genpd_add_callbacks(struct device
 		struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
 
 		gpd_data->ops = *ops;
+		if (td)
+			gpd_data->td = *td;
 	} else {
 		ret = -EINVAL;
 	}
@@ -1258,10 +1274,11 @@ int pm_genpd_add_callbacks(struct device
 EXPORT_SYMBOL_GPL(pm_genpd_add_callbacks);
 
 /**
- * pm_genpd_remove_callbacks - Remove PM domain callbacks from a given device.
+ * __pm_genpd_remove_callbacks - Remove PM domain callbacks from a given device.
  * @dev: Device to remove the callbacks from.
+ * @clear_td: If set, clear the device's timing data too.
  */
-int pm_genpd_remove_callbacks(struct device *dev)
+int __pm_genpd_remove_callbacks(struct device *dev, bool clear_td)
 {
 	struct pm_domain_data *pdd;
 	int ret = 0;
@@ -1277,6 +1294,8 @@ int pm_genpd_remove_callbacks(struct dev
 		struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
 
 		gpd_data->ops = (struct gpd_dev_ops){ 0 };
+		if (clear_td)
+			gpd_data->td = (struct gpd_timing_data){ 0 };
 	} else {
 		ret = -EINVAL;
 	}
@@ -1286,7 +1305,7 @@ int pm_genpd_remove_callbacks(struct dev
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(pm_genpd_remove_callbacks);
+EXPORT_SYMBOL_GPL(__pm_genpd_remove_callbacks);
 
 /* Default device callbacks for generic PM domains. */
 
Index: linux/drivers/base/power/Makefile
===================================================================
--- linux.orig/drivers/base/power/Makefile
+++ linux/drivers/base/power/Makefile
@@ -3,7 +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_PM_GENERIC_DOMAINS)	+=  domain.o domain_governor.o
 obj-$(CONFIG_HAVE_CLK)	+= clock_ops.o
 
 ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
Index: linux/drivers/base/power/domain_governor.c
===================================================================
--- /dev/null
+++ linux/drivers/base/power/domain_governor.c
@@ -0,0 +1,33 @@
+/*
+ * drivers/base/power/domain_governor.c - Governors for device PM domains.
+ *
+ * Copyright (C) 2011 Rafael J. Wysocki <rjw@sisk.pl>, Renesas Electronics Corp.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_qos.h>
+
+/**
+ * default_stop_ok - Default PM domain governor routine for stopping devices.
+ * @dev: Device to check.
+ */
+bool default_stop_ok(struct device *dev)
+{
+	struct gpd_timing_data *td = &dev_gpd_data(dev)->td;
+
+	dev_dbg(dev, "%s()\n", __func__);
+
+	if (dev->power.max_time_suspended_ns < 0 || td->break_even_ns == 0)
+		return true;
+
+	return td->stop_latency_ns + td->start_latency_ns < td->break_even_ns
+		&& td->break_even_ns < dev->power.max_time_suspended_ns;
+}
+
+struct dev_power_governor simple_qos_governor = {
+	.stop_ok = default_stop_ok,
+};
Index: linux/arch/arm/mach-shmobile/pm-sh7372.c
===================================================================
--- linux.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux/arch/arm/mach-shmobile/pm-sh7372.c
@@ -169,6 +169,7 @@ static bool sh7372_power_down_forbidden(
 
 struct dev_power_governor sh7372_always_on_gov = {
 	.power_down_ok = sh7372_power_down_forbidden,
+	.stop_ok = default_stop_ok,
 };
 
 static int sh7372_stop_dev(struct device *dev)
@@ -203,8 +204,9 @@ static int sh7372_start_dev(struct devic
 void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd)
 {
 	struct generic_pm_domain *genpd = &sh7372_pd->genpd;
+	struct dev_power_governor *gov = sh7372_pd->gov;
 
-	pm_genpd_init(genpd, sh7372_pd->gov, false);
+	pm_genpd_init(genpd, gov ? : &simple_qos_governor, false);
 	genpd->dev_ops.stop = sh7372_stop_dev;
 	genpd->dev_ops.start = sh7372_start_dev;
 	genpd->dev_ops.active_wakeup = pd_active_wakeup;


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [update][PATCH 6/7] PM / Domains: Add default power off governor function (v4)
  2011-11-14  0:22 ` [update][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
                     ` (4 preceding siblings ...)
  2011-11-14  0:27   ` [update][PATCH 5/7] PM / Domains: Add device stop governor function (v4) Rafael J. Wysocki
@ 2011-11-14  0:27   ` Rafael J. Wysocki
  2011-11-14  0:28   ` [update][PATCH 7/7] PM / Domains: Automatically update overoptimistic latency information Rafael J. Wysocki
  2011-11-19 13:56   ` [Update 2x][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
  7 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-14  0:27 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

Add a function deciding whether or not a given PM domain should
be powered off on the basis of the PM QoS constraints of devices
belonging to it and their PM QoS timing data.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c          |   12 +++
 drivers/base/power/domain_governor.c |  110 +++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h            |    7 ++
 3 files changed, 129 insertions(+)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -57,8 +57,13 @@ struct generic_pm_domain {
 	bool suspend_power_off;	/* Power status before system suspend */
 	bool dev_irq_safe;	/* Device callbacks are IRQ-safe */
 	int (*power_off)(struct generic_pm_domain *domain);
+	s64 power_off_latency_ns;
 	int (*power_on)(struct generic_pm_domain *domain);
+	s64 power_on_latency_ns;
 	struct gpd_dev_ops dev_ops;
+	s64 break_even_ns;	/* Power break even for the entire domain. */
+	s64 max_off_time_ns;	/* Maximum allowed "suspended" time. */
+	ktime_t power_off_time;
 };
 
 static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
@@ -76,6 +81,8 @@ struct gpd_link {
 struct gpd_timing_data {
 	s64 stop_latency_ns;
 	s64 start_latency_ns;
+	s64 save_state_latency_ns;
+	s64 restore_state_latency_ns;
 	s64 break_even_ns;
 };
 
Index: linux/drivers/base/power/domain_governor.c
===================================================================
--- linux.orig/drivers/base/power/domain_governor.c
+++ linux/drivers/base/power/domain_governor.c
@@ -10,6 +10,7 @@
 #include <linux/kernel.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_qos.h>
+#include <linux/hrtimer.h>
 
 /**
  * default_stop_ok - Default PM domain governor routine for stopping devices.
@@ -28,6 +29,115 @@ bool default_stop_ok(struct device *dev)
 		&& td->break_even_ns < dev->power.max_time_suspended_ns;
 }
 
+/**
+ * default_power_down_ok - Default generic PM domain power off governor routine.
+ * @pd: PM domain to check.
+ *
+ * This routine must be executed under the PM domain's lock.
+ */
+static bool default_power_down_ok(struct dev_pm_domain *pd)
+{
+	struct generic_pm_domain *genpd = pd_to_genpd(pd);
+	struct gpd_link *link;
+	struct pm_domain_data *pdd;
+	s64 min_dev_off_time_ns;
+	s64 off_on_time_ns;
+	ktime_t time_now = ktime_get();
+
+	off_on_time_ns = genpd->power_off_latency_ns +
+				genpd->power_on_latency_ns;
+	/*
+	 * It doesn't make sense to remove power from the domain if saving
+	 * the state of all devices in it and the power off/power on operations
+	 * take too much time.
+	 *
+	 * All devices in this domain have been stopped already at this point.
+	 */
+	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
+		if (pdd->dev->driver)
+			off_on_time_ns +=
+				to_gpd_data(pdd)->td.save_state_latency_ns;
+	}
+
+	/*
+	 * Check if subdomains can be off for enough time.
+	 *
+	 * All subdomains have been powered off already at this point.
+	 */
+	list_for_each_entry(link, &genpd->master_links, master_node) {
+		struct generic_pm_domain *sd = link->slave;
+		s64 sd_max_off_ns = sd->max_off_time_ns;
+
+		if (sd_max_off_ns < 0)
+			continue;
+
+		sd_max_off_ns -= ktime_to_ns(ktime_sub(time_now,
+						       sd->power_off_time));
+		/*
+		 * Check if the subdomain is allowed to be off long enough for
+		 * the current domain to turn off and on (that's how much time
+		 * it will have to wait worst case).
+		 */
+		if (sd_max_off_ns <= off_on_time_ns)
+			return false;
+	}
+
+	/*
+	 * Check if the devices in the domain can be off enough time.
+	 */
+	min_dev_off_time_ns = -1;
+	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
+		struct gpd_timing_data *td;
+		struct device *dev = pdd->dev;
+		s64 dev_off_time_ns;
+
+		if (!dev->driver || dev->power.max_time_suspended_ns < 0)
+			continue;
+
+		td = &to_gpd_data(pdd)->td;
+		dev_off_time_ns = dev->power.max_time_suspended_ns -
+			(td->start_latency_ns + td->restore_state_latency_ns +
+				ktime_to_ns(ktime_sub(time_now,
+						dev->power.suspend_time)));
+		if (dev_off_time_ns <= off_on_time_ns)
+			return false;
+
+		if (min_dev_off_time_ns > dev_off_time_ns
+		    || min_dev_off_time_ns < 0)
+			min_dev_off_time_ns = dev_off_time_ns;
+	}
+
+	if (min_dev_off_time_ns < 0) {
+		/*
+		 * There are no latency constraints, so the domain can spend
+		 * arbitrary time in the "off" state.
+		 */
+		genpd->max_off_time_ns = -1;
+		return true;
+	}
+
+	/*
+	 * The difference between the computed minimum delta and the time needed
+	 * to turn the domain on is the maximum theoretical time this domain can
+	 * spend in the "off" state.
+	 */
+	min_dev_off_time_ns -= genpd->power_on_latency_ns;
+
+	/*
+	 * If the difference between the computed minimum delta and the time
+	 * needed to turn the domain off and back on on is smaller than the
+	 * domain's power break even time, removing power from the domain is not
+	 * worth it.
+	 */
+	if (genpd->break_even_ns >
+	    min_dev_off_time_ns - genpd->power_off_latency_ns)
+		return false;
+
+	genpd->max_off_time_ns = min_dev_off_time_ns;
+	return true;
+}
+
 struct dev_power_governor simple_qos_governor = {
 	.stop_ok = default_stop_ok,
+	.power_down_ok = default_power_down_ok,
 };
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -398,6 +398,17 @@ static int pm_genpd_poweroff(struct gene
 	}
 
 	genpd->status = GPD_STATE_POWER_OFF;
+	genpd->power_off_time = ktime_get();
+
+	/* Update PM QoS information for devices in the domain. */
+	list_for_each_entry_reverse(pdd, &genpd->dev_list, list_node) {
+		struct gpd_timing_data *td = &to_gpd_data(pdd)->td;
+
+		pm_runtime_update_max_time_suspended(pdd->dev,
+					td->start_latency_ns +
+					td->restore_state_latency_ns +
+					genpd->power_on_latency_ns);
+	}
 
 	list_for_each_entry(link, &genpd->slave_links, slave_node) {
 		genpd_sd_counter_dec(link->master);
@@ -1417,6 +1428,7 @@ void pm_genpd_init(struct generic_pm_dom
 	genpd->resume_count = 0;
 	genpd->device_count = 0;
 	genpd->suspended_count = 0;
+	genpd->max_off_time_ns = -1;
 	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;


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [update][PATCH 7/7] PM / Domains: Automatically update overoptimistic latency information
  2011-11-14  0:22 ` [update][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
                     ` (5 preceding siblings ...)
  2011-11-14  0:27   ` [update][PATCH 6/7] PM / Domains: Add default power off " Rafael J. Wysocki
@ 2011-11-14  0:28   ` Rafael J. Wysocki
  2011-11-19 13:56   ` [Update 2x][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
  7 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-14  0:28 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

Measure the time of execution of the .stop(), .start(), .save_state()
and .restore_state() PM domain device callbacks and if the result
is greater than the corresponding latency value stored in the
device's struct generic_pm_domain_data object, replace the inaccurate
value with the measured time.

Do analogously for the PM domains' .power_off() and .power_off()
callbacks.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |   39 +++++++++++++++++++++++++++++++++++----
 1 file changed, 35 insertions(+), 4 deletions(-)

Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -33,6 +33,17 @@
 	__ret;							\
 })
 
+#define GENPD_DEV_TIMED_CALLBACK(genpd, type, callback, dev, latency)	\
+({									\
+	ktime_t __start = ktime_get();					\
+	type __retval = GENPD_DEV_CALLBACK(genpd, type, callback, dev);	\
+	s64 __elapsed = ktime_to_ns(ktime_sub(ktime_get(), __start));	\
+	struct generic_pm_domain_data *__gpd_data = dev_gpd_data(dev);	\
+	if (__elapsed > __gpd_data->td.latency)				\
+		__gpd_data->td.latency = __elapsed;			\
+	__retval;							\
+})
+
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
 
@@ -48,22 +59,26 @@ struct generic_pm_domain *dev_to_genpd(s
 
 static int genpd_stop_dev(struct generic_pm_domain *genpd, struct device *dev)
 {
-	return GENPD_DEV_CALLBACK(genpd, int, stop, dev);
+	return GENPD_DEV_TIMED_CALLBACK(genpd, int, stop, dev,
+					stop_latency_ns);
 }
 
 static int genpd_start_dev(struct generic_pm_domain *genpd, struct device *dev)
 {
-	return GENPD_DEV_CALLBACK(genpd, int, start, dev);
+	return GENPD_DEV_TIMED_CALLBACK(genpd, int, start, dev,
+					start_latency_ns);
 }
 
 static int genpd_save_dev(struct generic_pm_domain *genpd, struct device *dev)
 {
-	return GENPD_DEV_CALLBACK(genpd, int, save_state, dev);
+	return GENPD_DEV_TIMED_CALLBACK(genpd, int, save_state, dev,
+					save_state_latency_ns);
 }
 
 static int genpd_restore_dev(struct generic_pm_domain *genpd, struct device *dev)
 {
-	return GENPD_DEV_CALLBACK(genpd, int, restore_state, dev);
+	return GENPD_DEV_TIMED_CALLBACK(genpd, int, restore_state, dev,
+					restore_state_latency_ns);
 }
 
 static bool genpd_sd_counter_dec(struct generic_pm_domain *genpd)
@@ -182,9 +197,16 @@ int __pm_genpd_poweron(struct generic_pm
 	}
 
 	if (genpd->power_on) {
+		ktime_t time_start = ktime_get();
+		s64 elapsed_ns;
+
 		ret = genpd->power_on(genpd);
 		if (ret)
 			goto err;
+
+		elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
+		if (elapsed_ns > genpd->power_on_latency_ns)
+			genpd->power_on_latency_ns = elapsed_ns;
 	}
 
 	genpd_set_active(genpd);
@@ -377,11 +399,16 @@ static int pm_genpd_poweroff(struct gene
 	}
 
 	if (genpd->power_off) {
+		ktime_t time_start;
+		s64 elapsed_ns;
+
 		if (atomic_read(&genpd->sd_count) > 0) {
 			ret = -EBUSY;
 			goto out;
 		}
 
+		time_start = ktime_get();
+
 		/*
 		 * If sd_count > 0 at this point, one of the subdomains hasn't
 		 * managed to call pm_genpd_poweron() for the master yet after
@@ -395,6 +422,10 @@ static int pm_genpd_poweroff(struct gene
 			genpd_set_active(genpd);
 			goto out;
 		}
+
+		elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
+		if (elapsed_ns > genpd->power_off_latency_ns)
+			genpd->power_off_latency_ns = elapsed_ns;
 	}
 
 	genpd->status = GPD_STATE_POWER_OFF;


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Update 2x][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS
  2011-11-14  0:22 ` [update][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
                     ` (6 preceding siblings ...)
  2011-11-14  0:28   ` [update][PATCH 7/7] PM / Domains: Automatically update overoptimistic latency information Rafael J. Wysocki
@ 2011-11-19 13:56   ` Rafael J. Wysocki
  2011-11-19 13:58     ` [Update 2x][PATCH 1/7] PM / Domains: Make it possible to use per-device domain callbacks Rafael J. Wysocki
                       ` (6 more replies)
  7 siblings, 7 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-19 13:56 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

Hi,

This is the third iteration of the patchset introducing per-device callbacks
and PM QoS support to PM domains.

All patches have been updated due to minor problems found since the previous
iteration.  Patch [7/7] now prints diagnostic messages when updating the
latency values.

On Monday, November 14, 2011, Rafael J. Wysocki wrote:
[...] 
> [1/7] - Make it possible to define per-device start/stop and .active_wakeup()
>         routines.
> 
> [2/7] - Introduce "save/restore state" device callbacks.
> 
> [3/7] - Introduce per-device PM domain callbacks for system suspend.
> 
> [4/7] - Make runtime PM core take device PM QoS constraints into account.
> 
> [5/7] - Add device "stop governor".
> 
> [6/7] - Add domain "power off governor".
> 
> [7/7] - Automatically update overoptimistic latency information.
> 
> The patchset have been tested for backwards compatibility, but it still needs
> some testing for the new features it adds.

If there are no objections, I'm going to queue up this patchset in linux-next
for the 3.3 merge window.

Thanks,
Rafael


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Update 2x][PATCH 1/7] PM / Domains: Make it possible to use per-device domain callbacks
  2011-11-19 13:56   ` [Update 2x][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
@ 2011-11-19 13:58     ` Rafael J. Wysocki
  2011-11-19 13:59     ` [Update 2x][PATCH 2/7] PM / Domains: Introduce "save/restore state" device callbacks Rafael J. Wysocki
                       ` (5 subsequent siblings)
  6 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-19 13:58 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

The current generic PM domains code requires that the same .stop(),
.start() and .active_wakeup() device callback routines be used for
all devices in the given domain, which is inflexible and may not
cover some specific use cases.  For this reason, make it possible to
use device specific .start()/.stop() and .active_wakeup() callback
routines by adding corresponding callback pointers to struct
generic_pm_domain_data.  Add a new helper routine,
pm_genpd_register_callbacks(), that can be used to populate
the new per-device callback pointers.

Modify the shmobile's power domains code to allow drivers to add
their own code to be run during the device stop and start operations
with the help of the new callback pointers.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/mach-shmobile/pm-sh7372.c |   40 ++++++++-
 drivers/base/power/domain.c        |  152 +++++++++++++++++++++++++++----------
 include/linux/pm_domain.h          |   27 +++++-
 3 files changed, 175 insertions(+), 44 deletions(-)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -23,6 +23,12 @@ struct dev_power_governor {
 	bool (*power_down_ok)(struct dev_pm_domain *domain);
 };
 
+struct gpd_dev_ops {
+	int (*start)(struct device *dev);
+	int (*stop)(struct device *dev);
+	bool (*active_wakeup)(struct device *dev);
+};
+
 struct generic_pm_domain {
 	struct dev_pm_domain domain;	/* PM domain operations */
 	struct list_head gpd_list_node;	/* Node in the global PM domains list */
@@ -45,9 +51,7 @@ struct generic_pm_domain {
 	bool dev_irq_safe;	/* Device callbacks are IRQ-safe */
 	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);
-	bool (*active_wakeup)(struct device *dev);
+	struct gpd_dev_ops dev_ops;
 };
 
 static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
@@ -64,6 +68,7 @@ struct gpd_link {
 
 struct generic_pm_domain_data {
 	struct pm_domain_data base;
+	struct gpd_dev_ops ops;
 	bool need_restore;
 };
 
@@ -73,6 +78,11 @@ static inline struct generic_pm_domain_d
 }
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS
+static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)
+{
+	return to_gpd_data(dev->power.subsys_data->domain_data);
+}
+
 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,
@@ -81,6 +91,8 @@ extern int pm_genpd_add_subdomain(struct
 				  struct generic_pm_domain *new_subdomain);
 extern int pm_genpd_remove_subdomain(struct generic_pm_domain *genpd,
 				     struct generic_pm_domain *target);
+extern int pm_genpd_add_callbacks(struct device *dev, struct gpd_dev_ops *ops);
+extern int pm_genpd_remove_callbacks(struct device *dev);
 extern void pm_genpd_init(struct generic_pm_domain *genpd,
 			  struct dev_power_governor *gov, bool is_off);
 extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
@@ -105,6 +117,15 @@ static inline int pm_genpd_remove_subdom
 {
 	return -ENOSYS;
 }
+static inline int pm_genpd_add_callbacks(struct device *dev,
+					 struct gpd_dev_ops *ops)
+{
+	return -ENOSYS;
+}
+static inline int pm_genpd_remove_callbacks(struct device *dev)
+{
+	return -ENOSYS;
+}
 static inline void pm_genpd_init(struct generic_pm_domain *genpd,
 				 struct dev_power_governor *gov, bool is_off) {}
 static inline int pm_genpd_poweron(struct generic_pm_domain *genpd)
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -15,6 +15,23 @@
 #include <linux/err.h>
 #include <linux/sched.h>
 #include <linux/suspend.h>
+#include <linux/export.h>
+
+#define GENPD_DEV_CALLBACK(genpd, type, callback, dev)		\
+({								\
+	type (*__routine)(struct device *__d); 			\
+	type __ret = (type)0;					\
+								\
+	__routine = genpd->dev_ops.callback; 			\
+	if (__routine) {					\
+		__ret = __routine(dev); 			\
+	} else {						\
+		__routine = dev_gpd_data(dev)->ops.callback;	\
+		if (__routine) 					\
+			__ret = __routine(dev);			\
+	}							\
+	__ret;							\
+})
 
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
@@ -29,6 +46,16 @@ static struct generic_pm_domain *dev_to_
 	return pd_to_genpd(dev->pm_domain);
 }
 
+static int genpd_stop_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, stop, dev);
+}
+
+static int genpd_start_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, start, dev);
+}
+
 static bool genpd_sd_counter_dec(struct generic_pm_domain *genpd)
 {
 	bool ret = false;
@@ -199,13 +226,9 @@ static int __pm_genpd_save_device(struct
 	mutex_unlock(&genpd->lock);
 
 	if (drv && drv->pm && drv->pm->runtime_suspend) {
-		if (genpd->start_device)
-			genpd->start_device(dev);
-
+		genpd_start_dev(genpd, dev);
 		ret = drv->pm->runtime_suspend(dev);
-
-		if (genpd->stop_device)
-			genpd->stop_device(dev);
+		genpd_stop_dev(genpd, dev);
 	}
 
 	mutex_lock(&genpd->lock);
@@ -235,13 +258,9 @@ static void __pm_genpd_restore_device(st
 	mutex_unlock(&genpd->lock);
 
 	if (drv && drv->pm && drv->pm->runtime_resume) {
-		if (genpd->start_device)
-			genpd->start_device(dev);
-
+		genpd_start_dev(genpd, dev);
 		drv->pm->runtime_resume(dev);
-
-		if (genpd->stop_device)
-			genpd->stop_device(dev);
+		genpd_stop_dev(genpd, dev);
 	}
 
 	mutex_lock(&genpd->lock);
@@ -413,6 +432,7 @@ static void genpd_power_off_work_fn(stru
 static int pm_genpd_runtime_suspend(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
+	int ret;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
@@ -422,11 +442,9 @@ static int pm_genpd_runtime_suspend(stru
 
 	might_sleep_if(!genpd->dev_irq_safe);
 
-	if (genpd->stop_device) {
-		int ret = genpd->stop_device(dev);
-		if (ret)
-			return ret;
-	}
+	ret = genpd_stop_dev(genpd, dev);
+	if (ret)
+		return ret;
 
 	/*
 	 * If power.irq_safe is set, this routine will be run with interrupts
@@ -502,8 +520,7 @@ static int pm_genpd_runtime_resume(struc
 	mutex_unlock(&genpd->lock);
 
  out:
-	if (genpd->start_device)
-		genpd->start_device(dev);
+	genpd_start_dev(genpd, dev);
 
 	return 0;
 }
@@ -534,6 +551,12 @@ static inline void genpd_power_off_work_
 
 #ifdef CONFIG_PM_SLEEP
 
+static bool genpd_dev_active_wakeup(struct generic_pm_domain *genpd,
+				    struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, bool, active_wakeup, dev);
+}
+
 /**
  * pm_genpd_sync_poweroff - Synchronously power off a PM domain and its masters.
  * @genpd: PM domain to power off, if possible.
@@ -590,7 +613,7 @@ static bool resume_needed(struct device
 	if (!device_can_wakeup(dev))
 		return false;
 
-	active_wakeup = genpd->active_wakeup && genpd->active_wakeup(dev);
+	active_wakeup = genpd_dev_active_wakeup(genpd, dev);
 	return device_may_wakeup(dev) ? active_wakeup : !active_wakeup;
 }
 
@@ -646,7 +669,7 @@ static int pm_genpd_prepare(struct devic
 	/*
 	 * The PM domain must be in the GPD_STATE_ACTIVE state at this point,
 	 * so pm_genpd_poweron() will return immediately, but if the device
-	 * is suspended (e.g. it's been stopped by .stop_device()), we need
+	 * is suspended (e.g. it's been stopped by genpd_stop_dev()), we need
 	 * to make it operational.
 	 */
 	pm_runtime_resume(dev);
@@ -714,12 +737,10 @@ static int pm_genpd_suspend_noirq(struct
 	if (ret)
 		return ret;
 
-	if (dev->power.wakeup_path
-	    && genpd->active_wakeup && genpd->active_wakeup(dev))
+	if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
 		return 0;
 
-	if (genpd->stop_device)
-		genpd->stop_device(dev);
+	genpd_stop_dev(genpd, dev);
 
 	/*
 	 * Since all of the "noirq" callbacks are executed sequentially, it is
@@ -761,8 +782,7 @@ static int pm_genpd_resume_noirq(struct
 	 */
 	pm_genpd_poweron(genpd);
 	genpd->suspended_count--;
-	if (genpd->start_device)
-		genpd->start_device(dev);
+	genpd_start_dev(genpd, dev);
 
 	return pm_generic_resume_noirq(dev);
 }
@@ -836,8 +856,7 @@ static int pm_genpd_freeze_noirq(struct
 	if (ret)
 		return ret;
 
-	if (genpd->stop_device)
-		genpd->stop_device(dev);
+	genpd_stop_dev(genpd, dev);
 
 	return 0;
 }
@@ -864,8 +883,7 @@ static int pm_genpd_thaw_noirq(struct de
 	if (genpd->suspend_power_off)
 		return 0;
 
-	if (genpd->start_device)
-		genpd->start_device(dev);
+	genpd_start_dev(genpd, dev);
 
 	return pm_generic_thaw_noirq(dev);
 }
@@ -938,12 +956,10 @@ static int pm_genpd_dev_poweroff_noirq(s
 	if (ret)
 		return ret;
 
-	if (dev->power.wakeup_path
-	    && genpd->active_wakeup && genpd->active_wakeup(dev))
+	if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
 		return 0;
 
-	if (genpd->stop_device)
-		genpd->stop_device(dev);
+	genpd_stop_dev(genpd, dev);
 
 	/*
 	 * Since all of the "noirq" callbacks are executed sequentially, it is
@@ -993,8 +1009,7 @@ static int pm_genpd_restore_noirq(struct
 
 	pm_genpd_poweron(genpd);
 	genpd->suspended_count--;
-	if (genpd->start_device)
-		genpd->start_device(dev);
+	genpd_start_dev(genpd, dev);
 
 	return pm_generic_restore_noirq(dev);
 }
@@ -1280,6 +1295,69 @@ int pm_genpd_remove_subdomain(struct gen
 }
 
 /**
+ * pm_genpd_add_callbacks - Add PM domain callbacks to a given device.
+ * @dev: Device to add the callbacks to.
+ * @ops: Set of callbacks to add.
+ */
+int pm_genpd_add_callbacks(struct device *dev, struct gpd_dev_ops *ops)
+{
+	struct pm_domain_data *pdd;
+	int ret = 0;
+
+	if (!(dev && dev->power.subsys_data && ops))
+		return -EINVAL;
+
+	pm_runtime_disable(dev);
+	device_pm_lock();
+
+	pdd = dev->power.subsys_data->domain_data;
+	if (pdd) {
+		struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
+
+		gpd_data->ops = *ops;
+	} else {
+		ret = -EINVAL;
+	}
+
+	device_pm_unlock();
+	pm_runtime_enable(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pm_genpd_add_callbacks);
+
+/**
+ * pm_genpd_remove_callbacks - Remove PM domain callbacks from a given device.
+ * @dev: Device to remove the callbacks from.
+ */
+int pm_genpd_remove_callbacks(struct device *dev)
+{
+	struct pm_domain_data *pdd;
+	int ret = 0;
+
+	if (!(dev && dev->power.subsys_data))
+		return -EINVAL;
+
+	pm_runtime_disable(dev);
+	device_pm_lock();
+
+	pdd = dev->power.subsys_data->domain_data;
+	if (pdd) {
+		struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
+
+		gpd_data->ops = (struct gpd_dev_ops){ 0 };
+	} else {
+		ret = -EINVAL;
+	}
+
+	device_pm_unlock();
+	pm_runtime_enable(dev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(pm_genpd_remove_callbacks);
+
+/**
  * 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).
Index: linux/arch/arm/mach-shmobile/pm-sh7372.c
===================================================================
--- linux.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux/arch/arm/mach-shmobile/pm-sh7372.c
@@ -156,7 +156,10 @@ static void sh7372_a4r_suspend(void)
 
 static bool pd_active_wakeup(struct device *dev)
 {
-	return true;
+	bool (*active_wakeup)(struct device *dev);
+
+	active_wakeup = dev_gpd_data(dev)->ops.active_wakeup;
+	return active_wakeup ? active_wakeup(dev) : true;
 }
 
 static bool sh7372_power_down_forbidden(struct dev_pm_domain *domain)
@@ -168,15 +171,44 @@ struct dev_power_governor sh7372_always_
 	.power_down_ok = sh7372_power_down_forbidden,
 };
 
+static int sh7372_stop_dev(struct device *dev)
+{
+	int (*stop)(struct device *dev);
+
+	stop = dev_gpd_data(dev)->ops.stop;
+	if (stop) {
+		int ret = stop(dev);
+		if (ret)
+			return ret;
+	}
+	return pm_clk_suspend(dev);
+}
+
+static int sh7372_start_dev(struct device *dev)
+{
+	int (*start)(struct device *dev);
+	int ret;
+
+	ret = pm_clk_resume(dev);
+	if (ret)
+		return ret;
+
+	start = dev_gpd_data(dev)->ops.start;
+	if (start)
+		ret = start(dev);
+
+	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, sh7372_pd->gov, false);
-	genpd->stop_device = pm_clk_suspend;
-	genpd->start_device = pm_clk_resume;
+	genpd->dev_ops.stop = sh7372_stop_dev;
+	genpd->dev_ops.start = sh7372_start_dev;
+	genpd->dev_ops.active_wakeup = pd_active_wakeup;
 	genpd->dev_irq_safe = true;
-	genpd->active_wakeup = pd_active_wakeup;
 	genpd->power_off = pd_power_down;
 	genpd->power_on = pd_power_up;
 	__pd_power_up(sh7372_pd, false);

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Update 2x][PATCH 2/7] PM / Domains: Introduce "save/restore state" device callbacks
  2011-11-19 13:56   ` [Update 2x][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
  2011-11-19 13:58     ` [Update 2x][PATCH 1/7] PM / Domains: Make it possible to use per-device domain callbacks Rafael J. Wysocki
@ 2011-11-19 13:59     ` Rafael J. Wysocki
  2011-11-19 13:59     ` [Update 2x][PATCH 3/7] PM / Domains: Rework system suspend callback routines Rafael J. Wysocki
                       ` (4 subsequent siblings)
  6 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-19 13:59 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

The current PM domains code uses device drivers' .runtime_suspend()
and .runtime_resume() callbacks as the "save device state" and
"restore device state" operations, which may not be appropriate in
general, because it forces drivers to assume that they always will
be used with generic PM domains.  However, in theory, the same
hardware may be used in devices that don't belong to any PM
domain, in which case it would be necessary to add "fake" PM
domains to satisfy the above assumption.  It also may be located in
a PM domain that's not handled with the help of the generic code.

To allow device drivers that may be used along with the generic PM
domains code of more flexibility, introduce new device callbacks,
.save_state() and .restore_state(), that can be supplied by the
drivers in addition to their "standard" runtime PM callbacks.  This
will allow the drivers to be designed to work with generic PM domains
as well as without them.

For backwards compatibility, introduce default .save_state() and
.restore_state() callback routines for PM domains that will execute
a device driver's .runtime_suspend() and .runtime_resume() callbacks,
respectively, for the given device if the driver doesn't provide its
own implementations of .save_state() and .restore_state().

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |   68 ++++++++++++++++++++++++++++++++++++--------
 include/linux/pm_domain.h   |    2 +
 2 files changed, 58 insertions(+), 12 deletions(-)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -26,6 +26,8 @@ struct dev_power_governor {
 struct gpd_dev_ops {
 	int (*start)(struct device *dev);
 	int (*stop)(struct device *dev);
+	int (*save_state)(struct device *dev);
+	int (*restore_state)(struct device *dev);
 	bool (*active_wakeup)(struct device *dev);
 };
 
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -56,6 +56,16 @@ static int genpd_start_dev(struct generi
 	return GENPD_DEV_CALLBACK(genpd, int, start, dev);
 }
 
+static int genpd_save_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, save_state, dev);
+}
+
+static int genpd_restore_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, restore_state, dev);
+}
+
 static bool genpd_sd_counter_dec(struct generic_pm_domain *genpd)
 {
 	bool ret = false;
@@ -217,7 +227,6 @@ static int __pm_genpd_save_device(struct
 {
 	struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
 	struct device *dev = pdd->dev;
-	struct device_driver *drv = dev->driver;
 	int ret = 0;
 
 	if (gpd_data->need_restore)
@@ -225,11 +234,9 @@ static int __pm_genpd_save_device(struct
 
 	mutex_unlock(&genpd->lock);
 
-	if (drv && drv->pm && drv->pm->runtime_suspend) {
-		genpd_start_dev(genpd, dev);
-		ret = drv->pm->runtime_suspend(dev);
-		genpd_stop_dev(genpd, dev);
-	}
+	genpd_start_dev(genpd, dev);
+	ret = genpd_save_dev(genpd, dev);
+	genpd_stop_dev(genpd, dev);
 
 	mutex_lock(&genpd->lock);
 
@@ -250,18 +257,15 @@ static void __pm_genpd_restore_device(st
 {
 	struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
 	struct device *dev = pdd->dev;
-	struct device_driver *drv = dev->driver;
 
 	if (!gpd_data->need_restore)
 		return;
 
 	mutex_unlock(&genpd->lock);
 
-	if (drv && drv->pm && drv->pm->runtime_resume) {
-		genpd_start_dev(genpd, dev);
-		drv->pm->runtime_resume(dev);
-		genpd_stop_dev(genpd, dev);
-	}
+	genpd_start_dev(genpd, dev);
+	genpd_restore_dev(genpd, dev);
+	genpd_stop_dev(genpd, dev);
 
 	mutex_lock(&genpd->lock);
 
@@ -1358,6 +1362,44 @@ int pm_genpd_remove_callbacks(struct dev
 EXPORT_SYMBOL_GPL(pm_genpd_remove_callbacks);
 
 /**
+ * pm_genpd_default_save_state - Default "save device state" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_save_state(struct device *dev)
+{
+	int (*cb)(struct device *__dev);
+	struct device_driver *drv = dev->driver;
+
+	cb = dev_gpd_data(dev)->ops.save_state;
+	if (cb)
+		return cb(dev);
+
+	if (drv && drv->pm && drv->pm->runtime_suspend)
+		return drv->pm->runtime_suspend(dev);
+
+	return 0;
+}
+
+/**
+ * pm_genpd_default_restore_state - Default PM domians "restore device state".
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_restore_state(struct device *dev)
+{
+	int (*cb)(struct device *__dev);
+	struct device_driver *drv = dev->driver;
+
+	cb = dev_gpd_data(dev)->ops.restore_state;
+	if (cb)
+		return cb(dev);
+
+	if (drv && drv->pm && drv->pm->runtime_resume)
+		return drv->pm->runtime_resume(dev);
+
+	return 0;
+}
+
+/**
  * 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).
@@ -1400,6 +1442,8 @@ void pm_genpd_init(struct generic_pm_dom
 	genpd->domain.ops.restore_noirq = pm_genpd_restore_noirq;
 	genpd->domain.ops.restore = pm_genpd_restore;
 	genpd->domain.ops.complete = pm_genpd_complete;
+	genpd->dev_ops.save_state = pm_genpd_default_save_state;
+	genpd->dev_ops.restore_state = pm_genpd_default_restore_state;
 	mutex_lock(&gpd_list_lock);
 	list_add(&genpd->gpd_list_node, &gpd_list);
 	mutex_unlock(&gpd_list_lock);


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Update 2x][PATCH 3/7] PM / Domains: Rework system suspend callback routines
  2011-11-19 13:56   ` [Update 2x][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
  2011-11-19 13:58     ` [Update 2x][PATCH 1/7] PM / Domains: Make it possible to use per-device domain callbacks Rafael J. Wysocki
  2011-11-19 13:59     ` [Update 2x][PATCH 2/7] PM / Domains: Introduce "save/restore state" device callbacks Rafael J. Wysocki
@ 2011-11-19 13:59     ` Rafael J. Wysocki
  2011-11-24  0:20       ` [Update 3x][PATCH 3/7] PM / Domains: Rework system suspend callback routines (v2) Rafael J. Wysocki
  2011-11-19 14:00     ` [Update 2x][PATCH 4/7] PM / Runtime: Use device PM QoS constraints Rafael J. Wysocki
                       ` (3 subsequent siblings)
  6 siblings, 1 reply; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-19 13:59 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

The current generic PM domains code attempts to use the generic
system suspend operations along with the domains' device stop/start
routines, which requires device drivers to assume that their
system suspend/resume (and hibernation/restore) callbacks will always
be used with generic PM domains.  However, in theory, the same
hardware may be used in devices that don't belong to any PM domain,
in which case it would be necessary to add "fake" PM domains to
satisfy the above assumption.  Also, the domain the hardware belongs
to may not be handled with the help of the generic code.

To allow device drivers that may be used along with the generic PM
domains code of more flexibility, add new device callbacks, .freeze(),
.freeze_late(), .thaw_early() and .thaw(), that can be supplied by
the drivers in addition to their "standard" system suspend and
hibernation callbacks.  These new callbacks, if defined, will be used
by the generic PM domains code for the handling of system suspend and
hibernation instead of the "standard" ones.  This will allow drivers
to be designed to work with generic PM domains as well as without
them.

For backwards compatibility, introduce default .freeze(), .thaw(),
.freeze_late() and .thaw_early() callback routines for PM domains that
will execute pm_generic_suspend(), pm_generic_resume(),
pm_generic_suspend_noirq() and pm_generic_resume_noirq(),
respectively, for the given device if its driver doesn't provide its
own implementations of .freeze(), .thaw(), .freeze_late() and
.thaw_early() (this works slightly differently from the current code,
but its existing users don't care anyway).

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |  189 +++++++++++++++++++-------------------------
 include/linux/pm_domain.h   |    4 
 2 files changed, 87 insertions(+), 106 deletions(-)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -28,6 +28,10 @@ struct gpd_dev_ops {
 	int (*stop)(struct device *dev);
 	int (*save_state)(struct device *dev);
 	int (*restore_state)(struct device *dev);
+	int (*freeze)(struct device *dev);
+	int (*freeze_late)(struct device *dev);
+	int (*thaw_early)(struct device *dev);
+	int (*thaw)(struct device *dev);
 	bool (*active_wakeup)(struct device *dev);
 };
 
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -561,6 +561,26 @@ static bool genpd_dev_active_wakeup(stru
 	return GENPD_DEV_CALLBACK(genpd, bool, active_wakeup, dev);
 }
 
+static int genpd_freeze_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, freeze, dev);
+}
+
+static int genpd_freeze_late(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, freeze_late, dev);
+}
+
+static int genpd_thaw_early(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, thaw_early, dev);
+}
+
+static int genpd_thaw_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, thaw, dev);
+}
+
 /**
  * pm_genpd_sync_poweroff - Synchronously power off a PM domain and its masters.
  * @genpd: PM domain to power off, if possible.
@@ -695,27 +715,6 @@ static int pm_genpd_prepare(struct devic
 }
 
 /**
- * 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.
  *
@@ -737,7 +736,7 @@ static int pm_genpd_suspend_noirq(struct
 	if (genpd->suspend_power_off)
 		return 0;
 
-	ret = pm_generic_suspend_noirq(dev);
+	ret = genpd_freeze_late(genpd, dev);
 	if (ret)
 		return ret;
 
@@ -788,28 +787,7 @@ static int pm_genpd_resume_noirq(struct
 	genpd->suspended_count--;
 	genpd_start_dev(genpd, dev);
 
-	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);
+	return genpd_thaw_early(genpd, dev);
 }
 
 /**
@@ -830,14 +808,14 @@ static int pm_genpd_freeze(struct device
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	return genpd->suspend_power_off ? 0 : pm_generic_freeze(dev);
+	return genpd->suspend_power_off ? 0 : genpd_freeze_dev(genpd, 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
+ * 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.
@@ -856,7 +834,7 @@ static int pm_genpd_freeze_noirq(struct
 	if (genpd->suspend_power_off)
 		return 0;
 
-	ret = pm_generic_freeze_noirq(dev);
+	ret = genpd_freeze_late(genpd, dev);
 	if (ret)
 		return ret;
 
@@ -869,7 +847,7 @@ static int pm_genpd_freeze_noirq(struct
  * 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
+ * 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.
@@ -889,7 +867,7 @@ static int pm_genpd_thaw_noirq(struct de
 
 	genpd_start_dev(genpd, dev);
 
-	return pm_generic_thaw_noirq(dev);
+	return genpd_thaw_early(genpd, dev);
 }
 
 /**
@@ -910,28 +888,7 @@ static int pm_genpd_thaw(struct device *
 	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);
+	return genpd->suspend_power_off ? 0 : genpd_thaw_dev(genpd, dev);
 }
 
 /**
@@ -945,7 +902,6 @@ static int pm_genpd_dev_poweroff(struct
 static int pm_genpd_dev_poweroff_noirq(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
-	int ret;
 
 	dev_dbg(dev, "%s()\n", __func__);
 
@@ -956,10 +912,6 @@ static int pm_genpd_dev_poweroff_noirq(s
 	if (genpd->suspend_power_off)
 		return 0;
 
-	ret = pm_generic_poweroff_noirq(dev);
-	if (ret)
-		return ret;
-
 	if (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
 		return 0;
 
@@ -1015,28 +967,7 @@ static int pm_genpd_restore_noirq(struct
 	genpd->suspended_count--;
 	genpd_start_dev(genpd, 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);
+	return genpd_thaw_early(genpd, dev);
 }
 
 /**
@@ -1078,18 +1009,14 @@ static void pm_genpd_complete(struct dev
 #else
 
 #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_dev_poweroff_noirq	NULL
-#define pm_genpd_dev_poweroff		NULL
 #define pm_genpd_restore_noirq		NULL
-#define pm_genpd_restore		NULL
 #define pm_genpd_complete		NULL
 
 #endif /* CONFIG_PM_SLEEP */
@@ -1361,6 +1288,8 @@ int pm_genpd_remove_callbacks(struct dev
 }
 EXPORT_SYMBOL_GPL(pm_genpd_remove_callbacks);
 
+/* Default device callbacks for generic PM domains. */
+
 /**
  * pm_genpd_default_save_state - Default "save device state" for PM domians.
  * @dev: Device to handle.
@@ -1400,6 +1329,50 @@ static int pm_genpd_default_restore_stat
 }
 
 /**
+ * pm_genpd_default_freeze - Default "device freeze" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_freeze(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.freeze;
+
+	return cb ? cb(dev) : pm_generic_suspend(dev);
+}
+
+/**
+ * pm_genpd_default_freeze_late - Default "late device freeze" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_freeze_late(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.freeze_late;
+
+	return cb ? cb(dev) : pm_generic_suspend_noirq(dev);
+}
+
+/**
+ * pm_genpd_default_thaw_early - Default "early device thaw" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_thaw_early(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.thaw_early;
+
+	return cb ? cb(dev) : pm_generic_resume_noirq(dev);
+}
+
+/**
+ * pm_genpd_default_thaw - Default "device thaw" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_thaw(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.thaw;
+
+	return cb ? cb(dev) : pm_generic_resume(dev);
+}
+
+/**
  * 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).
@@ -1429,21 +1402,25 @@ void pm_genpd_init(struct generic_pm_dom
 	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 = pm_genpd_freeze;
 	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.resume = pm_genpd_thaw;
 	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 = pm_genpd_freeze;
 	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.restore = pm_genpd_thaw;
 	genpd->domain.ops.complete = pm_genpd_complete;
 	genpd->dev_ops.save_state = pm_genpd_default_save_state;
 	genpd->dev_ops.restore_state = pm_genpd_default_restore_state;
+	genpd->dev_ops.freeze = pm_genpd_default_freeze;
+	genpd->dev_ops.freeze_late = pm_genpd_default_freeze_late;
+	genpd->dev_ops.thaw_early = pm_genpd_default_thaw_early;
+	genpd->dev_ops.thaw = pm_genpd_default_thaw;
 	mutex_lock(&gpd_list_lock);
 	list_add(&genpd->gpd_list_node, &gpd_list);
 	mutex_unlock(&gpd_list_lock);


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Update 2x][PATCH 4/7] PM / Runtime: Use device PM QoS constraints
  2011-11-19 13:56   ` [Update 2x][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
                       ` (2 preceding siblings ...)
  2011-11-19 13:59     ` [Update 2x][PATCH 3/7] PM / Domains: Rework system suspend callback routines Rafael J. Wysocki
@ 2011-11-19 14:00     ` Rafael J. Wysocki
  2011-11-30 23:20       ` [Update 3x][PATCH 4/7] PM / Runtime: Use device PM QoS constraints (v2) Rafael J. Wysocki
  2011-11-19 14:01     ` [Update 2x][PATCH 5/7] PM / Domains: Add device stop governor function (v4) Rafael J. Wysocki
                       ` (2 subsequent siblings)
  6 siblings, 1 reply; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-19 14:00 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

Make the runtime PM core use device PM QoS constraints to check if
it is allowed to suspend a given device, so that an error code is
returned if the device's own PM QoS constraint is negative or one of
its children has already been suspended for too long.  If this is
not the case, the maximum estimated time the device is allowed to be
suspended, computed as the minimum of the device's PM QoS constraint
and the PM QoS constraints of its children (reduced by the difference
between the current time and their suspend times) is stored in a new
device's PM field power.max_time_suspended_ns that can be used by
the device's subsystem or PM domain to decide whether or not to put
the device into lower-power (and presumably higher-latency) states
later (if the constraint is 0, which means "no constraint", the
power.max_time_suspended_ns is set to -1).

Additionally, the time of execution of the subsystem-level
.runtime_suspend() callback for the device is recorded in the new
power.suspend_time field for later use by the device's subsystem or
PM domain along with power.max_time_suspended_ns (it also is used
by the core code when the device's parent is suspended).

Introduce a new helper function,
pm_runtime_update_max_time_suspended(), allowing subsystems and PM
domains (or device drivers) to update the power.max_time_suspended_ns
field, for example after changing the power state of a suspended
device.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/qos.c     |   24 +++++++---
 drivers/base/power/runtime.c |   94 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/pm.h           |    2 
 include/linux/pm_qos.h       |    3 +
 include/linux/pm_runtime.h   |    5 ++
 5 files changed, 120 insertions(+), 8 deletions(-)

Index: linux/include/linux/pm.h
===================================================================
--- linux.orig/include/linux/pm.h
+++ linux/include/linux/pm.h
@@ -482,6 +482,8 @@ struct dev_pm_info {
 	unsigned long		active_jiffies;
 	unsigned long		suspended_jiffies;
 	unsigned long		accounting_timestamp;
+	ktime_t			suspend_time;
+	s64			max_time_suspended_ns;
 #endif
 	struct pm_subsys_data	*subsys_data;  /* Owned by the subsystem. */
 	struct pm_qos_constraints *constraints;
Index: linux/drivers/base/power/runtime.c
===================================================================
--- linux.orig/drivers/base/power/runtime.c
+++ linux/drivers/base/power/runtime.c
@@ -279,6 +279,47 @@ static int rpm_callback(int (*cb)(struct
 	return retval != -EACCES ? retval : -EIO;
 }
 
+struct rpm_qos_data {
+	ktime_t time_now;
+	s64 constraint_ns;
+};
+
+/**
+ * rpm_update_qos_constraint - Update a given PM QoS constraint data.
+ * @dev: Device whose timing data to use.
+ * @data: PM QoS constraint data to update.
+ *
+ * Use the suspend timing data of @dev to update PM QoS constraint data pointed
+ * to by @data.
+ */
+static int rpm_update_qos_constraint(struct device *dev, void *data)
+{
+	struct rpm_qos_data *qos = data;
+	unsigned long flags;
+	s64 delta_ns;
+	int ret = 0;
+
+	spin_lock_irqsave_nested(&dev->power.lock, flags, SINGLE_DEPTH_NESTING);
+
+	if (dev->power.max_time_suspended_ns < 0)
+		goto out;
+
+	delta_ns = dev->power.max_time_suspended_ns -
+		ktime_to_ns(ktime_sub(qos->time_now, dev->power.suspend_time));
+	if (delta_ns <= 0) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	if (qos->constraint_ns > delta_ns || qos->constraint_ns == 0)
+		qos->constraint_ns = delta_ns;
+
+ out:
+	spin_unlock_irqrestore(&dev->power.lock, flags);
+
+	return ret;
+}
+
 /**
  * rpm_suspend - Carry out runtime suspend of given device.
  * @dev: Device to suspend.
@@ -305,6 +346,7 @@ static int rpm_suspend(struct device *de
 {
 	int (*callback)(struct device *);
 	struct device *parent = NULL;
+	struct rpm_qos_data qos;
 	int retval;
 
 	trace_rpm_suspend(dev, rpmflags);
@@ -400,6 +442,25 @@ static int rpm_suspend(struct device *de
 		goto out;
 	}
 
+	qos.constraint_ns = __dev_pm_qos_read_value(dev);
+	if (qos.constraint_ns < 0) {
+		/* Negative constraint means "never suspend". */
+		retval = -EPERM;
+		goto out;
+	}
+	qos.constraint_ns *= NSEC_PER_USEC;
+	qos.time_now = ktime_get();
+
+	if (!dev->power.ignore_children) {
+		retval = device_for_each_child(dev, &qos,
+					       rpm_update_qos_constraint);
+		if (retval)
+			goto out;
+	}
+
+	dev->power.suspend_time = qos.time_now;
+	dev->power.max_time_suspended_ns = qos.constraint_ns ? : -1;
+
 	__update_runtime_status(dev, RPM_SUSPENDING);
 
 	if (dev->pm_domain)
@@ -416,6 +477,8 @@ static int rpm_suspend(struct device *de
 	retval = rpm_callback(callback, dev);
 	if (retval) {
 		__update_runtime_status(dev, RPM_ACTIVE);
+		dev->power.suspend_time = ktime_set(0, 0);
+		dev->power.max_time_suspended_ns = -1;
 		dev->power.deferred_resume = false;
 		if (retval == -EAGAIN || retval == -EBUSY) {
 			dev->power.runtime_error = 0;
@@ -620,6 +683,9 @@ static int rpm_resume(struct device *dev
 	if (dev->power.no_callbacks)
 		goto no_callback;	/* Assume success. */
 
+	dev->power.suspend_time = ktime_set(0, 0);
+	dev->power.max_time_suspended_ns = -1;
+
 	__update_runtime_status(dev, RPM_RESUMING);
 
 	if (dev->pm_domain)
@@ -1279,6 +1345,9 @@ void pm_runtime_init(struct device *dev)
 	setup_timer(&dev->power.suspend_timer, pm_suspend_timer_fn,
 			(unsigned long)dev);
 
+	dev->power.suspend_time = ktime_set(0, 0);
+	dev->power.max_time_suspended_ns = -1;
+
 	init_waitqueue_head(&dev->power.wait_queue);
 }
 
@@ -1296,3 +1365,28 @@ void pm_runtime_remove(struct device *de
 	if (dev->power.irq_safe && dev->parent)
 		pm_runtime_put_sync(dev->parent);
 }
+
+/**
+ * pm_runtime_update_max_time_suspended - Update device's suspend time data.
+ * @dev: Device to handle.
+ * @delta_ns: Value to subtract from the device's max_time_suspended_ns field.
+ *
+ * Update the device's power.max_time_suspended_ns field by subtracting
+ * @delta_ns from it.  The resulting value of power.max_time_suspended_ns is
+ * never negative.
+ */
+void pm_runtime_update_max_time_suspended(struct device *dev, s64 delta_ns)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->power.lock, flags);
+
+	if (delta_ns > 0 && dev->power.max_time_suspended_ns > 0) {
+		if (dev->power.max_time_suspended_ns > delta_ns)
+			dev->power.max_time_suspended_ns -= delta_ns;
+		else
+			dev->power.max_time_suspended_ns = 0;
+	}
+
+	spin_unlock_irqrestore(&dev->power.lock, flags);
+}
Index: linux/include/linux/pm_runtime.h
===================================================================
--- linux.orig/include/linux/pm_runtime.h
+++ linux/include/linux/pm_runtime.h
@@ -45,6 +45,8 @@ extern void pm_runtime_irq_safe(struct d
 extern void __pm_runtime_use_autosuspend(struct device *dev, bool use);
 extern void pm_runtime_set_autosuspend_delay(struct device *dev, int delay);
 extern unsigned long pm_runtime_autosuspend_expiration(struct device *dev);
+extern void pm_runtime_update_max_time_suspended(struct device *dev,
+						 s64 delta_ns);
 
 static inline bool pm_children_suspended(struct device *dev)
 {
@@ -148,6 +150,9 @@ static inline void pm_runtime_set_autosu
 static inline unsigned long pm_runtime_autosuspend_expiration(
 				struct device *dev) { return 0; }
 
+static inline void pm_runtime_update_max_time_suspended(struct device *dev,
+							s64 delta_ns) {}
+
 #endif /* !CONFIG_PM_RUNTIME */
 
 static inline int pm_runtime_idle(struct device *dev)
Index: linux/drivers/base/power/qos.c
===================================================================
--- linux.orig/drivers/base/power/qos.c
+++ linux/drivers/base/power/qos.c
@@ -47,21 +47,29 @@ static DEFINE_MUTEX(dev_pm_qos_mtx);
 static BLOCKING_NOTIFIER_HEAD(dev_pm_notifiers);
 
 /**
- * dev_pm_qos_read_value - Get PM QoS constraint for a given device.
+ * __dev_pm_qos_read_value - Get PM QoS constraint for a given device.
+ * @dev: Device to get the PM QoS constraint value for.
+ *
+ * This routine must be called with dev->power.lock held.
+ */
+s32 __dev_pm_qos_read_value(struct device *dev)
+{
+	struct pm_qos_constraints *c = dev->power.constraints;
+
+	return c ? pm_qos_read_value(c) : 0;
+}
+
+/**
+ * dev_pm_qos_read_value - Get PM QoS constraint for a given device (locked).
  * @dev: Device to get the PM QoS constraint value for.
  */
 s32 dev_pm_qos_read_value(struct device *dev)
 {
-	struct pm_qos_constraints *c;
 	unsigned long flags;
-	s32 ret = 0;
+	s32 ret;
 
 	spin_lock_irqsave(&dev->power.lock, flags);
-
-	c = dev->power.constraints;
-	if (c)
-		ret = pm_qos_read_value(c);
-
+	ret = __dev_pm_qos_read_value(dev);
 	spin_unlock_irqrestore(&dev->power.lock, flags);
 
 	return ret;
Index: linux/include/linux/pm_qos.h
===================================================================
--- linux.orig/include/linux/pm_qos.h
+++ linux/include/linux/pm_qos.h
@@ -78,6 +78,7 @@ int pm_qos_remove_notifier(int pm_qos_cl
 int pm_qos_request_active(struct pm_qos_request *req);
 s32 pm_qos_read_value(struct pm_qos_constraints *c);
 
+s32 __dev_pm_qos_read_value(struct device *dev);
 s32 dev_pm_qos_read_value(struct device *dev);
 int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req,
 			   s32 value);
@@ -119,6 +120,8 @@ static inline int pm_qos_request_active(
 static inline s32 pm_qos_read_value(struct pm_qos_constraints *c)
 			{ return 0; }
 
+static inline s32 __dev_pm_qos_read_value(struct device *dev)
+			{ return 0; }
 static inline s32 dev_pm_qos_read_value(struct device *dev)
 			{ return 0; }
 static inline int dev_pm_qos_add_request(struct device *dev,


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Update 2x][PATCH 5/7] PM / Domains: Add device stop governor function (v4)
  2011-11-19 13:56   ` [Update 2x][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
                       ` (3 preceding siblings ...)
  2011-11-19 14:00     ` [Update 2x][PATCH 4/7] PM / Runtime: Use device PM QoS constraints Rafael J. Wysocki
@ 2011-11-19 14:01     ` Rafael J. Wysocki
  2011-11-19 14:01     ` [Update 2x][PATCH 6/7] PM / Domains: Add default power off " Rafael J. Wysocki
  2011-11-19 14:02     ` [Update 2x][PATCH 7/7] PM / Domains: Automatically update overoptimistic latency information Rafael J. Wysocki
  6 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-19 14:01 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

Add a function deciding whether or not devices should be stopped in
pm_genpd_runtime_suspend() depending on their PM QoS constraints
and stop/start timing values.  Make it possible to add information
used by this function to device objects.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/arm/mach-shmobile/pm-sh7372.c   |    4 +-
 drivers/base/power/Makefile          |    2 -
 drivers/base/power/domain.c          |   33 ++++++++++++++----
 drivers/base/power/domain_governor.c |   33 ++++++++++++++++++
 include/linux/pm_domain.h            |   63 ++++++++++++++++++++++++++++++-----
 5 files changed, 118 insertions(+), 17 deletions(-)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -21,6 +21,7 @@ enum gpd_status {
 
 struct dev_power_governor {
 	bool (*power_down_ok)(struct dev_pm_domain *domain);
+	bool (*stop_ok)(struct device *dev);
 };
 
 struct gpd_dev_ops {
@@ -72,9 +73,16 @@ struct gpd_link {
 	struct list_head slave_node;
 };
 
+struct gpd_timing_data {
+	s64 stop_latency_ns;
+	s64 start_latency_ns;
+	s64 break_even_ns;
+};
+
 struct generic_pm_domain_data {
 	struct pm_domain_data base;
 	struct gpd_dev_ops ops;
+	struct gpd_timing_data td;
 	bool need_restore;
 };
 
@@ -89,20 +97,48 @@ static inline struct generic_pm_domain_d
 	return to_gpd_data(dev->power.subsys_data->domain_data);
 }
 
-extern int pm_genpd_add_device(struct generic_pm_domain *genpd,
-			       struct device *dev);
+extern struct dev_power_governor simple_qos_governor;
+
+extern struct generic_pm_domain *dev_to_genpd(struct device *dev);
+extern int __pm_genpd_add_device(struct generic_pm_domain *genpd,
+				 struct device *dev,
+				 struct gpd_timing_data *td);
+
+static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
+				      struct device *dev)
+{
+	return __pm_genpd_add_device(genpd, dev, NULL);
+}
+
 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 int pm_genpd_add_callbacks(struct device *dev, struct gpd_dev_ops *ops);
-extern int pm_genpd_remove_callbacks(struct device *dev);
+extern int pm_genpd_add_callbacks(struct device *dev,
+				  struct gpd_dev_ops *ops,
+				  struct gpd_timing_data *td);
+extern int __pm_genpd_remove_callbacks(struct device *dev, bool clear_td);
 extern void pm_genpd_init(struct generic_pm_domain *genpd,
 			  struct dev_power_governor *gov, bool is_off);
+
 extern int pm_genpd_poweron(struct generic_pm_domain *genpd);
+
+extern bool default_stop_ok(struct device *dev);
+
 #else
+
+static inline struct generic_pm_domain *dev_to_genpd(struct device *dev)
+{
+	return ERR_PTR(-ENOSYS);
+}
+static inline int __pm_genpd_add_device(struct generic_pm_domain *genpd,
+					struct device *dev,
+					struct gpd_timing_data *td)
+{
+	return -ENOSYS;
+}
 static inline int pm_genpd_add_device(struct generic_pm_domain *genpd,
 				      struct device *dev)
 {
@@ -124,22 +160,33 @@ static inline int pm_genpd_remove_subdom
 	return -ENOSYS;
 }
 static inline int pm_genpd_add_callbacks(struct device *dev,
-					 struct gpd_dev_ops *ops)
+					 struct gpd_dev_ops *ops,
+					 struct gpd_timing_data *td)
 {
 	return -ENOSYS;
 }
-static inline int pm_genpd_remove_callbacks(struct device *dev)
+static inline int __pm_genpd_remove_callbacks(struct device *dev, bool clear_td)
 {
 	return -ENOSYS;
 }
-static inline void pm_genpd_init(struct generic_pm_domain *genpd,
-				 struct dev_power_governor *gov, bool is_off) {}
+static inline void pm_genpd_init(struct generic_pm_domain *genpd, bool is_off)
+{
+}
 static inline int pm_genpd_poweron(struct generic_pm_domain *genpd)
 {
 	return -ENOSYS;
 }
+static inline bool default_stop_ok(struct device *dev)
+{
+	return false;
+}
 #endif
 
+static inline int pm_genpd_remove_callbacks(struct device *dev)
+{
+	return __pm_genpd_remove_callbacks(dev, true);
+}
+
 #ifdef CONFIG_PM_GENERIC_DOMAINS_RUNTIME
 extern void genpd_queue_power_off_work(struct generic_pm_domain *genpd);
 extern void pm_genpd_poweroff_unused(void);
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -38,7 +38,7 @@ static DEFINE_MUTEX(gpd_list_lock);
 
 #ifdef CONFIG_PM
 
-static struct generic_pm_domain *dev_to_genpd(struct device *dev)
+struct generic_pm_domain *dev_to_genpd(struct device *dev)
 {
 	if (IS_ERR_OR_NULL(dev->pm_domain))
 		return ERR_PTR(-EINVAL);
@@ -436,6 +436,7 @@ static void genpd_power_off_work_fn(stru
 static int pm_genpd_runtime_suspend(struct device *dev)
 {
 	struct generic_pm_domain *genpd;
+	bool (*stop_ok)(struct device *__dev);
 	int ret;
 
 	dev_dbg(dev, "%s()\n", __func__);
@@ -446,10 +447,17 @@ static int pm_genpd_runtime_suspend(stru
 
 	might_sleep_if(!genpd->dev_irq_safe);
 
+	stop_ok = genpd->gov ? genpd->gov->stop_ok : NULL;
+	if (stop_ok && !stop_ok(dev))
+		return -EBUSY;
+
 	ret = genpd_stop_dev(genpd, dev);
 	if (ret)
 		return ret;
 
+	pm_runtime_update_max_time_suspended(dev,
+				dev_gpd_data(dev)->td.start_latency_ns);
+
 	/*
 	 * If power.irq_safe is set, this routine will be run with interrupts
 	 * off, so it can't use mutexes.
@@ -1022,11 +1030,13 @@ static void pm_genpd_complete(struct dev
 #endif /* CONFIG_PM_SLEEP */
 
 /**
- * pm_genpd_add_device - Add a device to an I/O PM domain.
+ * __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.
+ * @td: Set of PM QoS timing parameters to attach to the device.
  */
-int pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev)
+int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
+			  struct gpd_timing_data *td)
 {
 	struct generic_pm_domain_data *gpd_data;
 	struct pm_domain_data *pdd;
@@ -1069,6 +1079,8 @@ int pm_genpd_add_device(struct generic_p
 	gpd_data->base.dev = dev;
 	gpd_data->need_restore = false;
 	list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);
+	if (td)
+		gpd_data->td = *td;
 
  out:
 	genpd_release_lock(genpd);
@@ -1229,8 +1241,10 @@ int pm_genpd_remove_subdomain(struct gen
  * pm_genpd_add_callbacks - Add PM domain callbacks to a given device.
  * @dev: Device to add the callbacks to.
  * @ops: Set of callbacks to add.
+ * @td: Timing data to add to the device along with the callbacks (optional).
  */
-int pm_genpd_add_callbacks(struct device *dev, struct gpd_dev_ops *ops)
+int pm_genpd_add_callbacks(struct device *dev, struct gpd_dev_ops *ops,
+			   struct gpd_timing_data *td)
 {
 	struct pm_domain_data *pdd;
 	int ret = 0;
@@ -1246,6 +1260,8 @@ int pm_genpd_add_callbacks(struct device
 		struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
 
 		gpd_data->ops = *ops;
+		if (td)
+			gpd_data->td = *td;
 	} else {
 		ret = -EINVAL;
 	}
@@ -1258,10 +1274,11 @@ int pm_genpd_add_callbacks(struct device
 EXPORT_SYMBOL_GPL(pm_genpd_add_callbacks);
 
 /**
- * pm_genpd_remove_callbacks - Remove PM domain callbacks from a given device.
+ * __pm_genpd_remove_callbacks - Remove PM domain callbacks from a given device.
  * @dev: Device to remove the callbacks from.
+ * @clear_td: If set, clear the device's timing data too.
  */
-int pm_genpd_remove_callbacks(struct device *dev)
+int __pm_genpd_remove_callbacks(struct device *dev, bool clear_td)
 {
 	struct pm_domain_data *pdd;
 	int ret = 0;
@@ -1277,6 +1294,8 @@ int pm_genpd_remove_callbacks(struct dev
 		struct generic_pm_domain_data *gpd_data = to_gpd_data(pdd);
 
 		gpd_data->ops = (struct gpd_dev_ops){ 0 };
+		if (clear_td)
+			gpd_data->td = (struct gpd_timing_data){ 0 };
 	} else {
 		ret = -EINVAL;
 	}
@@ -1286,7 +1305,7 @@ int pm_genpd_remove_callbacks(struct dev
 
 	return ret;
 }
-EXPORT_SYMBOL_GPL(pm_genpd_remove_callbacks);
+EXPORT_SYMBOL_GPL(__pm_genpd_remove_callbacks);
 
 /* Default device callbacks for generic PM domains. */
 
Index: linux/drivers/base/power/Makefile
===================================================================
--- linux.orig/drivers/base/power/Makefile
+++ linux/drivers/base/power/Makefile
@@ -3,7 +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_PM_GENERIC_DOMAINS)	+=  domain.o domain_governor.o
 obj-$(CONFIG_HAVE_CLK)	+= clock_ops.o
 
 ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
Index: linux/drivers/base/power/domain_governor.c
===================================================================
--- /dev/null
+++ linux/drivers/base/power/domain_governor.c
@@ -0,0 +1,33 @@
+/*
+ * drivers/base/power/domain_governor.c - Governors for device PM domains.
+ *
+ * Copyright (C) 2011 Rafael J. Wysocki <rjw@sisk.pl>, Renesas Electronics Corp.
+ *
+ * This file is released under the GPLv2.
+ */
+
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_qos.h>
+
+/**
+ * default_stop_ok - Default PM domain governor routine for stopping devices.
+ * @dev: Device to check.
+ */
+bool default_stop_ok(struct device *dev)
+{
+	struct gpd_timing_data *td = &dev_gpd_data(dev)->td;
+
+	dev_dbg(dev, "%s()\n", __func__);
+
+	if (dev->power.max_time_suspended_ns < 0 || td->break_even_ns == 0)
+		return true;
+
+	return td->stop_latency_ns + td->start_latency_ns < td->break_even_ns
+		&& td->break_even_ns < dev->power.max_time_suspended_ns;
+}
+
+struct dev_power_governor simple_qos_governor = {
+	.stop_ok = default_stop_ok,
+};
Index: linux/arch/arm/mach-shmobile/pm-sh7372.c
===================================================================
--- linux.orig/arch/arm/mach-shmobile/pm-sh7372.c
+++ linux/arch/arm/mach-shmobile/pm-sh7372.c
@@ -169,6 +169,7 @@ static bool sh7372_power_down_forbidden(
 
 struct dev_power_governor sh7372_always_on_gov = {
 	.power_down_ok = sh7372_power_down_forbidden,
+	.stop_ok = default_stop_ok,
 };
 
 static int sh7372_stop_dev(struct device *dev)
@@ -203,8 +204,9 @@ static int sh7372_start_dev(struct devic
 void sh7372_init_pm_domain(struct sh7372_pm_domain *sh7372_pd)
 {
 	struct generic_pm_domain *genpd = &sh7372_pd->genpd;
+	struct dev_power_governor *gov = sh7372_pd->gov;
 
-	pm_genpd_init(genpd, sh7372_pd->gov, false);
+	pm_genpd_init(genpd, gov ? : &simple_qos_governor, false);
 	genpd->dev_ops.stop = sh7372_stop_dev;
 	genpd->dev_ops.start = sh7372_start_dev;
 	genpd->dev_ops.active_wakeup = pd_active_wakeup;


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Update 2x][PATCH 6/7] PM / Domains: Add default power off governor function (v4)
  2011-11-19 13:56   ` [Update 2x][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
                       ` (4 preceding siblings ...)
  2011-11-19 14:01     ` [Update 2x][PATCH 5/7] PM / Domains: Add device stop governor function (v4) Rafael J. Wysocki
@ 2011-11-19 14:01     ` Rafael J. Wysocki
  2011-11-19 14:02     ` [Update 2x][PATCH 7/7] PM / Domains: Automatically update overoptimistic latency information Rafael J. Wysocki
  6 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-19 14:01 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

Add a function deciding whether or not a given PM domain should
be powered off on the basis of the PM QoS constraints of devices
belonging to it and their PM QoS timing data.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c          |   12 +++
 drivers/base/power/domain_governor.c |  110 +++++++++++++++++++++++++++++++++++
 include/linux/pm_domain.h            |    7 ++
 3 files changed, 129 insertions(+)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -57,8 +57,13 @@ struct generic_pm_domain {
 	bool suspend_power_off;	/* Power status before system suspend */
 	bool dev_irq_safe;	/* Device callbacks are IRQ-safe */
 	int (*power_off)(struct generic_pm_domain *domain);
+	s64 power_off_latency_ns;
 	int (*power_on)(struct generic_pm_domain *domain);
+	s64 power_on_latency_ns;
 	struct gpd_dev_ops dev_ops;
+	s64 break_even_ns;	/* Power break even for the entire domain. */
+	s64 max_off_time_ns;	/* Maximum allowed "suspended" time. */
+	ktime_t power_off_time;
 };
 
 static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)
@@ -76,6 +81,8 @@ struct gpd_link {
 struct gpd_timing_data {
 	s64 stop_latency_ns;
 	s64 start_latency_ns;
+	s64 save_state_latency_ns;
+	s64 restore_state_latency_ns;
 	s64 break_even_ns;
 };
 
Index: linux/drivers/base/power/domain_governor.c
===================================================================
--- linux.orig/drivers/base/power/domain_governor.c
+++ linux/drivers/base/power/domain_governor.c
@@ -10,6 +10,7 @@
 #include <linux/kernel.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_qos.h>
+#include <linux/hrtimer.h>
 
 /**
  * default_stop_ok - Default PM domain governor routine for stopping devices.
@@ -28,6 +29,115 @@ bool default_stop_ok(struct device *dev)
 		&& td->break_even_ns < dev->power.max_time_suspended_ns;
 }
 
+/**
+ * default_power_down_ok - Default generic PM domain power off governor routine.
+ * @pd: PM domain to check.
+ *
+ * This routine must be executed under the PM domain's lock.
+ */
+static bool default_power_down_ok(struct dev_pm_domain *pd)
+{
+	struct generic_pm_domain *genpd = pd_to_genpd(pd);
+	struct gpd_link *link;
+	struct pm_domain_data *pdd;
+	s64 min_dev_off_time_ns;
+	s64 off_on_time_ns;
+	ktime_t time_now = ktime_get();
+
+	off_on_time_ns = genpd->power_off_latency_ns +
+				genpd->power_on_latency_ns;
+	/*
+	 * It doesn't make sense to remove power from the domain if saving
+	 * the state of all devices in it and the power off/power on operations
+	 * take too much time.
+	 *
+	 * All devices in this domain have been stopped already at this point.
+	 */
+	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
+		if (pdd->dev->driver)
+			off_on_time_ns +=
+				to_gpd_data(pdd)->td.save_state_latency_ns;
+	}
+
+	/*
+	 * Check if subdomains can be off for enough time.
+	 *
+	 * All subdomains have been powered off already at this point.
+	 */
+	list_for_each_entry(link, &genpd->master_links, master_node) {
+		struct generic_pm_domain *sd = link->slave;
+		s64 sd_max_off_ns = sd->max_off_time_ns;
+
+		if (sd_max_off_ns < 0)
+			continue;
+
+		sd_max_off_ns -= ktime_to_ns(ktime_sub(time_now,
+						       sd->power_off_time));
+		/*
+		 * Check if the subdomain is allowed to be off long enough for
+		 * the current domain to turn off and on (that's how much time
+		 * it will have to wait worst case).
+		 */
+		if (sd_max_off_ns <= off_on_time_ns)
+			return false;
+	}
+
+	/*
+	 * Check if the devices in the domain can be off enough time.
+	 */
+	min_dev_off_time_ns = -1;
+	list_for_each_entry(pdd, &genpd->dev_list, list_node) {
+		struct gpd_timing_data *td;
+		struct device *dev = pdd->dev;
+		s64 dev_off_time_ns;
+
+		if (!dev->driver || dev->power.max_time_suspended_ns < 0)
+			continue;
+
+		td = &to_gpd_data(pdd)->td;
+		dev_off_time_ns = dev->power.max_time_suspended_ns -
+			(td->start_latency_ns + td->restore_state_latency_ns +
+				ktime_to_ns(ktime_sub(time_now,
+						dev->power.suspend_time)));
+		if (dev_off_time_ns <= off_on_time_ns)
+			return false;
+
+		if (min_dev_off_time_ns > dev_off_time_ns
+		    || min_dev_off_time_ns < 0)
+			min_dev_off_time_ns = dev_off_time_ns;
+	}
+
+	if (min_dev_off_time_ns < 0) {
+		/*
+		 * There are no latency constraints, so the domain can spend
+		 * arbitrary time in the "off" state.
+		 */
+		genpd->max_off_time_ns = -1;
+		return true;
+	}
+
+	/*
+	 * The difference between the computed minimum delta and the time needed
+	 * to turn the domain on is the maximum theoretical time this domain can
+	 * spend in the "off" state.
+	 */
+	min_dev_off_time_ns -= genpd->power_on_latency_ns;
+
+	/*
+	 * If the difference between the computed minimum delta and the time
+	 * needed to turn the domain off and back on on is smaller than the
+	 * domain's power break even time, removing power from the domain is not
+	 * worth it.
+	 */
+	if (genpd->break_even_ns >
+	    min_dev_off_time_ns - genpd->power_off_latency_ns)
+		return false;
+
+	genpd->max_off_time_ns = min_dev_off_time_ns;
+	return true;
+}
+
 struct dev_power_governor simple_qos_governor = {
 	.stop_ok = default_stop_ok,
+	.power_down_ok = default_power_down_ok,
 };
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -398,6 +398,17 @@ static int pm_genpd_poweroff(struct gene
 	}
 
 	genpd->status = GPD_STATE_POWER_OFF;
+	genpd->power_off_time = ktime_get();
+
+	/* Update PM QoS information for devices in the domain. */
+	list_for_each_entry_reverse(pdd, &genpd->dev_list, list_node) {
+		struct gpd_timing_data *td = &to_gpd_data(pdd)->td;
+
+		pm_runtime_update_max_time_suspended(pdd->dev,
+					td->start_latency_ns +
+					td->restore_state_latency_ns +
+					genpd->power_on_latency_ns);
+	}
 
 	list_for_each_entry(link, &genpd->slave_links, slave_node) {
 		genpd_sd_counter_dec(link->master);
@@ -1417,6 +1428,7 @@ void pm_genpd_init(struct generic_pm_dom
 	genpd->resume_count = 0;
 	genpd->device_count = 0;
 	genpd->suspended_count = 0;
+	genpd->max_off_time_ns = -1;
 	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;


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Update 2x][PATCH 7/7] PM / Domains: Automatically update overoptimistic latency information
  2011-11-19 13:56   ` [Update 2x][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
                       ` (5 preceding siblings ...)
  2011-11-19 14:01     ` [Update 2x][PATCH 6/7] PM / Domains: Add default power off " Rafael J. Wysocki
@ 2011-11-19 14:02     ` Rafael J. Wysocki
  6 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-19 14:02 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

Measure the time of execution of the .stop(), .start(), .save_state()
and .restore_state() PM domain device callbacks and if the result
is greater than the corresponding latency value stored in the
device's struct generic_pm_domain_data object, replace the inaccurate
value with the measured time.

Do analogously for the PM domains' .power_off() and .power_off()
callbacks.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/base/power/domain.c |   43 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 4 deletions(-)

Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -33,6 +33,20 @@
 	__ret;							\
 })
 
+#define GENPD_DEV_TIMED_CALLBACK(genpd, type, callback, dev, field, name)	\
+({										\
+	ktime_t __start = ktime_get();						\
+	type __retval = GENPD_DEV_CALLBACK(genpd, type, callback, dev);		\
+	s64 __elapsed = ktime_to_ns(ktime_sub(ktime_get(), __start));		\
+	struct generic_pm_domain_data *__gpd_data = dev_gpd_data(dev);		\
+	if (__elapsed > __gpd_data->td.field) {					\
+		__gpd_data->td.field = __elapsed;				\
+		dev_warn(dev, name " latency exceeded, new value %lld ns\n",	\
+			__elapsed);						\
+	}									\
+	__retval;								\
+})
+
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
 
@@ -48,22 +62,27 @@ struct generic_pm_domain *dev_to_genpd(s
 
 static int genpd_stop_dev(struct generic_pm_domain *genpd, struct device *dev)
 {
-	return GENPD_DEV_CALLBACK(genpd, int, stop, dev);
+	return GENPD_DEV_TIMED_CALLBACK(genpd, int, stop, dev,
+					stop_latency_ns, "stop");
 }
 
 static int genpd_start_dev(struct generic_pm_domain *genpd, struct device *dev)
 {
-	return GENPD_DEV_CALLBACK(genpd, int, start, dev);
+	return GENPD_DEV_TIMED_CALLBACK(genpd, int, start, dev,
+					start_latency_ns, "start");
 }
 
 static int genpd_save_dev(struct generic_pm_domain *genpd, struct device *dev)
 {
-	return GENPD_DEV_CALLBACK(genpd, int, save_state, dev);
+	return GENPD_DEV_TIMED_CALLBACK(genpd, int, save_state, dev,
+					save_state_latency_ns, "state save");
 }
 
 static int genpd_restore_dev(struct generic_pm_domain *genpd, struct device *dev)
 {
-	return GENPD_DEV_CALLBACK(genpd, int, restore_state, dev);
+	return GENPD_DEV_TIMED_CALLBACK(genpd, int, restore_state, dev,
+					restore_state_latency_ns,
+					"state restore");
 }
 
 static bool genpd_sd_counter_dec(struct generic_pm_domain *genpd)
@@ -182,9 +201,16 @@ int __pm_genpd_poweron(struct generic_pm
 	}
 
 	if (genpd->power_on) {
+		ktime_t time_start = ktime_get();
+		s64 elapsed_ns;
+
 		ret = genpd->power_on(genpd);
 		if (ret)
 			goto err;
+
+		elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
+		if (elapsed_ns > genpd->power_on_latency_ns)
+			genpd->power_on_latency_ns = elapsed_ns;
 	}
 
 	genpd_set_active(genpd);
@@ -377,11 +403,16 @@ static int pm_genpd_poweroff(struct gene
 	}
 
 	if (genpd->power_off) {
+		ktime_t time_start;
+		s64 elapsed_ns;
+
 		if (atomic_read(&genpd->sd_count) > 0) {
 			ret = -EBUSY;
 			goto out;
 		}
 
+		time_start = ktime_get();
+
 		/*
 		 * If sd_count > 0 at this point, one of the subdomains hasn't
 		 * managed to call pm_genpd_poweron() for the master yet after
@@ -395,6 +426,10 @@ static int pm_genpd_poweroff(struct gene
 			genpd_set_active(genpd);
 			goto out;
 		}
+
+		elapsed_ns = ktime_to_ns(ktime_sub(ktime_get(), time_start));
+		if (elapsed_ns > genpd->power_off_latency_ns)
+			genpd->power_off_latency_ns = elapsed_ns;
 	}
 
 	genpd->status = GPD_STATE_POWER_OFF;


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Update 3x][PATCH 3/7] PM / Domains: Rework system suspend callback routines (v2)
  2011-11-19 13:59     ` [Update 2x][PATCH 3/7] PM / Domains: Rework system suspend callback routines Rafael J. Wysocki
@ 2011-11-24  0:20       ` Rafael J. Wysocki
  0 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-24  0:20 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

The current generic PM domains code attempts to use the generic
system suspend operations along with the domains' device stop/start
routines, which requires device drivers to assume that their
system suspend/resume (and hibernation/restore) callbacks will always
be used with generic PM domains.  However, in theory, the same
hardware may be used in devices that don't belong to any PM domain,
in which case it would be necessary to add "fake" PM domains to
satisfy the above assumption.  Also, the domain the hardware belongs
to may not be handled with the help of the generic code.

To allow device drivers that may be used along with the generic PM
domains code of more flexibility, add new device callbacks,
.suspend(), .suspend_late(), .resume_early(), .resume(), .freeze(),
.freeze_late(), .thaw_early(), and .thaw(), that can be supplied by
the drivers in addition to their "standard" system suspend and
hibernation callbacks.  These new callbacks, if defined, will be used
by the generic PM domains code for the handling of system suspend and
hibernation instead of the "standard" ones.  This will allow drivers
to be designed to work with generic PM domains as well as without
them.

For backwards compatibility, introduce default implementations of the
new callbacks for PM domains that will execute pm_generic_suspend(),
pm_generic_suspend_noirq(), pm_generic_resume_noirq(),
pm_generic_resume(), pm_generic_freeze(), pm_generic_freeze_noirq(),
pm_generic_thaw_noirq(), and pm_generic_thaw(), respectively, for the
given device if its driver doesn't define those callbacks.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---

When I started to work on documentation updates, I realized that
.freeze()/.thaw() callbacks (and their late/early versions) couldn't
be used as .suspend()/.resume() for two reasons.  First, for some
devices .suspend() has to make wakeup preparations, while .freeze()
need not (and even shouldn't) do that.  Second, .resume() ususally
has to restore the device's state from memory, while .thaw() need
not (and even shouldn't) do that.  Therefore, a complete set of
suspend/resume _and_ freeze/thaw device callbacks for PM domains
is needed.

Thanks,
Rafael

---
 drivers/base/power/domain.c |  249 ++++++++++++++++++++++++++------------------
 include/linux/pm_domain.h   |    8 +
 2 files changed, 158 insertions(+), 99 deletions(-)

Index: linux/include/linux/pm_domain.h
===================================================================
--- linux.orig/include/linux/pm_domain.h
+++ linux/include/linux/pm_domain.h
@@ -28,6 +28,14 @@ struct gpd_dev_ops {
 	int (*stop)(struct device *dev);
 	int (*save_state)(struct device *dev);
 	int (*restore_state)(struct device *dev);
+	int (*suspend)(struct device *dev);
+	int (*suspend_late)(struct device *dev);
+	int (*resume_early)(struct device *dev);
+	int (*resume)(struct device *dev);
+	int (*freeze)(struct device *dev);
+	int (*freeze_late)(struct device *dev);
+	int (*thaw_early)(struct device *dev);
+	int (*thaw)(struct device *dev);
 	bool (*active_wakeup)(struct device *dev);
 };
 
Index: linux/drivers/base/power/domain.c
===================================================================
--- linux.orig/drivers/base/power/domain.c
+++ linux/drivers/base/power/domain.c
@@ -561,6 +561,46 @@ static bool genpd_dev_active_wakeup(stru
 	return GENPD_DEV_CALLBACK(genpd, bool, active_wakeup, dev);
 }
 
+static int genpd_suspend_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, suspend, dev);
+}
+
+static int genpd_suspend_late(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, suspend_late, dev);
+}
+
+static int genpd_resume_early(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, resume_early, dev);
+}
+
+static int genpd_resume_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, resume, dev);
+}
+
+static int genpd_freeze_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, freeze, dev);
+}
+
+static int genpd_freeze_late(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, freeze_late, dev);
+}
+
+static int genpd_thaw_early(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, thaw_early, dev);
+}
+
+static int genpd_thaw_dev(struct generic_pm_domain *genpd, struct device *dev)
+{
+	return GENPD_DEV_CALLBACK(genpd, int, thaw, dev);
+}
+
 /**
  * pm_genpd_sync_poweroff - Synchronously power off a PM domain and its masters.
  * @genpd: PM domain to power off, if possible.
@@ -712,7 +752,7 @@ static int pm_genpd_suspend(struct devic
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	return genpd->suspend_power_off ? 0 : pm_generic_suspend(dev);
+	return genpd->suspend_power_off ? 0 : genpd_suspend_dev(genpd, dev);
 }
 
 /**
@@ -737,7 +777,7 @@ static int pm_genpd_suspend_noirq(struct
 	if (genpd->suspend_power_off)
 		return 0;
 
-	ret = pm_generic_suspend_noirq(dev);
+	ret = genpd_suspend_late(genpd, dev);
 	if (ret)
 		return ret;
 
@@ -788,7 +828,7 @@ static int pm_genpd_resume_noirq(struct
 	genpd->suspended_count--;
 	genpd_start_dev(genpd, dev);
 
-	return pm_generic_resume_noirq(dev);
+	return genpd_resume_early(genpd, dev);
 }
 
 /**
@@ -809,7 +849,7 @@ static int pm_genpd_resume(struct device
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	return genpd->suspend_power_off ? 0 : pm_generic_resume(dev);
+	return genpd->suspend_power_off ? 0 : genpd_resume_dev(genpd, dev);
 }
 
 /**
@@ -830,7 +870,7 @@ static int pm_genpd_freeze(struct device
 	if (IS_ERR(genpd))
 		return -EINVAL;
 
-	return genpd->suspend_power_off ? 0 : pm_generic_freeze(dev);
+	return genpd->suspend_power_off ? 0 : genpd_freeze_dev(genpd, dev);
 }
 
 /**
@@ -856,7 +896,7 @@ static int pm_genpd_freeze_noirq(struct
 	if (genpd->suspend_power_off)
 		return 0;
 
-	ret = pm_generic_freeze_noirq(dev);
+	ret = genpd_freeze_late(genpd, dev);
 	if (ret)
 		return ret;
 
@@ -889,7 +929,7 @@ static int pm_genpd_thaw_noirq(struct de
 
 	genpd_start_dev(genpd, dev);
 
-	return pm_generic_thaw_noirq(dev);
+	return genpd_thaw_early(genpd, dev);
 }
 
 /**
@@ -910,70 +950,7 @@ static int pm_genpd_thaw(struct device *
 	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 (dev->power.wakeup_path && genpd_dev_active_wakeup(genpd, dev))
-		return 0;
-
-	genpd_stop_dev(genpd, 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;
+	return genpd->suspend_power_off ? 0 : genpd_thaw_dev(genpd, dev);
 }
 
 /**
@@ -1015,28 +992,7 @@ static int pm_genpd_restore_noirq(struct
 	genpd->suspended_count--;
 	genpd_start_dev(genpd, 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);
+	return genpd_resume_early(genpd, dev);
 }
 
 /**
@@ -1086,10 +1042,7 @@ static void pm_genpd_complete(struct dev
 #define pm_genpd_freeze_noirq		NULL
 #define pm_genpd_thaw_noirq		NULL
 #define pm_genpd_thaw			NULL
-#define pm_genpd_dev_poweroff_noirq	NULL
-#define pm_genpd_dev_poweroff		NULL
 #define pm_genpd_restore_noirq		NULL
-#define pm_genpd_restore		NULL
 #define pm_genpd_complete		NULL
 
 #endif /* CONFIG_PM_SLEEP */
@@ -1361,6 +1314,8 @@ int pm_genpd_remove_callbacks(struct dev
 }
 EXPORT_SYMBOL_GPL(pm_genpd_remove_callbacks);
 
+/* Default device callbacks for generic PM domains. */
+
 /**
  * pm_genpd_default_save_state - Default "save device state" for PM domians.
  * @dev: Device to handle.
@@ -1400,6 +1355,94 @@ static int pm_genpd_default_restore_stat
 }
 
 /**
+ * pm_genpd_default_suspend - Default "device suspend" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_suspend(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.freeze;
+
+	return cb ? cb(dev) : pm_generic_suspend(dev);
+}
+
+/**
+ * pm_genpd_default_suspend_late - Default "late device suspend" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_suspend_late(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.freeze_late;
+
+	return cb ? cb(dev) : pm_generic_suspend_noirq(dev);
+}
+
+/**
+ * pm_genpd_default_resume_early - Default "early device resume" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_resume_early(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.thaw_early;
+
+	return cb ? cb(dev) : pm_generic_resume_noirq(dev);
+}
+
+/**
+ * pm_genpd_default_resume - Default "device resume" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_resume(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.thaw;
+
+	return cb ? cb(dev) : pm_generic_resume(dev);
+}
+
+/**
+ * pm_genpd_default_freeze - Default "device freeze" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_freeze(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.freeze;
+
+	return cb ? cb(dev) : pm_generic_freeze(dev);
+}
+
+/**
+ * pm_genpd_default_freeze_late - Default "late device freeze" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_freeze_late(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.freeze_late;
+
+	return cb ? cb(dev) : pm_generic_freeze_noirq(dev);
+}
+
+/**
+ * pm_genpd_default_thaw_early - Default "early device thaw" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_thaw_early(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.thaw_early;
+
+	return cb ? cb(dev) : pm_generic_thaw_noirq(dev);
+}
+
+/**
+ * pm_genpd_default_thaw - Default "device thaw" for PM domians.
+ * @dev: Device to handle.
+ */
+static int pm_genpd_default_thaw(struct device *dev)
+{
+	int (*cb)(struct device *__dev) = dev_gpd_data(dev)->ops.thaw;
+
+	return cb ? cb(dev) : pm_generic_thaw(dev);
+}
+
+/**
  * 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).
@@ -1437,13 +1480,21 @@ void pm_genpd_init(struct generic_pm_dom
 	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.poweroff = pm_genpd_suspend;
+	genpd->domain.ops.poweroff_noirq = pm_genpd_suspend_noirq;
 	genpd->domain.ops.restore_noirq = pm_genpd_restore_noirq;
-	genpd->domain.ops.restore = pm_genpd_restore;
+	genpd->domain.ops.restore = pm_genpd_resume;
 	genpd->domain.ops.complete = pm_genpd_complete;
 	genpd->dev_ops.save_state = pm_genpd_default_save_state;
 	genpd->dev_ops.restore_state = pm_genpd_default_restore_state;
+	genpd->dev_ops.freeze = pm_genpd_default_suspend;
+	genpd->dev_ops.freeze_late = pm_genpd_default_suspend_late;
+	genpd->dev_ops.thaw_early = pm_genpd_default_resume_early;
+	genpd->dev_ops.thaw = pm_genpd_default_resume;
+	genpd->dev_ops.freeze = pm_genpd_default_freeze;
+	genpd->dev_ops.freeze_late = pm_genpd_default_freeze_late;
+	genpd->dev_ops.thaw_early = pm_genpd_default_thaw_early;
+	genpd->dev_ops.thaw = pm_genpd_default_thaw;
 	mutex_lock(&gpd_list_lock);
 	list_add(&genpd->gpd_list_node, &gpd_list);
 	mutex_unlock(&gpd_list_lock);


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [Update 3x][PATCH 4/7] PM / Runtime: Use device PM QoS constraints (v2)
  2011-11-19 14:00     ` [Update 2x][PATCH 4/7] PM / Runtime: Use device PM QoS constraints Rafael J. Wysocki
@ 2011-11-30 23:20       ` Rafael J. Wysocki
  0 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2011-11-30 23:20 UTC (permalink / raw)
  To: Linux PM list
  Cc: LKML, Linux-sh list, Magnus Damm, Guennadi Liakhovetski,
	Kevin Hilman, jean.pihet

From: Rafael J. Wysocki <rjw@sisk.pl>

Make the runtime PM core use device PM QoS constraints to check if
it is allowed to suspend a given device, so that an error code is
returned if the device's own PM QoS constraint is negative or one of
its children has already been suspended for too long.  If this is
not the case, the maximum estimated time the device is allowed to be
suspended, computed as the minimum of the device's PM QoS constraint
and the PM QoS constraints of its children (reduced by the difference
between the current time and their suspend times) is stored in a new
device's PM field power.max_time_suspended_ns that can be used by
the device's subsystem or PM domain to decide whether or not to put
the device into lower-power (and presumably higher-latency) states
later (if the constraint is 0, which means "no constraint", the
power.max_time_suspended_ns is set to -1).

Additionally, the time of execution of the subsystem-level
.runtime_suspend() callback for the device is recorded in the new
power.suspend_time field for later use by the device's subsystem or
PM domain along with power.max_time_suspended_ns (it also is used
by the core code when the device's parent is suspended).

Introduce a new helper function,
pm_runtime_update_max_time_suspended(), allowing subsystems and PM
domains (or device drivers) to update the power.max_time_suspended_ns
field, for example after changing the power state of a suspended
device.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---

Hi,

I had to rework the patch so that device_for_each_child() was not
run under dev->power.lock, which caused lockdep to complain loudly.

Thanks,
Rafael

---
 drivers/base/power/qos.c     |   24 ++++--
 drivers/base/power/runtime.c |  148 +++++++++++++++++++++++++++++++++++++------
 include/linux/pm.h           |    2 
 include/linux/pm_qos.h       |    3 
 include/linux/pm_runtime.h   |    5 +
 5 files changed, 154 insertions(+), 28 deletions(-)

Index: linux/include/linux/pm.h
===================================================================
--- linux.orig/include/linux/pm.h
+++ linux/include/linux/pm.h
@@ -521,6 +521,8 @@ struct dev_pm_info {
 	unsigned long		active_jiffies;
 	unsigned long		suspended_jiffies;
 	unsigned long		accounting_timestamp;
+	ktime_t			suspend_time;
+	s64			max_time_suspended_ns;
 #endif
 	struct pm_subsys_data	*subsys_data;  /* Owned by the subsystem. */
 	struct pm_qos_constraints *constraints;
Index: linux/drivers/base/power/runtime.c
===================================================================
--- linux.orig/drivers/base/power/runtime.c
+++ linux/drivers/base/power/runtime.c
@@ -279,6 +279,47 @@ static int rpm_callback(int (*cb)(struct
 	return retval != -EACCES ? retval : -EIO;
 }
 
+struct rpm_qos_data {
+	ktime_t time_now;
+	s64 constraint_ns;
+};
+
+/**
+ * rpm_update_qos_constraint - Update a given PM QoS constraint data.
+ * @dev: Device whose timing data to use.
+ * @data: PM QoS constraint data to update.
+ *
+ * Use the suspend timing data of @dev to update PM QoS constraint data pointed
+ * to by @data.
+ */
+static int rpm_update_qos_constraint(struct device *dev, void *data)
+{
+	struct rpm_qos_data *qos = data;
+	unsigned long flags;
+	s64 delta_ns;
+	int ret = 0;
+
+	spin_lock_irqsave(&dev->power.lock, flags);
+
+	if (dev->power.max_time_suspended_ns < 0)
+		goto out;
+
+	delta_ns = dev->power.max_time_suspended_ns -
+		ktime_to_ns(ktime_sub(qos->time_now, dev->power.suspend_time));
+	if (delta_ns <= 0) {
+		ret = -EBUSY;
+		goto out;
+	}
+
+	if (qos->constraint_ns > delta_ns || qos->constraint_ns == 0)
+		qos->constraint_ns = delta_ns;
+
+ out:
+	spin_unlock_irqrestore(&dev->power.lock, flags);
+
+	return ret;
+}
+
 /**
  * rpm_suspend - Carry out runtime suspend of given device.
  * @dev: Device to suspend.
@@ -305,6 +346,7 @@ static int rpm_suspend(struct device *de
 {
 	int (*callback)(struct device *);
 	struct device *parent = NULL;
+	struct rpm_qos_data qos;
 	int retval;
 
 	trace_rpm_suspend(dev, rpmflags);
@@ -400,8 +442,38 @@ static int rpm_suspend(struct device *de
 		goto out;
 	}
 
+	qos.constraint_ns = __dev_pm_qos_read_value(dev);
+	if (qos.constraint_ns < 0) {
+		/* Negative constraint means "never suspend". */
+		retval = -EPERM;
+		goto out;
+	}
+	qos.constraint_ns *= NSEC_PER_USEC;
+	qos.time_now = ktime_get();
+
 	__update_runtime_status(dev, RPM_SUSPENDING);
 
+	if (!dev->power.ignore_children) {
+		if (dev->power.irq_safe)
+			spin_unlock(&dev->power.lock);
+		else
+			spin_unlock_irq(&dev->power.lock);
+
+		retval = device_for_each_child(dev, &qos,
+					       rpm_update_qos_constraint);
+
+		if (dev->power.irq_safe)
+			spin_lock(&dev->power.lock);
+		else
+			spin_lock_irq(&dev->power.lock);
+
+		if (retval)
+			goto fail;
+	}
+
+	dev->power.suspend_time = qos.time_now;
+	dev->power.max_time_suspended_ns = qos.constraint_ns ? : -1;
+
 	if (dev->pm_domain)
 		callback = dev->pm_domain->ops.runtime_suspend;
 	else if (dev->type && dev->type->pm)
@@ -414,27 +486,9 @@ static int rpm_suspend(struct device *de
 		callback = NULL;
 
 	retval = rpm_callback(callback, dev);
-	if (retval) {
-		__update_runtime_status(dev, RPM_ACTIVE);
-		dev->power.deferred_resume = false;
-		if (retval == -EAGAIN || retval == -EBUSY) {
-			dev->power.runtime_error = 0;
+	if (retval)
+		goto fail;
 
-			/*
-			 * If the callback routine failed an autosuspend, and
-			 * if the last_busy time has been updated so that there
-			 * is a new autosuspend expiration time, automatically
-			 * reschedule another autosuspend.
-			 */
-			if ((rpmflags & RPM_AUTO) &&
-			    pm_runtime_autosuspend_expiration(dev) != 0)
-				goto repeat;
-		} else {
-			pm_runtime_cancel_pending(dev);
-		}
-		wake_up_all(&dev->power.wait_queue);
-		goto out;
-	}
  no_callback:
 	__update_runtime_status(dev, RPM_SUSPENDED);
 	pm_runtime_deactivate_timer(dev);
@@ -466,6 +520,29 @@ static int rpm_suspend(struct device *de
 	trace_rpm_return_int(dev, _THIS_IP_, retval);
 
 	return retval;
+
+ fail:
+	__update_runtime_status(dev, RPM_ACTIVE);
+	dev->power.suspend_time = ktime_set(0, 0);
+	dev->power.max_time_suspended_ns = -1;
+	dev->power.deferred_resume = false;
+	if (retval == -EAGAIN || retval == -EBUSY) {
+		dev->power.runtime_error = 0;
+
+		/*
+		 * If the callback routine failed an autosuspend, and
+		 * if the last_busy time has been updated so that there
+		 * is a new autosuspend expiration time, automatically
+		 * reschedule another autosuspend.
+		 */
+		if ((rpmflags & RPM_AUTO) &&
+		    pm_runtime_autosuspend_expiration(dev) != 0)
+			goto repeat;
+	} else {
+		pm_runtime_cancel_pending(dev);
+	}
+	wake_up_all(&dev->power.wait_queue);
+	goto out;
 }
 
 /**
@@ -620,6 +697,9 @@ static int rpm_resume(struct device *dev
 	if (dev->power.no_callbacks)
 		goto no_callback;	/* Assume success. */
 
+	dev->power.suspend_time = ktime_set(0, 0);
+	dev->power.max_time_suspended_ns = -1;
+
 	__update_runtime_status(dev, RPM_RESUMING);
 
 	if (dev->pm_domain)
@@ -1279,6 +1359,9 @@ void pm_runtime_init(struct device *dev)
 	setup_timer(&dev->power.suspend_timer, pm_suspend_timer_fn,
 			(unsigned long)dev);
 
+	dev->power.suspend_time = ktime_set(0, 0);
+	dev->power.max_time_suspended_ns = -1;
+
 	init_waitqueue_head(&dev->power.wait_queue);
 }
 
@@ -1296,3 +1379,28 @@ void pm_runtime_remove(struct device *de
 	if (dev->power.irq_safe && dev->parent)
 		pm_runtime_put_sync(dev->parent);
 }
+
+/**
+ * pm_runtime_update_max_time_suspended - Update device's suspend time data.
+ * @dev: Device to handle.
+ * @delta_ns: Value to subtract from the device's max_time_suspended_ns field.
+ *
+ * Update the device's power.max_time_suspended_ns field by subtracting
+ * @delta_ns from it.  The resulting value of power.max_time_suspended_ns is
+ * never negative.
+ */
+void pm_runtime_update_max_time_suspended(struct device *dev, s64 delta_ns)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->power.lock, flags);
+
+	if (delta_ns > 0 && dev->power.max_time_suspended_ns > 0) {
+		if (dev->power.max_time_suspended_ns > delta_ns)
+			dev->power.max_time_suspended_ns -= delta_ns;
+		else
+			dev->power.max_time_suspended_ns = 0;
+	}
+
+	spin_unlock_irqrestore(&dev->power.lock, flags);
+}
Index: linux/include/linux/pm_runtime.h
===================================================================
--- linux.orig/include/linux/pm_runtime.h
+++ linux/include/linux/pm_runtime.h
@@ -45,6 +45,8 @@ extern void pm_runtime_irq_safe(struct d
 extern void __pm_runtime_use_autosuspend(struct device *dev, bool use);
 extern void pm_runtime_set_autosuspend_delay(struct device *dev, int delay);
 extern unsigned long pm_runtime_autosuspend_expiration(struct device *dev);
+extern void pm_runtime_update_max_time_suspended(struct device *dev,
+						 s64 delta_ns);
 
 static inline bool pm_children_suspended(struct device *dev)
 {
@@ -148,6 +150,9 @@ static inline void pm_runtime_set_autosu
 static inline unsigned long pm_runtime_autosuspend_expiration(
 				struct device *dev) { return 0; }
 
+static inline void pm_runtime_update_max_time_suspended(struct device *dev,
+							s64 delta_ns) {}
+
 #endif /* !CONFIG_PM_RUNTIME */
 
 static inline int pm_runtime_idle(struct device *dev)
Index: linux/drivers/base/power/qos.c
===================================================================
--- linux.orig/drivers/base/power/qos.c
+++ linux/drivers/base/power/qos.c
@@ -47,21 +47,29 @@ static DEFINE_MUTEX(dev_pm_qos_mtx);
 static BLOCKING_NOTIFIER_HEAD(dev_pm_notifiers);
 
 /**
- * dev_pm_qos_read_value - Get PM QoS constraint for a given device.
+ * __dev_pm_qos_read_value - Get PM QoS constraint for a given device.
+ * @dev: Device to get the PM QoS constraint value for.
+ *
+ * This routine must be called with dev->power.lock held.
+ */
+s32 __dev_pm_qos_read_value(struct device *dev)
+{
+	struct pm_qos_constraints *c = dev->power.constraints;
+
+	return c ? pm_qos_read_value(c) : 0;
+}
+
+/**
+ * dev_pm_qos_read_value - Get PM QoS constraint for a given device (locked).
  * @dev: Device to get the PM QoS constraint value for.
  */
 s32 dev_pm_qos_read_value(struct device *dev)
 {
-	struct pm_qos_constraints *c;
 	unsigned long flags;
-	s32 ret = 0;
+	s32 ret;
 
 	spin_lock_irqsave(&dev->power.lock, flags);
-
-	c = dev->power.constraints;
-	if (c)
-		ret = pm_qos_read_value(c);
-
+	ret = __dev_pm_qos_read_value(dev);
 	spin_unlock_irqrestore(&dev->power.lock, flags);
 
 	return ret;
Index: linux/include/linux/pm_qos.h
===================================================================
--- linux.orig/include/linux/pm_qos.h
+++ linux/include/linux/pm_qos.h
@@ -78,6 +78,7 @@ int pm_qos_remove_notifier(int pm_qos_cl
 int pm_qos_request_active(struct pm_qos_request *req);
 s32 pm_qos_read_value(struct pm_qos_constraints *c);
 
+s32 __dev_pm_qos_read_value(struct device *dev);
 s32 dev_pm_qos_read_value(struct device *dev);
 int dev_pm_qos_add_request(struct device *dev, struct dev_pm_qos_request *req,
 			   s32 value);
@@ -119,6 +120,8 @@ static inline int pm_qos_request_active(
 static inline s32 pm_qos_read_value(struct pm_qos_constraints *c)
 			{ return 0; }
 
+static inline s32 __dev_pm_qos_read_value(struct device *dev)
+			{ return 0; }
 static inline s32 dev_pm_qos_read_value(struct device *dev)
 			{ return 0; }
 static inline int dev_pm_qos_add_request(struct device *dev,


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 4/7] PM / Domains: Rework system suspend callback routines
  2011-11-07  0:08 ` [PATCH 4/7] PM / Domains: Rework system suspend callback routines Rafael J. Wysocki
@ 2012-02-17 19:29   ` Pavel Machek
  2012-02-17 21:01     ` Rafael J. Wysocki
  0 siblings, 1 reply; 35+ messages in thread
From: Pavel Machek @ 2012-02-17 19:29 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux PM list, LKML, Linux-sh list, Magnus Damm,
	Guennadi Liakhovetski, Kevin Hilman, jean.pihet

On Mon 2011-11-07 01:08:11, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> The current generic PM domains code attempts to use the generic
> system suspend operations along with the domains' device stop/start
> routines, which requires device drivers to assume that their
> system suspend/resume (and hibernation/restore) callbacks will always
> be used with generic PM domains.  However, in theory, the same
> hardware may be used in devices that don't belong to any PM domain,
> in which case it would be necessary to add "fake" PM domains to
> satisfy the above assumption.  Also, the domain the hardware belongs
> to may not be handled with the help of the generic code.
> 
> To allow device drivers that may be used along with the generic PM
> domains code of more flexibility, add new device callbacks, .freeze(),
> .freeze_late(), .thaw_early() and .thaw(), that can be supplied by
> the drivers in addition to their "standard" system suspend and
> hibernation callbacks.  These new callbacks, if defined, will be used
> by the generic PM domains code for the handling of system suspend and
> hibernation instead of the "standard" ones.  This will allow drivers
> to be designed to work with generic PM domains as well as without
> them.

Should this go to Documentation/ somewhere? May concern is that we
have way too many callbacks these days. Why is fake PM domain such a
bad thing?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 4/7] PM / Domains: Rework system suspend callback routines
  2012-02-17 19:29   ` Pavel Machek
@ 2012-02-17 21:01     ` Rafael J. Wysocki
  0 siblings, 0 replies; 35+ messages in thread
From: Rafael J. Wysocki @ 2012-02-17 21:01 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Linux PM list, LKML, Linux-sh list, Magnus Damm,
	Guennadi Liakhovetski, Kevin Hilman, jean.pihet

On Friday, February 17, 2012, Pavel Machek wrote:
> On Mon 2011-11-07 01:08:11, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rjw@sisk.pl>
> > 
> > The current generic PM domains code attempts to use the generic
> > system suspend operations along with the domains' device stop/start
> > routines, which requires device drivers to assume that their
> > system suspend/resume (and hibernation/restore) callbacks will always
> > be used with generic PM domains.  However, in theory, the same
> > hardware may be used in devices that don't belong to any PM domain,
> > in which case it would be necessary to add "fake" PM domains to
> > satisfy the above assumption.  Also, the domain the hardware belongs
> > to may not be handled with the help of the generic code.
> > 
> > To allow device drivers that may be used along with the generic PM
> > domains code of more flexibility, add new device callbacks, .freeze(),
> > .freeze_late(), .thaw_early() and .thaw(), that can be supplied by
> > the drivers in addition to their "standard" system suspend and
> > hibernation callbacks.  These new callbacks, if defined, will be used
> > by the generic PM domains code for the handling of system suspend and
> > hibernation instead of the "standard" ones.  This will allow drivers
> > to be designed to work with generic PM domains as well as without
> > them.
> 
> Should this go to Documentation/ somewhere? May concern is that we
> have way too many callbacks these days. Why is fake PM domain such a
> bad thing?

I'm not sure what you mean, really.  This particular patch only affects
the generic PM domains framework, which only has a few users now.

Thanks,
Rafael

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2012-02-17 20:57 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-07  0:01 [PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
2011-11-07  0:06 ` [PATCH 1/7] PM / Domains: Make it possible to use per-device start/stop routines Rafael J. Wysocki
2011-11-08  9:30   ` Guennadi Liakhovetski
2011-11-08 20:38     ` Rafael J. Wysocki
2011-11-07  0:06 ` [PATCH 2/7] PM / Domains: Make it possible to use per-device .active_wakeup() Rafael J. Wysocki
2011-11-08 10:27   ` Guennadi Liakhovetski
2011-11-08 20:40     ` Rafael J. Wysocki
2011-11-09  8:52       ` Guennadi Liakhovetski
2011-11-09 22:40         ` Rafael J. Wysocki
2011-11-09 23:02           ` Guennadi Liakhovetski
2011-11-07  0:07 ` [PATCH 3/7] PM / Domains: Introduce "save/restore state" device callbacks Rafael J. Wysocki
2011-11-07  0:08 ` [PATCH 4/7] PM / Domains: Rework system suspend callback routines Rafael J. Wysocki
2012-02-17 19:29   ` Pavel Machek
2012-02-17 21:01     ` Rafael J. Wysocki
2011-11-07  0:08 ` [PATCH 5/7] PM / Domains: Add device stop governor function (v3) Rafael J. Wysocki
2011-11-07  0:09 ` [PATCH 6/7] PM / Domains: Add default power off " Rafael J. Wysocki
2011-11-07  0:10 ` [PATCH 7/7] PM / Domains: Automatically update overoptimistic latency information Rafael J. Wysocki
2011-11-14  0:22 ` [update][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
2011-11-14  0:23   ` [update][PATCH 1/7] PM / Domains: Make it possible to use per-device domain callbacks Rafael J. Wysocki
2011-11-14  0:24   ` [update][PATCH 2/7] PM / Domains: Introduce "save/restore state" device callbacks Rafael J. Wysocki
2011-11-14  0:25   ` [update][PATCH 3/7] PM / Domains: Rework system suspend callback routines Rafael J. Wysocki
2011-11-14  0:26   ` [update][PATCH 4/7] PM / Runtime: Use device PM QoS constraints Rafael J. Wysocki
2011-11-14  0:27   ` [update][PATCH 5/7] PM / Domains: Add device stop governor function (v4) Rafael J. Wysocki
2011-11-14  0:27   ` [update][PATCH 6/7] PM / Domains: Add default power off " Rafael J. Wysocki
2011-11-14  0:28   ` [update][PATCH 7/7] PM / Domains: Automatically update overoptimistic latency information Rafael J. Wysocki
2011-11-19 13:56   ` [Update 2x][PATCH 0/7] PM / Domains: Per-device callbacks and PM QoS Rafael J. Wysocki
2011-11-19 13:58     ` [Update 2x][PATCH 1/7] PM / Domains: Make it possible to use per-device domain callbacks Rafael J. Wysocki
2011-11-19 13:59     ` [Update 2x][PATCH 2/7] PM / Domains: Introduce "save/restore state" device callbacks Rafael J. Wysocki
2011-11-19 13:59     ` [Update 2x][PATCH 3/7] PM / Domains: Rework system suspend callback routines Rafael J. Wysocki
2011-11-24  0:20       ` [Update 3x][PATCH 3/7] PM / Domains: Rework system suspend callback routines (v2) Rafael J. Wysocki
2011-11-19 14:00     ` [Update 2x][PATCH 4/7] PM / Runtime: Use device PM QoS constraints Rafael J. Wysocki
2011-11-30 23:20       ` [Update 3x][PATCH 4/7] PM / Runtime: Use device PM QoS constraints (v2) Rafael J. Wysocki
2011-11-19 14:01     ` [Update 2x][PATCH 5/7] PM / Domains: Add device stop governor function (v4) Rafael J. Wysocki
2011-11-19 14:01     ` [Update 2x][PATCH 6/7] PM / Domains: Add default power off " Rafael J. Wysocki
2011-11-19 14:02     ` [Update 2x][PATCH 7/7] PM / Domains: Automatically update overoptimistic latency information Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).