* [PATCH 1/3] rework pm_ops pm_disk_mode, kill misuse
2007-03-21 14:53 [PATCH 0/3] suspend pm_ops cleanup Johannes Berg
@ 2007-03-21 14:53 ` Johannes Berg
2007-03-21 23:45 ` Pavel Machek
2007-03-21 14:53 ` [PATCH 2/3] power management: remove firmware disk mode Johannes Berg
2007-03-21 14:53 ` [PATCH 3/3] power management: implement pm_ops.valid for everybody Johannes Berg
2 siblings, 1 reply; 8+ messages in thread
From: Johannes Berg @ 2007-03-21 14:53 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-pm, Pavel Machek
[-- Attachment #1: 011-pm-ops-diskmode.patch --]
[-- Type: text/plain, Size: 11426 bytes --]
The pm_ops.pm_disk_mode is used in totally bogus ways since
nobody really seems to understand what it actually does.
This patch clarifies the pm_disk_mode description.
It also removes all the arm and sh users that think they can veto
suspend to disk via pm_ops; not so since the user can always
do echo shutdown > /sys/power/disk, they need to find a better
way involving Kconfig or such.
ACPI is the only user left with a non-zero pm_disk_mode.
The patch also sets the default mode to shutdown again, but
when a new pm_ops is registered its pm_disk_mode is selected
as default, that way the default stays for ACPI where it is
apparently required.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Cc: David Brownell <david-b@pacbell.net>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: linux-pm@lists.linux-foundation.org
---
arch/arm/common/sharpsl_pm.c | 1
arch/arm/mach-at91/pm.c | 1
arch/arm/mach-omap1/pm.c | 1
arch/arm/mach-omap2/pm.c | 1
arch/arm/mach-pxa/pm.c | 4 ---
arch/arm/mach-sa1100/pm.c | 7 -----
arch/arm/plat-s3c24xx/pm.c | 9 ------
arch/sh/boards/hp6xx/pm.c | 7 -----
include/linux/pm.h | 23 +++++++++--------
kernel/power/disk.c | 56 ++++++++++++++++++++++++++++---------------
kernel/power/main.c | 6 +++-
11 files changed, 54 insertions(+), 62 deletions(-)
--- linux-2.6.orig/include/linux/pm.h 2007-03-20 12:41:38.423214909 +0100
+++ linux-2.6/include/linux/pm.h 2007-03-20 12:42:02.253214909 +0100
@@ -112,6 +112,8 @@ typedef int __bitwise suspend_state_t;
typedef int __bitwise suspend_disk_method_t;
+/* invalid must be 0 so struct pm_ops initialisers can leave it out */
+#define PM_DISK_INVALID ((__force suspend_disk_method_t) 0)
#define PM_DISK_FIRMWARE ((__force suspend_disk_method_t) 1)
#define PM_DISK_PLATFORM ((__force suspend_disk_method_t) 2)
#define PM_DISK_SHUTDOWN ((__force suspend_disk_method_t) 3)
@@ -137,17 +139,16 @@ typedef int __bitwise suspend_disk_metho
* @finish: Called when the system has left the given state and all devices
* are resumed. The return value is ignored.
*
- * @pm_disk_mode: Set to the disk method that the user should be able to
- * configure for suspend-to-disk. Since %PM_DISK_SHUTDOWN,
- * %PM_DISK_REBOOT, %PM_DISK_TEST and %PM_DISK_TESTPROC
- * are always allowed, currently only %PM_DISK_PLATFORM
- * makes sense. If the user then choses %PM_DISK_PLATFORM,
- * the @prepare call will be called before suspending to disk
- * (if present), the @enter call should be present and will
- * be called after all state has been saved and the machine
- * is ready to be shut down/suspended/..., and the @finish
- * callback is called after state has been restored. All
- * these calls are called with %PM_SUSPEND_DISK as the state.
+ * @pm_disk_mode: The generic code always allows one of the shutdown methods
+ * %PM_DISK_SHUTDOWN, %PM_DISK_REBOOT, %PM_DISK_TEST and
+ * %PM_DISK_TESTPROC. If this variable is set, the mode it is set
+ * to is allowed in addition to those modes and is also made default.
+ * When this mode is sent selected, the @prepare call will be called
+ * before suspending to disk (if present), the @enter call should be
+ * present and will be called after all state has been saved and the
+ * machine is ready to be powered off; the @finish callback is called
+ * after state has been restored. All these calls are called with
+ * %PM_SUSPEND_DISK as the state.
*/
struct pm_ops {
int (*valid)(suspend_state_t state);
--- linux-2.6.orig/kernel/power/disk.c 2007-03-20 12:41:38.463214909 +0100
+++ linux-2.6/kernel/power/disk.c 2007-03-20 12:42:02.263214909 +0100
@@ -39,7 +39,13 @@ static inline int platform_prepare(void)
{
int error = 0;
- if (pm_disk_mode == PM_DISK_PLATFORM) {
+ switch (pm_disk_mode) {
+ case PM_DISK_TEST:
+ case PM_DISK_TESTPROC:
+ case PM_DISK_SHUTDOWN:
+ case PM_DISK_REBOOT:
+ break;
+ default:
if (pm_ops && pm_ops->prepare)
error = pm_ops->prepare(PM_SUSPEND_DISK);
}
@@ -48,31 +54,33 @@ static inline int platform_prepare(void)
/**
* power_down - Shut machine down for hibernate.
- * @mode: Suspend-to-disk mode
*
- * Use the platform driver, if configured so, and return gracefully if it
- * fails.
- * Otherwise, try to power off and reboot. If they fail, halt the machine,
- * there ain't no turning back.
+ * Use the platform driver, if configured so; otherwise try
+ * to power off or reboot.
*/
-static void power_down(suspend_disk_method_t mode)
+static void power_down(void)
{
disable_nonboot_cpus();
- switch(mode) {
- case PM_DISK_PLATFORM:
- if (pm_ops && pm_ops->enter) {
- kernel_shutdown_prepare(SYSTEM_SUSPEND_DISK);
- pm_ops->enter(PM_SUSPEND_DISK);
- break;
- }
+
+ switch (pm_disk_mode) {
+ case PM_DISK_TEST:
+ case PM_DISK_TESTPROC:
+ break;
case PM_DISK_SHUTDOWN:
kernel_power_off();
break;
case PM_DISK_REBOOT:
kernel_restart(NULL);
break;
+ default:
+ if (pm_ops && pm_ops->enter) {
+ kernel_shutdown_prepare(SYSTEM_SUSPEND_DISK);
+ pm_ops->enter(PM_SUSPEND_DISK);
+ break;
+ }
}
+
kernel_halt();
/* Valid image is on the disk, if we continue we risk serious data corruption
after resume. */
@@ -82,7 +90,13 @@ static void power_down(suspend_disk_meth
static inline void platform_finish(void)
{
- if (pm_disk_mode == PM_DISK_PLATFORM) {
+ switch (pm_disk_mode) {
+ case PM_DISK_TEST:
+ case PM_DISK_TESTPROC:
+ case PM_DISK_SHUTDOWN:
+ case PM_DISK_REBOOT:
+ break;
+ default:
if (pm_ops && pm_ops->finish)
pm_ops->finish(PM_SUSPEND_DISK);
}
@@ -167,7 +181,7 @@ int pm_suspend_disk(void)
pr_debug("PM: writing image.\n");
error = swsusp_write();
if (!error)
- power_down(pm_disk_mode);
+ power_down();
else {
swsusp_free();
goto Thaw;
@@ -347,10 +361,14 @@ static ssize_t disk_store(struct subsyst
}
}
if (mode) {
- if (mode == PM_DISK_SHUTDOWN || mode == PM_DISK_REBOOT ||
- mode == PM_DISK_TEST || mode == PM_DISK_TESTPROC) {
+ switch (mode) {
+ case PM_DISK_SHUTDOWN:
+ case PM_DISK_REBOOT:
+ case PM_DISK_TEST:
+ case PM_DISK_TESTPROC:
pm_disk_mode = mode;
- } else {
+ break;
+ default:
if (pm_ops && pm_ops->enter &&
(mode == pm_ops->pm_disk_mode))
pm_disk_mode = mode;
--- linux-2.6.orig/kernel/power/main.c 2007-03-20 12:41:38.543214909 +0100
+++ linux-2.6/kernel/power/main.c 2007-03-20 12:42:02.263214909 +0100
@@ -30,7 +30,7 @@
DEFINE_MUTEX(pm_mutex);
struct pm_ops *pm_ops;
-suspend_disk_method_t pm_disk_mode = PM_DISK_PLATFORM;
+suspend_disk_method_t pm_disk_mode = PM_DISK_SHUTDOWN;
/**
* pm_set_ops - Set the global power method table.
@@ -41,6 +41,10 @@ void pm_set_ops(struct pm_ops * ops)
{
mutex_lock(&pm_mutex);
pm_ops = ops;
+ if (ops && ops->pm_disk_mode != PM_DISK_INVALID) {
+ pm_disk_mode = ops->pm_disk_mode;
+ } else
+ pm_disk_mode = PM_DISK_SHUTDOWN;
mutex_unlock(&pm_mutex);
}
--- linux-2.6.orig/arch/arm/common/sharpsl_pm.c 2007-03-20 12:41:38.673214909 +0100
+++ linux-2.6/arch/arm/common/sharpsl_pm.c 2007-03-20 12:42:02.263214909 +0100
@@ -766,7 +766,6 @@ static void sharpsl_apm_get_power_status
}
static struct pm_ops sharpsl_pm_ops = {
- .pm_disk_mode = PM_DISK_FIRMWARE,
.prepare = pxa_pm_prepare,
.enter = corgi_pxa_pm_enter,
.finish = pxa_pm_finish,
--- linux-2.6.orig/arch/arm/mach-at91/pm.c 2007-03-20 12:41:38.743214909 +0100
+++ linux-2.6/arch/arm/mach-at91/pm.c 2007-03-20 12:42:02.263214909 +0100
@@ -201,7 +201,6 @@ error:
static struct pm_ops at91_pm_ops ={
- .pm_disk_mode = 0,
.valid = at91_pm_valid_state,
.prepare = at91_pm_prepare,
.enter = at91_pm_enter,
--- linux-2.6.orig/arch/arm/mach-omap1/pm.c 2007-03-20 12:41:38.763214909 +0100
+++ linux-2.6/arch/arm/mach-omap1/pm.c 2007-03-20 12:42:02.263214909 +0100
@@ -698,7 +698,6 @@ static struct irqaction omap_wakeup_irq
static struct pm_ops omap_pm_ops ={
- .pm_disk_mode = 0,
.prepare = omap_pm_prepare,
.enter = omap_pm_enter,
.finish = omap_pm_finish,
--- linux-2.6.orig/arch/arm/mach-omap2/pm.c 2007-03-20 12:41:38.833214909 +0100
+++ linux-2.6/arch/arm/mach-omap2/pm.c 2007-03-20 12:42:02.263214909 +0100
@@ -370,7 +370,6 @@ static int omap2_pm_finish(suspend_state
}
static struct pm_ops omap_pm_ops = {
- .pm_disk_mode = 0,
.prepare = omap2_pm_prepare,
.enter = omap2_pm_enter,
.finish = omap2_pm_finish,
--- linux-2.6.orig/arch/arm/mach-pxa/pm.c 2007-03-20 12:41:38.853214909 +0100
+++ linux-2.6/arch/arm/mach-pxa/pm.c 2007-03-20 12:42:02.263214909 +0100
@@ -223,11 +223,7 @@ int pxa_pm_finish(suspend_state_t state)
EXPORT_SYMBOL_GPL(pxa_pm_finish);
-/*
- * Set to PM_DISK_FIRMWARE so we can quickly veto suspend-to-disk.
- */
static struct pm_ops pxa_pm_ops = {
- .pm_disk_mode = PM_DISK_FIRMWARE,
.prepare = pxa_pm_prepare,
.enter = pxa_pm_enter,
.finish = pxa_pm_finish,
--- linux-2.6.orig/arch/arm/mach-sa1100/pm.c 2007-03-20 12:41:38.903214909 +0100
+++ linux-2.6/arch/arm/mach-sa1100/pm.c 2007-03-20 12:42:02.273214909 +0100
@@ -59,9 +59,6 @@ static int sa11x0_pm_enter(suspend_state
unsigned long gpio, sleep_save[SLEEP_SAVE_SIZE];
struct timespec delta, rtc;
- if (state != PM_SUSPEND_MEM)
- return -EINVAL;
-
/* preserve current time */
rtc.tv_sec = RCNR;
rtc.tv_nsec = 0;
@@ -134,11 +131,7 @@ unsigned long sleep_phys_sp(void *sp)
return virt_to_phys(sp);
}
-/*
- * Set to PM_DISK_FIRMWARE so we can quickly veto suspend-to-disk.
- */
static struct pm_ops sa11x0_pm_ops = {
- .pm_disk_mode = PM_DISK_FIRMWARE,
.enter = sa11x0_pm_enter,
};
--- linux-2.6.orig/arch/arm/plat-s3c24xx/pm.c 2007-03-20 12:41:38.953214909 +0100
+++ linux-2.6/arch/arm/plat-s3c24xx/pm.c 2007-03-20 12:42:02.273214909 +0100
@@ -511,11 +511,6 @@ static int s3c2410_pm_enter(suspend_stat
return -EINVAL;
}
- if (state != PM_SUSPEND_MEM) {
- printk(KERN_ERR PFX "error: only PM_SUSPEND_MEM supported\n");
- return -EINVAL;
- }
-
/* check if we have anything to wake-up with... bad things seem
* to happen if you suspend with no wakeup (system will often
* require a full power-cycle)
@@ -633,11 +628,7 @@ static int s3c2410_pm_finish(suspend_sta
return 0;
}
-/*
- * Set to PM_DISK_FIRMWARE so we can quickly veto suspend-to-disk.
- */
static struct pm_ops s3c2410_pm_ops = {
- .pm_disk_mode = PM_DISK_FIRMWARE,
.prepare = s3c2410_pm_prepare,
.enter = s3c2410_pm_enter,
.finish = s3c2410_pm_finish,
--- linux-2.6.orig/arch/sh/boards/hp6xx/pm.c 2007-03-20 12:41:39.163214909 +0100
+++ linux-2.6/arch/sh/boards/hp6xx/pm.c 2007-03-20 12:42:02.273214909 +0100
@@ -27,9 +27,6 @@ static int hp6x0_pm_enter(suspend_state_
u16 hd64461_stbcr;
#endif
- if (state != PM_SUSPEND_MEM)
- return -EINVAL;
-
#ifdef CONFIG_HD64461_ENABLER
outb(0, HD64461_PCC1CSCIER);
@@ -70,11 +67,7 @@ static int hp6x0_pm_enter(suspend_state_
return 0;
}
-/*
- * Set to PM_DISK_FIRMWARE so we can quickly veto suspend-to-disk.
- */
static struct pm_ops hp6x0_pm_ops = {
- .pm_disk_mode = PM_DISK_FIRMWARE,
.enter = hp6x0_pm_enter,
};
--
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 2/3] power management: remove firmware disk mode
2007-03-21 14:53 [PATCH 0/3] suspend pm_ops cleanup Johannes Berg
2007-03-21 14:53 ` [PATCH 1/3] rework pm_ops pm_disk_mode, kill misuse Johannes Berg
@ 2007-03-21 14:53 ` Johannes Berg
2007-03-21 23:49 ` Pavel Machek
2007-03-21 14:53 ` [PATCH 3/3] power management: implement pm_ops.valid for everybody Johannes Berg
2 siblings, 1 reply; 8+ messages in thread
From: Johannes Berg @ 2007-03-21 14:53 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-pm, Pavel Machek
[-- Attachment #1: 012-remove-firmware-disk-mode.patch --]
[-- Type: text/plain, Size: 9812 bytes --]
This patch removes the firmware disk suspend mode which is the wrong
approach, it is supposed to be used for implementing firmware-based disk
suspend but cannot actually be used for that.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: linux-pm@lists.linux-foundation.org
---
Documentation/power/interface.txt | 21 +++++----------------
Documentation/power/states.txt | 13 +++++++------
Documentation/power/swsusp.txt | 14 +++++---------
include/linux/pm.h | 13 ++++++-------
kernel/power/disk.c | 27 +++++++++++----------------
5 files changed, 34 insertions(+), 54 deletions(-)
--- linux-2.6.orig/include/linux/pm.h 2007-03-21 15:44:54.663148946 +0100
+++ linux-2.6/include/linux/pm.h 2007-03-21 15:45:04.403148946 +0100
@@ -114,13 +114,12 @@ typedef int __bitwise suspend_disk_metho
/* invalid must be 0 so struct pm_ops initialisers can leave it out */
#define PM_DISK_INVALID ((__force suspend_disk_method_t) 0)
-#define PM_DISK_FIRMWARE ((__force suspend_disk_method_t) 1)
-#define PM_DISK_PLATFORM ((__force suspend_disk_method_t) 2)
-#define PM_DISK_SHUTDOWN ((__force suspend_disk_method_t) 3)
-#define PM_DISK_REBOOT ((__force suspend_disk_method_t) 4)
-#define PM_DISK_TEST ((__force suspend_disk_method_t) 5)
-#define PM_DISK_TESTPROC ((__force suspend_disk_method_t) 6)
-#define PM_DISK_MAX ((__force suspend_disk_method_t) 7)
+#define PM_DISK_PLATFORM ((__force suspend_disk_method_t) 1)
+#define PM_DISK_SHUTDOWN ((__force suspend_disk_method_t) 2)
+#define PM_DISK_REBOOT ((__force suspend_disk_method_t) 3)
+#define PM_DISK_TEST ((__force suspend_disk_method_t) 4)
+#define PM_DISK_TESTPROC ((__force suspend_disk_method_t) 5)
+#define PM_DISK_MAX ((__force suspend_disk_method_t) 6)
/**
* struct pm_ops - Callbacks for managing platform dependent suspend states.
--- linux-2.6.orig/kernel/power/disk.c 2007-03-21 15:44:54.693148946 +0100
+++ linux-2.6/kernel/power/disk.c 2007-03-21 15:48:06.073148946 +0100
@@ -123,8 +123,6 @@ static int prepare_processes(void)
/**
* pm_suspend_disk - The granpappy of hibernation power management.
*
- * If we're going through the firmware, then get it over with quickly.
- *
* If not, then call swsusp to do its thing, then figure out how
* to power down the system.
*/
@@ -301,7 +299,6 @@ late_initcall(software_resume);
static const char * const pm_disk_modes[] = {
- [PM_DISK_FIRMWARE] = "firmware",
[PM_DISK_PLATFORM] = "platform",
[PM_DISK_SHUTDOWN] = "shutdown",
[PM_DISK_REBOOT] = "reboot",
@@ -312,27 +309,25 @@ static const char * const pm_disk_modes[
/**
* disk - Control suspend-to-disk mode
*
- * Suspend-to-disk can be handled in several ways. The greatest
- * distinction is who writes memory to disk - the firmware or the OS.
- * If the firmware does it, we assume that it also handles suspending
- * the system.
- * If the OS does it, then we have three options for putting the system
- * to sleep - using the platform driver (e.g. ACPI or other PM registers),
- * powering off the system or rebooting the system (for testing).
+ * Suspend-to-disk can be handled in several ways. We have a few options
+ * for putting the system to sleep - using the platform driver (e.g. ACPI
+ * or other pm_ops), powering off the system or rebooting the system
+ * (for testing) as well as the two test modes.
*
- * The system will support either 'firmware' or 'platform', and that is
- * known a priori (and encoded in pm_ops). But, the user may choose
- * 'shutdown' or 'reboot' as alternatives.
+ * The system can support 'platform', and that is known a priori (and
+ * encoded in pm_ops). However, the user may choose 'shutdown' or 'reboot'
+ * as alternatives, as well as the test modes 'test' and 'testproc'.
*
* show() will display what the mode is currently set to.
* store() will accept one of
*
- * 'firmware'
* 'platform'
* 'shutdown'
* 'reboot'
+ * 'test'
+ * 'testproc'
*
- * It will only change to 'firmware' or 'platform' if the system
+ * It will only change to 'platform' if the system
* supports it (as determined from pm_ops->pm_disk_mode).
*/
@@ -354,7 +349,7 @@ static ssize_t disk_store(struct subsyst
len = p ? p - buf : n;
mutex_lock(&pm_mutex);
- for (i = PM_DISK_FIRMWARE; i < PM_DISK_MAX; i++) {
+ for (i = PM_DISK_PLATFORM; i < PM_DISK_MAX; i++) {
if (!strncmp(buf, pm_disk_modes[i], len)) {
mode = i;
break;
--- linux-2.6.orig/Documentation/power/interface.txt 2007-03-21 15:45:17.873148946 +0100
+++ linux-2.6/Documentation/power/interface.txt 2007-03-21 15:49:26.673148946 +0100
@@ -18,17 +18,10 @@ states.
/sys/power/disk controls the operating mode of the suspend-to-disk
-mechanism. Suspend-to-disk can be handled in several ways. The
-greatest distinction is who writes memory to disk - the firmware or
-the kernel. If the firmware does it, we assume that it also handles
-suspending the system.
-
-If the kernel does it, then we have three options for putting the system
-to sleep - using the platform driver (e.g. ACPI or other PM
-registers), powering off the system or rebooting the system (for
-testing). The system will support either 'firmware' or 'platform', and
-that is known a priori. But, the user may choose 'shutdown' or
-'reboot' as alternatives.
+mechanism. Suspend-to-disk can be handled in several ways. We have a
+few options for putting the system to sleep - using the platform driver
+(e.g. ACPI or other pm_ops), powering off the system or rebooting the
+system (for testing).
Additionally, /sys/power/disk can be used to turn on one of the two testing
modes of the suspend-to-disk mechanism: 'testproc' or 'test'. If the
@@ -44,16 +37,12 @@ is being slow and which device drivers a
Reading from this file will display what the mode is currently set
to. Writing to this file will accept one of
- 'firmware'
- 'platform'
+ 'platform' (only if the platform supports it)
'shutdown'
'reboot'
'testproc'
'test'
-It will only change to 'firmware' or 'platform' if the system supports
-it.
-
/sys/power/image_size controls the size of the image created by
the suspend-to-disk mechanism. It can be written a string
representing a non-negative integer that will be used as an upper
--- linux-2.6.orig/Documentation/power/states.txt 2007-03-21 15:45:17.993148946 +0100
+++ linux-2.6/Documentation/power/states.txt 2007-03-21 15:51:18.763148946 +0100
@@ -62,17 +62,18 @@ setup via another operating system for i
inconvenience, this method requires minimal work by the kernel, since
the firmware will also handle restoring memory contents on resume.
-If the kernel is responsible for persistently saving state, a mechanism
-called 'swsusp' (Swap Suspend) is used to write memory contents to
-free swap space. swsusp has some restrictive requirements, but should
-work in most cases. Some, albeit outdated, documentation can be found
-in Documentation/power/swsusp.txt.
+For suspend-to-disk, a mechanism called swsusp called 'swsusp' (Swap
+Suspend) is used to write memory contents to free swap space.
+swsusp has some restrictive requirements, but should work in most
+cases. Some, albeit outdated, documentation can be found in
+Documentation/power/swsusp.txt. Alternatively, userspace can do most
+of the actual suspend to disk work, see userland-swsusp.txt.
Once memory state is written to disk, the system may either enter a
low-power state (like ACPI S4), or it may simply power down. Powering
down offers greater savings, and allows this mechanism to work on any
system. However, entering a real low-power state allows the user to
-trigger wake up events (e.g. pressing a key or opening a laptop lid).
+trigger wake up events (e.g. pressing a key or opening a laptop lid).
A transition from Suspend-to-Disk to the On state should take about 30
seconds, though it's typically a bit more with the current
--- linux-2.6.orig/Documentation/power/swsusp.txt 2007-03-21 15:45:18.133148946 +0100
+++ linux-2.6/Documentation/power/swsusp.txt 2007-03-21 15:52:20.423148946 +0100
@@ -156,8 +156,7 @@ instead set the PF_NOFREEZE process flag
be very careful).
-Q: What is the difference between "platform", "shutdown" and
-"firmware" in /sys/power/disk?
+Q: What is the difference between "platform" and "shutdown"?
A:
@@ -166,11 +165,8 @@ shutdown: save state in linux, then tell
platform: save state in linux, then tell bios to powerdown and blink
"suspended led"
-firmware: tell bios to save state itself [needs BIOS-specific suspend
- partition, and has very little to do with swsusp]
-
-"platform" is actually right thing to do, but "shutdown" is most
-reliable.
+"platform" is actually right thing to do where supported, but
+"shutdown" is most reliable (except on ACPI systems).
Q: I do not understand why you have such strong objections to idea of
selective suspend.
@@ -388,8 +384,8 @@ while the system is asleep, maintaining
modes like "suspend-to-RAM" or "standby". (Don't write "disk" to the
/sys/power/state file; write "standby" or "mem".) We've not seen any
hardware that can use these modes through software suspend, although in
-theory some systems might support "platform" or "firmware" modes that
-won't break the USB connections.
+theory some systems might support "platform" modes that won't break the
+USB connections.
Remember that it's always a bad idea to unplug a disk drive containing a
mounted filesystem. That's true even when your system is asleep! The
--
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 3/3] power management: implement pm_ops.valid for everybody
2007-03-21 14:53 [PATCH 0/3] suspend pm_ops cleanup Johannes Berg
2007-03-21 14:53 ` [PATCH 1/3] rework pm_ops pm_disk_mode, kill misuse Johannes Berg
2007-03-21 14:53 ` [PATCH 2/3] power management: remove firmware disk mode Johannes Berg
@ 2007-03-21 14:53 ` Johannes Berg
2007-03-21 23:51 ` Pavel Machek
2 siblings, 1 reply; 8+ messages in thread
From: Johannes Berg @ 2007-03-21 14:53 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-pm, Pavel Machek
[-- Attachment #1: 013-implement-valid.patch --]
[-- Type: text/plain, Size: 7606 bytes --]
Almost all users of pm_ops only support mem sleep, don't check in .valid
and don't reject any others in .prepare so users can be confused if they
check /sys/power/state, especially when new states are added (these would
then result in s-t-r although they're supposed to be something different).
This patch implements a generic pm_valid_only_mem function that is then
exported for users and puts it to use in almost all existing pm_ops.
Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Cc: David Brownell <david-b@pacbell.net>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: linux-pm@lists.linux-foundation.org
---
arch/arm/common/sharpsl_pm.c | 1 +
arch/arm/mach-omap1/pm.c | 1 +
arch/arm/mach-omap2/pm.c | 1 +
arch/arm/mach-pnx4008/pm.c | 35 +----------------------------------
arch/arm/mach-pxa/pm.c | 1 +
arch/arm/mach-sa1100/pm.c | 1 +
arch/arm/plat-s3c24xx/pm.c | 19 +------------------
arch/sh/boards/hp6xx/pm.c | 1 +
drivers/acpi/sleep/main.c | 13 +++++++++++--
include/linux/pm.h | 4 ++++
kernel/power/main.c | 13 +++++++++++++
11 files changed, 36 insertions(+), 54 deletions(-)
--- linux-2.6.orig/include/linux/pm.h 2007-03-20 12:42:04.813214909 +0100
+++ linux-2.6/include/linux/pm.h 2007-03-20 12:42:05.693214909 +0100
@@ -128,6 +128,9 @@ typedef int __bitwise suspend_disk_metho
* always valid and never passed to this call.
* If not assigned, all suspend states are advertised as valid
* in /sys/power/state (but can still be rejected by prepare or enter.)
+ * Since new states can be added for other platforms, you should
+ * assign this callback. There is a %pm_valid_only_mem function
+ * available if you only implemented mem sleep.
*
* @prepare: Prepare the platform for the given suspend state. Can return a
* negative error code if necessary.
@@ -165,6 +168,7 @@ extern void pm_set_ops(struct pm_ops *pm
extern struct pm_ops *pm_ops;
extern int pm_suspend(suspend_state_t state);
+extern int pm_valid_only_mem(suspend_state_t state);
/*
* Device power management
--- linux-2.6.orig/arch/arm/common/sharpsl_pm.c 2007-03-20 12:42:02.263214909 +0100
+++ linux-2.6/arch/arm/common/sharpsl_pm.c 2007-03-20 12:42:05.693214909 +0100
@@ -769,6 +769,7 @@ static struct pm_ops sharpsl_pm_ops = {
.prepare = pxa_pm_prepare,
.enter = corgi_pxa_pm_enter,
.finish = pxa_pm_finish,
+ .valid = pm_valid_only_mem,
};
static int __init sharpsl_pm_probe(struct platform_device *pdev)
--- linux-2.6.orig/arch/arm/mach-omap1/pm.c 2007-03-20 12:42:02.263214909 +0100
+++ linux-2.6/arch/arm/mach-omap1/pm.c 2007-03-20 12:42:05.703214909 +0100
@@ -701,6 +701,7 @@ static struct pm_ops omap_pm_ops ={
.prepare = omap_pm_prepare,
.enter = omap_pm_enter,
.finish = omap_pm_finish,
+ .valid = pm_valid_only_mem,
};
static int __init omap_pm_init(void)
--- linux-2.6.orig/arch/arm/mach-omap2/pm.c 2007-03-20 12:42:02.263214909 +0100
+++ linux-2.6/arch/arm/mach-omap2/pm.c 2007-03-20 12:42:05.703214909 +0100
@@ -373,6 +373,7 @@ static struct pm_ops omap_pm_ops = {
.prepare = omap2_pm_prepare,
.enter = omap2_pm_enter,
.finish = omap2_pm_finish,
+ .valid = pm_valid_only_mem,
};
int __init omap2_pm_init(void)
--- linux-2.6.orig/arch/arm/mach-pnx4008/pm.c 2007-03-20 12:41:36.133214909 +0100
+++ linux-2.6/arch/arm/mach-pnx4008/pm.c 2007-03-20 12:42:05.703214909 +0100
@@ -115,42 +115,9 @@ static int pnx4008_pm_enter(suspend_stat
return 0;
}
-/*
- * Called after processes are frozen, but before we shut down devices.
- */
-static int pnx4008_pm_prepare(suspend_state_t state)
-{
- switch (state) {
- case PM_SUSPEND_STANDBY:
- case PM_SUSPEND_MEM:
- break;
-
- case PM_SUSPEND_DISK:
- return -ENOTSUPP;
- break;
-
- default:
- return -EINVAL;
- break;
- }
- return 0;
-}
-
-/*
- * Called after devices are re-setup, but before processes are thawed.
- */
-static int pnx4008_pm_finish(suspend_state_t state)
-{
- return 0;
-}
-
-/*
- * Set to PM_DISK_FIRMWARE so we can quickly veto suspend-to-disk.
- */
static struct pm_ops pnx4008_pm_ops = {
- .prepare = pnx4008_pm_prepare,
.enter = pnx4008_pm_enter,
- .finish = pnx4008_pm_finish,
+ .valid = pm_valid_only_mem,
};
static int __init pnx4008_pm_init(void)
--- linux-2.6.orig/arch/arm/mach-pxa/pm.c 2007-03-20 12:42:02.263214909 +0100
+++ linux-2.6/arch/arm/mach-pxa/pm.c 2007-03-20 12:42:05.703214909 +0100
@@ -227,6 +227,7 @@ static struct pm_ops pxa_pm_ops = {
.prepare = pxa_pm_prepare,
.enter = pxa_pm_enter,
.finish = pxa_pm_finish,
+ .valid = pm_valid_only_mem,
};
static int __init pxa_pm_init(void)
--- linux-2.6.orig/arch/arm/mach-sa1100/pm.c 2007-03-20 12:42:02.273214909 +0100
+++ linux-2.6/arch/arm/mach-sa1100/pm.c 2007-03-20 12:42:05.703214909 +0100
@@ -133,6 +133,7 @@ unsigned long sleep_phys_sp(void *sp)
static struct pm_ops sa11x0_pm_ops = {
.enter = sa11x0_pm_enter,
+ .valid = pm_valid_only_mem,
};
static int __init sa11x0_pm_init(void)
--- linux-2.6.orig/arch/arm/plat-s3c24xx/pm.c 2007-03-20 12:42:02.273214909 +0100
+++ linux-2.6/arch/arm/plat-s3c24xx/pm.c 2007-03-20 12:42:05.703214909 +0100
@@ -612,26 +612,9 @@ static int s3c2410_pm_enter(suspend_stat
return 0;
}
-/*
- * Called after processes are frozen, but before we shut down devices.
- */
-static int s3c2410_pm_prepare(suspend_state_t state)
-{
- return 0;
-}
-
-/*
- * Called after devices are re-setup, but before processes are thawed.
- */
-static int s3c2410_pm_finish(suspend_state_t state)
-{
- return 0;
-}
-
static struct pm_ops s3c2410_pm_ops = {
- .prepare = s3c2410_pm_prepare,
.enter = s3c2410_pm_enter,
- .finish = s3c2410_pm_finish,
+ .valid = pm_valid_only_mem,
};
/* s3c2410_pm_init
--- linux-2.6.orig/arch/sh/boards/hp6xx/pm.c 2007-03-20 12:42:02.273214909 +0100
+++ linux-2.6/arch/sh/boards/hp6xx/pm.c 2007-03-20 12:42:05.713214909 +0100
@@ -69,6 +69,7 @@ static int hp6x0_pm_enter(suspend_state_
static struct pm_ops hp6x0_pm_ops = {
.enter = hp6x0_pm_enter,
+ .valid = pm_valid_only_mem,
};
static int __init hp6x0_pm_init(void)
--- linux-2.6.orig/drivers/acpi/sleep/main.c 2007-03-20 12:41:36.863214909 +0100
+++ linux-2.6/drivers/acpi/sleep/main.c 2007-03-20 12:42:05.713214909 +0100
@@ -168,9 +168,18 @@ int acpi_suspend(u32 acpi_state)
static int acpi_pm_state_valid(suspend_state_t pm_state)
{
- u32 acpi_state = acpi_suspend_states[pm_state];
+ u32 acpi_state;
- return sleep_states[acpi_state];
+ switch (pm_state) {
+ case PM_SUSPEND_ON:
+ case PM_SUSPEND_STANDBY:
+ case PM_SUSPEND_MEM:
+ acpi_state = acpi_suspend_states[pm_state];
+
+ return sleep_states[acpi_state];
+ default:
+ return 0;
+ }
}
static struct pm_ops acpi_pm_ops = {
--- linux-2.6.orig/kernel/power/main.c 2007-03-20 12:42:02.263214909 +0100
+++ linux-2.6/kernel/power/main.c 2007-03-20 12:42:05.713214909 +0100
@@ -48,6 +48,19 @@ void pm_set_ops(struct pm_ops * ops)
mutex_unlock(&pm_mutex);
}
+/**
+ * pm_valid_only_mem - generic memory-only valid callback
+ *
+ * pm_ops drivers that implement mem suspend only and only need
+ * to check for that in their .valid callback can use this instead
+ * of rolling their own .valid callback.
+ */
+int pm_valid_only_mem(suspend_state_t state)
+{
+ return state == PM_SUSPEND_MEM;
+}
+
+
static inline void pm_finish(suspend_state_t state)
{
if (pm_ops->finish)
--
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 3/3] power management: implement pm_ops.valid for everybody
2007-03-21 14:53 ` [PATCH 3/3] power management: implement pm_ops.valid for everybody Johannes Berg
@ 2007-03-21 23:51 ` Pavel Machek
2007-03-22 1:01 ` Andrew Morton
0 siblings, 1 reply; 8+ messages in thread
From: Pavel Machek @ 2007-03-21 23:51 UTC (permalink / raw)
To: Johannes Berg; +Cc: Andrew Morton, linux-pm
On Wed 2007-03-21 15:53:31, Johannes Berg wrote:
> Almost all users of pm_ops only support mem sleep, don't check in .valid
> and don't reject any others in .prepare so users can be confused if they
> check /sys/power/state, especially when new states are added (these would
> then result in s-t-r although they're supposed to be something different).
>
> This patch implements a generic pm_valid_only_mem function that is then
> exported for users and puts it to use in almost all existing pm_ops.
>
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> Cc: David Brownell <david-b@pacbell.net>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: linux-pm@lists.linux-foundation.org
Looks ok to me. ACK. (But I guess it would be nice to cc rmk or
something, so we don't conflict with his patches etc?)
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] power management: implement pm_ops.valid for everybody
2007-03-21 23:51 ` Pavel Machek
@ 2007-03-22 1:01 ` Andrew Morton
0 siblings, 0 replies; 8+ messages in thread
From: Andrew Morton @ 2007-03-22 1:01 UTC (permalink / raw)
To: Pavel Machek; +Cc: Johannes Berg, linux-pm, Russell King
On Thu, 22 Mar 2007 00:51:54 +0100 Pavel Machek <pavel@ucw.cz> wrote:
> On Wed 2007-03-21 15:53:31, Johannes Berg wrote:
> > Almost all users of pm_ops only support mem sleep, don't check in .valid
> > and don't reject any others in .prepare so users can be confused if they
> > check /sys/power/state, especially when new states are added (these would
> > then result in s-t-r although they're supposed to be something different).
> >
> > This patch implements a generic pm_valid_only_mem function that is then
> > exported for users and puts it to use in almost all existing pm_ops.
> >
> > Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> > Cc: David Brownell <david-b@pacbell.net>
> > Cc: Pavel Machek <pavel@ucw.cz>
> > Cc: linux-pm@lists.linux-foundation.org
>
> Looks ok to me. ACK. (But I guess it would be nice to cc rmk or
> something, so we don't conflict with his patches etc?)
>
I copied Russell and Paul on all three patches.
^ permalink raw reply [flat|nested] 8+ messages in thread