Skip to contentSkip to conversation

ACPI: re-name acpi_pm_ops to acpi_suspend_ops

PATCH In review Len Brown 3 participants 3 messages Apr 29, 2008 – May 1, 2008

Subsystems (6)
lore
Share & tools
lore

Len Brown

author reviewer
#0 lore raw

PATCH 1/1 ACPI: re-name acpi_pm_ops to acpi_suspend_ops

Pavel, Rafael,
I re-named the acpi_pm_* ops to be acpi_suspend_* below.

I think also that main.c isn't the best name for this file, how about acpi_platform_suspend.c, or maybe just acpi_suspend.c?

thanks,
-Len

commit 2c6e33c366bff2f839df60d9235ff09143e28dd9

Author: Len Brown <[email protected]>
Date:   Wed Apr 23 18:02:52 2008 -0400

ACPI: re-name acpi_pm_ops to acpi_suspend_ops

... as they are platform_suspend_ops after all.

cosmetic re-name only, no functional change.

drivers/acpi/sleep/main.c +21 -21
index 71183ee..c3b0cd8 100644
@@ -51,7 +51,7 @@ static int acpi_sleep_prepare(u32 acpi_state)
 }
 
 #ifdef CONFIG_SUSPEND
-static struct platform_suspend_ops acpi_pm_ops;
+static struct platform_suspend_ops acpi_suspend_ops;
 
 extern void do_suspend_lowlevel(void);
 
@@ -65,11 +65,11 @@ static u32 acpi_suspend_states[] = {
 static int init_8259A_after_S1;
 
 /**
- *	acpi_pm_begin - Set the target system sleep state to the state
+ *	acpi_suspend_begin - Set the target system sleep state to the state
  *		associated with given @pm_state, if supported.
  */
 
-static int acpi_pm_begin(suspend_state_t pm_state)
+static int acpi_suspend_begin(suspend_state_t pm_state)
 {
 	u32 acpi_state = acpi_suspend_states[pm_state];
 	int error = 0;
@@ -85,13 +85,13 @@ static int acpi_pm_begin(suspend_state_t pm_state)
 }
 
 /**
- *	acpi_pm_prepare - Do preliminary suspend work.
+ *	acpi_suspend_prepare - Do preliminary suspend work.
  *
  *	If necessary, set the firmware waking vector and do arch-specific
  *	nastiness to get the wakeup code to the waking vector.
  */
 
-static int acpi_pm_prepare(void)
+static int acpi_suspend_prepare(void)
 {
 	int error = acpi_sleep_prepare(acpi_target_sleep_state);
 
@@ -104,7 +104,7 @@ static int acpi_pm_prepare(void)
 }
 
 /**
- *	acpi_pm_enter - Actually enter a sleep state.
+ *	acpi_suspend_enter - Actually enter a sleep state.
  *	@pm_state: ignored
  *
  *	Flush caches and go to sleep. For STR we have to call arch-specific
@@ -112,7 +112,7 @@ static int acpi_pm_prepare(void)
  *	It's unfortunate, but it works. Please fix if you're feeling frisky.
  */
 
-static int acpi_pm_enter(suspend_state_t pm_state)
+static int acpi_suspend_enter(suspend_state_t pm_state)
 {
 	acpi_status status = AE_OK;
 	unsigned long flags = 0;
@@ -169,13 +169,13 @@ static int acpi_pm_enter(suspend_state_t pm_state)
 }
 
 /**
- *	acpi_pm_finish - Instruct the platform to leave a sleep state.
+ *	acpi_suspend_finish - Instruct the platform to leave a sleep state.
  *
  *	This is called after we wake back up (or if entering the sleep state
  *	failed). 
  */
 
-static void acpi_pm_finish(void)
+static void acpi_suspend_finish(void)
 {
 	u32 acpi_state = acpi_target_sleep_state;
 
@@ -196,19 +196,19 @@ static void acpi_pm_finish(void)
 }
 
 /**
- *	acpi_pm_end - Finish up suspend sequence.
+ *	acpi_suspend_end - Finish up suspend sequence.
  */
 
-static void acpi_pm_end(void)
+static void acpi_suspend_end(void)
 {
 	/*
-	 * This is necessary in case acpi_pm_finish() is not called during a
+	 * This is necessary in case acpi_suspend_finish() is not called during a
 	 * failing transition to a sleep state.
 	 */
 	acpi_target_sleep_state = ACPI_STATE_S0;
 }
 
-static int acpi_pm_state_valid(suspend_state_t pm_state)
+static int acpi_suspend_state_valid(suspend_state_t pm_state)
 {
 	u32 acpi_state;
 
@@ -224,13 +224,13 @@ static int acpi_pm_state_valid(suspend_state_t pm_state)
 	}
 }
 
-static struct platform_suspend_ops acpi_pm_ops = {
-	.valid = acpi_pm_state_valid,
-	.begin = acpi_pm_begin,
-	.prepare = acpi_pm_prepare,
-	.enter = acpi_pm_enter,
-	.finish = acpi_pm_finish,
-	.end = acpi_pm_end,
+static struct platform_suspend_ops acpi_suspend_ops = {
+	.valid = acpi_suspend_state_valid,
+	.begin = acpi_suspend_begin,
+	.prepare = acpi_suspend_prepare,
+	.enter = acpi_suspend_enter,
+	.finish = acpi_suspend_finish,
+	.end = acpi_suspend_end,
 };
 
 /*
@@ -492,7 +492,7 @@ int __init acpi_sleep_init(void)
 		}
 	}
 
-	suspend_set_ops(&acpi_pm_ops);
+	suspend_set_ops(&acpi_suspend_ops);
 #endif
 
 #ifdef CONFIG_HIBERNATION
#1 lore raw
Quoted 5 lines · Len Brown

Pavel, Rafael,
I re-named the acpi_pm_* ops to be acpi_suspend_* below.

I think also that main.c isn't the best name for this file, how about acpi_platform_suspend.c, or maybe just acpi_suspend.c?

Well, I have a patch in the works that conflicts with these changes. This is a patch to add 'acpi_old_suspend_ordering' so that people can use the ACPI 1.0 ordering of suspend code.

Unfortunately, it depends on a couple of patches in the Greg's tree, so I thought I would submit it later, but well, please find it below.

Thanks,
Rafael

---
From: Rafael J. Wysocki <[email protected]>

There are some systems out there that don't work correctly with our current suspend/hibernation code ordering. Provide a workaround for these systems allowing them to pass the 'acpi_old_suspend_ordering' command line parameter to the kernel that will make it use the pre-ACPI 2.0 suspend code ordering.

Unfortunately, that requires us to add a platform hook to the resuming of devices for recovering the platform in case one of the device drivers' .suspend() routines returns error code. Namely, ACPI 1.0 specifies that _PTS should be called before suspending devices, but _WAK still should be called before resuming them in order to undo the changes made by _PTS. However, if there is an error during suspending devices, they are automatically resumed without returning control to the PM core, so the _WAK has to be called from within device_resume() in that cases.

The patch also reorders and refactors the ACPI suspend/hibernation code to avoid duplication as far as reasonably possible.

---
 Documentation/kernel-parameters.txt |    5 
 drivers/acpi/sleep/main.c           |  310 +++++++++++++++++++++---------------
 drivers/base/power/main.c           |   15 +
 include/linux/pm.h                  |    2 
 4 files changed, 205 insertions(+), 127 deletions(-)
Documentation/kernel-parameters.txt +5 -0
Index: linux-2.6/Documentation/kernel-parameters.txt
===================================================================
@@ -170,6 +170,11 @@ and is between 256 and 4096 characters. 
 	acpi_irq_isa=	[HW,ACPI] If irq_balance, mark listed IRQs used by ISA
 			Format: <irq>,<irq>...
 
+       acpi_old_suspend_ordering [HW,ACPI]
+                       Enforce the ACPI 1.0 ordering of the _PTS control
+                       method wrt putting devices into low power states
+                       default: ACPI 2.0 ordering of _PTS
+
 	acpi_no_auto_ssdt	[HW,ACPI] Disable automatic loading of SSDT
 
 	acpi_os_name=	[HW,ACPI] Tell ACPI BIOS the name of the OS
drivers/acpi/sleep/main.c +183 -127

406 lines not on this page · raw message

drivers/base/power/main.c +15 -0
Index: linux-2.6/drivers/base/power/main.c
===================================================================
@@ -28,6 +28,20 @@
 #include "power.h"
 
 /*
+ * Some platforms (most importantly ACPI) may need to be prepared for
+ * resuming devices in case of a failure to suspend them.  They should set
+ * this pointer to the routine that will always be called just prior to resuming
+ * devices.
+ */
+void (*platform_pm_recover)(void);
+
+static inline void recover_platform(void)
+{
+	if (platform_pm_recover)
+		platform_pm_recover();
+}
+
+/*
  * The entries in the dpm_list list are in a depth first order, simply
  * because children are guaranteed to be discovered after parents, and
  * are inserted at the back of the list on discovery.
@@ -511,6 +525,7 @@ static void dpm_complete(pm_message_t st
 void device_resume(pm_message_t state)
 {
 	might_sleep();
+	recover_platform();
 	dpm_resume(state);
 	dpm_complete(state);
 }
include/linux/pm.h +2 -0
Index: linux-2.6/include/linux/pm.h
===================================================================
@@ -447,6 +447,8 @@ struct dev_pm_info {
  */
 
 #ifdef CONFIG_PM_SLEEP
+extern void (*platform_pm_recover)(void);
+
 extern void device_pm_lock(void);
 extern void device_power_up(pm_message_t state);
 extern void device_resume(pm_message_t state);
#2 lore raw
In reply to Len Brown

Hi!

Quoted 5 lines

Pavel, Rafael,
I re-named the acpi_pm_* ops to be acpi_suspend_* below.

I think also that main.c isn't the best name for this file, how about acpi_platform_suspend.c, or maybe just acpi_suspend.c?

sleep/main.c sounds quite sensible, but I guess acpi_suspend.c would be okay, too.

Quoted 17 lines

commit 2c6e33c366bff2f839df60d9235ff09143e28dd9

Author: Len Brown <[email protected]>
Date:   Wed Apr 23 18:02:52 2008 -0400

ACPI: re-name acpi_pm_ops to acpi_suspend_ops

... as they are platform_suspend_ops after all.

cosmetic re-name only, no functional change.

drivers/acpi/sleep/main.c +0 -0
index 71183ee..c3b0cd8 100644

drivers/acpi/sleep/main.c static int acpi_sleep_prepare(u32 acpi_state) · line 55

Quoted 6 lines
drivers/acpi/sleep/main.c +1 -1
@@ -51,7 +51,7 @@ static int acpi_sleep_prepare(u32 acpi_state)
 }
 
 #ifdef CONFIG_SUSPEND
-static struct platform_suspend_ops acpi_pm_ops;
+static struct platform_suspend_ops acpi_suspend_ops;
 

ACK.

drivers/acpi/sleep/main.c static u32 acpi_suspend_states[] = { · line 72

Quoted 13 lines
drivers/acpi/sleep/main.c +2 -2
 extern void do_suspend_lowlevel(void);
 
@@ -65,11 +65,11 @@ static u32 acpi_suspend_states[] = {
 static int init_8259A_after_S1;
 
 /**
- *	acpi_pm_begin - Set the target system sleep state to the state
+ *	acpi_suspend_begin - Set the target system sleep state to the state
  *		associated with given @pm_state, if supported.
  */
 
-static int acpi_pm_begin(suspend_state_t pm_state)
+static int acpi_suspend_begin(suspend_state_t pm_state)

And yes, this means it is very good idea.
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html