[PATCH][2.4] acpi S5 poweroff fix

["willy tarreau" <wtarreau-Qt13gs6zZMY@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2494 bytes --]

Hello Andy,

since I have this new laptop (sony vaio pcg-fx705), ACPI has never allowed
me to shutdown correctly. It always hangs and I have to maintain the power
button pressed to power off, whatever the version of your acpi patch I was
using. Today, I had a bit of free time, so I investigated into the code and
finally fixed the problem :-)

Basically, two unrelated things :
1) I noticed that "echo 5 >/proc/acpi/sleep" would oops, so I manually copied
   the oops and discovered that acpi_restore_state_mem() crashed, trying to
   affet 'pmd' a NULL value. Then I realized that it was because on this path,
   acpi_save_state_mem() was never called in S5. I first thought this was
   intentional because S5 is not expected to return, but reading through the
   code, I found a conflict such as "if (state < 5) { ... if (state != 5) ..."
   so I was sure that the first one should have included 5, and I changed the
   '<' to '<=' so that acpi_save_state_mem() is now called.
   It still didn't work, and I noticed that in acpi_suspend(), the call to
   acpi_system_suspend() is expected to do the right job and certainly not to
   return. But acpi_system_suspend() does nothing if called with state=5 !
   So I added the check for ACPI_STATE_S5 at the same level as _S1 because it
   semt right to me.
   => with these 2 changes, echoing 5 in /proc/acpi/sleep now turns my notebook
   OFF !!!
   => I diffed original (20030424) and my changes and put them in the first
      attached patch.

2) The sad thing is that even after that, neither Alt-SysRq-O nor halt would
   power off. I saw that acpi_poweroff() was a simpler than the code path
   executed from acpi_suspend(5). I tried to duplicate part of the code, but
   no lock. I think I missed some bits. So I did it far simpler : now
   acpi_poweroff() only calls acpi_suspend(ACPI_STATE_S5) and everything works
   fine !  => hence the second patch, still against 20030424.

Although I'm not sure that the second case affects everybody, because it might
be caused by side-effects, but from what I've understood from the code, I'm
pretty sure that nobody can power off by echoing 5 to /proc/acpi/sleep !

Here are the two patches, please review them and apply them if you agree !

Cheers,
Willy (happy with a notebook that powers down for the first time in months !)


___________________________________________________________
Do You Yahoo!? -- Une adresse @yahoo.fr gratuite et en français !
Yahoo! Mail : http://fr.mail.yahoo.com

[-- Attachment #2: acpi-20030424-S5-fix.diff --]
[-- Type: application/octet-stream, Size: 531 bytes --]

--- ./drivers/acpi/system.c-orig	Tue Apr 29 17:39:34 2003
+++ ./drivers/acpi/system.c	Tue Apr 29 19:08:09 2003
@@ -180,7 +180,7 @@
 			return AE_ERROR;
 	}
 
-	if (state < ACPI_STATE_S5) {
+	if (state <= ACPI_STATE_S5) {
 		/* Tell devices to stop I/O and actually save their state.
 		 * It is theoretically possible that something could fail,
 		 * so handle that gracefully..
@@ -277,6 +277,7 @@
 
 	switch (state) {
 	case ACPI_STATE_S1:
+	case ACPI_STATE_S5:
 		barrier();
 		status = acpi_enter_sleep_state(state);
 		break;

[-- Attachment #3: acpi-20030424-pwoff-use-suspend.diff --]
[-- Type: application/octet-stream, Size: 339 bytes --]

--- ./drivers/acpi/system.c-working	Tue Apr 29 19:09:19 2003
+++ ./drivers/acpi/system.c	Tue Apr 29 19:36:08 2003
@@ -90,9 +90,7 @@
 static void
 acpi_power_off (void)
 {
-	acpi_enter_sleep_state_prep(ACPI_STATE_S5);
-	ACPI_DISABLE_IRQS();
-	acpi_enter_sleep_state(ACPI_STATE_S5);
+	acpi_suspend(ACPI_STATE_S5);
 }
 
 #endif /*CONFIG_PM*/