lkml.org 
[lkml]   [2005]   [Dec]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [linux-pm] [patch] pm: fix runtime powermanagement's /sys interface


    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.

    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.

    One way or another, you want the drivers to export the power states that
    they support in some fashion. Not all devices support PM, and the current
    interface is admittedly lacking in that respect. As I mentioned, the
    proper thing to do would be to not add this file for *every* device, but
    leave it up to the buses to add it for devices that support PM (and that
    have drivers bound to them that implement it).

    The reason is that some drivers and devices will support more than just
    "on" and "suspended". which states those are, the power savings that they
    offer, and the tradeoff in latency for resuming from those states are real
    values and things to be considered for wanting to enter those states.

    Your patch does nothing to actually help support those things, and doesn't
    do anything to improve the broken interface.

    > 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
    runtime power management interface for every single type of device. The
    power states are inherently specific to the bus that they are on. Some of
    them are specific to the device.

    This is not suspend - you won't be able to get away with a few arbitrary
    values that work for most systems. The proper interface should allow the
    buses and drivers to use *factual* identifiers to express the states they
    support. Anything else (including the current interface, which I wrote) is
    simply a hack.

    Thanks,


    Patrick

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-12-28 05:24    [W:0.024 / U:0.332 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site