Re: [linux-pm] [patch] pm: fix runtime powermanagement's /sys interface

[Pavel Machek <pavel@ucw.cz>]

On Út 27-12-05 20:22:04, Patrick Mochel wrote:
> 
> 
> On Tue, 27 Dec 2005, Pavel Machek wrote:
> 
> > Hi!
> >
> > > >  static ssize_t state_show(struct device * dev, struct device_attribute *attr, char * buf)
> > > >  {
> > > > -       return sprintf(buf, "%u\n", dev->power.power_state.event);
> > > > +       if (dev->power.power_state.event)
> > > > +               return sprintf(buf, "suspend\n");
> > > > +       else
> > > > +               return sprintf(buf, "on\n");
> > > >  }
> > >
> > > Are you sure that having only 2 options (suspend/on) is enough at the
> > > core level? I could envision having more levels, like "poweroff", etc?
> >
> > Note that interface is 0/2, currently, so this is improvement :-).
> 
> Heh, not really. You're not really solving any problems, only giving the
> illusion that you've actually fixed something.

Except perhaps userspace passing invalid values down to drivers in
pm_message_t.event?

> As I mentioned in the thread (currently happening, BTW) on the linux-pm
> list, what you want to do is accept a string that reflects an actual state
> that the device supports. For PCI devices that support low-power states,
> this would be "D1", "D2", "D3", etc. For USB devices, which only support
> an "on" and "suspended" state, the values that this patch parses would
> actually work.

We want _common_ values, anyway. So, we do not want "D0", "D1", "D2",
"D3hot" in PCI cases. We probably want "on", "D1", "D2", "suspend",
and I'm not sure about those "D1" and "D2" parts. Userspace should not
have to know about details, it will mostly use "on"/"suspend" anyway.

> > One day, when we find device that needs it, we may want to add more
> > states. I don't know about such device currently.
> 
> There are many devices already do - there are PCI, PCI-X, PCI Express,
> ACPI devices, etc that do. But, you simply cannot create a single
> decent

I asked for an example.
								Pavel
-- 
Thanks, Sharp!