[RFC/PATCH 0/3] Integrating struct class_device into power management framework

[RFC/PATCH 0/3] Integrating struct class_device into power management framework

[Dmitry Torokhov]

Hi,

The following patches integrate class devices into common PM framework.
Classes can define 2 new methods: pause() and restart() that will be
called at suspend and resume time respectively. This will ease hardware
driver's tasks at suspend and resume time by moving common code into
classes.

The advantages of integrating class_device over moving to struct device:

 - no need to reshuffle entire kernel, subsystems can be converted
   one by one without a flag day;
 - struct device will not become a kitchen sink.

--
Dmitry

[RFC/PATCH 1/3] Use kobject_name() to access kobject names

[Dmitry Torokhov]

[-- Attachment #1: dpm-use-kobject-name.patch --]
[-- Type: text/plain, Size: 1277 bytes --]

PM: use kobject_name() to access kobject names

Noone should use kobj.name directly since it may contain garbage.
Objects with longer names have them stored in memory pointed by
kobj->k_name.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/base/power/main.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

Index: work/drivers/base/power/main.c
===================================================================
--- work.orig/drivers/base/power/main.c
+++ work/drivers/base/power/main.c
@@ -54,7 +54,8 @@ int device_pm_add(struct device * dev)
 	int error;
 
 	pr_debug("PM: Adding info for %s:%s\n",
-		 dev->bus ? dev->bus->name : "No Bus", dev->kobj.name);
+		 dev->bus ? dev->bus->name : "No Bus",
+		 kobject_name(&dev->kobj));
 	down(&dpm_list_sem);
 	list_add_tail(&dev->power.entry, &dpm_active);
 	device_pm_set_parent(dev, dev->parent);
@@ -67,7 +68,8 @@ int device_pm_add(struct device * dev)
 void device_pm_remove(struct device * dev)
 {
 	pr_debug("PM: Removing info for %s:%s\n",
-		 dev->bus ? dev->bus->name : "No Bus", dev->kobj.name);
+		 dev->bus ? dev->bus->name : "No Bus",
+		 kobject_name(&dev->kobj));
 	down(&dpm_list_sem);
 	dpm_sysfs_remove(dev);
 	put_device(dev->power.pm_parent);

[RFC/PATCH 2/3] Allow objects other than "struct device" in pm list

[Dmitry Torokhov]

[-- Attachment #1: dpm-make-mutiobject.patch --]
[-- Type: text/plain, Size: 20037 bytes --]

PM: allow objects other than "struct device" in pm list

Any object with embedded dev_pm_info structure can be added to
ower management list and have its suspend/resume methods called
automatically by driver core.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/base/power/main.c    |   69 ++++++++++++++++++++++++++--------
 drivers/base/power/power.h   |   23 ++++++++++-
 drivers/base/power/resume.c  |   61 +++++++++++++++++-------------
 drivers/base/power/runtime.c |   30 ++++++++-------
 drivers/base/power/suspend.c |   85 ++++++++++++++++++++++++-------------------
 drivers/base/power/trace.c   |   17 +++++---
 drivers/pcmcia/ds.c          |    8 ++--
 include/linux/pm.h           |   21 +++++++---
 include/linux/resume-trace.h |   10 ++---
 9 files changed, 207 insertions(+), 117 deletions(-)

Index: work/drivers/base/power/main.c
===================================================================
--- work.orig/drivers/base/power/main.c
+++ work/drivers/base/power/main.c
@@ -31,8 +31,8 @@ DECLARE_MUTEX(dpm_list_sem);
 
 /**
  *	device_pm_set_parent - Specify power dependency.
- *	@dev:		Device who needs power.
- *	@parent:	Device that supplies power.
+ *	@dpm:		Object who needs power.
+ *	@parent:	Object that supplies power.
  *
  *	This function is used to manually describe a power-dependency
  *	relationship. It may be used to specify a transversal relationship
@@ -42,26 +42,63 @@ DECLARE_MUTEX(dpm_list_sem);
  *	before the power dependent.
  */
 
-void device_pm_set_parent(struct device * dev, struct device * parent)
+void dpm_set_parent(struct dev_pm_info *dpm, struct dev_pm_info *parent)
 {
-	put_device(dev->power.pm_parent);
-	dev->power.pm_parent = get_device(parent);
+	if (dpm->pm_parent)
+		kobject_put(dpm->pm_parent->pm_object);
+	if (parent)
+		kobject_get(parent->pm_object);
+	dpm->pm_parent = parent;
 }
-EXPORT_SYMBOL_GPL(device_pm_set_parent);
+EXPORT_SYMBOL_GPL(dpm_set_parent);
 
-int device_pm_add(struct device * dev)
+int dpm_add_object(struct dev_pm_info *dpm)
 {
+	down(&dpm_list_sem);
+	list_add_tail(&dpm->entry, &dpm_active);
+	up(&dpm_list_sem);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(dpm_add_object);
+
+void dpm_delete_object(struct dev_pm_info *dpm)
+{
+	down(&dpm_list_sem);
+	dpm_set_parent(dpm, NULL);
+	list_del_init(&dpm->entry);
+	up(&dpm_list_sem);
+}
+EXPORT_SYMBOL_GPL(dpm_delete_object);
+
+static const struct dev_pm_ops device_pm_ops = {
+	.suspend	= suspend_device,
+	.suspend_late	= suspend_device_late,
+	.resume		= resume_device,
+	.resume_early	= resume_device_early,
+};
+
+int device_pm_add(struct device *dev)
+{
+	struct dev_pm_info *dpm = &dev->power;
 	int error;
 
 	pr_debug("PM: Adding info for %s:%s\n",
 		 dev->bus ? dev->bus->name : "No Bus",
 		 kobject_name(&dev->kobj));
-	down(&dpm_list_sem);
-	list_add_tail(&dev->power.entry, &dpm_active);
-	device_pm_set_parent(dev, dev->parent);
-	if ((error = dpm_sysfs_add(dev)))
-		list_del(&dev->power.entry);
-	up(&dpm_list_sem);
+
+	dpm->pm_object = &dev->kobj;
+	dpm->pm_ops = &device_pm_ops;
+	if (dev->parent)
+		dpm_set_parent(dpm, &dev->parent->power);
+	error = dpm_add_object(dpm);
+	if (error)
+		return error;
+
+	error = dpm_sysfs_add(dev);
+	if (error)
+		dpm_delete_object(dpm);
+
 	return error;
 }
 
@@ -70,11 +107,9 @@ void device_pm_remove(struct device * de
 	pr_debug("PM: Removing info for %s:%s\n",
 		 dev->bus ? dev->bus->name : "No Bus",
 		 kobject_name(&dev->kobj));
-	down(&dpm_list_sem);
+
 	dpm_sysfs_remove(dev);
-	put_device(dev->power.pm_parent);
-	list_del_init(&dev->power.entry);
-	up(&dpm_list_sem);
+	dpm_delete_object(&dev->power);
 }
 
 
Index: work/drivers/base/power/resume.c
===================================================================
--- work.orig/drivers/base/power/resume.c
+++ work/drivers/base/power/resume.c
@@ -20,26 +20,28 @@
  *
  */
 
-int resume_device(struct device * dev)
+int resume_device(struct dev_pm_info *dpm)
 {
 	int error = 0;
+	struct device *dev = container_of(dpm, struct device, power);
+	struct dev_pm_info *pm_parent = dpm->pm_parent;
 
-	TRACE_DEVICE(dev);
+	TRACE_DEVICE(dpm);
 	TRACE_RESUME(0);
 	down(&dev->sem);
-	if (dev->power.pm_parent
-			&& dev->power.pm_parent->power.power_state.event) {
-		dev_err(dev, "PM: resume from %d, parent %s still %d\n",
-			dev->power.power_state.event,
-			dev->power.pm_parent->bus_id,
-			dev->power.pm_parent->power.power_state.event);
+
+	if (pm_parent && pm_parent->power_state.event) {
+		pm_err(dpm, "resume from %d, parent %s still %d\n",
+			dpm->power_state.event,
+			kobject_name(pm_parent->pm_object),
+			pm_parent->power_state.event);
 	}
 	if (dev->bus && dev->bus->resume) {
-		dev_dbg(dev,"resuming\n");
+		pm_dbg(dpm, "resuming\n");
 		error = dev->bus->resume(dev);
 	}
 	if (dev->class && dev->class->resume) {
-		dev_dbg(dev,"class resume\n");
+		pm_dbg(dpm, "class resume\n");
 		error = dev->class->resume(dev);
 	}
 	up(&dev->sem);
@@ -47,12 +49,12 @@ int resume_device(struct device * dev)
 	return error;
 }
 
-
-static int resume_device_early(struct device * dev)
+int resume_device_early(struct dev_pm_info *dpm)
 {
 	int error = 0;
+	struct device *dev = container_of(dpm, struct device, power);
 
-	TRACE_DEVICE(dev);
+	TRACE_DEVICE(dpm);
 	TRACE_RESUME(0);
 	if (dev->bus && dev->bus->resume_early) {
 		dev_dbg(dev,"EARLY resume\n");
@@ -62,6 +64,7 @@ static int resume_device_early(struct de
 	return error;
 }
 
+
 /*
  * Resume the devices that have either not gone through
  * the late suspend, or that did go through it but also
@@ -69,19 +72,21 @@ static int resume_device_early(struct de
  */
 void dpm_resume(void)
 {
-	down(&dpm_list_sem);
-	while(!list_empty(&dpm_off)) {
-		struct list_head * entry = dpm_off.next;
-		struct device * dev = to_device(entry);
+	struct dev_pm_info *dpm;
 
-		get_device(dev);
-		list_move_tail(entry, &dpm_active);
+	down(&dpm_list_sem);
+	while (!list_empty(&dpm_off)) {
+		dpm = list_entry(dpm_off.next, struct dev_pm_info, entry);
 
+		kobject_get(dpm->pm_object);
+		list_move_tail(&dpm->entry, &dpm_active);
 		up(&dpm_list_sem);
-		if (!dev->power.prev_state.event)
-			resume_device(dev);
+
+		if (!dpm->prev_state.event)
+			dpm->pm_ops->resume(dpm);
+
 		down(&dpm_list_sem);
-		put_device(dev);
+		kobject_put(dpm->pm_object);
 	}
 	up(&dpm_list_sem);
 }
@@ -118,12 +123,14 @@ EXPORT_SYMBOL_GPL(device_resume);
 
 void dpm_power_up(void)
 {
-	while(!list_empty(&dpm_off_irq)) {
-		struct list_head * entry = dpm_off_irq.next;
-		struct device * dev = to_device(entry);
+	struct dev_pm_info *dpm;
+
+	while (!list_empty(&dpm_off_irq)) {
+		dpm = list_entry(dpm_off_irq.next, struct dev_pm_info, entry);
 
-		list_move_tail(entry, &dpm_off);
-		resume_device_early(dev);
+		list_move_tail(&dpm->entry, &dpm_off);
+		if (dpm->pm_ops->resume_early)
+			dpm->pm_ops->resume_early(dpm);
 	}
 }
 
Index: work/drivers/base/power/suspend.c
===================================================================
--- work.orig/drivers/base/power/suspend.c
+++ work/drivers/base/power/suspend.c
@@ -46,28 +46,29 @@ static inline char *suspend_verb(u32 eve
  *	@state:	Power state device is entering.
  */
 
-int suspend_device(struct device * dev, pm_message_t state)
+int suspend_device(struct dev_pm_info *dpm, pm_message_t state)
 {
 	int error = 0;
+	struct device *dev = container_of(dpm, struct device, power);
+	struct dev_pm_info *dpm_parent = dpm->pm_parent;
 
 	down(&dev->sem);
-	if (dev->power.power_state.event) {
-		dev_dbg(dev, "PM: suspend %d-->%d\n",
-			dev->power.power_state.event, state.event);
-	}
-	if (dev->power.pm_parent
-			&& dev->power.pm_parent->power.power_state.event) {
-		dev_err(dev,
-			"PM: suspend %d->%d, parent %s already %d\n",
-			dev->power.power_state.event, state.event,
-			dev->power.pm_parent->bus_id,
-			dev->power.pm_parent->power.power_state.event);
-	}
 
-	dev->power.prev_state = dev->power.power_state;
+	if (dpm->power_state.event)
+		pm_dbg(dpm, "suspend %d-->%d\n",
+			dpm->power_state.event, state.event);
+
+	if (dpm_parent && dpm_parent->power_state.event)
+		pm_err(dpm,
+			"suspend %d->%d, parent %s already %d\n",
+			dpm->power_state.event, state.event,
+			kobject_name(dpm_parent->pm_object),
+			dpm_parent->power_state.event);
+
+	dpm->prev_state = dpm->power_state;
 
-	if (dev->class && dev->class->suspend && !dev->power.power_state.event) {
-		dev_dbg(dev, "class %s%s\n",
+	if (dev->class && dev->class->suspend && !dpm->power_state.event) {
+		pm_dbg(dpm, "class %s%s\n",
 			suspend_verb(state.event),
 			((state.event == PM_EVENT_SUSPEND)
 					&& device_may_wakeup(dev))
@@ -78,8 +79,8 @@ int suspend_device(struct device * dev, 
 		suspend_report_result(dev->class->suspend, error);
 	}
 
-	if (!error && dev->bus && dev->bus->suspend && !dev->power.power_state.event) {
-		dev_dbg(dev, "%s%s\n",
+	if (!error && dev->bus && dev->bus->suspend && !dpm->power_state.event) {
+		pm_dbg(dpm, "%s%s\n",
 			suspend_verb(state.event),
 			((state.event == PM_EVENT_SUSPEND)
 					&& device_may_wakeup(dev))
@@ -89,22 +90,23 @@ int suspend_device(struct device * dev, 
 		error = dev->bus->suspend(dev, state);
 		suspend_report_result(dev->bus->suspend, error);
 	}
+
 	up(&dev->sem);
 	return error;
 }
 
-
 /*
  * This is called with interrupts off, only a single CPU
  * running. We can't do down() on a semaphore (and we don't
  * need the protection)
  */
-static int suspend_device_late(struct device *dev, pm_message_t state)
+int suspend_device_late(struct dev_pm_info *dpm, pm_message_t state)
 {
 	int error = 0;
+	struct device *dev = container_of(dpm, struct device, power);
 
 	if (dev->bus && dev->bus->suspend_late && !dev->power.power_state.event) {
-		dev_dbg(dev, "LATE %s%s\n",
+		pm_dbg(dpm, "LATE %s%s\n",
 			suspend_verb(state.event),
 			((state.event == PM_EVENT_SUSPEND)
 					&& device_may_wakeup(dev))
@@ -139,34 +141,40 @@ static int suspend_device_late(struct de
 int device_suspend(pm_message_t state)
 {
 	int error = 0;
+	struct dev_pm_info *dpm;
 
 	might_sleep();
+
 	down(&dpm_sem);
 	down(&dpm_list_sem);
+
 	while (!list_empty(&dpm_active) && error == 0) {
-		struct list_head * entry = dpm_active.prev;
-		struct device * dev = to_device(entry);
 
-		get_device(dev);
+		dpm = list_entry(dpm_active.prev, struct dev_pm_info, entry);
+		kobject_get(dpm->pm_object);
+
 		up(&dpm_list_sem);
 
-		error = suspend_device(dev, state);
+		error = dpm->pm_ops->suspend(dpm, state);
 
 		down(&dpm_list_sem);
 
 		/* Check if the device got removed */
-		if (!list_empty(&dev->power.entry)) {
+		if (!list_empty(&dpm->entry)) {
 			/* Move it to the dpm_off list */
 			if (!error)
-				list_move(&dev->power.entry, &dpm_off);
+				list_move(&dpm->entry, &dpm_off);
 		}
+
 		if (error)
 			printk(KERN_ERR "Could not suspend device %s: "
 				"error %d%s\n",
-				kobject_name(&dev->kobj), error,
+				kobject_name(dpm->pm_object), error,
 				error == -EAGAIN ? " (please convert to suspend_late)" : "");
-		put_device(dev);
+
+		kobject_put(dpm->pm_object);
 	}
+
 	up(&dpm_list_sem);
 	if (error)
 		dpm_resume();
@@ -189,16 +197,19 @@ EXPORT_SYMBOL_GPL(device_suspend);
 int device_power_down(pm_message_t state)
 {
 	int error = 0;
-	struct device * dev;
+	struct dev_pm_info *dpm;
 
 	while (!list_empty(&dpm_off)) {
-		struct list_head * entry = dpm_off.prev;
 
-		dev = to_device(entry);
-		error = suspend_device_late(dev, state);
-		if (error)
-			goto Error;
-		list_move(&dev->power.entry, &dpm_off_irq);
+		dpm = list_entry(dpm_off.prev, struct dev_pm_info, entry);
+
+		if (dpm->pm_ops->suspend_late) {
+			error = dpm->pm_ops->suspend_late(dpm, state);
+			if (error)
+				goto Error;
+		}
+
+		list_move(&dpm->entry, &dpm_off_irq);
 	}
 
 	error = sysdev_suspend(state);
@@ -206,7 +217,7 @@ int device_power_down(pm_message_t state
 	return error;
  Error:
 	printk(KERN_ERR "Could not power down device %s: "
-		"error %d\n", kobject_name(&dev->kobj), error);
+		"error %d\n", kobject_name(dpm->pm_object), error);
 	dpm_power_up();
 	goto Done;
 }
Index: work/include/linux/pm.h
===================================================================
--- work.orig/include/linux/pm.h
+++ work/include/linux/pm.h
@@ -206,12 +206,21 @@ struct dev_pm_info {
 #ifdef	CONFIG_PM
 	unsigned		should_wakeup:1;
 	pm_message_t		prev_state;
-	void			* saved_state;
-	struct device		* pm_parent;
+	void			*saved_state;
+	struct kobject		*pm_object;
+	struct dev_pm_info	*pm_parent;
 	struct list_head	entry;
+	const struct dev_pm_ops	*pm_ops;
 #endif
 };
 
+struct dev_pm_ops {
+	int (*suspend)(struct dev_pm_info *dpm, pm_message_t state);
+	int (*suspend_late)(struct dev_pm_info *dpm, pm_message_t state);
+	int (*resume_early)(struct dev_pm_info *dpm);
+	int (*resume)(struct dev_pm_info *dpm);
+};
+
 extern void device_pm_set_parent(struct device * dev, struct device * parent);
 
 extern int device_power_down(pm_message_t state);
@@ -229,8 +238,8 @@ extern int device_prepare_suspend(pm_mes
 #define device_may_wakeup(dev) \
 	(device_can_wakeup(dev) && (dev)->power.should_wakeup)
 
-extern int dpm_runtime_suspend(struct device *, pm_message_t);
-extern void dpm_runtime_resume(struct device *);
+extern int dpm_runtime_suspend(struct dev_pm_info *, pm_message_t);
+extern void dpm_runtime_resume(struct dev_pm_info *);
 extern void __suspend_report_result(const char *function, void *fn, int ret);
 
 #define suspend_report_result(fn, ret)					\
@@ -248,12 +257,12 @@ static inline int device_suspend(pm_mess
 #define device_set_wakeup_enable(dev,val)	do{}while(0)
 #define device_may_wakeup(dev)			(0)
 
-static inline int dpm_runtime_suspend(struct device * dev, pm_message_t state)
+static inline int dpm_runtime_suspend(struct dev_pm_indo *dpm, pm_message_t state)
 {
 	return 0;
 }
 
-static inline void dpm_runtime_resume(struct device * dev)
+static inline void dpm_runtime_resume(struct dev_pm_info *dpm)
 {
 }
 
Index: work/drivers/base/power/power.h
===================================================================
--- work.orig/drivers/base/power/power.h
+++ work/drivers/base/power/power.h
@@ -7,6 +7,23 @@ extern void device_shutdown(void);
 
 #ifdef CONFIG_PM
 
+#ifdef DEBUG
+#define pm_dbg(dpm, format, arg...)			\
+	printk(KERN_DEBUG "%s PM: " format,		\
+		kobject_name(dpm->pm_object), ## arg)
+#else
+#define pm_dbg(dpm, format, arg...) do { (void)(dpm); } while (0)
+#endif
+
+#define pm_err(dpm, format, arg...)			\
+	printk(KERN_ERR "%s PM: " format,		\
+		kobject_name(dpm->pm_object), ## arg)
+
+#define pm_info(dpm, format, arg...)			\
+	printk(KERN_INFO, "%s PM: " format,		\
+		kobject_name(dpm->pm_object), ## arg)
+
+
 /*
  * main.c
  */
@@ -55,12 +72,14 @@ extern void dpm_sysfs_remove(struct devi
 
 extern void dpm_resume(void);
 extern void dpm_power_up(void);
-extern int resume_device(struct device *);
+extern int resume_device(struct dev_pm_info *);
+extern int resume_device_early(struct dev_pm_info *);
 
 /*
  * suspend.c
  */
-extern int suspend_device(struct device *, pm_message_t);
+extern int suspend_device(struct dev_pm_info *, pm_message_t);
+extern int suspend_device_late(struct dev_pm_info *, pm_message_t);
 
 
 /*
Index: work/drivers/base/power/runtime.c
===================================================================
--- work.orig/drivers/base/power/runtime.c
+++ work/drivers/base/power/runtime.c
@@ -10,13 +10,15 @@
 #include "power.h"
 
 
-static void runtime_resume(struct device * dev)
+static void runtime_resume(struct dev_pm_info *dpm)
 {
-	dev_dbg(dev, "resuming\n");
-	if (!dev->power.power_state.event)
+	pm_dbg(dpm, "resuming\n");
+
+	if (!dpm->power_state.event)
 		return;
-	if (!resume_device(dev))
-		dev->power.power_state = PMSG_ON;
+
+	if (dpm->pm_ops->resume(dpm) == 0)
+		dpm->power_state = PMSG_ON;
 }
 
 
@@ -30,10 +32,10 @@ static void runtime_resume(struct device
  *	FIXME: We need to handle devices that are in an unknown state.
  */
 
-void dpm_runtime_resume(struct device * dev)
+void dpm_runtime_resume(struct dev_pm_info *dpm)
 {
 	down(&dpm_sem);
-	runtime_resume(dev);
+	runtime_resume(dpm);
 	up(&dpm_sem);
 }
 EXPORT_SYMBOL(dpm_runtime_resume);
@@ -45,19 +47,21 @@ EXPORT_SYMBOL(dpm_runtime_resume);
  *	@state:	State to enter.
  */
 
-int dpm_runtime_suspend(struct device * dev, pm_message_t state)
+int dpm_runtime_suspend(struct dev_pm_info *dpm, pm_message_t state)
 {
 	int error = 0;
 
 	down(&dpm_sem);
-	if (dev->power.power_state.event == state.event)
+
+	if (dpm->power_state.event == state.event)
 		goto Done;
 
-	if (dev->power.power_state.event)
-		runtime_resume(dev);
+	if (dpm->power_state.event)
+		runtime_resume(dpm);
 
-	if (!(error = suspend_device(dev, state)))
-		dev->power.power_state = state;
+	error = dpm->pm_ops->suspend(dpm, state);
+	if (!error)
+		dpm->power_state = state;
  Done:
 	up(&dpm_sem);
 	return error;
Index: work/drivers/pcmcia/ds.c
===================================================================
--- work.orig/drivers/pcmcia/ds.c
+++ work/drivers/pcmcia/ds.c
@@ -968,9 +968,9 @@ static ssize_t pcmcia_store_pm_state(str
                 return -EINVAL;
 
 	if ((!p_dev->suspended) && !strncmp(buf, "off", 3))
-		ret = dpm_runtime_suspend(dev, PMSG_SUSPEND);
+		ret = dpm_runtime_suspend(&dev->power, PMSG_SUSPEND);
 	else if (p_dev->suspended && !strncmp(buf, "on", 2))
-		dpm_runtime_resume(dev);
+		dpm_runtime_resume(&dev->power);
 
 	return ret ? ret : count;
 }
@@ -1095,7 +1095,7 @@ static int pcmcia_bus_suspend_callback(s
 	if (p_dev->socket != skt)
 		return 0;
 
-	return dpm_runtime_suspend(dev, PMSG_SUSPEND);
+	return dpm_runtime_suspend(&dev->power, PMSG_SUSPEND);
 }
 
 static int pcmcia_bus_resume_callback(struct device *dev, void * _data)
@@ -1106,7 +1106,7 @@ static int pcmcia_bus_resume_callback(st
 	if (p_dev->socket != skt)
 		return 0;
 
-	dpm_runtime_resume(dev);
+	dpm_runtime_resume(&dev->power);
 
 	return 0;
 }
Index: work/drivers/base/power/trace.c
===================================================================
--- work.orig/drivers/base/power/trace.c
+++ work/drivers/base/power/trace.c
@@ -138,9 +138,10 @@ static unsigned int hash_string(unsigned
 	return seed % mod;
 }
 
-void set_trace_device(struct device *dev)
+void set_trace_device(struct dev_pm_info *dpm)
 {
-	dev_hash_value = hash_string(DEVSEED, dev->bus_id, DEVHASH);
+	dev_hash_value = hash_string(DEVSEED, kobject_name(dpm->pm_object),
+				     DEVHASH);
 }
 
 /*
@@ -185,13 +186,17 @@ static int show_file_hash(unsigned int v
 static int show_dev_hash(unsigned int value)
 {
 	int match = 0;
-	struct list_head * entry = dpm_active.prev;
+	struct list_head *entry = dpm_active.prev;
+	struct dev_pm_info *dpm;
+	unsigned int hash;
 
 	while (entry != &dpm_active) {
-		struct device * dev = to_device(entry);
-		unsigned int hash = hash_string(DEVSEED, dev->bus_id, DEVHASH);
+		dpm = container_of(entry, struct dev_pm_info, entry);
+		hash = hash_string(DEVSEED, kobject_name(dpm->pm_object),
+				   DEVHASH);
 		if (hash == value) {
-			printk("  hash matches device %s\n", dev->bus_id);
+			printk("  hash matches device %s\n",
+				kobject_name(dpm->pm_object));
 			match++;
 		}
 		entry = entry->prev;
Index: work/include/linux/resume-trace.h
===================================================================
--- work.orig/include/linux/resume-trace.h
+++ work/include/linux/resume-trace.h
@@ -5,11 +5,11 @@
 
 extern int pm_trace_enabled;
 
-struct device;
-extern void set_trace_device(struct device *);
+struct dev_pm_info;
+extern void set_trace_device(struct dev_pm_info *);
 extern void generate_resume_trace(void *tracedata, unsigned int user);
 
-#define TRACE_DEVICE(dev) set_trace_device(dev)
+#define TRACE_DEVICE(dpm) set_trace_device(dpm)
 #define TRACE_RESUME(user) do {					\
 	if (pm_trace_enabled) {					\
 		void *tracedata;				\
@@ -26,8 +26,8 @@ extern void generate_resume_trace(void *
 
 #else
 
-#define TRACE_DEVICE(dev) do { } while (0)
-#define TRACE_RESUME(dev) do { } while (0)
+#define TRACE_DEVICE(dpm) do { } while (0)
+#define TRACE_RESUME(user) do { } while (0)
 
 #endif
 

Re: [RFC/PATCH 2/3] Allow objects other than "struct device" in pm list

[David Brownell]

On Tuesday 10 October 2006 11:01 pm, Dmitry Torokhov wrote:
> PM: allow objects other than "struct device" in pm list

For what it's worth I'd like to see that global list go away.  It's only
a shortcut to simplify the "walk the whole device tree" during system-wide
sleep state transitions, and it doesn't cooperate all that well with the
notion that large chunks of that tree might already be suspended.

It's straightforward to construct the relevant list on the fly, and in a
way that could let subtrees suspend and resume based on the demands of
runtime PM.  (Like, hmm, a tree of USB devices ...)  Which could achieve
goals like suspending and resuming into the same system state, rather than
having resume force all devices into full power state (even ones that had
previously been in low power states).  Heck, there might not _be_ enough
power available to turn everything back on!


> Any object with embedded dev_pm_info structure can be added to
> ower management list and have its suspend/resume methods called
> automatically by driver core.
> 
> Signed-off-by: Dmitry Torokhov <dtor@mail.ru>

This patch was interesting ... was there followup that I missed?
(The patch 3/3, teaching class_device how to do PM, seems like it's
not a good approach, now that class suspend/resume can work.)

The thing is that the driver model tree parent mostly reflects one kind of
relationship -- I'll call it "control", it's at least "suspends after" plus
"resumes before" -- while power management can involve many relationships.

For example, the clock tree never shows up in the driver model tree.  (In
fact it doesn't "show up" much of anywhere, despite being essential  ...)
And for various reasons, folk tend to hide the bus hierarchy found inside
a given SOC ... and the way it's common for devices to use one bus for
volume data transfers, and another for control transfers.  (Power saving
points for powering both bus interfaces on/off at runtime...)

And the only infrastructure that currently exists to reflect other kinds
of PM relationship is the pm_parent ... which never worked for much of
anything that I can see.

Something like this patch might actually be able to make that mechanism
become useful for something.  If not ... then maybe pm_parent should be
removed.


> --- work.orig/drivers/base/power/main.c
> +++ work/drivers/base/power/main.c
> @@ -31,8 +31,8 @@ DECLARE_MUTEX(dpm_list_sem);
>  
>  /**
>   *	device_pm_set_parent - Specify power dependency.
> - *	@dev:		Device who needs power.
> - *	@parent:	Device that supplies power.
> + *	@dpm:		Object who needs power.
> + *	@parent:	Object that supplies power.

I'm not 100% sure that "supplies power" is the interesting relationship.
Or that there's necessarily only one such supplier; it's common for
devices to use more than one voltage.

- Dave


> --- work.orig/include/linux/pm.h
> +++ work/include/linux/pm.h
> @@ -206,12 +206,21 @@ struct dev_pm_info {
>  #ifdef	CONFIG_PM
>  	unsigned		should_wakeup:1;
>  	pm_message_t		prev_state;
> -	void			* saved_state;
> -	struct device		* pm_parent;
> +	void			*saved_state;
> +	struct kobject		*pm_object;
> +	struct dev_pm_info	*pm_parent;
>  	struct list_head	entry;
> +	const struct dev_pm_ops	*pm_ops;
>  #endif
>  };
>  
> +struct dev_pm_ops {
> +	int (*suspend)(struct dev_pm_info *dpm, pm_message_t state);
> +	int (*suspend_late)(struct dev_pm_info *dpm, pm_message_t state);
> +	int (*resume_early)(struct dev_pm_info *dpm);
> +	int (*resume)(struct dev_pm_info *dpm);
> +};
> +
>  extern void device_pm_set_parent(struct device * dev, struct device * parent);
>  
>  extern int device_power_down(pm_message_t state);

Re: [RFC/PATCH 2/3] Allow objects other than "struct device" in pm list

[Dmitry Torokhov]

Hi David,

It appears that your unread lists backlog is even longer than mine ;)

On 2/28/07, David Brownell <david-b@pacbell.net> wrote:
>
> > Any object with embedded dev_pm_info structure can be added to
> > ower management list and have its suspend/resume methods called
> > automatically by driver core.
> >
> > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
>
> This patch was interesting ... was there followup that I missed?

Yes, there was:
http://lists.osdl.org/pipermail/linux-pm/2006-October/003927.html

> (The patch 3/3, teaching class_device how to do PM, seems like it's
> not a good approach, now that class suspend/resume can work.)
>

Now the point is moot; I was just trying to do class device
suspend/resume without wholesale conversion to struct device requiring
entire tree shakeup.

-- 
Dmitry

Re: [RFC/PATCH 2/3] Allow objects other than "struct device" in pm list

[David Brownell]

On Wednesday 28 February 2007 1:20 pm, Dmitry Torokhov wrote:
> Hi David,
> 
> It appears that your unread lists backlog is even longer than mine ;)

I had to tune out linux-PM for a while due to volume (and travel).  In
retrospect, that was a good choice because those discussions were so
inconclusive.  Ergo one might say it was more noise than signal.  :)


> On 2/28/07, David Brownell <david-b@pacbell.net> wrote:
> >
> > > Any object with embedded dev_pm_info structure can be added to
> > > ower management list and have its suspend/resume methods called
> > > automatically by driver core.
> > >
> > > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> >
> > This patch was interesting ... was there followup that I missed?
> 
> Yes, there was:
> http://lists.osdl.org/pipermail/linux-pm/2006-October/003927.html

The list archive just showed these patches, no direct followups.
Too bad.  :(

 
> > (The patch 3/3, teaching class_device how to do PM, seems like it's
> > not a good approach, now that class suspend/resume can work.)
> >
> 
> Now the point is moot; I was just trying to do class device
> suspend/resume without wholesale conversion to struct device requiring
> entire tree shakeup.

It seems that's not going to happen for class_device.  I take it you
were looking at having the input subsystem use class suspend/resume?

- Dave

Re: [RFC/PATCH 2/3] Allow objects other than "struct device" in pm list

[Dmitry Torokhov]

On 2/28/07, David Brownell <david-b@pacbell.net> wrote:
> On Wednesday 28 February 2007 1:20 pm, Dmitry Torokhov wrote:
> > Hi David,
> >
> > It appears that your unread lists backlog is even longer than mine ;)
>
> I had to tune out linux-PM for a while due to volume (and travel).  In
> retrospect, that was a good choice because those discussions were so
> inconclusive.  Ergo one might say it was more noise than signal.  :)
>
>
> > On 2/28/07, David Brownell <david-b@pacbell.net> wrote:
> > >
> > > > Any object with embedded dev_pm_info structure can be added to
> > > > ower management list and have its suspend/resume methods called
> > > > automatically by driver core.
> > > >
> > > > Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
> > >
> > > This patch was interesting ... was there followup that I missed?
> >
> > Yes, there was:
> > http://lists.osdl.org/pipermail/linux-pm/2006-October/003927.html
>
> The list archive just showed these patches, no direct followups.
> Too bad.  :(
>
>
> > > (The patch 3/3, teaching class_device how to do PM, seems like it's
> > > not a good approach, now that class suspend/resume can work.)
> > >
> >
> > Now the point is moot; I was just trying to do class device
> > suspend/resume without wholesale conversion to struct device requiring
> > entire tree shakeup.
>
> It seems that's not going to happen for class_device.  I take it you
> were looking at having the input subsystem use class suspend/resume?
>

Not at that time. I was just questioning the need to dump "struct
class_device" and switch everything to "stuct device" fattening it in
the process.

-- 
Dmitry

[RFC/PATCH 3/3] add class_devices to power management list

[Dmitry Torokhov]

[-- Attachment #1: class_device-to-dpm-tree.patch --]
[-- Type: text/plain, Size: 7439 bytes --]

PM: add class_devices to power management list

pause() and restart() methods are added to struct class. These
methods are called for each class_device that belongs to class
that has them defined. pause() is called at suspend time and
restart() at resume time.

Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
---

 drivers/base/class.c         |    9 +++++++++
 drivers/base/power/main.c    |   33 +++++++++++++++++++++++++++++++++
 drivers/base/power/power.h   |   14 ++++++++++++++
 drivers/base/power/resume.c  |   31 +++++++++++++++++++++++++++++++
 drivers/base/power/suspend.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/device.h       |    6 ++++++
 6 files changed, 135 insertions(+)

Index: work/drivers/base/class.c
===================================================================
--- work.orig/drivers/base/class.c
+++ work/drivers/base/class.c
@@ -18,6 +18,7 @@
 #include <linux/err.h>
 #include <linux/slab.h>
 #include "base.h"
+#include "power/power.h"
 
 extern struct subsystem devices_subsys;
 
@@ -612,6 +613,10 @@ int class_device_add(struct class_device
 	if (error)
 		goto out8;
 
+	error = class_device_pm_add(class_dev);
+	if (error)
+		goto out9;
+
 	kobject_uevent(&class_dev->kobj, KOBJ_ADD);
 
 	/* notify any interfaces this device is now here */
@@ -625,6 +630,8 @@ int class_device_add(struct class_device
 
 	goto out1;
 
+ out9:
+	class_device_remove_groups(class_dev);
  out8:
 	if (class_dev->dev)
 		sysfs_remove_link(&class_dev->kobj, class_name);
@@ -734,6 +741,8 @@ void class_device_del(struct class_devic
 		up(&parent_class->sem);
 	}
 
+	class_device_pm_remove(class_dev);
+
 	if (class_dev->dev) {
 		class_name = make_class_name(class_dev->class->name,
 					     &class_dev->kobj);
Index: work/drivers/base/power/main.c
===================================================================
--- work.orig/drivers/base/power/main.c
+++ work/drivers/base/power/main.c
@@ -112,4 +112,37 @@ void device_pm_remove(struct device * de
 	dpm_delete_object(&dev->power);
 }
 
+static const struct dev_pm_ops class_device_pm_ops = {
+	.suspend	= suspend_class_device,
+	.resume		= resume_class_device,
+};
+
+int class_device_pm_add(struct class_device *dev)
+{
+	int error;
+
+	pr_debug("PM: Adding info for %s:%s\n",
+		 dev->class ? dev->class->name : "No Class",
+		 kobject_name(&dev->kobj));
+
+	dev->power.pm_object = &dev->kobj;
+	dev->power.pm_ops = &class_device_pm_ops;
+	if (dev->parent)
+		dpm_set_parent(&dev->power, &dev->parent->power);
+	error = dpm_add_object(&dev->power);
+	if (error)
+		return error;
+
+	return error;
+}
+
+void class_device_pm_remove(struct class_device * dev)
+{
+	pr_debug("PM: Removing info for %s:%s\n",
+		 dev->class ? dev->class->name : "No Class",
+		 kobject_name(&dev->kobj));
+
+	dpm_delete_object(&dev->power);
+}
+
 
Index: work/drivers/base/power/power.h
===================================================================
--- work.orig/drivers/base/power/power.h
+++ work/drivers/base/power/power.h
@@ -59,6 +59,9 @@ static inline struct device * to_device(
 extern int device_pm_add(struct device *);
 extern void device_pm_remove(struct device *);
 
+extern int class_device_pm_add(struct class_device *);
+extern void class_device_pm_remove(struct class_device *);
+
 /*
  * sysfs.c
  */
@@ -74,12 +77,14 @@ extern void dpm_resume(void);
 extern void dpm_power_up(void);
 extern int resume_device(struct dev_pm_info *);
 extern int resume_device_early(struct dev_pm_info *);
+extern int resume_class_device(struct dev_pm_info *);
 
 /*
  * suspend.c
  */
 extern int suspend_device(struct dev_pm_info *, pm_message_t);
 extern int suspend_device_late(struct dev_pm_info *, pm_message_t);
+extern int suspend_class_device(struct dev_pm_info *, pm_message_t);
 
 
 /*
@@ -98,4 +103,13 @@ static inline void device_pm_remove(stru
 
 }
 
+static inline int class_device_pm_add(struct class_device * dev)
+{
+	return 0;
+}
+static inline void class_device_pm_remove(struct class_device * dev)
+{
+
+}
+
 #endif
Index: work/drivers/base/power/suspend.c
===================================================================
--- work.orig/drivers/base/power/suspend.c
+++ work/drivers/base/power/suspend.c
@@ -120,6 +120,48 @@ int suspend_device_late(struct dev_pm_in
 }
 
 /**
+ *	suspend_class_device - Save state of one device.
+ *	@dev:	Device.
+ *	@state:	Power state device is entering.
+ */
+
+int suspend_class_device(struct dev_pm_info *dpm, pm_message_t state)
+{
+	int error = 0;
+	struct class_device *dev = container_of(dpm, struct class_device,
+						power);
+	struct dev_pm_info *dpm_parent;
+
+	if (dpm->power_state.event)
+		pm_dbg(dpm, "PM: suspend %d-->%d\n",
+			dpm->power_state.event, state.event);
+
+	dpm_parent = dev->power.pm_parent;
+	if (dpm_parent && dpm_parent->power_state.event)
+		pm_err(dpm, "PM: suspend %d->%d, parent %s already %d\n",
+			dpm->power_state.event, state.event,
+			kobject_name(dpm_parent->pm_object),
+			dpm_parent->power_state.event);
+
+	dpm->prev_state = dpm->power_state;
+
+	if (dev->class && dev->class->pause && !dpm->power_state.event) {
+		pm_dbg(dpm, "%s%s\n",
+			suspend_verb(state.event),
+			((state.event == PM_EVENT_SUSPEND)
+					&& device_may_wakeup(dev))
+				? ", may wakeup"
+				: ""
+			);
+		error = dev->class->pause(dev);
+		suspend_report_result(dev->class->pause, error);
+	}
+
+	return error;
+}
+
+
+/**
  *	device_suspend - Save state and stop all devices in system.
  *	@state:		Power state to put each device in.
  *
Index: work/include/linux/device.h
===================================================================
--- work.orig/include/linux/device.h
+++ work/include/linux/device.h
@@ -171,6 +171,9 @@ struct class {
 
 	int	(*suspend)(struct device *, pm_message_t state);
 	int	(*resume)(struct device *);
+
+	int	(*pause)(struct class_device *dev);
+	int	(*restart)(struct class_device *dev);
 };
 
 extern int __must_check class_register(struct class *);
@@ -241,9 +244,12 @@ struct class_device {
 	struct class_device	*parent;	/* parent of this child device, if there is one */
 	struct attribute_group  ** groups;	/* optional groups */
 
+	struct dev_pm_info	power;
+
 	void	(*release)(struct class_device *dev);
 	int	(*uevent)(struct class_device *dev, char **envp,
 			   int num_envp, char *buffer, int buffer_size);
+
 	char	class_id[BUS_ID_SIZE];	/* unique to this class */
 };
 
Index: work/drivers/base/power/resume.c
===================================================================
--- work.orig/drivers/base/power/resume.c
+++ work/drivers/base/power/resume.c
@@ -64,6 +64,37 @@ int resume_device_early(struct dev_pm_in
 	return error;
 }
 
+/**
+ *	resume_class_device - Restore state for one device.
+ *	@dev:	Device.
+ *
+ */
+
+int resume_class_device(struct dev_pm_info *dpm)
+{
+	int error = 0;
+	struct class_device *dev = container_of(dpm, struct class_device,
+						power);
+	struct dev_pm_info *pm_parent = dpm->pm_parent;
+
+	TRACE_DEVICE(dpm);
+	TRACE_RESUME(0);
+
+	if (pm_parent && pm_parent->power_state.event) {
+		pm_err(dpm, "resume from %d, parent %s still %d\n",
+			dpm->power_state.event,
+			kobject_name(pm_parent->pm_object),
+			pm_parent->power_state.event);
+	}
+
+	if (dev->class && dev->class->restart) {
+		pm_dbg(dpm, "class restart\n");
+		error = dev->class->restart(dev);
+	}
+
+	TRACE_RESUME(error);
+	return error;
+}
 
 /*
  * Resume the devices that have either not gone through