Re: Runtime device power management in userspace

[Pavel Machek <pavel@suse.cz>]

On Pá 23-12-05 07:12:35, Patrick Mochel wrote:
> On Fri, 23 Dec 2005, Holger Macht wrote:
> > We implemented device runtime power management in a userspace application
> > (the powersave daemon). In this specific case, it means to successively
> > put pci devices into D3 powersave mode with writing a numerical '3' to the
> > corresponding power state file.
> >
> > There are two main reasons for us to even doing this:
> >
> >   1. At first, the obvious reason. As mentioned in our research regarding
> >      power consumption on this list, there is a very huge potential to
> >      save battery power.
> >
> >   2. Due to the fact that this is AFAIK a heavily untested area, as a side
> >      effect, we like to get reports about broken modules/drivers and maybe
> >      get them fixed.
> 
> That's great!
> 
> Please note that D3 is only relevant for PCI devices and for ACPI devices.
> The fact that it's the same value for every device in the system is a
> design flaw. Please be aware that the value to write to the device file
> could change, and will be dependent on the type (bus) of device, and quite
> possibly on the device itself. It may not even be '3' for all PCI devices
> in the future, or may be a string rather than 1 character, or simply a '1'
> into a different file.

Is there enough locking in driver core to make this safe?

Ouch and BTW *right* value is _2_, today... because state_store() uses
value from user as a pm_message_t.event. We should at least supply
some hint that it is runtime suspend in pm_message_t.flags --
unfortunately we do not have pm_message_t.flags yet.

Something like this is needed -- userspace should not play with
pm_message_t.state directly. [not even compile tested, have to sleep.]

I guess we should also audit the drivers... It needs locking against
user supplying requests concurently.

> Also, is there source available?

Should be, we are trying to do *Open*SUSE these days ;-))).

								Pavel

diff --git a/drivers/base/power/sysfs.c b/drivers/base/power/sysfs.c
--- a/drivers/base/power/sysfs.c
+++ b/drivers/base/power/sysfs.c
@@ -34,12 +34,14 @@ static ssize_t state_store(struct device
 {
 	pm_message_t state;
 	char * rest;
-	int error = 0;
+	int error = 0, i;
 
-	state.event = simple_strtoul(buf, &rest, 10);
+	i = simple_strtoul(buf, &rest, 10);
+	state.event = PM_EVENT_SUSPEND;
+	state.flags = PM_FLAGS_RUNTIME;
 	if (*rest)
 		return -EINVAL;
-	if (state.event)
+	if (i)
 		error = dpm_runtime_suspend(dev, state);
 	else
 		dpm_runtime_resume(dev);
diff --git a/include/linux/pm.h b/include/linux/pm.h
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -140,6 +140,7 @@ struct device;
 
 typedef struct pm_message {
 	int event;
+	int flags;
 } pm_message_t;
 
 /*
@@ -165,6 +166,8 @@ typedef struct pm_message {
 #define PM_EVENT_FREEZE 1
 #define PM_EVENT_SUSPEND 2
 
+#define PM_FLAGS_RUNTIME 1
+
 #define PMSG_FREEZE	((struct pm_message){ .event = PM_EVENT_FREEZE, })
 #define PMSG_SUSPEND	((struct pm_message){ .event = PM_EVENT_SUSPEND, })
 #define PMSG_ON		((struct pm_message){ .event = PM_EVENT_ON, })

-- 
Thanks, Sharp!