On ?t 05-01-06 16:04:07, Patrick Mochel wrote:
>
> On Fri, 6 Jan 2006, Pavel Machek wrote:
>
> > On Pá 06-01-06 00:46:29, Dominik Brodowski wrote:
> > > On Fri, Jan 06, 2006 at 12:08:49AM +0100, Pavel Machek wrote:
> > > > Ok, so lets at least add value-checking to .../power file, and prevent
> > > > userspace see changes to PM_EVENT_SUSPEND value. 2 and 0 are now
> > > > "arbitrary cookies". I'd like to use "on" and "off", but pcmcia
> > > > apparently depends on "2" and "0", so...
> > > >
> > > > Any objections?
> > >
> > > Sorry, but yes -- the same as before, minus the PCMCIA breakage.
> >
> > I don't understand at this point.
> >
> > Current code takes value from user, and passes it down to driver,
> > without any checking. If user writes 666 into the file, it will
> > happily pass down 666 to the driver. Driver does not expect 666 in
> > pm_message_t.event. It may oops, BUG_ON() or anything like that.
> >
> > Shall I change
> >
> > #define PM_EVENT_SUSPEND 2
> >
> > to
> >
> > #define PM_EVENT_SUSPEND 1324
> >
> > to get my point across? This is kernel-specific value, it should not
> > be exported to userland.
>
> A better point, and one that would actually be useful, would be to remove
> the file altogether. Let Dominik export a power file, with complete
> control over the values, for each pcmcia device. Then you never have to
> worry about breaking PCMCIA again.
Fine with me.
[except that now you we went from supporting 2 runtime device states
to supporting just 1... I don't think this is much of progress...]
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
@@ -6,49 +6,6 @@
#include <linux/string.h>
#include "power.h"
-
-/**
- * state - Control current power state of device
- *
- * show() returns the current power state of the device. '0' indicates
- * the device is on. Other values (1-3) indicate the device is in a low
- * power state.
- *
- * store() sets the current power state, which is an integer value
- * between 0-3. If the device is on ('0'), and the value written is
- * greater than 0, then the device is placed directly into the low-power
- * state (via its driver's ->suspend() method).
- * If the device is currently in a low-power state, and the value is 0,
- * the device is powered back on (via the ->resume() method).
- * If the device is in a low-power state, and a different low-power state
- * is requested, the device is first resumed, then suspended into the new
- * low-power state.
- */
-
-static ssize_t state_show(struct device * dev, struct device_attribute *attr, char * buf)
-{
- return sprintf(buf, "%u\n", dev->power.power_state.event);
-}
-
-static ssize_t state_store(struct device * dev, struct device_attribute *attr, const char * buf, size_t n)
-{
- pm_message_t state;
- char * rest;
- int error = 0;
-
- state.event = simple_strtoul(buf, &rest, 10);
- if (*rest)
- return -EINVAL;
- if (state.event)
- error = dpm_runtime_suspend(dev, state);
- else
- dpm_runtime_resume(dev);
- return error ? error : n;
-}
-
-static DEVICE_ATTR(state, 0644, state_show, state_store);
-
-
/*
* wakeup - Report/change current wakeup option for device
*
@@ -122,7 +79,6 @@ static DEVICE_ATTR(wakeup, 0644, wake_sh
static struct attribute * power_attrs[] = {
- &dev_attr_state.attr,
&dev_attr_wakeup.attr,
NULL,
};
--
Thanks, Sharp!
