RE: [PATCH][2.4] acpi S5 poweroff fix

Yu, Luming
2003-05-12T08:35:13Z

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

To keep the consistency of the code, below patch could be a better solution.

Thanks,
Luming

-----Original Message-----
From: willy tarreau [mailto:wtarreau-Qt13gs6zZMY@public.gmane.org]
Sent: 2003?4?30? 2:15
To: Grover, Andrew
Cc: Acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Subject: [ACPI] [PATCH][2.4] acpi S5 poweroff fix

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: s5oops.diff --]
[-- Type: application/octet-stream, Size: 449 bytes --]

diff -Bru 1/drivers/acpi/system.c 2/drivers/acpi/system.c
--- 1/drivers/acpi/system.c 2003-03-28 14:44:35.000000000 +0800
+++ 2/drivers/acpi/system.c 2003-03-31 09:47:38.000000000 +0800
@@ -113,6 +128,10 @@
if (state != ACPI_STATE_S1 && state != ACPI_STATE_S5)
return AE_ERROR;

+ if(state == ACPI_STATE_S5){
+ acpi_power_off();
+ return;
+ }
acpi_enter_sleep_state_prep(state);

/* disable interrupts and flush caches */