[PATCH 0/6] ACPI / PM: Simplifications of core suspend and hibernate code

[PATCH 0/6] ACPI / PM: Simplifications of core suspend and hibernate code

[Rafael J. Wysocki]

Hi,

The following series of patches reduces complexity of the core suspend
and hibernate code a bit.

[1/6] - Remove acpi_restore_state_mem() which isn't used.
[2/6] - Do not turn interrupts on/off in acpi_suspend_enter().
[3/6] - Move the "back to C" message and make it meaningful.
[4/6] - Move the call to acpi_save_state_mem() towards do_suspend_lowlevel().
[5/6] - Merge do_suspend_lowlevel() into acpi_save_state_mem().
[6/6] - Do not turn interrupts on/off in acpi_hibernation_enter().

I'm regarding these patches as 2.6.39 material, if there are no objections.

Thanks,
Rafael

[PATCH 1/6] ACPI / PM: Drop acpi_restore_state_mem()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The function acpi_restore_state_mem() has never been and most likely
never will be used, so remove it.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/ia64/include/asm/acpi.h |    4 ++--
 arch/ia64/kernel/acpi.c      |    5 -----
 arch/x86/include/asm/acpi.h  |    3 +--
 arch/x86/kernel/acpi/sleep.c |    8 --------
 drivers/acpi/sleep.c         |    4 ----
 5 files changed, 3 insertions(+), 21 deletions(-)

Index: linux-2.6/drivers/acpi/sleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep.c
+++ linux-2.6/drivers/acpi/sleep.c
@@ -293,10 +293,6 @@ static int acpi_suspend_enter(suspend_st
 	local_irq_restore(flags);
 	printk(KERN_DEBUG "Back to C!\n");
 
-	/* restore processor state */
-	if (acpi_state == ACPI_STATE_S3)
-		acpi_restore_state_mem();
-
 	suspend_nvs_restore();
 
 	return ACPI_SUCCESS(status) ? 0 : -EFAULT;
Index: linux-2.6/arch/ia64/include/asm/acpi.h
===================================================================
--- linux-2.6.orig/arch/ia64/include/asm/acpi.h
+++ linux-2.6/arch/ia64/include/asm/acpi.h
@@ -128,9 +128,9 @@ static inline const char *acpi_get_sysna
 int acpi_request_vector (u32 int_type);
 int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
 
-/* routines for saving/restoring kernel state */
+/* Routine for saving kernel state during suspend. */
 extern int acpi_save_state_mem(void);
-extern void acpi_restore_state_mem(void);
+
 extern unsigned long acpi_wakeup_address;
 
 /*
Index: linux-2.6/arch/ia64/kernel/acpi.c
===================================================================
--- linux-2.6.orig/arch/ia64/kernel/acpi.c
+++ linux-2.6/arch/ia64/kernel/acpi.c
@@ -1041,11 +1041,6 @@ EXPORT_SYMBOL(acpi_unregister_ioapic);
 int acpi_save_state_mem(void) { return 0; } 
 
 /*
- * acpi_restore_state()
- */
-void acpi_restore_state_mem(void) {}
-
-/*
  * do_suspend_lowlevel()
  */
 void do_suspend_lowlevel(void) {}
Index: linux-2.6/arch/x86/include/asm/acpi.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/acpi.h
+++ linux-2.6/arch/x86/include/asm/acpi.h
@@ -112,9 +112,8 @@ static inline void acpi_disable_pci(void
 	acpi_noirq_set();
 }
 
-/* routines for saving/restoring kernel state */
+/* Routine for saving kernel state during suspend. */
 extern int acpi_save_state_mem(void);
-extern void acpi_restore_state_mem(void);
 
 extern unsigned long acpi_wakeup_address;
 
Index: linux-2.6/arch/x86/kernel/acpi/sleep.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/acpi/sleep.c
+++ linux-2.6/arch/x86/kernel/acpi/sleep.c
@@ -112,14 +112,6 @@ int acpi_save_state_mem(void)
 	return 0;
 }
 
-/*
- * acpi_restore_state - undo effects of acpi_save_state_mem
- */
-void acpi_restore_state_mem(void)
-{
-}
-
-
 /**
  * acpi_reserve_wakeup_memory - do _very_ early ACPI initialisation
  *

[PATCH 2/6] ACPI / PM: Do not switch interrupts off/on in acpi_suspend_enter()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The function acpi_suspend_enter() is always called with interrupts
off, so it doesn't need to switch them off and on.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/sleep.c |    3 ---
 1 file changed, 3 deletions(-)

Index: linux-2.6/drivers/acpi/sleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep.c
+++ linux-2.6/drivers/acpi/sleep.c
@@ -243,7 +243,6 @@ static int acpi_suspend_begin(suspend_st
 static int acpi_suspend_enter(suspend_state_t pm_state)
 {
 	acpi_status status = AE_OK;
-	unsigned long flags = 0;
 	u32 acpi_state = acpi_target_sleep_state;
 
 	ACPI_FLUSH_CPU_CACHE();
@@ -256,7 +255,6 @@ static int acpi_suspend_enter(suspend_st
 			return error;
 	}
 
-	local_irq_save(flags);
 	switch (acpi_state) {
 	case ACPI_STATE_S1:
 		barrier();
@@ -290,7 +288,6 @@ static int acpi_suspend_enter(suspend_st
 	/* Allow EC transactions to happen. */
 	acpi_ec_unblock_transactions_early();
 
-	local_irq_restore(flags);
 	printk(KERN_DEBUG "Back to C!\n");
 
 	suspend_nvs_restore();

[PATCH 3/6] ACPI / PM: Modify the "low-level resume finished" message

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Move the low-level resume completion message to the point where
control goes back to acpi_suspend_enter() during resume and change
it so that it's more informative.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/sleep.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Index: linux-2.6/drivers/acpi/sleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep.c
+++ linux-2.6/drivers/acpi/sleep.c
@@ -263,6 +263,7 @@ static int acpi_suspend_enter(suspend_st
 
 	case ACPI_STATE_S3:
 		do_suspend_lowlevel();
+		pr_info(PREFIX "Low-level resume complete\n");
 		break;
 	}
 
@@ -288,8 +289,6 @@ static int acpi_suspend_enter(suspend_st
 	/* Allow EC transactions to happen. */
 	acpi_ec_unblock_transactions_early();
 
-	printk(KERN_DEBUG "Back to C!\n");
-
 	suspend_nvs_restore();
 
 	return ACPI_SUCCESS(status) ? 0 : -EFAULT;

[PATCH 4/6] ACPI / PM: Call acpi_save_state_mem() right before low-level suspend

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

Since acpi_save_state_mem() is only called by acpi_suspend_enter()
if the target sleep state is S3, it's better to call it under the
switch (acpi_state), right before do_suspend_lowlevel().

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/sleep.c |   12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

Index: linux-2.6/drivers/acpi/sleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep.c
+++ linux-2.6/drivers/acpi/sleep.c
@@ -244,17 +244,10 @@ static int acpi_suspend_enter(suspend_st
 {
 	acpi_status status = AE_OK;
 	u32 acpi_state = acpi_target_sleep_state;
+	int error;
 
 	ACPI_FLUSH_CPU_CACHE();
 
-	/* Do arch specific saving of state. */
-	if (acpi_state == ACPI_STATE_S3) {
-		int error = acpi_save_state_mem();
-
-		if (error)
-			return error;
-	}
-
 	switch (acpi_state) {
 	case ACPI_STATE_S1:
 		barrier();
@@ -262,6 +255,9 @@ static int acpi_suspend_enter(suspend_st
 		break;
 
 	case ACPI_STATE_S3:
+		error = acpi_save_state_mem();
+		if (error)
+			return error;
 		do_suspend_lowlevel();
 		pr_info(PREFIX "Low-level resume complete\n");
 		break;

[PATCH 5/6] ACPI / PM: Merge do_suspend_lowlevel() into acpi_save_state_mem()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The function do_suspend_lowlevel() is specific to x86 and defined in
assembly code, so it should be called from the x86 low-level suspend
code rather than from acpi_suspend_enter().

Merge do_suspend_lowlevel() into the x86's acpi_save_state_mem() and
change the name of the latter to acpi_suspend_lowlevel(), so that the
function's purpose is better reflected by its name.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 arch/ia64/include/asm/acpi.h |    4 ++--
 arch/ia64/kernel/acpi.c      |    9 ++-------
 arch/x86/include/asm/acpi.h  |    4 ++--
 arch/x86/kernel/acpi/sleep.c |    5 +++--
 arch/x86/kernel/acpi/sleep.h |    2 ++
 drivers/acpi/sleep.c         |    5 +----
 6 files changed, 12 insertions(+), 17 deletions(-)

Index: linux-2.6/drivers/acpi/sleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep.c
+++ linux-2.6/drivers/acpi/sleep.c
@@ -199,8 +199,6 @@ static void acpi_pm_end(void)
 #endif /* CONFIG_ACPI_SLEEP */
 
 #ifdef CONFIG_SUSPEND
-extern void do_suspend_lowlevel(void);
-
 static u32 acpi_suspend_states[] = {
 	[PM_SUSPEND_ON] = ACPI_STATE_S0,
 	[PM_SUSPEND_STANDBY] = ACPI_STATE_S1,
@@ -255,10 +253,9 @@ static int acpi_suspend_enter(suspend_st
 		break;
 
 	case ACPI_STATE_S3:
-		error = acpi_save_state_mem();
+		error = acpi_suspend_lowlevel();
 		if (error)
 			return error;
-		do_suspend_lowlevel();
 		pr_info(PREFIX "Low-level resume complete\n");
 		break;
 	}
Index: linux-2.6/arch/ia64/include/asm/acpi.h
===================================================================
--- linux-2.6.orig/arch/ia64/include/asm/acpi.h
+++ linux-2.6/arch/ia64/include/asm/acpi.h
@@ -128,8 +128,8 @@ static inline const char *acpi_get_sysna
 int acpi_request_vector (u32 int_type);
 int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
 
-/* Routine for saving kernel state during suspend. */
-extern int acpi_save_state_mem(void);
+/* Low-level suspend routine. */
+extern int acpi_suspend_lowlevel(void);
 
 extern unsigned long acpi_wakeup_address;
 
Index: linux-2.6/arch/ia64/kernel/acpi.c
===================================================================
--- linux-2.6.orig/arch/ia64/kernel/acpi.c
+++ linux-2.6/arch/ia64/kernel/acpi.c
@@ -1034,13 +1034,8 @@ int acpi_unregister_ioapic(acpi_handle h
 EXPORT_SYMBOL(acpi_unregister_ioapic);
 
 /*
- * acpi_save_state_mem() - save kernel state
+ * acpi_suspend_lowlevel() - save kernel state and suspend.
  *
  * TBD when when IA64 starts to support suspend...
  */
-int acpi_save_state_mem(void) { return 0; } 
-
-/*
- * do_suspend_lowlevel()
- */
-void do_suspend_lowlevel(void) {}
+int acpi_suspend_lowlevel(void) { return 0; }
Index: linux-2.6/arch/x86/include/asm/acpi.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/acpi.h
+++ linux-2.6/arch/x86/include/asm/acpi.h
@@ -112,8 +112,8 @@ static inline void acpi_disable_pci(void
 	acpi_noirq_set();
 }
 
-/* Routine for saving kernel state during suspend. */
-extern int acpi_save_state_mem(void);
+/* Low-level suspend routine. */
+extern int acpi_suspend_lowlevel(void);
 
 extern unsigned long acpi_wakeup_address;
 
Index: linux-2.6/arch/x86/kernel/acpi/sleep.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/acpi/sleep.c
+++ linux-2.6/arch/x86/kernel/acpi/sleep.c
@@ -31,14 +31,14 @@ static char temp_stack[4096];
 #endif
 
 /**
- * acpi_save_state_mem - save kernel state
+ * acpi_suspend_lowlevel - save kernel state
  *
  * Create an identity mapped page table and copy the wakeup routine to
  * low memory.
  *
  * Note that this is too late to change acpi_wakeup_address.
  */
-int acpi_save_state_mem(void)
+int acpi_suspend_lowlevel(void)
 {
 	struct wakeup_header *header;
 
@@ -109,6 +109,7 @@ int acpi_save_state_mem(void)
        saved_magic = 0x123456789abcdef0L;
 #endif /* CONFIG_64BIT */
 
+	do_suspend_lowlevel();
 	return 0;
 }
 
Index: linux-2.6/arch/x86/kernel/acpi/sleep.h
===================================================================
--- linux-2.6.orig/arch/x86/kernel/acpi/sleep.h
+++ linux-2.6/arch/x86/kernel/acpi/sleep.h
@@ -14,3 +14,5 @@ extern char swsusp_pg_dir[PAGE_SIZE];
 
 extern unsigned long acpi_copy_wakeup_routine(unsigned long);
 extern void wakeup_long64(void);
+
+extern void do_suspend_lowlevel(void);

[PATCH 6/6] ACPI / PM: Do not switch interrupts off/on in acpi_hibernation_enter()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rjw@sisk.pl>

The function acpi_hibernation_enter() is always called with
interrupts off, so it doesn't need to switch them off and on.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/sleep.c |    3 ---
 1 file changed, 3 deletions(-)

Index: linux-2.6/drivers/acpi/sleep.c
===================================================================
--- linux-2.6.orig/drivers/acpi/sleep.c
+++ linux-2.6/drivers/acpi/sleep.c
@@ -457,16 +457,13 @@ static int acpi_hibernation_begin(void)
 static int acpi_hibernation_enter(void)
 {
 	acpi_status status = AE_OK;
-	unsigned long flags = 0;
 
 	ACPI_FLUSH_CPU_CACHE();
 
-	local_irq_save(flags);
 	/* This shouldn't return.  If it returns, we have a problem */
 	status = acpi_enter_sleep_state(ACPI_STATE_S4);
 	/* Reprogram control registers and execute _BFS */
 	acpi_leave_sleep_state_prep(ACPI_STATE_S4);
-	local_irq_restore(flags);
 
 	return ACPI_SUCCESS(status) ? 0 : -EFAULT;
 }

Re: [PATCH 1/6] ACPI / PM: Drop acpi_restore_state_mem()

[Pavel Machek]

On Mon 2011-02-07 23:18:02, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> The function acpi_restore_state_mem() has never been and most likely
> never will be used, so remove it.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

ack.

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

Re: [PATCH 2/6] ACPI / PM: Do not switch interrupts off/on in acpi_suspend_enter()

[Pavel Machek]

Hi!

> The function acpi_suspend_enter() is always called with interrupts
> off, so it doesn't need to switch them off and on.

Maybe add 'this function always runs with irqs disabled' comment?

Otherwise ack.

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

Re: [PATCH 3/6] ACPI / PM: Modify the "low-level resume finished" message

[Pavel Machek]

Hi!

> Move the low-level resume completion message to the point where
> control goes back to acpi_suspend_enter() during resume and change
> it so that it's more informative.

Well, doing some printk as soon as reasonably possible was helpful for
debugging in the past. I'd move it sooner, not later.

(If you see that message, it means that normal printk debugging can be
used, not exotic stuff like hashes in NVRAM).

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

Re: [PATCH 4/6] ACPI / PM: Call acpi_save_state_mem() right before low-level suspend

[Pavel Machek]

On Mon 2011-02-07 23:20:53, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> Since acpi_save_state_mem() is only called by acpi_suspend_enter()
> if the target sleep state is S3, it's better to call it under the
> switch (acpi_state), right before do_suspend_lowlevel().

ack.

> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
> ---
>  drivers/acpi/sleep.c |   12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> Index: linux-2.6/drivers/acpi/sleep.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/sleep.c
> +++ linux-2.6/drivers/acpi/sleep.c
> @@ -244,17 +244,10 @@ static int acpi_suspend_enter(suspend_st
>  {
>  	acpi_status status = AE_OK;
>  	u32 acpi_state = acpi_target_sleep_state;
> +	int error;
>  
>  	ACPI_FLUSH_CPU_CACHE();
>  
> -	/* Do arch specific saving of state. */
> -	if (acpi_state == ACPI_STATE_S3) {
> -		int error = acpi_save_state_mem();
> -
> -		if (error)
> -			return error;
> -	}
> -
>  	switch (acpi_state) {
>  	case ACPI_STATE_S1:
>  		barrier();
> @@ -262,6 +255,9 @@ static int acpi_suspend_enter(suspend_st
>  		break;
>  
>  	case ACPI_STATE_S3:
> +		error = acpi_save_state_mem();
> +		if (error)
> +			return error;
>  		do_suspend_lowlevel();
>  		pr_info(PREFIX "Low-level resume complete\n");
>  		break;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

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

Re: [PATCH 5/6] ACPI / PM: Merge do_suspend_lowlevel() into acpi_save_state_mem()

[Pavel Machek]

On Mon 2011-02-07 23:21:37, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> The function do_suspend_lowlevel() is specific to x86 and defined in
> assembly code, so it should be called from the x86 low-level suspend
> code rather than from acpi_suspend_enter().
> 
> Merge do_suspend_lowlevel() into the x86's acpi_save_state_mem() and
> change the name of the latter to acpi_suspend_lowlevel(), so that the
> function's purpose is better reflected by its name.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Ack.

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

Re: [PATCH 6/6] ACPI / PM: Do not switch interrupts off/on in acpi_hibernation_enter()

[Pavel Machek]

On Mon 2011-02-07 23:22:46, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rjw@sisk.pl>
> 
> The function acpi_hibernation_enter() is always called with
> interrupts off, so it doesn't need to switch them off and on.

ack... and maybe comment?
							       Pavel

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

Re: [PATCH 3/6] ACPI / PM: Modify the "low-level resume finished" message

[Rafael J. Wysocki]

On Friday, March 04, 2011, Pavel Machek wrote:
> Hi!
> 
> > Move the low-level resume completion message to the point where
> > control goes back to acpi_suspend_enter() during resume and change
> > it so that it's more informative.
> 
> Well, doing some printk as soon as reasonably possible was helpful for
> debugging in the past. I'd move it sooner, not later.
> 
> (If you see that message, it means that normal printk debugging can be
> used, not exotic stuff like hashes in NVRAM).

That's what the patch is doing, isn't it?