Hello,
After testing rc8, I noticed that I couldn't power off the computer

directly, it only got halted and I had to press the power button

manually. Just before displaying "System halted", the following message

is displayed:
  ACPI : PCI interrupt for device 0000:02:08.0 disabled
I had to first revert 5a50fe709d527f31169263e36601dd83446d5744 then

f216cc3748a3a22c2b99390fddcdafa0583791a2 (handling of Sx states) to

recover previous behaviour.
Attached are the dmesg for rc7, rc8 and rc8 with the two patches

reverted.
--

Damien Wyart

"diff -u dmesg.rc8_revert dmesg.rc8" says:
--- dmesg.rc8_revert	2007-09-25 00:31:33.000000000 -0700

+++ dmesg.rc8	2007-09-25 00:31:30.000000000 -0700

@@ -1,4 +1,4 @@

-Linux version 2.6.23-rc8-25092007dw (root@brouette) (gcc version 4.2.1 (Debian 4.2.1-5)) #2 SMP Tue Sep 25 08:31:09 CEST 2007

+Linux version 2.6.23-rc8-25092007dw (root@brouette) (gcc version 4.2.1 (Debian 4.2.1-5)) #1 SMP Tue Sep 25 07:27:10 CEST 2007

 BIOS-provided physical RAM map:

  BIOS-e820: 0000000000000000 - 00000000000a0000 (usable)

  BIOS-e820: 00000000000f0000 - 0000000000100000 (reserved)

@@ -72,18 +72,18 @@

 console [tty0] enabled

 Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)

 Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)

-Memory: 2074780k/2096592k available (2175k kernel code, 20712k reserved, 815k data, 192k init, 1179088k highmem)

+Memory: 2074780k/2096592k available (2175k kernel code, 20712k reserved, 816k data, 192k init, 1179088k highmem)

 virtual kernel memory layout:

     fixmap  : 0xfff9b000 - 0xfffff000   ( 400 kB)

     pkmap   : 0xff800000 - 0xffc00000   (4096 kB)

     vmalloc : 0xf8800000 - 0xff7fe000   ( 111 MB)

     lowmem  : 0xc0000000 - 0xf8000000   ( 896 MB)

       .init : 0xc03f0000 - 0xc0420000   ( 192 kB)

-      .data : 0xc031fd5c - 0xc03ebd04   ( 815 kB)

-      .text : 0xc0100000 - 0xc031fd5c   (2175 kB)

+      .data : 0xc031fcd4 - 0xc03ebd04   ( 816 kB)

+      .text : 0xc0100000 - 0xc031fcd4   (2175 kB)

 Checking if this processor honours the WP bit even in supervisor mode... Ok.

 SLUB: Genslabs=22, HWalign=64, Order=0-1, MinObjects=4, CPUs=2, Nodes=1

-Calibrating delay using timer specific routine.. 5987.66 BogoMIPS (lpj=2993833)

+Calibrating delay using timer specific routine.. 5987.60 BogoMIPS (lpj=2993802)

 Mount-cache hash table entries: 512

 CPU: After generic identify, caps: bfebfbff 00000000 00000000 00000000 0000041d 00000000 00000000 00000000

 monitor/mwait feature present.

@@ -104,7 +104,7 @@

 Booting processor 1/1 eip 2000

 CP...
[ message continues ]

Hmm. Those things *do* seem to be suspicious.
For example, those commits seem to move code that used to be inside

CONFIG_PM (which pretty much *everybody* has) to be inside

CONFIG_ACPI_SLEEP (which is a totally different thing, and depends on

whether the user asked for suspend support or not!
Damien - does it work if you ask for SUSPEND or HIBERNATION support?
Len - why are you guys moving stuff into CONFIG_PM_SLEEP? I know you seem

to think that absolutely *everybody* should always support suspend and

hibernation, but the fact is, not everybody does. And it's a totally

separate thing for normal ACPI CPU runstate support that people have used

to manage a *running* CPU (and shutting it down).
		Linus

-

Boot time cut in half.

My PC no longer 'wakes up' angrily. My wife does that, I'm going to

start sleeping with the P4, its more agreeable now.
P4 HT with generic Intel chipset.
What fun is this when nothing breaks?
Thank you all :)
-

This was a mistake and fixes have already been posted:
http://marc.info/?l=linux-acpi&m=119052970904643&w=4

http://marc.info/?l=linux-acpi&m=119073173625910&w=4
Greetings,

Rafael

-

As this whole "separate suspend and hibernate" change seems to have been

trickier than expected, I have done some additional testing using

2.6.23-rc8 plus both patches referred to above.
I compiled kernels (i386) for my laptop (Toshiba Satellite A40) with:

A) PM + SUSPEND + HIBERNATION + ACPI

B) PM + SUSPEND + ACPI

C) PM + HIBERNATION + ACPI

D) PM + ACPI

E) PM + SUSPEND + HIBERNATION but _without_ ACPI

F) without PM
In all cases CONFIG_APM was not set; all compiled without errors.
With all I have have booted, tried to suspend (close lid, which runs sleep

script, which does 'echo mem >/sys/power/state'), tried to hibernate ('echo

disk >/sys/power/state') and powered off (all from KDE sessions).
RESULTS

	boot	s2ram	s2disk	off

A)	OK	OK	OK	OK

B)	OK	OK	N/A	OK

C)	OK	OK 1)	OK	OK

D)	OK 2)	OK 1)	N/A	OK

E)	OK 3)	N/A 4)	??? 5)	Only halted 6)

F)	OK	N/A	N/A	Only halted 6)
Comments:

1) sleep script ran, but laptop stayed on

2) dmesg lists S0,S5 supported

   /proc/acpi/{sleep,wakeup} do not exist (is that correct?)

3) /proc/acpi/{sleep,wakeup} do not exist; /sys/power/state only has 'mem'

   so SUSPEND seems missing even though compiled in

4) sleep script did not even run (as expected)

5) System did suspend, but power stayed on with on console:

     Shutdown: hda \ Power down. \ Shutdown: hda \ hda: lost interrupt (2x)

     \ psmouse.c: Failed to deactivate mouse on isa0060/serio1

   System powered off manually and resumed correctly after that

6) System halted correctly, but did not power off automatically
The results for E and F are possibly correct as basically the config

(without ACPI) was broken for this laptop. In fact, I was somewhat

surprised how well it behaved without ACPI.
I suspect that whether a system powers off without ACPI or not can depend on

the system?
I have dmesg after boot and some other info available for all kernels if

anybody is interested. If any additional tests are wanted, please ask.
Cheers,

FJP

-

I think that your results reflect the code pretty well, ie. no big surprises.
On ACPI machines suspend doesn't actually work without ACPI and that's why

'mem' is missing from kernel/power/state with E).  Hibernation theoretically

works without ACPI, but the box cannot be powered off due to the lack of

support for that.
For the same reason, the system doesn't power off with E), F).
The errors from device drivers in the last phase of hibernation with E) may be

related to some confusion cause by the lack of ACPI methods expected to be

present.
Greetings,

Rafael

-

You're welcome. Until 2.6.20 or .21 s2ram never worked for me and even then

sound did not work after resume. Now it seems to be supported perfectly, so

my thanks to all of you!

Pity this laptop is slowly nearing end of life (3.5 years old, though still 
Great. That was what I was thinking. Thanks for confirming.

-

Do you have CONFIG_ACPI_SLEEP enabled in your .config?
-

I'm answering that too, because I suspect that my "2.6.23-rc7-mm1 does

not power off"-error might have the same cause.
No, I do not have CONFIG_ACPI_SLEEP set,

because I do not have CONFIG_PM_SLEEP set,

because I do not want SUSPEND and/or HIBERNATION.
.config from 2.6.23-rc7-mm1 attached.
Torsten

This is not the reason. SUSPEND is controlled with CONFIG_SUSPEND and

HIBERNATION is controlled with CONFIG_HIBERNATION.
-

What I meant with SUSPEND and/or HIBERNATION was CONFIG_SUSPEND /

CONFIG_HIBERNATION.
And the relations where from Kconfig:

from drivers/acpi/Kconfig:

config ACPI_SLEEP

        bool

        depends on PM_SLEEP

        default y
-> no PM_SLEEP means no ACPI_SLEEP
from kernel/power/Kconfig:

config PM_SLEEP

        bool

        depends on SUSPEND || HIBERNATION

        default y
-> No SUSPEND and/or HIBERNATION means no PM_SLEEP
And if I select SUSPEND and/or HIBERNATION I will not only build this

feature into the kernel, but also HOTPLUG_CPU and I want to avoid

that.
It's exactly as Linus said in his mail: Not everybody wants SUSPEND...
I should have formulated that better in my mail, but that was what I

wanted to say.
Back to debugging this:

http://marc.info/?l=linux-acpi&m=119052970904643&w=4

fails to apply against 2.6.23-rc7-mm1, but moving that function by

hand was not to difficult. ;)

(With only the second patch I got a link error...)
http://marc.info/?l=linux-acpi&m=119073173625910&w=4

applied, and a test showed that my system now powers off again.
Torsten

-

Good,

thanks, Alex.

-

Understood, patches are available, please test.
Regards,

Alex.

-

Same answer from my side: I do not have CONFIG_ACPI_SLEEP for the same

reason (and this worked fine without them in rc7). I do not think these

settings should have changed between rc7 and rc8.
--

Damien Wyart

-

Also, another test I just did: on another computer, rc8 is fine

regarding ACPI power off, even if CONFIG_ACPI_SLEEP is not set. I can

provide config if needed.
Best,
--

Damien Wyart

-

On the box that fails to power off, can you please test -rc8 with these two

commits reverted:
commit 5a50fe709d527f31169263e36601dd83446d5744

ACPI: suspend: consolidate handling of Sx states addendum
commit f216cc3748a3a22c2b99390fddcdafa0583791a2

ACPI: suspend: consolidate handling of Sx states.
and see if it works?
Greetings,

Rafael

-

I had done these tests in the first place (see my first mail), so yes,

reverting makes the box power off fine.
Will test this evening the patch you pointed in your next message.
--

Damien Wyart

-

> Will test this evening the patch you pointed in your next message.
Ok, with both patches (including the very latest one from Alexey ---

tried the "stylistically correct" one), machine halts fine again. Thanks

to all for having looked at this!
--

Damien Wyart

-

On Tue, 25 Sep 2007 17:05:00 +0200
Same here, applying the two patches in this thread fixed the problem

for me, on my Intel 965P based system. It now does the acpi power off

correctly once again, and I have noticed no other problems with

2.6.23-rc8.
"move acpi_sleep_prepare outside of CONFIG_SUSPEND" and the last

"ACPI: suspend: fix ACPI_SLEEP states"
Thanks Alexey and Rafael.
Mike Houston

-

If it does, please test the patch from this message
http://marc.info/?l=linux-kernel&m=119052978117735&w=4
on top of vanilla 2.6.23-rc8.
Greetings,

Rafael

-

You will need one more patch on top of just mentioned one.
Regards,

Hm, why did you put acpi_target_sleep_state under CONFIG_SUSPEND?
CONFIG_HIBERNATION needs acpi_target_sleep_state  too.
Greetings,

Rafael

-

Agree, attaching updated patch.
Thanks,

Alex.

Well, please use "ifdef CONFIG_PM_SLEEP" instead of

"if defined(CONFIG_SUSPEND)||defined(CONFIG_HIBERNATION)",

as you did with the second block.
Greetings,

Rafael

-

I was thinking about that, but it seem to be less clear...

We need this variable only for suspend or hibernation, nothing else.

with pm_sleep it is not visible at all.
Thoughts?

-

Well, PM_SLEEP is defined as (SUSPEND || HIBERNATION), please have a look

at kernel/power/Kconfig, and it was introduced exactly for the conditions like

this.
IOW, if we want something to be used for anything else than suspend or

hibernation, it shouldn't be defined under PM_SLEEP.
Greetings,

Rafael

-

I've seen this then I wrote the patch :) See my point, it is not clear,

that PM_SLEEP is equivalent to SUSPEND || HIBERNATION, one needs to

grep Kconfig files to find that -- it means that code becomes less readable,

Agree, but we should distinguish there it is better to use PM_SLEEP,

and there it is better to use (SUSPEND || HIBERNATION) just to be more expressive...
Regards,

Alex.

-

I see your point.  Still, you are using PM_SLEEP in the same file, so someone

reading the code for the first time will have to find out what it is anyway.
OTOH, the only function of PM_SLEEP is to be a replacement for

(SUSPEND || HIBERNATION).  It has no other meaning whatsoever.
Well, since PM_SLEEP is used as (SUSPEND || HIBERNATION) everywhere else,

I think that it would actually be confusing not to use it here. :-)
Greetings,

Rafael

-

Ok, patch is here.
Regards,

Alex.

I get a reject applying this to current mainline.  Easy enough to fix it,

but I worry that the fix might be incorrect when applied to some tree other

than that which you were working on.
-

Andrew,
There are 2 patches, this is the second.

Above, Rafael gave link to first. Here it is again:

http://marc.info/?l=linux-kernel&m=119052978117735&w=4
Sorry for confusion,

Alex.
-

Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
and thanks for fixing this.

-