2004-04-29 20:27:17

by Todd Poynor

[permalink] [raw]
Subject: [PATCH] Hotplug for device power state changes

A patch to call a hotplug device-power agent when the power state of a
device is modified at runtime (that is, individually via sysfs or by a
driver call, not as part of a system suspend/resume). Allows a power
management application to be informed of changes in device power needs.
This can be useful on platforms with dependencies between system
clock/voltage settings and operation of certain devices (such as
PXA27x), or, for example, on a cell phone where voiceband or network
devices going inactive signals an opportunity to lower platform power
levels to conserve battery life.

Implemented via new class device-power, with which all devices register.
I'm interested in comments on this approach and in alternate
suggestions. Perhaps device-power should be an "opt-in" class, since
power state changes won't be a concern for most devices/platforms/applications.


--- linux-2.6.5-orig/drivers/base/power/runtime.c 2004-03-11 15:02:22.000000000 -0800
+++ linux-2.6.5-pm/drivers/base/power/runtime.c 2004-04-28 16:51:37.000000000 -0700
@@ -33,6 +33,7 @@
down(&dpm_sem);
runtime_resume(dev);
up(&dpm_sem);
+ dpm_notify(dev);
}


@@ -53,8 +54,10 @@
if (dev->power.power_state)
runtime_resume(dev);

- if (!(error = suspend_device(dev,state)))
+ if (!(error = suspend_device(dev,state))) {
dev->power.power_state = state;
+ dpm_notify(dev);
+ }
Done:
up(&dpm_sem);
return error;

--- linux-2.6.5-orig/drivers/base/power/sysfs.c 2004-03-11 14:57:55.000000000 -0800
+++ linux-2.6.5-pm/drivers/base/power/sysfs.c 2004-04-29 12:41:32.962625032 -0700
@@ -3,6 +3,7 @@
*/

#include <linux/device.h>
+#include <linux/init.h>
#include "power.h"


@@ -57,12 +58,81 @@
.attrs = power_attrs,
};

+#ifdef CONFIG_HOTPLUG
+static int
+device_power_hotplug(struct class_device *class_dev, char **envp,
+ int num_envp, char *buffer, int buffer_size)
+{
+ struct device * dev = (struct device *) class_dev->class_data;
+ int i = 0;
+ int length = 0;
+
+ envp[i++] = buffer;
+ length += scnprintf (buffer, buffer_size - length, "STATE=%d",
+ dev->power.power_state);
+ if ((buffer_size - length <= 0) || (i >= num_envp))
+ return -ENOMEM;
+ ++length;
+
+ envp[i] = 0;
+
+ return 0;
+}
+#endif
+
+static void
+device_power_dev_release(struct class_device *class_dev)
+{
+ if (class_dev)
+ kfree(class_dev);
+}
+
+void dpm_notify(struct device * dev)
+{
+#ifdef CONFIG_HOTPLUG
+ kobject_hotplug("state-change", &dev->power.class_dev->kobj);
+#endif
+}
+
+static struct class device_power_class = {
+ .name = "device-power",
+#ifdef CONFIG_HOTPLUG
+ .hotplug = device_power_hotplug,
+#endif
+ .release = device_power_dev_release,
+};
+
+
int dpm_sysfs_add(struct device * dev)
{
+ struct class_device *class_dev = kmalloc(sizeof(struct class_device), GFP_KERNEL);
+
+ if (class_dev) {
+ memset(class_dev, 0, sizeof (*class_dev));
+ dev->power.class_dev = class_dev;
+ class_dev->class = &device_power_class;
+ class_dev->class_data = dev;
+ strlcpy(class_dev->class_id, dev->bus_id, BUS_ID_SIZE);
+ class_device_register(class_dev);
+ }
+
return sysfs_create_group(&dev->kobj,&pm_attr_group);
}

void dpm_sysfs_remove(struct device * dev)
{
+ struct class_device *class_dev = dev->power.class_dev;
+
+ if (class_dev) {
+ class_device_unregister(class_dev);
+ dev->power.class_dev = NULL;
+ }
+
sysfs_remove_group(&dev->kobj,&pm_attr_group);
}
+
+int __init dpm_init(void)
+{
+ return class_register(&device_power_class);
+}
+

--- linux-2.6.5-orig/drivers/base/power/power.h 2004-03-11 14:57:55.000000000 -0800
+++ linux-2.6.5-pm/drivers/base/power/power.h 2004-04-28 16:53:52.000000000 -0700
@@ -54,6 +54,7 @@

extern int dpm_sysfs_add(struct device *);
extern void dpm_sysfs_remove(struct device *);
+extern void dpm_notify(struct device *);

/*
* resume.c

--- linux-2.6.5-orig/drivers/base/init.c 2004-03-11 15:02:22.000000000 -0800
+++ linux-2.6.5-pm/drivers/base/init.c 2004-04-28 16:56:25.000000000 -0700
@@ -14,6 +14,7 @@
extern int buses_init(void);
extern int classes_init(void);
extern int firmware_init(void);
+extern int dpm_init(void);
extern int platform_bus_init(void);
extern int system_bus_init(void);
extern int cpu_dev_init(void);
@@ -31,6 +32,7 @@
devices_init();
buses_init();
classes_init();
+ dpm_init();
firmware_init();

/* These are also core pieces, but must come after the

--- linux-2.6.5-orig/include/linux/pm.h 2004-03-11 14:58:50.000000000 -0800
+++ linux-2.6.5-pm/include/linux/pm.h 2004-04-28 16:55:31.000000000 -0700
@@ -227,6 +227,7 @@
*/

struct device;
+struct class_device;

struct dev_pm_info {
#ifdef CONFIG_PM
@@ -234,6 +235,7 @@
u8 * saved_state;
atomic_t pm_users;
struct device * pm_parent;
+ struct class_device * class_dev;
struct list_head entry;
#endif
};


--
Todd Poynor
MontaVista Software


2004-04-29 21:43:06

by Russell King

[permalink] [raw]
Subject: Re: [PATCH] Hotplug for device power state changes

On Thu, Apr 29, 2004 at 01:26:54PM -0700, Todd Poynor wrote:
> A patch to call a hotplug device-power agent when the power state of a
> device is modified at runtime (that is, individually via sysfs or by a
> driver call, not as part of a system suspend/resume). Allows a power
> management application to be informed of changes in device power needs.
> This can be useful on platforms with dependencies between system
> clock/voltage settings and operation of certain devices (such as
> PXA27x), or, for example, on a cell phone where voiceband or network
> devices going inactive signals an opportunity to lower platform power
> levels to conserve battery life.

Note that we should run this synchronously with userspace - ie, wait
for the userspace hotplug script to finish executing before moving
on to the next device. Why?

Think of the case where we're suspending the complete system. If you
go round and asynchonously try to run userspace scripts, chances are
you'll have the CPU asleep before _any_ of the scripts have run, which
means (eg) your DHCP client couldn't tell the server that its released
its allocation.

Also, should we be telling userspace about suspend before we actually
suspend the device?

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2004-04-29 22:36:59

by Todd Poynor

[permalink] [raw]
Subject: Re: [PATCH] Hotplug for device power state changes

Russell King wrote:

> Note that we should run this synchronously with userspace - ie, wait
> for the userspace hotplug script to finish executing before moving
> on to the next device. Why?
>
> Think of the case where we're suspending the complete system. If you
> go round and asynchonously try to run userspace scripts, chances are
> you'll have the CPU asleep before _any_ of the scripts have run, which
> means (eg) your DHCP client couldn't tell the server that its released
> its allocation.

I figured system suspend/resume would need to be a separate event and
isn't covered by this patch, which is for "runtime" individual device
suspend/resume only. Also, the flood of notifications of all devices
suspending/resuming might not be useful -- the single system
suspend/resume event could imply these device events, although perhaps
in some cases something would want to know exactly which devices were
operable at system suspend time. I can also send a patch for system
suspend/resume hotplug if there's interest.

Now that you mention it, device power hotplug should be synchronous, to
make sure the power management application has reacted to the changed
state prior to the device going into actual service (in the case of a
resume).

> Also, should we be telling userspace about suspend before we actually
> suspend the device?

I suppose that, depending on the reason notification is needed, "just
before" or "just after" might be the right answer. Now that you bring
this up, it was originally my intention to notify of suspend "just
after" (so power mgr can change power state, knowing that the associated
device no longer has any requirements), and notify of resume "just
before" (so power mgr can adjust power state to match requirements about
to be put into service). I'd be interested in hearing about other usage
scenarios that might need a different notification order. Thanks,

--
Todd Poynor
MontaVista Software

2004-04-30 00:58:30

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH] Hotplug for device power state changes


> I figured system suspend/resume would need to be a separate event and
> isn't covered by this patch, which is for "runtime" individual device
> suspend/resume only. Also, the flood of notifications of all devices
> suspending/resuming might not be useful -- the single system
> suspend/resume event could imply these device events, although perhaps
> in some cases something would want to know exactly which devices were
> operable at system suspend time. I can also send a patch for system
> suspend/resume hotplug if there's interest.
>
> Now that you mention it, device power hotplug should be synchronous, to
> make sure the power management application has reacted to the changed
> state prior to the device going into actual service (in the case of a
> resume).

This is dangerous.

If the device you are suspending is on the VM path in any way,
beeing synchronous with a userland call can deadlock you solid.

This is even more true for system suspend where we are suspending
all devices including the main swap/storage.

There are various cases where I would have loved to get userland
more involved in the suspend/resume process for various reasons,
but in the end, I always got bitten by that problem. Userland cannot
be relied upon unless the process is made completely resident as soon
as we start the suspend dance.

More to this: If you use the "common" code in kernel/power, which I
don't (yet) use on pmac for suspend-to-ram, you'll also stop all
userland processes before notifying drivers (and suspend-to-disk
expects that).

Ben.

2004-04-30 08:30:23

by Russell King

[permalink] [raw]
Subject: Re: [PATCH] Hotplug for device power state changes

On Fri, Apr 30, 2004 at 10:50:26AM +1000, Benjamin Herrenschmidt wrote:
> This is dangerous.
>
> If the device you are suspending is on the VM path in any way,
> beeing synchronous with a userland call can deadlock you solid.

And not being synchronous means that there's no point in calling
userland, because userland won't run before the machine has
suspended, so there's no point in calling it in the first place.
Also consider the case where you suspend, and asynchronously queue
up all these suspend scripts to run. Then you resume and queue up
the resume scripts to run. What order do the suspend and resume
scripts ultimately end up being run?

What if the scripts have side effects like releasing and re-acquiring
your DHCP allocation - what would be the effect of the suspend script
completing after the resume script?

What about the case where suspend/resume scripts bring up/tear down
any communication protocol?

Maybe we should have a two-pass approach, where the first pass
synchronously tells userspace about the suspend, and the second
pass does the actual suspend. Then for resume the opposite.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2004-04-30 19:07:20

by Todd Poynor

[permalink] [raw]
Subject: Re: [PATCH] Hotplug for device power state changes

Benjamin Herrenschmidt wrote:

>>Now that you mention it, device power hotplug should be synchronous, to
>>make sure the power management application has reacted to the changed
>>state prior to the device going into actual service (in the case of a
>>resume).
>
>
> This is dangerous.
>
> If the device you are suspending is on the VM path in any way,
> beeing synchronous with a userland call can deadlock you solid.
>
> This is even more true for system suspend where we are suspending
> all devices including the main swap/storage.

Well, this feature is intended to allow power management of appropriate
devices; using sysfs or a driver call to individually suspend a device
required for proper system operation would be a danger, hotplug
notification or no. And the individual device notifications provided by
the patch under discussion are not for use during a system-wide
suspend/resume sequence. I would imagine system suspend/resume would be
separate events that probably would not notify of the individual device
suspends/resumes performed as a consequence. At any rate, yes, this
would occur outside of the code path that freezes processes and such.

--
Todd Poynor
MontaVista Software

2004-04-30 20:00:38

by Todd Poynor

[permalink] [raw]
Subject: Re: [PATCH] Hotplug for device power state changes

Russell King wrote:

> And not being synchronous means that there's no point in calling
> userland, because userland won't run before the machine has
> suspended, so there's no point in calling it in the first place.
> Also consider the case where you suspend, and asynchronously queue
> up all these suspend scripts to run. Then you resume and queue up
> the resume scripts to run. What order do the suspend and resume
> scripts ultimately end up being run?
...
> Maybe we should have a two-pass approach, where the first pass
> synchronously tells userspace about the suspend, and the second
> pass does the actual suspend. Then for resume the opposite.

I would argue that a system suspend/resume event does not need to also
inform of the individual device suspend/resume events, since these can
be implied. But if we were to include individual device suspend/resume
hotplug events as part of system suspend/resume then I would agree with
a two-phase model, since notification at the time of actual hardware
suspend does not work once something critical to userspace notification
is shutdown.

So I'm planning to resubmit patches with the following:

* Individual device resume events signalled before, not after, the
resume, so that userspace can react to any new requirements before the
device is placed into service.

* Individual device suspend and resume events converted to synchronous
events (that wait for hotplug processing to complete before continuing).

* Changes to kobject to allow kobject hotplug to optionally be
synchronous if desired. I'd assume this is a new hotplug_ops field.

* Synchronous hotplug events for system suspend and resume (without
individual device notifications). These events can probably be
generated by the kobject hotplug methods by the existing power subsys
(once the above enhancement is in place).

Any comments on this course of action welcomed.


--
Todd Poynor
MontaVista Software

2004-04-30 21:57:27

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] Hotplug for device power state changes

On Fri, Apr 30, 2004 at 12:59:40PM -0700, Todd Poynor wrote:
>
> * Changes to kobject to allow kobject hotplug to optionally be
> synchronous if desired. I'd assume this is a new hotplug_ops field.

Ick.

> * Synchronous hotplug events for system suspend and resume (without
> individual device notifications). These events can probably be
> generated by the kobject hotplug methods by the existing power subsys
> (once the above enhancement is in place).

But why? Do you really need this? Have you actually tested a system to
see if it is needed?

thanks,

greg k-h

2004-05-01 00:14:40

by Nigel Cunningham

[permalink] [raw]
Subject: Re: [PATCH] Hotplug for device power state changes

Hi.

Sorry for getting in on this conversion a little late; I've only just
noticed it.

The usual way in which userspace notification of suspending/resuming is
handled at the moment is via scripts which are run prior to suspending and
after resuming. As has been noted, the first thing the kernel side
implementations does is freeze userspace, keeping things static until post
resume. This seems to me to be a good, simple model. DHCP releases can be
handled from user space, prior to echo 4 > /proc/acpi/sleep (or
alternatives) and the whole difficulty regarding interactions between
userspace and kernelspace just goes away.

Note too that the actual invocation of a suspend can still be in response
to kernel events. An ACPI event can be sent to the userspace ACPI daemon,
which does userspace preparations and then invokes the kernel suspend
mechanism. After resume, it can also do userspace reinitialisation.

Given this model, I would suggest that hotplug should silently drop any
events that happen while suspending, and queue events that occur while
resuming until the kernelspace part of resuming is complete and userspace
can run as normal. It shouldn't rely upon device suspend/resume
notifications because they can and do happen while we're still in the
process of suspending and resuming. The means to detect whether we're
suspending or resuming or running normally could be implemented as a
simple function that could test the status of the different suspend
implementations.

Is that at all helpful?

Regards,

Nigel

--
Nigel Cunningham
C/- Westminster Presbyterian Church Belconnen
61 Templeton Street, Cook, ACT 2614, Australia.
+61 (2) 6251 7727 (wk)

At just the right time, while we were still powerless, Christ
died for the ungodly. (Romans 5:6)

2004-05-01 01:16:38

by Todd Poynor

[permalink] [raw]
Subject: Re: [PATCH] Hotplug for device power state changes

Greg KH wrote:
> On Fri, Apr 30, 2004 at 12:59:40PM -0700, Todd Poynor wrote:
>
>>* Changes to kobject to allow kobject hotplug to optionally be
>>synchronous if desired. I'd assume this is a new hotplug_ops field.
>
>
> Ick.

Is the objection to using kobject for synchronous hotplug events, or to
using a hotplug_ops flag to indicate which kind is needed? Would the
addition of a kobject_hotplug_sync function be better? Or a
handshake-like interface as with firmware downloads?

>>* Synchronous hotplug events for system suspend and resume (without
>>individual device notifications). These events can probably be
>>generated by the kobject hotplug methods by the existing power subsys
>>(once the above enhancement is in place).
>
>
> But why? Do you really need this? Have you actually tested a system to
> see if it is needed?

This is something that was requested of me by others who build Linux
into consumer electronics devices. Perhaps some of the interested
parties may speak up here to add more insight. Among the intended uses
that I'm aware of are: saving application state to stable storage (for
example, to be prepared in case the battery dies during an extended
"suspended" period, and such gadgets often do not have a device suitable
for a complete system suspend-to-disk), terminating applications that
reside in memory banks to be powered off during the suspend (this also
relies on other enhancements to allocate memory accordingly), and
dropping network connections in order to conserve resources on servers
that support mobile devices. It sounds like the folks that deal with
ACPI power management have found use for such a mechanism in the
server/desktop world as well.

Thanks,

--
Todd Poynor
MontaVista Software

2004-05-01 01:49:14

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] Hotplug for device power state changes

On Fri, Apr 30, 2004 at 06:16:22PM -0700, Todd Poynor wrote:
> Greg KH wrote:
> >On Fri, Apr 30, 2004 at 12:59:40PM -0700, Todd Poynor wrote:
> >
> >>* Changes to kobject to allow kobject hotplug to optionally be
> >>synchronous if desired. I'd assume this is a new hotplug_ops field.
> >
> >
> >Ick.
>
> Is the objection to using kobject for synchronous hotplug events, or to
> using a hotplug_ops flag to indicate which kind is needed? Would the
> addition of a kobject_hotplug_sync function be better? Or a
> handshake-like interface as with firmware downloads?

To add an option to the kobject_hotplug() function for a sync call is
one thing. To make the option for the main kobject add and remove call
to be sync is a horribly misguided thought (the reason why is left as an
exercise for the reader...like go read the udev code for many reasons
why...)

I don't have an objection to add such a new paramater (or even a new
function call like you suggested), just don't go messing with the main
kobject hotplug call without thinking everything through :)

> >>* Synchronous hotplug events for system suspend and resume (without
> >>individual device notifications). These events can probably be
> >>generated by the kobject hotplug methods by the existing power subsys
> >>(once the above enhancement is in place).
> >
> >
> >But why? Do you really need this? Have you actually tested a system to
> >see if it is needed?
>
> This is something that was requested of me by others who build Linux
> into consumer electronics devices. Perhaps some of the interested
> parties may speak up here to add more insight.

Please encourage them to speak up. I hear _nothing_ from any embedded
developers, and I am really interested in how the driver model and
hotplug works (or doesn't) for them. Without that feedback, we are in
the dark as to what their needs/hates are.

thanks,

greg k-h