driver model wakeup support

This is a refresh of a patch I sent before. I suspect it'd be appropriate to merge this, given the agreement I thought I heard last time I posted it.

Follow-on patches would update other subsystems to use this:

* USB, replacing existing HCD support, when activating configurations that support remote wakeup, and when deciding whether to activate remote wakeup for a device as it's being suspended;

* ACPI, since there's a table flagging devices that can support wakeup, and that info is basically invisible to the drivers for those devices (unless the device also supports PCI PM);

* Networking, where I suspect the "ethtool" commands to set wake-on-LAN should update the "may_wakeup" bit

* ... and surely more; this after all only affects the policy applying to a device wakeup, it doesn't do anything to have wakeup actually work on the hardware.

The main thing I dislike about this patch is that it always creates the sysfs attribute, even for devices for which it's meaningful. That should be fixable by just adding the attributes one-at-a-time.

- Dave

This is a refresh of an earlier patch to add "wakeup" support to to the PM-core model:

* struct device_pm_info has two bits that are initialized as part of setting up the enclosing struct device:
- "can_wakeup", reflecting system hardware capabilities
- "may_wakeup", the policy setting (when CONFIG_PM)

* there's a writeable sysfs "wakeup" file, with one of three values:
- "enabled", when the policy is to allow wakeup
- "disabled", when the policy is not to allow it
- "" (empty string), when the hardware doesn't support wakeup

* this patch includes support to initialize these bits for devices that support PCI PM.

It would be preferable to not have the attribute for devices that aren't wired with wakeup support, but that'd involve extra sysfs magic.

@@ -210,6 +210,7 @@
 {
 	kobj_set_kset_s(dev, devices_subsys);
 	kobject_init(&dev->kobj);
+	device_init_wakeup(dev, 0);
 	INIT_LIST_HEAD(&dev->node);
 	INIT_LIST_HEAD(&dev->children);
 	INIT_LIST_HEAD(&dev->driver_list);

@@ -466,6 +466,10 @@
 	if (!pm) 
 		return enable ? -EIO : 0; 
 
+	/* don't enable unless policy set through driver core allows it */
+	if (!device_may_wakeup(&dev->dev) && enable)
+		return -EROFS;
+
 	/* Check device's ability to generate PME# */
 	pci_read_config_word(dev,pm+PCI_PM_PMC,&value);
 

@@ -544,6 +544,7 @@
 static int pci_setup_device(struct pci_dev * dev)
 {
 	u32 class;
+	u16 pm;
 
 	sprintf(pci_name(dev), "%04x:%02x:%02x.%d", pci_domain_nr(dev->bus),
 		dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));

@@ -603,6 +604,14 @@
 		dev->class = PCI_CLASS_NOT_DEFINED;
 	}
 
+	/* with PCI PM capability, it can maybe issue PME# */
+	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
+	if (pm) {
+		pci_read_config_word(dev, pm + PCI_PM_PMC, &pm);
+		if (pm & PCI_PM_CAP_PME)
+			device_init_wakeup(&dev->dev, 1);
+	}
+
 	/* We found a fine healthy device, go go go... */
 	return 0;
 }

@@ -722,11 +731,11 @@
 	/* Assume 32-bit PCI; let 64-bit PCI cards (which are far rarer)
 	   set this higher, assuming the system even supports it.  */
 	dev->dma_mask = 0xffffffff;
+	device_initialize(&dev->dev);
 	if (pci_setup_device(dev) < 0) {
 		kfree(dev);
 		return NULL;
 	}
-	device_initialize(&dev->dev);
 	dev->dev.release = pci_release_dev;
 	pci_dev_get(dev);
 

@@ -211,7 +211,9 @@
 
 struct dev_pm_info {
 	pm_message_t		power_state;
+	unsigned		can_wakeup:1;
 #ifdef	CONFIG_PM
+	unsigned		should_wakeup:1;
 	pm_message_t		prev_state;
 	void			* saved_state;
 	atomic_t		pm_users;

@@ -227,6 +229,22 @@
 extern void device_power_up(void);
 extern void device_resume(void);
 
+/* wakeup changes take effect on device's next pm state change */
+#define device_init_wakeup(dev,val) \
+	((dev)->power.can_wakeup = !!(val),((dev)->power.should_wakeup = 0))
+#define device_can_wakeup(dev) \
+	((dev)->power.can_wakeup)
+
+#ifdef CONFIG_PM
+#define device_set_wakeup_enable(dev,val) \
+	((dev)->power.should_wakeup = (dev)->power.can_wakeup ? !!(val) : 0)
+#define device_may_wakeup(dev) \
+	(device_can_wakeup(dev) && (dev)->power.should_wakeup)
+
+#else /* !CONFIG_PM */
+#define device_set_wakeup_enable(dev,val)	do()while(0)
+#define device_may_wakeup(dev)			(0)
+#endif
 
 #endif /* __KERNEL__ */
 

I actually was working on a similar patch, but decided to stop. I have some concerns about the generic-ness of "wakeup". For example, some devices may have multiple wakeup modes or wakeup criteria. Under these situations having a global "wakeup" parameter might become clumsy. So should drivers be exporting these sort of things on their own?

"Wake-on-lan", as an example, could have 4 or 5 different packet types to look for. So when the user specifies one of these, he or she is expecting "wake-on-lan" to just work. In the case of your patch, I think the sysfs attribute would also need to be configured.

"wakeup" seems to fit most ACPI and PCI devices well. I'm not sure how it would apply to usb, firewire, pcmcia, etc., especially when determining if it supports wake. It may just be something that only the driver knows.

A few more points:

Wakeup configuration may vary between system states, how can we account for this? Should wakeup information be reconfigured by userspace before entering a sleep or off state?

Would there be any value in exporting a list of system states in which wake is supported on a per-device basis?

This probably isn't too difficult. The question is how consistent we want the interface to be.

Thanks,
Adam

As I commented. Note that the various WOL options that "ethtool" exposes apply to the "ethN" device (in sysfs, /sys/class/net/* devices) which are not affected by this patch, not to real /sys/devices/... ones which are.

This patch was partially written to solve a big gaping hole in the PM framework that I noticed from USB. It certainly behaves with USB! (That is, with USB parts not included in that core patch.)

I have a hard time seeing how a simple policy setting could NOT work for other wakeup-capable frameworks (or devices), though I suppose an argument could be made that there should be an "always enabled" option to export perverse hardware to userspace.

USB devices have a single "remote wakeup" mechanism, which not all devices support. If they support it, it's listed in the descriptor for the active configuration (the one being suspended). That's info that's known by usbcore. Network adapters often use it (with WOL), as do keyboards and mice.

What do you mean by "wakeup configuration"?

I don't see any benefit to having state-specific policies; if the device is going to be able to wake the system from sleep, the user isn't really going to care how deep a sleep is involved. They may be annoyed when it can wake from some states and not others ... but that's basically going to be an inescapable hardware limitation. (Example: sleep states that let USB controllers retain root hub power can support wakeup by USB. But sleep states which power off the hubs, maybe "deeper" sleep, can't.)

How would normal users -- "joe six-pack", "aunt tilly", or whoever -- use such things? I don't see many folk wanting to think about such issues. They'll care if they can wake it up by touching the keyboard, and likely remember that sometimes they have to use the front panel power switch (for classic swsusp) rather than the one on the keyboard.

I'd say it differently: the question is how we want to trade off types of consistency versus memory usage. At one point Patrick estimated that each sysfs attribute instance cost about 1 KByte. That adds up quickly, especially on embedded systems ... and it's probably more consistent to not have attributes if they're not meaningful, than to have them always and to require userspace to always check for null values (etc).

- Dave

Hi!

Yes, not having empty wakeup file might be nice. Or place "<not supported>" there if we are doing strings anyway.
Pavel

Boycott Kodak -- for their patent abuse against Java.

Maybe I haven't been paying attention (likely), but I don't see that
there has been any official activity on this patch. Is it headed to
the -mm branch or headed to the bit bucket?

Jordan

Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>

Nobody really commented on it this time. But then, not many folk are actually looking at wakeup support lately ... there's so much attention on the "power down my laptop with swsusp" case that the other PM scenarios -- "real PM" vs "off" -- often get ignored.

I'll see about only putting that "wakeup" attribute on devices that actually need it, and then submit that. With infrastructure in place, folk can start using it.

- Dave

I volunteer to be an additional test victim if needed.

Jordan

Here's a refresh of my driver model wakeup patch. Two changes from last time:

- It sets up USB too, not just PCI. So keyboards and hubs, plus a few network adapters, will have these attributes.

- Only devices that are known to have wakeup capabilities
have the attribute. So "find /sys/devices -name wakeup"
returns only "interesting" devices.

At this time, wakeup mechanisms need to be explicitly enabled in userspace code; so this is in a sense a policy setting API affecting runtime (and suspend-to-ram time) device-specific PM.

There will be some usbcore changes to follow this, assuming this gets on the 2.6.14 train ... replacing current kluges.

Please merge...

- Dave

This is a refresh of an earlier patch to add "wakeup" support to to the PM-core model:

* "struct device_pm_info" has two bits that are initialized as part of setting up the enclosing struct device:
- "can_wakeup", reflecting hardware capabilities
- "may_wakeup", the policy setting (when CONFIG_PM)

* There's a writeable sysfs "wakeup" file, with one of two values:
- "enabled", when the policy is to allow wakeup
- "disabled", when the policy is not to allow it
The file is nonexistent when the device doesn't support wakeup.

* this patch includes support to initialize these bits for devices that support PCI PM, or USB configurations handle remote wakeup.

Thing of it as providing one policy control knob affecting a driver. In the future there may be others, with the overall device specific policy satisfying several constraints defined from userspace.

@@ -213,7 +213,9 @@ typedef u32 __bitwise pm_message_t;
 
 struct dev_pm_info {
 	pm_message_t		power_state;
+	unsigned		can_wakeup:1;
 #ifdef	CONFIG_PM
+	unsigned		should_wakeup:1;
 	pm_message_t		prev_state;
 	void			* saved_state;
 	atomic_t		pm_users;

@@ -228,13 +230,30 @@ extern int device_power_down(pm_message_
 extern void device_power_up(void);
 extern void device_resume(void);
 
+/* wakeup changes take effect on device's next pm state change */
+#define device_init_wakeup(dev,val) \
+	((dev)->power.can_wakeup = !!(val),((dev)->power.should_wakeup = 0))
+#define device_can_wakeup(dev) \
+	((dev)->power.can_wakeup)
+
 #ifdef CONFIG_PM
 extern int device_suspend(pm_message_t state);
-#else
+
+#define device_set_wakeup_enable(dev,val) \
+	((dev)->power.should_wakeup = (dev)->power.can_wakeup ? !!(val) : 0)
+#define device_may_wakeup(dev) \
+	(device_can_wakeup(dev) && (dev)->power.should_wakeup)
+
+#else /* !CONFIG_PM */
+
 static inline int device_suspend(pm_message_t state)
 {
 	return 0;
 }
+
+#define device_set_wakeup_enable(dev,val)	do{}while(0)
+#define device_may_wakeup(dev)			(0)
+
 #endif
 
 #endif /* __KERNEL__ */

@@ -207,6 +207,7 @@ void device_initialize(struct device *de
 {
 	kobj_set_kset_s(dev, devices_subsys);
 	kobject_init(&dev->kobj);
+	device_init_wakeup(dev, 0);
 	klist_init(&dev->klist_children);
 	INIT_LIST_HEAD(&dev->dma_pools);
 	init_MUTEX(&dev->sem);

@@ -476,6 +476,10 @@ int pci_enable_wake(struct pci_dev *dev,
 	if (!pm) 
 		return enable ? -EIO : 0; 
 
+	/* don't enable unless policy set through driver core allows it */
+	if (!device_may_wakeup(&dev->dev) && enable)
+		return -EROFS;
+
 	/* Check device's ability to generate PME# */
 	pci_read_config_word(dev,pm+PCI_PM_PMC,&value);
 

@@ -571,6 +571,7 @@ static void pci_read_irq(struct pci_dev 
 static int pci_setup_device(struct pci_dev * dev)
 {
 	u32 class;
+	u16 pm;
 
 	sprintf(pci_name(dev), "%04x:%02x:%02x.%d", pci_domain_nr(dev->bus),
 		dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));

@@ -630,6 +631,14 @@ static int pci_setup_device(struct pci_d
 		dev->class = PCI_CLASS_NOT_DEFINED;
 	}
 
+	/* with PCI PM capability, it can maybe issue PME# */
+	pm = pci_find_capability(dev, PCI_CAP_ID_PM);
+	if (pm) {
+		pci_read_config_word(dev, pm + PCI_PM_PMC, &pm);
+		if (pm & PCI_PM_CAP_PME)
+			device_init_wakeup(&dev->dev, 1);
+	}
+
 	/* We found a fine healthy device, go go go... */
 	return 0;
 }

@@ -749,11 +758,11 @@ pci_scan_device(struct pci_bus *bus, int
 	/* Assume 32-bit PCI; let 64-bit PCI cards (which are far rarer)
 	   set this higher, assuming the system even supports it.  */
 	dev->dma_mask = 0xffffffff;
+	device_initialize(&dev->dev);
 	if (pci_setup_device(dev) < 0) {
 		kfree(dev);
 		return NULL;
 	}
-	device_initialize(&dev->dev);
 	dev->dev.release = pci_release_dev;
 	pci_dev_get(dev);
 

@@ -1398,6 +1398,8 @@ free_interfaces:
 				 dev->bus->busnum, dev->devpath,
 				 configuration,
 				 alt->desc.bInterfaceNumber);
+			if (cp->desc.bmAttributes & USB_CONFIG_ATT_WAKEUP)
+				device_init_wakeup(&intf->dev, 1);
 		}
 		kfree(new_interfaces);
 

Nice to see this finally get into the kernel.

The PCI part of this patch should try to use ACPI to identify devices capable of platform-specific non-PME wakeup signalling. I don't have the faintest idea how to do this; maybe Len can help.

I'll hope ... :)

The same logic that creates /proc/acpi/wakeup will catch both those non-PME PCI cases and several non-PCI cases.

- Dave