BUG in include/linux/pm.h: remote wakeup settings

BUG in include/linux/pm.h: remote wakeup settings

[Alan Stern]

Currently include/linux/pm.h doesn't define the should_wakeup field in 
dev_pm_info, or the related accessor macros, unless CONFIG_PM_SLEEP is 
set.

This is wrong.  Even though the system can't go to sleep, individual
devices may be placed in low-power states (runtime PM).  Thus the
various wakeup settings should be available whenever CONFIG_PM is on.

I don't want to submit a patch to fix the problem right now, because it
would clash with ongoing changes that Rafael is making.  But the bug
needs to be fixed in time for 2.6.26.

Alan Stern

Re: BUG in include/linux/pm.h: remote wakeup settings

[Rafael J. Wysocki]

On Monday, 10 of March 2008, Alan Stern wrote:
> Currently include/linux/pm.h doesn't define the should_wakeup field in 
> dev_pm_info, or the related accessor macros, unless CONFIG_PM_SLEEP is 
> set.
> 
> This is wrong.  Even though the system can't go to sleep, individual
> devices may be placed in low-power states (runtime PM).  Thus the
> various wakeup settings should be available whenever CONFIG_PM is on.
> 
> I don't want to submit a patch to fix the problem right now, because it
> would clash with ongoing changes that Rafael is making.  But the bug
> needs to be fixed in time for 2.6.26.

Well, I'm going to prepare a "for 2.6.26" series of patches to send to Greg
(that won't include the "separate suspend and hibernation callbacks" patch
yet).

If you post a patch for this (with a changelog and s-o-b), I'll rebase it (if
necessary) add it to the series.

Thanks,
Rafael

Re: BUG in include/linux/pm.h: remote wakeup settings

[David Brownell]

On Monday 10 March 2008, Alan Stern wrote:
> Currently include/linux/pm.h doesn't define the should_wakeup field in 
> dev_pm_info, or the related accessor macros, unless CONFIG_PM_SLEEP is 
> set.

Right, and I certainly recall pointing that out sometime last year...
so it's not exactly a new discovery.


> This is wrong.  Even though the system can't go to sleep, individual
> devices may be placed in low-power states (runtime PM).  Thus the
> various wakeup settings should be available whenever CONFIG_PM is on.
> 
> I don't want to submit a patch to fix the problem right now, because it
> would clash with ongoing changes that Rafael is making.  But the bug
> needs to be fixed in time for 2.6.26.
> 
> Alan Stern
> 

[PATCH] PM: make wakeup flags available whenever CONFIG_PM is set

[Alan Stern]

The various wakeup flags and their accessor macros in struct
dev_pm_info should be available whenever CONFIG_PM is enabled, not
just when CONFIG_PM_SLEEP is on.  Otherwise remote wakeup won't always
be configurable for runtime power management.  This patch (as1056)
fixes the oversight.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: David Brownell <david-b@pacbell.net>

---

Index: usb-2.6/include/linux/pm.h
===================================================================
--- usb-2.6.orig/include/linux/pm.h
+++ usb-2.6/include/linux/pm.h
@@ -183,8 +183,8 @@ typedef struct pm_message {
 struct dev_pm_info {
 	pm_message_t		power_state;
 	unsigned		can_wakeup:1;
-#ifdef	CONFIG_PM_SLEEP
 	unsigned		should_wakeup:1;
+#ifdef	CONFIG_PM_SLEEP
 	struct list_head	entry;
 #endif
 };
@@ -197,11 +197,6 @@ extern void device_resume(void);
 extern int device_suspend(pm_message_t state);
 extern int device_prepare_suspend(pm_message_t state);
 
-#define device_set_wakeup_enable(dev,val) \
-	((dev)->power.should_wakeup = !!(val))
-#define device_may_wakeup(dev) \
-	(device_can_wakeup(dev) && (dev)->power.should_wakeup)
-
 extern void __suspend_report_result(const char *function, void *fn, int ret);
 
 #define suspend_report_result(fn, ret)					\
@@ -209,6 +204,35 @@ extern void __suspend_report_result(cons
 		__suspend_report_result(__FUNCTION__, fn, ret);		\
 	} while (0)
 
+#else /* !CONFIG_PM_SLEEP */
+
+static inline int device_suspend(pm_message_t state)
+{
+	return 0;
+}
+
+#define suspend_report_result(fn, ret) 		do {} while (0)
+
+#endif /* !CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_PM
+
+/* changes to device_may_wakeup take effect on the next pm state change.
+ * by default, devices should wakeup if they can.
+ */
+#define device_can_wakeup(dev) \
+	((dev)->power.can_wakeup)
+#define device_init_wakeup(dev,val) \
+	do { \
+		device_can_wakeup(dev) = !!(val); \
+		device_set_wakeup_enable(dev,val); \
+	} while(0)
+
+#define device_set_wakeup_enable(dev,val) \
+	((dev)->power.should_wakeup = !!(val))
+#define device_may_wakeup(dev) \
+	(device_can_wakeup(dev) && (dev)->power.should_wakeup)
+
 /*
  * Platform hook to activate device wakeup capability, if that's not already
  * handled by enable_irq_wake() etc.
@@ -223,35 +247,19 @@ static inline int call_platform_enable_w
 	return 0;
 }
 
-#else /* !CONFIG_PM_SLEEP */
-
-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)
+#else /* !CONFIG_PM */
 
-#define suspend_report_result(fn, ret) do { } while (0)
+#define device_can_wakeup(dev)			0
+#define device_init_wakeup(dev,val)		do {} while (0)
+#define device_set_wakeup_enable(dev,val)	do {} while (0)
+#define device_may_wakeup(dev)			0
 
 static inline int call_platform_enable_wakeup(struct device *dev, int is_on)
 {
 	return 0;
 }
 
-#endif /* !CONFIG_PM_SLEEP */
-
-/* changes to device_may_wakeup take effect on the next pm state change.
- * by default, devices should wakeup if they can.
- */
-#define device_can_wakeup(dev) \
-	((dev)->power.can_wakeup)
-#define device_init_wakeup(dev,val) \
-	do { \
-		device_can_wakeup(dev) = !!(val); \
-		device_set_wakeup_enable(dev,val); \
-	} while(0)
+#endif /* !CONFIG_PM */
 
 /*
  * Global Power Management flags
Index: usb-2.6/drivers/base/power/main.c
===================================================================
--- usb-2.6.orig/drivers/base/power/main.c
+++ usb-2.6/drivers/base/power/main.c
@@ -56,8 +56,6 @@ static DEFINE_MUTEX(dpm_list_mtx);
 
 static DECLARE_RWSEM(pm_sleep_rwsem);
 
-int (*platform_enable_wakeup)(struct device *dev, int is_on);
-
 /**
  *	device_pm_add - add a device to the list of active devices
  *	@dev:	Device to be added to the list
Index: usb-2.6/drivers/base/power/sysfs.c
===================================================================
--- usb-2.6.orig/drivers/base/power/sysfs.c
+++ usb-2.6/drivers/base/power/sysfs.c
@@ -6,6 +6,8 @@
 #include <linux/string.h>
 #include "power.h"
 
+int (*platform_enable_wakeup)(struct device *dev, int is_on);
+
 
 /*
  *	wakeup - Report/change current wakeup option for device