[PATCH][2.4] acpi S5 poweroff fix

willy tarreau
2003-04-29T18:15:18Z

[-- 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*/