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!
