Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
On Tue, 25 Sep 2007 08:51:09 +0200 Damien Wyart <[EMAIL PROTECTED]> wrote: > 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 :02:08.0 disabled > > I had to first revert 5a50fe709d527f31169263e36601dd83446d5744 then > f216cc3748a3a22c2b99390fddcdafa0583791a2 (handling of Sx states) to > recover previous behaviour. Let's add some cc's. > Attached are the dmesg for rc7, rc8 and rc8 with the two patches > reverted. "diff -u dmesg.rc8_revert dmesg.rc8" says: --- dmesg.rc8_revert2007-09-25 00:31:33.0 -0700 +++ dmesg.rc8 2007-09-25 00:31:30.0 -0700 @@ -1,4 +1,4 @@ -Linux version 2.6.23-rc8-25092007dw ([EMAIL PROTECTED]) (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 ([EMAIL PROTECTED]) (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: - 000a (usable) BIOS-e820: 000f - 0010 (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 - 0xf000 ( 400 kB) pkmap : 0xff80 - 0xffc0 (4096 kB) vmalloc : 0xf880 - 0xff7fe000 ( 111 MB) lowmem : 0xc000 - 0xf800 ( 896 MB) .init : 0xc03f - 0xc042 ( 192 kB) - .data : 0xc031fd5c - 0xc03ebd04 ( 815 kB) - .text : 0xc010 - 0xc031fd5c (2175 kB) + .data : 0xc031fcd4 - 0xc03ebd04 ( 816 kB) + .text : 0xc010 - 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 041d monitor/mwait feature present. @@ -104,7 +104,7 @@ Booting processor 1/1 eip 2000 CPU 1 irqstacks, hard=c0426000 soft=c0424000 Initializing CPU#1 -Calibrating delay using timer specific routine.. 5984.18 BogoMIPS (lpj=2992093) +Calibrating delay using timer specific routine.. 5984.16 BogoMIPS (lpj=2992084) CPU: After generic identify, caps: bfebfbff 041d monitor/mwait feature present. CPU: Trace cache: 12K uops, L1 D cache: 16K @@ -116,7 +116,7 @@ CPU1: Intel P4/Xeon Extended MCE MSRs (12) available CPU1: Thermal monitoring enabled CPU1: Intel(R) Pentium(R) 4 CPU 3.00GHz stepping 03 -Total of 2 processors activated (11971.85 BogoMIPS). +Total of 2 processors activated (11971.77 BogoMIPS). ENABLING IO-APIC IRQs ..TIMER: vector=0x31 apic1=0 pin1=2 apic2=-1 pin2=-1 checking TSC synchronization [CPU#0 -> CPU#1]: passed. @@ -279,6 +279,7 @@ EXT3-fs: mounted filesystem with ordered data mode. VFS: Mounted root (ext3 filesystem) readonly. Freeing unused kernel memory: 192k freed +input: PC Speaker as /devices/platform/pcspkr/input/input4 sr0: scsi3-mmc drive: 1x/48x cd/rw xa/form2 cdda tray Uniform CD-ROM driver Revision: 3.20 sr 1:0:0:0: Attached scsi CD-ROM sr0 @@ -287,7 +288,6 @@ usbcore: registered new interface driver usbfs usbcore: registered new interface driver hub usbcore: registered new device driver usb -input: PC Speaker as /devices/platform/pcspkr/input/input4 parport_pc 00:0a: reported by Plug and Play ACPI parport0: PC-style at 0x378 (0x778), irq 7, using FIFO [PCSPP,TRISTATE,COMPAT,ECP] USB Universal Host Controller Interface driver v3.0 @@ -299,46 +299,42 @@ usb usb1: configuration #1 chosen from 1 choice hub 1-0:1.0: USB hub found hub 1-0:1.0: 2 ports detected -ACPI: PCI Interrupt :00:1d.1[B] -> GSI 19 (level, low) -> IRQ 20 -PCI: Setting latency timer of device :00:1d.1 to 64 -uhci_hcd :00:1d.1: UHCI Host Controller -uhci_hcd :00:1d.1: new USB bus registered, assigned bus number 2 -uhci_hcd :00:1d.1: irq 20, io base 0xff60 +ACPI: PCI Interrupt :00:1d.7[D] -> GSI 23 (level, low) -> IRQ 20 +PCI: Setting latency timer of device :00:1d.7 to 64 +ehci_hcd :00:1d.7: EHCI Host Controller +ehci_hcd :00:1d.7: new USB bus registered, assigned bus number 2 +ehci_hcd :00:1d.7: debug port 1 +PCI: cache line size of 128 is not supported b
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Do you have CONFIG_ACPI_SLEEP enabled in your .config? Andrew Morton wrote: > On Tue, 25 Sep 2007 08:51:09 +0200 Damien Wyart <[EMAIL PROTECTED]> wrote: > >> 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 :02:08.0 disabled >> >> I had to first revert 5a50fe709d527f31169263e36601dd83446d5744 then >> f216cc3748a3a22c2b99390fddcdafa0583791a2 (handling of Sx states) to >> recover previous behaviour. > > Let's add some cc's. > >> Attached are the dmesg for rc7, rc8 and rc8 with the two patches >> reverted. > > "diff -u dmesg.rc8_revert dmesg.rc8" says: > > --- dmesg.rc8_revert 2007-09-25 00:31:33.0 -0700 > +++ dmesg.rc8 2007-09-25 00:31:30.0 -0700 > @@ -1,4 +1,4 @@ > -Linux version 2.6.23-rc8-25092007dw ([EMAIL PROTECTED]) (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 ([EMAIL PROTECTED]) (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: - 000a (usable) > BIOS-e820: 000f - 0010 (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 - 0xf000 ( 400 kB) > pkmap : 0xff80 - 0xffc0 (4096 kB) > vmalloc : 0xf880 - 0xff7fe000 ( 111 MB) > lowmem : 0xc000 - 0xf800 ( 896 MB) >.init : 0xc03f - 0xc042 ( 192 kB) > - .data : 0xc031fd5c - 0xc03ebd04 ( 815 kB) > - .text : 0xc010 - 0xc031fd5c (2175 kB) > + .data : 0xc031fcd4 - 0xc03ebd04 ( 816 kB) > + .text : 0xc010 - 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 > 041d > monitor/mwait feature present. > @@ -104,7 +104,7 @@ > Booting processor 1/1 eip 2000 > CPU 1 irqstacks, hard=c0426000 soft=c0424000 > Initializing CPU#1 > -Calibrating delay using timer specific routine.. 5984.18 BogoMIPS > (lpj=2992093) > +Calibrating delay using timer specific routine.. 5984.16 BogoMIPS > (lpj=2992084) > CPU: After generic identify, caps: bfebfbff > 041d > monitor/mwait feature present. > CPU: Trace cache: 12K uops, L1 D cache: 16K > @@ -116,7 +116,7 @@ > CPU1: Intel P4/Xeon Extended MCE MSRs (12) available > CPU1: Thermal monitoring enabled > CPU1: Intel(R) Pentium(R) 4 CPU 3.00GHz stepping 03 > -Total of 2 processors activated (11971.85 BogoMIPS). > +Total of 2 processors activated (11971.77 BogoMIPS). > ENABLING IO-APIC IRQs > ..TIMER: vector=0x31 apic1=0 pin1=2 apic2=-1 pin2=-1 > checking TSC synchronization [CPU#0 -> CPU#1]: passed. > @@ -279,6 +279,7 @@ > EXT3-fs: mounted filesystem with ordered data mode. > VFS: Mounted root (ext3 filesystem) readonly. > Freeing unused kernel memory: 192k freed > +input: PC Speaker as /devices/platform/pcspkr/input/input4 > sr0: scsi3-mmc drive: 1x/48x cd/rw xa/form2 cdda tray > Uniform CD-ROM driver Revision: 3.20 > sr 1:0:0:0: Attached scsi CD-ROM sr0 > @@ -287,7 +288,6 @@ > usbcore: registered new interface driver usbfs > usbcore: registered new interface driver hub > usbcore: registered new device driver usb > -input: PC Speaker as /devices/platform/pcspkr/input/input4 > parport_pc 00:0a: reported by Plug and Play ACPI > parport0: PC-style at 0x378 (0x778), irq 7, using FIFO > [PCSPP,TRISTATE,COMPAT,ECP] > USB Universal Host Controller Interface driver v3.0 > @@ -299,46 +299,42 @@ > usb usb1: configuration #1 chosen from 1 choice > hub 1-0:1.0: USB hub found > hub 1-0:1.0: 2 ports detected > -ACPI: PCI Interrupt :00:1d.1[B] -> GSI 19 (level, low) -> IRQ 20 > -PCI: Setting latency timer of device :00:1d.1 to 64 > -uhci_hcd :00:1d.1: UHCI Host Controller > -uhci_hcd :00:1d.1: new USB bus registered, assigned bus number 2 > -uhci_hcd :00:1d.1: irq 20, io base 0xff60 > +ACPI: PCI Interrupt :00:1d.7[D] -> GSI 23 (le
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Hello, > On 9/25/07, Alexey Starikovskiy <[EMAIL PROTECTED]> wrote: > > Do you have CONFIG_ACPI_SLEEP enabled in your .config? * Torsten Kaiser <[EMAIL PROTECTED]> [2007-09-25 11:08]: > 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. 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 - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
> > 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. > 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. 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 - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Torsten Kaiser wrote: > On 9/25/07, Alexey Starikovskiy <[EMAIL PROTECTED]> wrote: >> 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. This is not the reason. SUSPEND is controlled with CONFIG_SUSPEND and HIBERNATION is controlled with CONFIG_HIBERNATION. But if you want S5 ACPI sleep state you might want to enable ACPI_SLEEP... > > .config from 2.6.23-rc7-mm1 attached. > > Torsten > - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
On Tuesday, 25 September 2007 11:58, Damien Wyart wrote: > > > 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. > > > 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. Well, we haven't changed much. > 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. 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 - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote: > On Tuesday, 25 September 2007 11:58, Damien Wyart wrote: > > > > 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. > > > > > 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. > > Well, we haven't changed much. > > > 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. > > 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? 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 - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
> 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? 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 - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Rafael J. Wysocki wrote: > On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote: >> On Tuesday, 25 September 2007 11:58, Damien Wyart wrote: > 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. 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. >> Well, we haven't changed much. >> >>> 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. >> 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? > > 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. You will need one more patch on top of just mentioned one. Regards, Alex. > > Greetings, > Rafael > - > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to [EMAIL PROTECTED] > More majordomo info at http://vger.kernel.org/majordomo-info.html > ACPI: suspend: fix ACPI_SLEEP states From: Alexey Starikovskiy <[EMAIL PROTECTED]> Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]> --- drivers/acpi/sleep/Makefile |2 +- drivers/acpi/sleep/main.c |7 +-- include/acpi/acpi_drivers.h |4 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/acpi/sleep/Makefile b/drivers/acpi/sleep/Makefile index ba9bd40..f1fb888 100644 --- a/drivers/acpi/sleep/Makefile +++ b/drivers/acpi/sleep/Makefile @@ -1,5 +1,5 @@ obj-y := wakeup.o -obj-$(CONFIG_ACPI_SLEEP) += main.o +obj-y += main.o obj-$(CONFIG_ACPI_SLEEP) += proc.o EXTRA_CFLAGS += $(ACPI_CFLAGS) diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c index c79edcb..86415f1 100644 --- a/drivers/acpi/sleep/main.c +++ b/drivers/acpi/sleep/main.c @@ -24,8 +24,6 @@ u8 sleep_states[ACPI_S_STATE_COUNT]; -static u32 acpi_target_sleep_state = ACPI_STATE_S0; - int acpi_sleep_prepare(u32 acpi_state) { #ifdef CONFIG_ACPI_SLEEP @@ -48,6 +46,9 @@ int acpi_sleep_prepare(u32 acpi_state) } #ifdef CONFIG_SUSPEND + +static u32 acpi_target_sleep_state = ACPI_STATE_S0; + static struct pm_ops acpi_pm_ops; extern void do_suspend_lowlevel(void); @@ -299,6 +300,7 @@ int acpi_suspend(u32 acpi_state) return -EINVAL; } +#ifdef CONFIG_PM_SLEEP /** * acpi_pm_device_sleep_state - return preferred power state of ACPI device * in the system sleep state given by %acpi_target_sleep_state @@ -373,6 +375,7 @@ int acpi_pm_device_sleep_state(struct device *dev, int wake, int *d_min_p) *d_min_p = d_min; return d_max; } +#endif static void acpi_power_off_prepare(void) { diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h index 202acb9..f85f77a 100644 --- a/include/acpi/acpi_drivers.h +++ b/include/acpi/acpi_drivers.h @@ -147,10 +147,6 @@ static inline void unregister_hotplug_dock_device(acpi_handle handle) /*-- Suspend/Resume -- */ -#ifdef CONFIG_ACPI_SLEEP extern int acpi_sleep_init(void); -#else -static inline int acpi_sleep_init(void) { return 0; } -#endif #endif /*__ACPI_DRIVERS_H__*/
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
On Tuesday, 25 September 2007 14:05, Alexey Starikovskiy wrote: > Rafael J. Wysocki wrote: > > On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote: > >> On Tuesday, 25 September 2007 11:58, Damien Wyart wrote: > > 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. > 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. > >> Well, we haven't changed much. > >> > >>> 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. > >> 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? > > > > 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. > You will need one more patch on top of just mentioned one. Hm, why did you put acpi_target_sleep_state under CONFIG_SUSPEND? CONFIG_HIBERNATION needs acpi_target_sleep_state too. Greetings, Rafael - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Rafael J. Wysocki wrote: > On Tuesday, 25 September 2007 14:05, Alexey Starikovskiy wrote: >> Rafael J. Wysocki wrote: >>> On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote: On Tuesday, 25 September 2007 11:58, Damien Wyart wrote: >>> 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. >> 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. Well, we haven't changed much. > 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. 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? >>> 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. >> You will need one more patch on top of just mentioned one. > > Hm, why did you put acpi_target_sleep_state under CONFIG_SUSPEND? > > CONFIG_HIBERNATION needs acpi_target_sleep_state too. Agree, attaching updated patch. Thanks, Alex. ACPI: suspend: fix ACPI_SLEEP states From: Alexey Starikovskiy <[EMAIL PROTECTED]> Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]> --- drivers/acpi/sleep/Makefile |2 +- drivers/acpi/sleep/main.c |5 + include/acpi/acpi_drivers.h |4 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/acpi/sleep/Makefile b/drivers/acpi/sleep/Makefile index ba9bd40..f1fb888 100644 --- a/drivers/acpi/sleep/Makefile +++ b/drivers/acpi/sleep/Makefile @@ -1,5 +1,5 @@ obj-y := wakeup.o -obj-$(CONFIG_ACPI_SLEEP) += main.o +obj-y += main.o obj-$(CONFIG_ACPI_SLEEP) += proc.o EXTRA_CFLAGS += $(ACPI_CFLAGS) diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c index c79edcb..6ea0628 100644 --- a/drivers/acpi/sleep/main.c +++ b/drivers/acpi/sleep/main.c @@ -24,7 +24,9 @@ u8 sleep_states[ACPI_S_STATE_COUNT]; +#if defined(CONFIG_SUSPEND)||defined(CONFIG_HIBERNATION) static u32 acpi_target_sleep_state = ACPI_STATE_S0; +#endif int acpi_sleep_prepare(u32 acpi_state) { @@ -48,6 +50,7 @@ int acpi_sleep_prepare(u32 acpi_state) } #ifdef CONFIG_SUSPEND + static struct pm_ops acpi_pm_ops; extern void do_suspend_lowlevel(void); @@ -299,6 +302,7 @@ int acpi_suspend(u32 acpi_state) return -EINVAL; } +#ifdef CONFIG_PM_SLEEP /** * acpi_pm_device_sleep_state - return preferred power state of ACPI device * in the system sleep state given by %acpi_target_sleep_state @@ -373,6 +377,7 @@ int acpi_pm_device_sleep_state(struct device *dev, int wake, int *d_min_p) *d_min_p = d_min; return d_max; } +#endif static void acpi_power_off_prepare(void) { diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h index 202acb9..f85f77a 100644 --- a/include/acpi/acpi_drivers.h +++ b/include/acpi/acpi_drivers.h @@ -147,10 +147,6 @@ static inline void unregister_hotplug_dock_device(acpi_handle handle) /*-- Suspend/Resume -- */ -#ifdef CONFIG_ACPI_SLEEP extern int acpi_sleep_init(void); -#else -static inline int acpi_sleep_init(void) { return 0; } -#endif #endif /*__ACPI_DRIVERS_H__*/
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
On Tuesday, 25 September 2007 14:53, Alexey Starikovskiy wrote: > Rafael J. Wysocki wrote: > > On Tuesday, 25 September 2007 14:05, Alexey Starikovskiy wrote: > >> Rafael J. Wysocki wrote: > >>> On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote: > On Tuesday, 25 September 2007 11:58, Damien Wyart wrote: > >>> 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. > >> 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. > Well, we haven't changed much. > > > 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. > 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? > >>> 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. > >> You will need one more patch on top of just mentioned one. > > > > Hm, why did you put acpi_target_sleep_state under CONFIG_SUSPEND? > > > > CONFIG_HIBERNATION needs acpi_target_sleep_state too. > Agree, attaching updated patch. 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 - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Rafael J. Wysocki wrote: > On Tuesday, 25 September 2007 14:53, Alexey Starikovskiy wrote: >> Rafael J. Wysocki wrote: >>> On Tuesday, 25 September 2007 14:05, Alexey Starikovskiy wrote: Rafael J. Wysocki wrote: > On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote: >> On Tuesday, 25 September 2007 11:58, Damien Wyart wrote: > 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. 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. >> Well, we haven't changed much. >> >>> 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. >> 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? > 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. You will need one more patch on top of just mentioned one. >>> Hm, why did you put acpi_target_sleep_state under CONFIG_SUSPEND? >>> >>> CONFIG_HIBERNATION needs acpi_target_sleep_state too. >> Agree, attaching updated patch. > > Well, please use "ifdef CONFIG_PM_SLEEP" instead of > "if defined(CONFIG_SUSPEND)||defined(CONFIG_HIBERNATION)", > as you did with the second block. 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? - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
On Tuesday, 25 September 2007 15:15, Alexey Starikovskiy wrote: > Rafael J. Wysocki wrote: > > On Tuesday, 25 September 2007 14:53, Alexey Starikovskiy wrote: > >> Rafael J. Wysocki wrote: > >>> On Tuesday, 25 September 2007 14:05, Alexey Starikovskiy wrote: > Rafael J. Wysocki wrote: > > On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote: > >> On Tuesday, 25 September 2007 11:58, Damien Wyart wrote: > > 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. > 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. > >> Well, we haven't changed much. > >> > >>> 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. > >> 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? > > 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. > You will need one more patch on top of just mentioned one. > >>> Hm, why did you put acpi_target_sleep_state under CONFIG_SUSPEND? > >>> > >>> CONFIG_HIBERNATION needs acpi_target_sleep_state too. > >> Agree, attaching updated patch. > > > > Well, please use "ifdef CONFIG_PM_SLEEP" instead of > > "if defined(CONFIG_SUSPEND)||defined(CONFIG_HIBERNATION)", > > as you did with the second block. > 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 - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Rafael J. Wysocki wrote: > On Tuesday, 25 September 2007 15:15, Alexey Starikovskiy wrote: >> Rafael J. Wysocki wrote: >>> On Tuesday, 25 September 2007 14:53, Alexey Starikovskiy wrote: Rafael J. Wysocki wrote: > On Tuesday, 25 September 2007 14:05, Alexey Starikovskiy wrote: >> Rafael J. Wysocki wrote: >>> On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote: On Tuesday, 25 September 2007 11:58, Damien Wyart wrote: >>> 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. >> 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. Well, we haven't changed much. > 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. 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? >>> 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. >> You will need one more patch on top of just mentioned one. > Hm, why did you put acpi_target_sleep_state under CONFIG_SUSPEND? > > CONFIG_HIBERNATION needs acpi_target_sleep_state too. Agree, attaching updated patch. >>> Well, please use "ifdef CONFIG_PM_SLEEP" instead of >>> "if defined(CONFIG_SUSPEND)||defined(CONFIG_HIBERNATION)", >>> as you did with the second block. >> 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. 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, and I would like to avoid that. > > IOW, if we want something to be used for anything else than suspend or > hibernation, it shouldn't be defined under PM_SLEEP. 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. - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
On Tuesday, 25 September 2007 16:19, Alexey Starikovskiy wrote: > Rafael J. Wysocki wrote: > > On Tuesday, 25 September 2007 15:15, Alexey Starikovskiy wrote: > >> Rafael J. Wysocki wrote: > >>> On Tuesday, 25 September 2007 14:53, Alexey Starikovskiy wrote: > Rafael J. Wysocki wrote: > > On Tuesday, 25 September 2007 14:05, Alexey Starikovskiy wrote: > >> Rafael J. Wysocki wrote: > >>> On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote: > On Tuesday, 25 September 2007 11:58, Damien Wyart wrote: > >>> 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. > >> 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. > Well, we haven't changed much. > > > 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. > 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? > >>> 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. > >> You will need one more patch on top of just mentioned one. > > Hm, why did you put acpi_target_sleep_state under CONFIG_SUSPEND? > > > > CONFIG_HIBERNATION needs acpi_target_sleep_state too. > Agree, attaching updated patch. > >>> Well, please use "ifdef CONFIG_PM_SLEEP" instead of > >>> "if defined(CONFIG_SUSPEND)||defined(CONFIG_HIBERNATION)", > >>> as you did with the second block. > >> 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. > 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, > and I would like to avoid that. 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, sorry, I couldn't invent a better name.] > > IOW, if we want something to be used for anything else than suspend or > > hibernation, it shouldn't be defined under PM_SLEEP. > 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... 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 - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Rafael J. Wysocki wrote: > On Tuesday, 25 September 2007 16:19, Alexey Starikovskiy wrote: >> Rafael J. Wysocki wrote: >>> On Tuesday, 25 September 2007 15:15, Alexey Starikovskiy wrote: Rafael J. Wysocki wrote: > On Tuesday, 25 September 2007 14:53, Alexey Starikovskiy wrote: >> Rafael J. Wysocki wrote: >>> On Tuesday, 25 September 2007 14:05, Alexey Starikovskiy wrote: Rafael J. Wysocki wrote: > On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote: >> On Tuesday, 25 September 2007 11:58, Damien Wyart wrote: > 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. 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. >> Well, we haven't changed much. >> >>> 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. >> 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? > 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. You will need one more patch on top of just mentioned one. >>> Hm, why did you put acpi_target_sleep_state under CONFIG_SUSPEND? >>> >>> CONFIG_HIBERNATION needs acpi_target_sleep_state too. >> Agree, attaching updated patch. > Well, please use "ifdef CONFIG_PM_SLEEP" instead of > "if defined(CONFIG_SUSPEND)||defined(CONFIG_HIBERNATION)", > as you did with the second block. 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. >> 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, >> and I would like to avoid that. > > 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. In the second place it depends on header file using PM_SLEEP, so it makes sense. > > OTOH, the only function of PM_SLEEP is to be a replacement for > (SUSPEND || HIBERNATION). It has no other meaning whatsoever. > > [Well, sorry, I couldn't invent a better name.] > >>> IOW, if we want something to be used for anything else than suspend or >>> hibernation, it shouldn't be defined under PM_SLEEP. >> 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... > > Well, since PM_SLEEP is used as (SUSPEND || HIBERNATION) everywhere else, > I think that it would actually be confusing not to use it here. :-) Ok, patch is here. Regards, Alex. ACPI: suspend: fix ACPI_SLEEP states From: Alexey Starikovskiy <[EMAIL PROTECTED]> Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]> --- drivers/acpi/sleep/Makefile |2 +- drivers/acpi/sleep/main.c |4 include/acpi/acpi_drivers.h |4 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/acpi/sleep/Makefile b/drivers/acpi/sleep/Makefile index ba9bd40..f1fb888 100644 --- a/drivers/acpi/sleep/Makefile +++ b/drivers/acpi/sleep/Makefile @@ -1,5 +1,5 @@ obj-y := wakeup.o -obj-$(CONFIG_ACPI_SLEEP) += main.o +obj-y += main.o obj-$(CONFIG_ACPI_SLEEP) += proc.o EXTRA_CFLAGS += $(ACPI_CFLAGS) diff --git a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c index c79edcb..2cbb9aa 100644 --- a/drivers/acpi/sleep/main.c +++ b/drivers/acpi/sleep/main.c @@ -24,7 +24,9 @@ u8 sleep_states[ACPI_S_STATE_COUNT]; +#ifdef CONFIG_PM_SLEEP static u32 acpi_target_sleep_state = ACPI_STATE_S0; +#endif int acpi_sleep_prepare(u32 acpi_state) { @@ -299,6 +301,7 @@ int acpi_suspend(u32 acpi_state) return -EINVAL; } +#ifdef CONFIG_PM_SLEEP /** * acpi_pm_device_sleep_state - return
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
> 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 - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
On Tue, 25 Sep 2007 08:51:09 +0200 Damien Wyart <[EMAIL PROTECTED]> wrote: > > 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 :02:08.0 disabled > > I had to first revert 5a50fe709d527f31169263e36601dd83446d5744 then > f216cc3748a3a22c2b99390fddcdafa0583791a2 (handling of Sx states) to > recover previous behaviour. 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 - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
On Tuesday, 25 September 2007 16:45, Alexey Starikovskiy wrote: > Rafael J. Wysocki wrote: > > On Tuesday, 25 September 2007 16:19, Alexey Starikovskiy wrote: > >> Rafael J. Wysocki wrote: > >>> On Tuesday, 25 September 2007 15:15, Alexey Starikovskiy wrote: > Rafael J. Wysocki wrote: > > On Tuesday, 25 September 2007 14:53, Alexey Starikovskiy wrote: > >> Rafael J. Wysocki wrote: > >>> On Tuesday, 25 September 2007 14:05, Alexey Starikovskiy wrote: > Rafael J. Wysocki wrote: > > On Tuesday, 25 September 2007 13:45, Rafael J. Wysocki wrote: > >> On Tuesday, 25 September 2007 11:58, Damien Wyart wrote: > > 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. > 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. > >> Well, we haven't changed much. > >> > >>> 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. > >> 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? > > 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. > You will need one more patch on top of just mentioned one. > >>> Hm, why did you put acpi_target_sleep_state under CONFIG_SUSPEND? > >>> > >>> CONFIG_HIBERNATION needs acpi_target_sleep_state too. > >> Agree, attaching updated patch. > > Well, please use "ifdef CONFIG_PM_SLEEP" instead of > > "if defined(CONFIG_SUSPEND)||defined(CONFIG_HIBERNATION)", > > as you did with the second block. > 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. > >> 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, > >> and I would like to avoid that. > > > > 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. > In the second place it depends on header file using PM_SLEEP, so it makes > sense. > > > > OTOH, the only function of PM_SLEEP is to be a replacement for > > (SUSPEND || HIBERNATION). It has no other meaning whatsoever. > > > > [Well, sorry, I couldn't invent a better name.] > > > >>> IOW, if we want something to be used for anything else than suspend or > >>> hibernation, it shouldn't be defined under PM_SLEEP. > >> 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... > > > > Well, since PM_SLEEP is used as (SUSPEND || HIBERNATION) everywhere else, > > I think that it would actually be confusing not to use it here. :-) > Ok, patch is here. Acked-by: Rafael J. Wysocki <[EMAIL PROTECTED]> and thanks for fixing this. - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
On Tuesday, 25 September 2007 17:40, Linus Torvalds wrote: > > On Tue, 25 Sep 2007 08:51:09 +0200 Damien Wyart <[EMAIL PROTECTED]> wrote: > > > > 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 :02:08.0 disabled > > > > I had to first revert 5a50fe709d527f31169263e36601dd83446d5744 then > > f216cc3748a3a22c2b99390fddcdafa0583791a2 (handling of Sx states) to > > recover previous behaviour. > > 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). 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 - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
On Tue, 25 Sep 2007 18:45:15 +0400 Alexey Starikovskiy <[EMAIL PROTECTED]> wrote: > [fix-ACPI_SLEEP_states.patch text/x-patch (2.0KB)] > ACPI: suspend: fix ACPI_SLEEP states > > From: Alexey Starikovskiy <[EMAIL PROTECTED]> > > Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]> > --- > > drivers/acpi/sleep/Makefile |2 +- > drivers/acpi/sleep/main.c |4 > include/acpi/acpi_drivers.h |4 > 3 files changed, 5 insertions(+), 5 deletions(-) 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. - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
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. Andrew Morton wrote: > On Tue, 25 Sep 2007 18:45:15 +0400 Alexey Starikovskiy <[EMAIL PROTECTED]> > wrote: > >> [fix-ACPI_SLEEP_states.patch text/x-patch (2.0KB)] >> ACPI: suspend: fix ACPI_SLEEP states >> >> From: Alexey Starikovskiy <[EMAIL PROTECTED]> >> >> Signed-off-by: Alexey Starikovskiy <[EMAIL PROTECTED]> >> --- >> >> drivers/acpi/sleep/Makefile |2 +- >> drivers/acpi/sleep/main.c |4 >> include/acpi/acpi_drivers.h |4 >> 3 files changed, 5 insertions(+), 5 deletions(-) > > 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. > > - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
On 9/25/07, Alexey Starikovskiy <[EMAIL PROTECTED]> wrote: > Torsten Kaiser wrote: > > 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. > This is not the reason. SUSPEND is controlled with CONFIG_SUSPEND and > HIBERNATION is controlled with CONFIG_HIBERNATION. > But if you want S5 ACPI sleep state you might want to enable ACPI_SLEEP... 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 - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Torsten Kaiser wrote: > On 9/25/07, Alexey Starikovskiy <[EMAIL PROTECTED]> wrote: >> Torsten Kaiser wrote: >>> 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. >> This is not the reason. SUSPEND is controlled with CONFIG_SUSPEND and >> HIBERNATION is controlled with CONFIG_HIBERNATION. >> But if you want S5 ACPI sleep state you might want to enable ACPI_SLEEP... > > 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 Understood, patches are available, please test. Regards, Alex. - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Torsten Kaiser wrote: > 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. Good, thanks, Alex. - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
On Tue, 25 Sep 2007 17:05:00 +0200 Damien Wyart <[EMAIL PROTECTED]> wrote: > > 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! 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 - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: ACPI power off regression in 2.6.23-rc8 (NOT in rc7)
Bah, its too damn stable. Break it and do it again. >From 2.6.20.3 : 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 :) On Tue, 2007-09-25 at 08:40 -0700, Linus Torvalds wrote: > > On Tue, 25 Sep 2007 08:51:09 +0200 Damien Wyart <[EMAIL PROTECTED]> wrote: > > > > 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 :02:08.0 disabled > > > > I had to first revert 5a50fe709d527f31169263e36601dd83446d5744 then > > f216cc3748a3a22c2b99390fddcdafa0583791a2 (handling of Sx states) to > > recover previous behaviour. > > 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 > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to [EMAIL PROTECTED] > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html