Re: fix ACPI poweroff in 2.4.25-rc2

Re: fix ACPI poweroff in 2.4.25-rc2

[Bjorn Helgaas]

The recent 2.4 patch breaks the build for ia64 because we don't
set CONFIG_ACPI_SLEEP, and acpi_system_save_state() is only
defined under #ifdef CONFIG_ACPI_SLEEP.  Can you add the following
patch to deal with this?

(Here's the original patch:
    http://linux.bkbits.net:8080/linux-2.4/cset-+MK1MFfKU9qYzr5aE/SGFA@public.gmane.org)

Thanks,
  Bjorn

===== drivers/acpi/system.c 1.18 vs edited =====
--- 1.18/drivers/acpi/system.c	Tue Mar  2 14:50:40 2004
+++ edited/drivers/acpi/system.c	Wed Mar  3 10:43:11 2004
@@ -95,7 +95,9 @@
 {
 	if (unlikely(in_interrupt())) 
 		BUG();
+#ifdef CONFIG_ACPI_SLEEP
 	acpi_system_save_state(ACPI_STATE_S5);
+#endif
 	acpi_enter_sleep_state_prep(ACPI_STATE_S5);
 	ACPI_DISABLE_IRQS();
 	acpi_enter_sleep_state(ACPI_STATE_S5);



-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id=1356&alloc_id=3438&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Matthew Wilcox]

On Wed, Mar 03, 2004 at 11:17:25AM -0700, Bjorn Helgaas wrote:
> ===== drivers/acpi/system.c 1.18 vs edited =====
> --- 1.18/drivers/acpi/system.c	Tue Mar  2 14:50:40 2004
> +++ edited/drivers/acpi/system.c	Wed Mar  3 10:43:11 2004
> @@ -95,7 +95,9 @@
>  {
>  	if (unlikely(in_interrupt())) 
>  		BUG();
> +#ifdef CONFIG_ACPI_SLEEP
>  	acpi_system_save_state(ACPI_STATE_S5);
> +#endif

Should we not rather define a null acpi_system_save_state() function if
CONFIG_ACPI_SLEEP is not set?

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Bjorn Helgaas]

On Wednesday 03 March 2004 12:46 pm, Matthew Wilcox wrote:
> On Wed, Mar 03, 2004 at 11:17:25AM -0700, Bjorn Helgaas wrote:
> > ===== drivers/acpi/system.c 1.18 vs edited =====
> > --- 1.18/drivers/acpi/system.c	Tue Mar  2 14:50:40 2004
> > +++ edited/drivers/acpi/system.c	Wed Mar  3 10:43:11 2004
> > @@ -95,7 +95,9 @@
> >  {
> >  	if (unlikely(in_interrupt())) 
> >  		BUG();
> > +#ifdef CONFIG_ACPI_SLEEP
> >  	acpi_system_save_state(ACPI_STATE_S5);
> > +#endif
> 
> Should we not rather define a null acpi_system_save_state() function if
> CONFIG_ACPI_SLEEP is not set?

Here's a patch to do it that way, if you prefer.  acpi_system_save_state()
isn't defined before use, so you get a warning about that, which I didn't
bother to fix (I expect x86 probably sees the same warning).

===== drivers/acpi/system.c 1.18 vs edited =====
--- 1.18/drivers/acpi/system.c	Tue Mar  2 14:50:40 2004
+++ edited/drivers/acpi/system.c	Wed Mar  3 13:03:01 2004
@@ -360,6 +360,15 @@
 	return status;
 }
 
+#else
+
+acpi_status
+acpi_system_save_state(
+	u32			state)
+{
+	return AE_OK;
+}
+
 #endif /* CONFIG_ACPI_SLEEP */
 
 



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Len Brown]

On Wed, 2004-03-03 at 15:05, Bjorn Helgaas wrote:
> On Wednesday 03 March 2004 12:46 pm, Matthew Wilcox wrote:
> > On Wed, Mar 03, 2004 at 11:17:25AM -0700, Bjorn Helgaas wrote:

> Here's a patch to do it that way, if you prefer.  acpi_system_save_state()
> isn't defined before use, so you get a warning about that, which I didn't
> bother to fix (I expect x86 probably sees the same warning).

I think you mean this, yes?
(note that CONFIG_X86 was redundant since CONFIG_ACPI_SLEEP depends on
it)

-Len

===== drivers/acpi/system.c 1.16 vs edited =====
--- 1.16/drivers/acpi/system.c	Sun Feb 15 06:38:17 2004
+++ edited/drivers/acpi/system.c	Sat Mar  6 00:57:18 2004
@@ -41,14 +41,18 @@
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
 
-#ifdef CONFIG_X86
 #ifdef CONFIG_ACPI_SLEEP
 #include <linux/mc146818rtc.h>
 #include <linux/irq.h>
 #include <asm/hw_irq.h>
-#endif
-#endif
 
+acpi_status acpi_system_save_state(u32);
+#else
+static inline acpi_status acpi_system_save_state(u32 state)
+{
+	return AE_OK;
+}
+#endif /* !CONFIG_ACPI_SLEEP */
 
 #define _COMPONENT		ACPI_SYSTEM_COMPONENT
 ACPI_MODULE_NAME		("acpi_system")





-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Willy Tarreau]

Hi,

On Wed, Mar 03, 2004 at 07:46:56PM +0000, Matthew Wilcox wrote:
> On Wed, Mar 03, 2004 at 11:17:25AM -0700, Bjorn Helgaas wrote:
> > ===== drivers/acpi/system.c 1.18 vs edited =====
> > --- 1.18/drivers/acpi/system.c	Tue Mar  2 14:50:40 2004
> > +++ edited/drivers/acpi/system.c	Wed Mar  3 10:43:11 2004
> > @@ -95,7 +95,9 @@
> >  {
> >  	if (unlikely(in_interrupt())) 
> >  		BUG();
> > +#ifdef CONFIG_ACPI_SLEEP
> >  	acpi_system_save_state(ACPI_STATE_S5);
> > +#endif
> 
> Should we not rather define a null acpi_system_save_state() function if
> CONFIG_ACPI_SLEEP is not set?

well, since I know of at least one machine on which this patch is not
enough, and since Pavel says that it might cause reboots on some systems,
I think that this line has a few chances to stay here anyway. I wish I had
some time to propose a cleaner version of the patch I sent last week.
Zwane proposed to try with set_cpus_allowed(). I don't know if this could work
here, because I don't know how to use it. Perhaps something like
  set_cpus_allowed(boot_cpu) ; schedule() ?

Regards,
Willy



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Len Brown]

Willy,
Zwane was right, set_cpus_allowed() fixes the SMP poweroff problem.  The
patch is here:

http://bugzilla.kernel.org/show_bug.cgi?id=1141

thanks,
-Len

On Wed, 2004-03-03 at 15:20, Willy Tarreau wrote:
> Hi,
> 
> On Wed, Mar 03, 2004 at 07:46:56PM +0000, Matthew Wilcox wrote:
> > On Wed, Mar 03, 2004 at 11:17:25AM -0700, Bjorn Helgaas wrote:
> > > ===== drivers/acpi/system.c 1.18 vs edited =====
> > > --- 1.18/drivers/acpi/system.c	Tue Mar  2 14:50:40 2004
> > > +++ edited/drivers/acpi/system.c	Wed Mar  3 10:43:11 2004
> > > @@ -95,7 +95,9 @@
> > >  {
> > >  	if (unlikely(in_interrupt())) 
> > >  		BUG();
> > > +#ifdef CONFIG_ACPI_SLEEP
> > >  	acpi_system_save_state(ACPI_STATE_S5);
> > > +#endif
> > 
> > Should we not rather define a null acpi_system_save_state() function if
> > CONFIG_ACPI_SLEEP is not set?
> 
> well, since I know of at least one machine on which this patch is not
> enough, and since Pavel says that it might cause reboots on some systems,
> I think that this line has a few chances to stay here anyway. I wish I had
> some time to propose a cleaner version of the patch I sent last week.
> Zwane proposed to try with set_cpus_allowed(). I don't know if this could work
> here, because I don't know how to use it. Perhaps something like
>   set_cpus_allowed(boot_cpu) ; schedule() ?
> 
> Regards,
> Willy
> 
> 
> 
> -------------------------------------------------------
> This SF.Net email is sponsored by: IBM Linux Tutorials
> Free Linux tutorial presented by Daniel Robbins, President and CEO of
> GenToo technologies. Learn everything from fundamentals to system
> administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click
> _______________________________________________
> Acpi-devel mailing list
> Acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/acpi-devel



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Willy Tarreau]

On Sat, Mar 06, 2004 at 01:21:26AM -0500, Len Brown wrote:
> Willy,
> Zwane was right, set_cpus_allowed() fixes the SMP poweroff problem.  The
> patch is here:
> 
> http://bugzilla.kernel.org/show_bug.cgi?id=1141

Great ! I didn't know that set_cpus_allowed() was so easy to use !
Unfortunately I cannot test it anymore on the P4 HT because it's been
racked at a colo. But I'll try this right here on my dual athlon at least
to confirm that it still works.

BTW, I've seen in the machine_restart() code that there were some cases
were the authors prefered to use boot_cpu instead of CPU 0, perhaps because
of particular machines which boot on another CPU. Wouldn't it be better to
use boot_cpu here, since we know for sure that it can access ACPI without
problem ?

Another thing : if we do a AltGr-SysRq-O, is 'current' valid, and can we
schedule ? or are we just in an interrupt handler ? I'll try this right
here anyway.

Thanks for the good news,
Willy



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Willy Tarreau]

On Sat, Mar 06, 2004 at 01:21:26AM -0500, Len Brown wrote:
> Willy,
> Zwane was right, set_cpus_allowed() fixes the SMP poweroff problem.  The
> patch is here:
> 
> http://bugzilla.kernel.org/show_bug.cgi?id=1141

OK, just tested it on the dual athlon, and it still works OK. SysRQ-O works
OK too. And since I've read at least 2 or 3 reports on bugzilla stating that
it fixed some P4 HTs, I suppose it's OK too for mine.

Here's the 2.4 backport against your latest BK patch :

 
--- ./drivers/acpi/system.c.old	Sat Mar  6 10:22:25 2004
+++ ./drivers/acpi/system.c	Sat Mar  6 10:22:31 2004
@@ -40,6 +40,7 @@
 #include <asm/acpi.h>
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
+#include <linux/sched.h>
 
 #ifdef CONFIG_X86
 #ifdef CONFIG_ACPI_SLEEP
@@ -95,6 +96,10 @@
 {
 	if (unlikely(in_interrupt())) 
 		BUG();
+#ifdef CONFIG_SMP
+	/* Some SMP machines only can poweroff in boot CPU */
+	set_cpus_allowed(current, cpumask_of_cpu(0));
+#endif
 	acpi_system_save_state(ACPI_STATE_S5);
 	acpi_enter_sleep_state_prep(ACPI_STATE_S5);
 	ACPI_DISABLE_IRQS();

Cheers,
Willy



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Willy Tarreau]

On Sat, Mar 06, 2004 at 01:21:26AM -0500, Len Brown wrote:
> Willy,
> Zwane was right, set_cpus_allowed() fixes the SMP poweroff problem.  The
> patch is here:

Len, my last mail was totally stupid : my dual athlon uses APM to power off,
so the code was not even compiled ! And guess what ? it does not compile :-(
So please use this one instead, which compiles in 2.4 :

--- linux-2.4.25-wt3/drivers/acpi/system.c.old	Sat Mar  6 10:22:25 2004
+++ linux-2.4.25-wt3/drivers/acpi/system.c	Sat Mar  6 11:56:28 2004
@@ -40,6 +40,7 @@
 #include <asm/acpi.h>
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
+#include <linux/sched.h>
 
 #ifdef CONFIG_X86
 #ifdef CONFIG_ACPI_SLEEP
@@ -95,6 +96,10 @@
 {
 	if (unlikely(in_interrupt())) 
 		BUG();
+#ifdef CONFIG_SMP
+	/* Some SMP machines only can poweroff in boot CPU */
+	set_cpus_allowed(current, 1 << cpu_logical_map(0));
+#endif
 	acpi_system_save_state(ACPI_STATE_S5);
 	acpi_enter_sleep_state_prep(ACPI_STATE_S5);
 	ACPI_DISABLE_IRQS();

Now the SMP athlon *really* powers off with this code since APM is disabled.

Sorry for the confusion,
Willy



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Pavel Machek]

Hu!
> The recent 2.4 patch breaks the build for ia64 because we don't
> set CONFIG_ACPI_SLEEP, and acpi_system_save_state() is only
> defined under #ifdef CONFIG_ACPI_SLEEP.  Can you add the following
> patch to deal with this?

Original patch should be reverted. Having different
behaviour of poweroff depending on ACPI_SLEEP would
be wrong.
				Pavel
-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Marcelo Tosatti]

On Mon, 8 Mar 2004, Pavel Machek wrote:

> Hu!
> > The recent 2.4 patch breaks the build for ia64 because we don't
> > set CONFIG_ACPI_SLEEP, and acpi_system_save_state() is only
> > defined under #ifdef CONFIG_ACPI_SLEEP.  Can you add the following
> > patch to deal with this?
> 
> Original patch should be reverted. Having different
> behaviour of poweroff depending on ACPI_SLEEP would
> be wrong.

Hi Pavel,

Original patches fixes a real problem, so its not as easy as "lets revert 
it", yes? 






-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Pavel Machek]

Hi!

> > > The recent 2.4 patch breaks the build for ia64 because we don't
> > > set CONFIG_ACPI_SLEEP, and acpi_system_save_state() is only
> > > defined under #ifdef CONFIG_ACPI_SLEEP.  Can you add the following
> > > patch to deal with this?
> > 
> > Original patch should be reverted. Having different
> > behaviour of poweroff depending on ACPI_SLEEP would
> > be wrong.
> 
> Hi Pavel,
> 
> Original patches fixes a real problem, so its not as easy as "lets revert 
> it", yes? 

Well, it is.

Real problem was that someone's machine was not able to suspend with
APIC enabled (IIRC). He found that calling acpi_system_save_state()
accidentally fixes it for him (probably AML disables APIC or something
like that).

Now, right fix is to make linux shutdown APIC (etc) prior to
powerdown. Calling random functions is not the way to go.

							Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Pavel Machek]

On Po 08-03-04 15:15:58, Marcelo Tosatti wrote:

> > > The recent 2.4 patch breaks the build for ia64 because we don't
> > > set CONFIG_ACPI_SLEEP, and acpi_system_save_state() is only
> > > defined under #ifdef CONFIG_ACPI_SLEEP.  Can you add the following
> > > patch to deal with this?
> > 
> > Original patch should be reverted. Having different
> > behaviour of poweroff depending on ACPI_SLEEP would
> > be wrong.
> 
> Hi Pavel,
> 
> Original patches fixes a real problem, so its not as easy as "lets revert 
> it", yes? 

To put it simply:

there's broken machine where calling acpi_system_save_state() with
invalid argument fixes it by accident.

It can do random stuff on any other machine...
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Marcelo Tosatti]

On Mon, 8 Mar 2004, Pavel Machek wrote:

> On Po 08-03-04 15:15:58, Marcelo Tosatti wrote:
> 
> > > > The recent 2.4 patch breaks the build for ia64 because we don't
> > > > set CONFIG_ACPI_SLEEP, and acpi_system_save_state() is only
> > > > defined under #ifdef CONFIG_ACPI_SLEEP.  Can you add the following
> > > > patch to deal with this?
> > > 
> > > Original patch should be reverted. Having different
> > > behaviour of poweroff depending on ACPI_SLEEP would
> > > be wrong.
> > 
> > Hi Pavel,
> > 
> > Original patches fixes a real problem, so its not as easy as "lets revert 
> > it", yes? 
> 
> To put it simply:
> 
> there's broken machine where calling acpi_system_save_state() with
> invalid argument fixes it by accident.
> 
> It can do random stuff on any other machine...

What random stuff ?





-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Pavel Machek]

Hi!

> > > > > The recent 2.4 patch breaks the build for ia64 because we don't
> > > > > set CONFIG_ACPI_SLEEP, and acpi_system_save_state() is only
> > > > > defined under #ifdef CONFIG_ACPI_SLEEP.  Can you add the following
> > > > > patch to deal with this?
> > > > 
> > > > Original patch should be reverted. Having different
> > > > behaviour of poweroff depending on ACPI_SLEEP would
> > > > be wrong.
> > > 
> > > Hi Pavel,
> > > 
> > > Original patches fixes a real problem, so its not as easy as "lets revert 
> > > it", yes? 
> > 
> > To put it simply:
> > 
> > there's broken machine where calling acpi_system_save_state() with
> > invalid argument fixes it by accident.
> > 
> > It can do random stuff on any other machine...
> 
> What random stuff ?

Okay, I was wrong. acpi_system_save_state does not do what I thought.

Anyway, #ifdef CONFIG_ACPI_SLEEP around acpi_system_sleep_state is
wrong fix. If its needed, its needed regardless of
CONFIG_ACPI_SLEEP...

Replacing it with
	pm_send_all(PM_SAVE_STATE, (void *)3);
	pm_send_all(PM_SUSPEND, (void *)3);
should do the trick.

								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Willy Tarreau]

Hi Pavel,

On Mon, Mar 08, 2004 at 07:32:28PM +0100, Pavel Machek wrote:
 
> there's broken machine where calling acpi_system_save_state() with
> invalid argument fixes it by accident.

In fact it does not fix it by accident, it's the previous code which
broke it by accident. Older ACPI poweroff code has been working well
on this machine, and suddenly stopped working. So i looked at what
changed and noticed that the acpi_poweroff() function changed from
_I don't remember what function_ to acpi_suspend(S5), and that
acpi_system_save_state(), which was called implicitly in the previous
function got lost during the change. Taking it back fixed my problem,
but I cannot say for sure that it will not break anything else, but its
use seems legitimate at first glance.

I agree that disabling local APIC from there would be another alternative,
but it may also be regarded as a workaround because I cannot explain
why this sh*tty notebook doesn't like it.

My intention was to write a more generic machine_prepare_stop() function
which would disable sensible things such as APIC, stop every non-boot CPU
and get the best conditions to stop the system. It could even be used for
the restart and for kexec. But calling it from ACPI and writing a new
arch-specific function for every arch will for sure break some other archs.

Anyway, up to 2.4.24, on the 3 machines I know that support ACPI, none of
them could poweroff. Now, with both this fix and the SMP fix, they all shutdown
cleanly. So I'm hopeful that we're in the right direction.

Cheers,
Willy



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Pavel Machek]

Hi!

> > there's broken machine where calling acpi_system_save_state() with
> > invalid argument fixes it by accident.
> 
> In fact it does not fix it by accident, it's the previous code which
> broke it by accident. Older ACPI poweroff code has been working well
> on this machine, and suddenly stopped working. So i looked at what
> changed and noticed that the acpi_poweroff() function changed from
> _I don't remember what function_ to acpi_suspend(S5), and that
> acpi_system_save_state(), which was called implicitly in the previous
> function got lost during the change. Taking it back fixed my problem,
> but I cannot say for sure that it will not break anything else, but its
> use seems legitimate at first glance.

Can you replace

acpi_system_save_state() with

        pm_send_all(PM_SAVE_STATE, (void *)3);
        pm_send_all(PM_SUSPEND, (void *)3);

?

That should fix it, too, and does not depend on CONFIG_ACPI_SLEEP.

							Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: Re: fix ACPI poweroff in 2.4.25-rc2

[Willy Tarreau]

On Mon, Mar 08, 2004 at 09:54:44PM +0100, Pavel Machek wrote:
 
> Can you replace
> 
> acpi_system_save_state() with
> 
>         pm_send_all(PM_SAVE_STATE, (void *)3);
>         pm_send_all(PM_SUSPEND, (void *)3);
> 
> ?
> 
> That should fix it, too, and does not depend on CONFIG_ACPI_SLEEP.

OK, I see what you mean, because I also wondered if it wouln't be equivalent
when I first tried the other function. I'll try, and I think it will work. If
it eliminates a dependancy, of course it should be better.

Cheers,
Willy



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

[PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Willy Tarreau]

Hi Pavel,

On Mon, Mar 08, 2004 at 09:54:44PM +0100, Pavel Machek wrote:
> Can you replace
> 
> acpi_system_save_state() with
> 
>         pm_send_all(PM_SAVE_STATE, (void *)3);
>         pm_send_all(PM_SUSPEND, (void *)3);
> 
> ?
> 
> That should fix it, too, and does not depend on CONFIG_ACPI_SLEEP.

Just checked, and I still can poweroff (which was a bit expected anyway, but
now confirmed). So with this little patch against 2.4.26-pre2, poweroff does
not depend on CONFIG_ACPI_SLEEP anymore, as you fairly requested (thank you
for having insisted).


diff -urN linux-2.4.26-pre2/drivers/acpi/system.c linux-2.4.26-pre2-pwoff/drivers/acpi/system.c
--- linux-2.4.26-pre2/drivers/acpi/system.c	Wed Mar 10 00:36:20 2004
+++ linux-2.4.26-pre2-pwoff/drivers/acpi/system.c	Wed Mar 10 00:43:48 2004
@@ -45,13 +45,6 @@
 #include <linux/mc146818rtc.h>
 #include <linux/irq.h>
 #include <asm/hw_irq.h>
-
-acpi_status acpi_system_save_state(u32);
-#else
-static inline acpi_status acpi_system_save_state(u32 state)
-{
-	return AE_OK;
-}
 #endif /* !CONFIG_ACPI_SLEEP */
 
 #define _COMPONENT		ACPI_SYSTEM_COMPONENT
@@ -99,7 +92,8 @@
 {
 	if (unlikely(in_interrupt())) 
 		BUG();
-	acpi_system_save_state(ACPI_STATE_S5);
+	pm_send_all(PM_SAVE_STATE, (void *)3);
+	pm_send_all(PM_SUSPEND, (void *)3);
 	acpi_enter_sleep_state_prep(ACPI_STATE_S5);
 	ACPI_DISABLE_IRQS();
 	acpi_enter_sleep_state(ACPI_STATE_S5);


Len, do you have any objection against this one ?
BTW, did you get the previous one dedicated to SMP ? Here it is
rediffed after this one, just in case :


diff -urN linux-2.4.26-pre2-pwoff/drivers/acpi/system.c linux-2.4.26-pre2-pwoff-smp/drivers/acpi/system.c
--- linux-2.4.26-pre2-pwoff/drivers/acpi/system.c	Wed Mar 10 00:43:48 2004
+++ linux-2.4.26-pre2-pwoff-smp/drivers/acpi/system.c	Wed Mar 10 00:44:26 2004
@@ -40,6 +40,7 @@
 #include <asm/acpi.h>
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
+#include <linux/sched.h>
 
 #ifdef CONFIG_ACPI_SLEEP
 #include <linux/mc146818rtc.h>
@@ -92,6 +93,10 @@
 {
 	if (unlikely(in_interrupt())) 
 		BUG();
+#ifdef CONFIG_SMP
+	/* Some SMP machines only can poweroff in boot CPU */
+	set_cpus_allowed(current, 1 << cpu_logical_map(0));
+#endif
 	pm_send_all(PM_SAVE_STATE, (void *)3);
 	pm_send_all(PM_SUSPEND, (void *)3);
 	acpi_enter_sleep_state_prep(ACPI_STATE_S5);


Cheers,
Willy



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Sérgio Monteiro Basto]

Hi 

One interesting test 
I have one Dell powerEdge with 2 CPU (I see 4) one Raid, 4GB Ram, 3
gigabits Ethernet Controller, and I have installed slackware 9.1 and
kernel 2.5.25 with smp, 64GB, raid and acpi , compiled unfortunately
with gcc 3.3.2 .

shutdown -h now 
doesn't poweroff but 
poweroff 
poweroff well.

I can't test with pre patches neither patches but what is the difference
of shutdown -h now and poweroff  ?
if any ? 


thanks 


-- 
Sérgio M. B.
-- 
Sérgio M. B.



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Bruno Ducrot]

On Wed, Mar 10, 2004 at 12:33:25AM +0100, Willy Tarreau wrote:
> Hi Pavel,
> 
> On Mon, Mar 08, 2004 at 09:54:44PM +0100, Pavel Machek wrote:
> > Can you replace
> > 
> > acpi_system_save_state() with
> > 
> >         pm_send_all(PM_SAVE_STATE, (void *)3);
> >         pm_send_all(PM_SUSPEND, (void *)3);
> > 
> > ?
> > 
> > That should fix it, too, and does not depend on CONFIG_ACPI_SLEEP.
> 
> Just checked, and I still can poweroff (which was a bit expected anyway, but
> now confirmed). So with this little patch against 2.4.26-pre2, poweroff does
> not depend on CONFIG_ACPI_SLEEP anymore, as you fairly requested (thank you
> for having insisted).

IMHO, that is much likely a bug somewhere in the notifer_reboot chain
than an ACPI problem.  For example, IDE disks would all been in standby
modes, and IDE hwif will be 'cleaned up' before even calling the
poweroff callback, yet with your patch the pm_send_all with
PM_SAVE_STATE would actually hang some of those IDE hwif.

-- 
Bruno Ducrot

--  Which is worse:  ignorance or apathy?
--  Don't know.  Don't care.


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Sérgio Monteiro Basto]

On Tue, 2004-03-09 at 23:33, Willy Tarreau wrote:
> On Mon, Mar 08, 2004 at 09:54:44PM +0100, Pavel Machek wrote:
> > Can you replace
> > 
> > acpi_system_save_state() with
> >         pm_send_all(PM_SAVE_STATE, (void *)3);
> >         pm_send_all(PM_SUSPEND, (void *)3);
> > ?
> > That should fix it, too, and does not depend on CONFIG_ACPI_SLEEP.

Patch 1:
> diff -urN linux-2.4.26-pre2/drivers/acpi/system.c linux-2.4.26-pre2-pwoff/drivers/acpi/system.c
> --- linux-2.4.26-pre2/drivers/acpi/system.c	Wed Mar 10 00:36:20 2004
> +++ linux-2.4.26-pre2-pwoff/drivers/acpi/system.c	Wed Mar 10 00:43:48 2004
> @@ -45,13 +45,6 @@
>  #include <linux/mc146818rtc.h>
>  #include <linux/irq.h>
>  #include <asm/hw_irq.h>
> -
> -acpi_status acpi_system_save_state(u32);
> -#else
> -static inline acpi_status acpi_system_save_state(u32 state)
> -{
> -	return AE_OK;
> -}
>  #endif /* !CONFIG_ACPI_SLEEP */
>  
>  #define _COMPONENT		ACPI_SYSTEM_COMPONENT
> @@ -99,7 +92,8 @@
>  {
>  	if (unlikely(in_interrupt())) 
>  		BUG();
> -	acpi_system_save_state(ACPI_STATE_S5);
> +	pm_send_all(PM_SAVE_STATE, (void *)3);
> +	pm_send_all(PM_SUSPEND, (void *)3);
>  	acpi_enter_sleep_state_prep(ACPI_STATE_S5);
>  	ACPI_DISABLE_IRQS();
>  	acpi_enter_sleep_state(ACPI_STATE_S5);
> 
> 
> Len, do you have any objection against this one ?
> BTW, did you get the previous one dedicated to SMP ? Here it is
> rediffed after this one, just in case :

Patch 2:
> diff -urN linux-2.4.26-pre2-pwoff/drivers/acpi/system.c linux-2.4.26-pre2-pwoff-smp/drivers/acpi/system.c
> --- linux-2.4.26-pre2-pwoff/drivers/acpi/system.c	Wed Mar 10 00:43:48 2004
> +++ linux-2.4.26-pre2-pwoff-smp/drivers/acpi/system.c	Wed Mar 10 00:44:26 2004
> @@ -40,6 +40,7 @@
>  #include <asm/acpi.h>
>  #include <acpi/acpi_bus.h>
>  #include <acpi/acpi_drivers.h>
> +#include <linux/sched.h>
>  
>  #ifdef CONFIG_ACPI_SLEEP
>  #include <linux/mc146818rtc.h>
> @@ -92,6 +93,10 @@
>  {
>  	if (unlikely(in_interrupt())) 
>  		BUG();
> +#ifdef CONFIG_SMP
> +	/* Some SMP machines only can poweroff in boot CPU */
> +	set_cpus_allowed(current, 1 << cpu_logical_map(0));
> +#endif
>  	pm_send_all(PM_SAVE_STATE, (void *)3);
>  	pm_send_all(PM_SUSPEND, (void *)3);
>  	acpi_enter_sleep_state_prep(ACPI_STATE_S5);

Hi 
the patch 1 can't be applied because 
we still have on line 341 of drivers/acpi/system.c :
    status = acpi_system_save_state(state);

the patch 2 has been applied on acpi-20040311-2.4.26.diff.gz,
   [ACPI] SMP poweroff (David Shaohua Li)
   http://bugzilla.kernel.org/show_bug.cgi?id=1141
and is a little different:
+       /* Some SMP machines only can poweroff in boot CPU */
+       set_cpus_allowed(current, 1UL << cpu_logical_map(0));
                                  ^^^

have you some suggestion for correction of patch 1 ?

thanks
-- 
Sérgio M. B.



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Willy Tarreau]

Hi,

On Sun, Mar 14, 2004 at 03:51:31PM +0000, S?rgio Monteiro Basto wrote:
> Hi 
> the patch 1 can't be applied because 
> we still have on line 341 of drivers/acpi/system.c :
>     status = acpi_system_save_state(state);
> 
> the patch 2 has been applied on acpi-20040311-2.4.26.diff.gz,
>    [ACPI] SMP poweroff (David Shaohua Li)
>    http://bugzilla.kernel.org/show_bug.cgi?id=1141
> and is a little different:
> +       /* Some SMP machines only can poweroff in boot CPU */
> +       set_cpus_allowed(current, 1UL << cpu_logical_map(0));
>                                   ^^^

OK it's equivalent, but cleaner with 1UL.

> have you some suggestion for correction of patch 1 ?

It's really trivial. Try the following one. BTW, be aware that Bruno believes
that this patch and the patch in BK might break IDE in some circumstances
(if interfaces are already powered down if I understand him correctly). So
even if what is done right here is relatively equivalent to current code in BK
without the dependency, it has great chances of not being definitive code.

Bruno, do you think that calling either one or the other pm_send line below
would still make sense and/or break IDE ? I haven't tested yet. IIRC my
notebook stopped correctly if I just disabled APIC in acpi_power_off().
Perhaps we should get back to something more APIC related, but I'm not
confident it will be enough for every hardware (at least it doesn't seem
to be enough if I use the atyfb on this notebook, I'll have to recheck).

Cheers,
Willy

--- linux-2.4.26-pre3-acpi/drivers/acpi/system.c	Mon Mar 15 21:54:55 2004
+++ linux-2.4.26-pre3-acpi-pwoff/drivers/acpi/system.c	Mon Mar 15 21:57:32 2004
@@ -46,13 +46,6 @@
 #include <linux/mc146818rtc.h>
 #include <linux/irq.h>
 #include <asm/hw_irq.h>
-
-acpi_status acpi_system_save_state(u32);
-#else
-static inline acpi_status acpi_system_save_state(u32 state)
-{
-	return AE_OK;
-}
 #endif /* !CONFIG_ACPI_SLEEP */
 
 #define _COMPONENT		ACPI_SYSTEM_COMPONENT
@@ -102,7 +95,8 @@
 		BUG();
 	/* Some SMP machines only can poweroff in boot CPU */
 	set_cpus_allowed(current, 1UL << cpu_logical_map(0));
-	acpi_system_save_state(ACPI_STATE_S5);
+	pm_send_all(PM_SAVE_STATE, (void *)3);
+	pm_send_all(PM_SUSPEND, (void *)3);
 	acpi_enter_sleep_state_prep(ACPI_STATE_S5);
 	ACPI_DISABLE_IRQS();
 	acpi_enter_sleep_state(ACPI_STATE_S5);




-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Sérgio Monteiro Basto]

Hi 
On Mon, 2004-03-15 at 21:07, Willy Tarreau wrote:
> --- linux-2.4.26-pre3-acpi/drivers/acpi/system.c	Mon Mar 15 21:54:55 2004
> +++ linux-2.4.26-pre3-acpi-pwoff/drivers/acpi/system.c	Mon Mar 15 21:57:32 2004
> @@ -46,13 +46,6 @@
>  #include <linux/mc146818rtc.h>
>  #include <linux/irq.h>
>  #include <asm/hw_irq.h>
> -
> -acpi_status acpi_system_save_state(u32);
> -#else
> -static inline acpi_status acpi_system_save_state(u32 state)
> -{
> -	return AE_OK;
> -}
>  #endif /* !CONFIG_ACPI_SLEEP */
>  
>  #define _COMPONENT		ACPI_SYSTEM_COMPONENT
> @@ -102,7 +95,8 @@
>  		BUG();
>  	/* Some SMP machines only can poweroff in boot CPU */
>  	set_cpus_allowed(current, 1UL << cpu_logical_map(0));
> -	acpi_system_save_state(ACPI_STATE_S5);
> +	pm_send_all(PM_SAVE_STATE, (void *)3);
> +	pm_send_all(PM_SUSPEND, (void *)3);
>  	acpi_enter_sleep_state_prep(ACPI_STATE_S5);
>  	ACPI_DISABLE_IRQS();
>  	acpi_enter_sleep_state(ACPI_STATE_S5);


and status = acpi_system_save_state(state); 
on line 341 of drivers/acpi/system.c ?
     
-- 
Sérgio M. B.



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Willy Tarreau]

On Mon, Mar 15, 2004 at 10:23:54PM +0000, S?rgio Monteiro Basto wrote:
 
> and status = acpi_system_save_state(state); 
> on line 341 of drivers/acpi/system.c ?

OK sorry, I didn't notice it. So you'll simply have to remove the
first hunk of the patch so that acpi_system_save_state() stays defined
at least when CONFIG_ACPI_SLEEP is defined.

Willy



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Pavel Machek]

Hi!

> > and status = acpi_system_save_state(state); 
> > on line 341 of drivers/acpi/system.c ?
> 
> OK sorry, I didn't notice it. So you'll simply have to remove the
> first hunk of the patch so that acpi_system_save_state() stays defined
> at least when CONFIG_ACPI_SLEEP is defined.

In such case... at least insert BUG() into fake acpi_system_save_state method.
-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Sérgio Monteiro Basto]

hi(gh)!
On Mon, 2004-03-22 at 10:59, Pavel Machek wrote:
> Hi!
> 
> > > and status = acpi_system_save_state(state); 
> > > on line 341 of drivers/acpi/system.c ?
> > 
> > OK sorry, I didn't notice it. So you'll simply have to remove the
> > first hunk of the patch so that acpi_system_save_state() stays defined
> > at least when CONFIG_ACPI_SLEEP is defined.
> 
> In such case... at least insert BUG() into fake acpi_system_save_state method.

Well you say that is a bug and I ask how do you translate 
status = acpi_system_save_state(state); if not I think that will be
inefficient. 
-- 
Sérgio M. B.



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Pavel Machek]

On Čt 25-03-04 22:22:07, Sérgio Monteiro Basto wrote:
> hi(gh)!
> On Mon, 2004-03-22 at 10:59, Pavel Machek wrote:
> > Hi!
> > 
> > > > and status = acpi_system_save_state(state); 
> > > > on line 341 of drivers/acpi/system.c ?
> > > 
> > > OK sorry, I didn't notice it. So you'll simply have to remove the
> > > first hunk of the patch so that acpi_system_save_state() stays defined
> > > at least when CONFIG_ACPI_SLEEP is defined.
> > 
> > In such case... at least insert BUG() into fake acpi_system_save_state method.
> 
> Well you say that is a bug and I ask how do you translate 
> status = acpi_system_save_state(state); if not I think that will be
> inefficient. 

? Sorry, parse error on my part.

*If* you replace acpi_system_save_state() with stub that does nothing
based on config option *and* someone tries to use it, they are not
getting the functionality they want, right? And therefore calling it
is a BUG().
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Sérgio Monteiro Basto]

so acpi_system_save_state(state) does do nothing at all ?
or does return nothing at all ?

On Thu, 2004-03-25 at 22:29, Pavel Machek wrote:
> On Čt 25-03-04 22:22:07, Sérgio Monteiro Basto wrote:
> > hi(gh)!
> > On Mon, 2004-03-22 at 10:59, Pavel Machek wrote:
> > > Hi!
> > > 
> > > > > and status = acacpi_system_save_state()pi_system_save_state(state); 
> > > > > on line 341 of drivers/acpi/system.c ?
> > > > 
> > > > OK sorry, I didn't notice it. So you'll simply have to remove the
> > > > first hunk of the patch so that acpi_system_save_state() stays defined
> > > > at least when CONFIG_ACPI_SLEEP is defined.
> > > 
> > > In such case... at least insert BUG() into fake acpi_system_save_state method.
> > 
> > Well you say that is a bug and I ask how do you translate 
> > status = acpi_system_save_state(state); if not I think that will be
> > inefficient. 
> 
> ? Sorry, parse error on my part.
> 
> *If* you replace acpi_system_save_state() with stub that does nothing
> based on config option *and* someone tries to use it, they are not
> getting the functionality they want, right? And therefore calling it
> is a BUG().
> 								Pavel
-- 
Sérgio M. B.



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Pavel Machek]

Hi!

> so acpi_system_save_state(state) does do nothing at all ?
> or does return nothing at all ?

IIRC there was patch that made acpi_system_save_state() do nothing at
all if !CONFIG_ACPI_SLEEP. I'm arguing thats very wrong.
								Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Sérgio Monteiro Basto]

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

Preamble :
I can be confused, but far as I know, you propose a patch for a _very
wrong_ thing, ( I believe that you are right ).
acpi_system_save_state is one reality, at least in kernel 2.4.26-pre2,
but what you propose looks like, that just resolve half of the problem.
if it a recently patch that made this problem I don't know.

Seeing the code yes, you are right.
I don't know if as make any sense to you but, what do you think put
acpi_system_save_state under #ifdef CONFIG_PM instead of CONFIG_SLEEP ?
like this 

thanks
On Thu, 2004-03-25 at 23:00, Pavel Machek wrote:
> Hi!
> 
> > so acpi_system_save_state(state) does do nothing at all ?
> > or does return nothing at all ?
> 
> IIRC there was patch that made acpi_system_save_state() do nothing at
> all if !CONFIG_ACPI_SLEEP. I'm arguing thats very wrong.
> 								Pavel
-- 
Sérgio M. B.

[-- Attachment #2: sys.diff --]
[-- Type: text/x-patch, Size: 3416 bytes --]

--- linux-2.4.26-pre2/drivers/acpi/system.c.orig	2004-03-26 00:47:29.000000000 +0000
+++ linux-2.4.26-pre2/drivers/acpi/system.c	2004-03-26 00:57:11.000000000 +0000
@@ -46,14 +46,16 @@
 #include <linux/mc146818rtc.h>
 #include <linux/irq.h>
 #include <asm/hw_irq.h>
+#endif /* CONFIG_ACPI_SLEEP */
 
+#ifdef CONFIG_PM
 acpi_status acpi_system_save_state(u32);
 #else
 static inline acpi_status acpi_system_save_state(u32 state)
 {
 	return AE_OK;
 }
-#endif /* !CONFIG_ACPI_SLEEP */
+#endif /* CONFIG_PM */ 
 
 #define _COMPONENT		ACPI_SYSTEM_COMPONENT
 ACPI_MODULE_NAME		("acpi_system")
@@ -110,62 +112,6 @@
 	printk(KERN_EMERG "ACPI: can not power off machine\n");
 }
 
-#endif /*CONFIG_PM*/
-
-
-#ifdef CONFIG_ACPI_SLEEP
-
-/**
- * acpi_system_restore_state - OS-specific restoration of state
- * @state:	sleep state we're exiting
- *
- * Note that if we're coming back from S4, the memory image should have
- * already been loaded from the disk and is already in place.  (Otherwise how
- * else would we be here?).
- */
-acpi_status
-acpi_system_restore_state(
-	u32			state)
-{
-	/* 
-	 * We should only be here if we're coming back from STR or STD.
-	 * And, in the case of the latter, the memory image should have already
-	 * been loaded from disk.
-	 */
-	if (state > ACPI_STATE_S1) {
-		acpi_restore_state_mem();
-
-		/* Do _early_ resume for irqs.  Required by
-		 * ACPI specs.
-		 */
-		/* TBD: call arch dependant reinitialization of the 
-		 * interrupts.
-		 */
-#ifdef CONFIG_X86
-		init_8259A(0);
-#endif
-		/* wait for power to come back */
-		mdelay(1000);
-
-	}
-
-	/* Be really sure that irqs are disabled. */
-	ACPI_DISABLE_IRQS();
-
-	/* Wait a little again, just in case... */
-	mdelay(1000);
-
-	/* enable interrupts once again */
-	ACPI_ENABLE_IRQS();
-
-	/* turn all the devices back on */
-	if (state > ACPI_STATE_S1)
-		pm_send_all(PM_RESUME, (void *)0);
-
-	return AE_OK;
-}
-
-
 /**
  * acpi_system_save_state - save OS specific state and power down devices
  * @state:	sleep state we're entering.
@@ -259,6 +205,63 @@
 	return error ? AE_ERROR : AE_OK;
 }
 
+#endif /*CONFIG_PM*/
+
+
+#ifdef CONFIG_ACPI_SLEEP
+
+/**
+ * acpi_system_restore_state - OS-specific restoration of state
+ * @state:	sleep state we're exiting
+ *
+ * Note that if we're coming back from S4, the memory image should have
+ * already been loaded from the disk and is already in place.  (Otherwise how
+ * else would we be here?).
+ */
+acpi_status
+acpi_system_restore_state(
+	u32			state)
+{
+	/* 
+	 * We should only be here if we're coming back from STR or STD.
+	 * And, in the case of the latter, the memory image should have already
+	 * been loaded from disk.
+	 */
+	if (state > ACPI_STATE_S1) {
+		acpi_restore_state_mem();
+
+		/* Do _early_ resume for irqs.  Required by
+		 * ACPI specs.
+		 */
+		/* TBD: call arch dependant reinitialization of the 
+		 * interrupts.
+		 */
+#ifdef CONFIG_X86
+		init_8259A(0);
+#endif
+		/* wait for power to come back */
+		mdelay(1000);
+
+	}
+
+	/* Be really sure that irqs are disabled. */
+	ACPI_DISABLE_IRQS();
+
+	/* Wait a little again, just in case... */
+	mdelay(1000);
+
+	/* enable interrupts once again */
+	ACPI_ENABLE_IRQS();
+
+	/* turn all the devices back on */
+	if (state > ACPI_STATE_S1)
+		pm_send_all(PM_RESUME, (void *)0);
+
+	return AE_OK;
+}
+
+
+
 
 /****************************************************************************
  *

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Sérgio Monteiro Basto]

Sorry about that patch this really doesn't make sence,

in kernels 2.4.x now in drives/acpi/Config.in

if [ "$CONFIG_ACPI" = "y" ]; then
ACPI_SLEEP = y

if acpi support (CONFIG_ACPI) is equal to no =>
the drives/acpi/system.c is not compiled at all.

the only thing just don't make sense (for me) is permission to compile
ACPI without CONFIG_PM ?

in conclusion without any patch acpi_system_save_state never is "return
ok", so you can ignore my patch.

and arch/i386/config.in shouldn't be like this:

if [ "$CONFIG_PM" = "y" ]; then
source drivers/acpi/Config.in
fi

thanks 
I will investe more ...

On Fri, 2004-03-26 at 01:01, Sérgio Monteiro Basto wrote:
> Preamble :
> I can be confused, but far as I know, you propose a patch for a _very
> wrong_ thing, ( I believe that you are right ).
> acpi_system_save_state is one reality, at least in kernel 2.4.26-pre2,
> but what you propose looks like, that just resolve half of the problem.
> if it a recently patch that made this problem I don't know.
> 
> Seeing the code yes, you are right.
> I don't know if as make any sense to you but, what do you think put
> acpi_system_save_state under #ifdef CONFIG_PM instead of CONFIG_SLEEP ?
> like this 
> 
> thanks
> On Thu, 2004-03-25 at 23:00, Pavel Machek wrote:
> > Hi!
> > 
> > > so acpi_system_save_state(state) does do nothing at all ?
> > > or does return nothing at all ?
> > 
> > IIRC there was patch that made acpi_system_save_state() do nothing at
> > all if !CONFIG_ACPI_SLEEP. I'm arguing thats very wrong.
> > 								Pavel
-- 
Sérgio M. B.



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Pavel Machek]

Hi!

> Sorry about that patch this really doesn't make sence,
> 
> in kernels 2.4.x now in drives/acpi/Config.in
> 
> if [ "$CONFIG_ACPI" = "y" ]; then
> ACPI_SLEEP = y

I thought ia64 had problem where there was CONFIG_ACPI but not
CONFIG_ACPI_SLEEP.
							Pavel
-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Sérgio Monteiro Basto]

On Fri, 2004-03-26 at 22:07, Pavel Machek wrote:
> Hi!
> 
> > Sorry about that patch this really doesn't make sence,
> > 
> > in kernels 2.4.x now in drives/acpi/Config.in
> > 
> > if [ "$CONFIG_ACPI" = "y" ]; then
> > ACPI_SLEEP = y
> 
> I thought ia64 had problem where there was CONFIG_ACPI but not
> CONFIG_ACPI_SLEEP.
> 							Pavel
sorry I just study i386 case, I forgot to see ia64 
-- 
Sérgio M. B.



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Sérgio Monteiro Basto]

Hi

On Fri, 2004-03-26 at 22:07, Pavel Machek wrote:
> I thought ia64 had problem where there was CONFIG_ACPI but not
> CONFIG_ACPI_SLEEP.

yes, So this problem is with ia64. 
I don't have any acknowledgment on ia64, so i give up.

IIRC: if I recall correctly 
we couldn't compile ACPI without Power management,
but now we can configure ACPI without PM this is right ?

or arch/i386/config.in shouldn't be like this:

if [ "$CONFIG_PM" = "y" ]; then
source drivers/acpi/Config.in
fi

thanks and sorry for the spamming  
-- 
Sérgio M. B.



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Willy Tarreau]

Hi Pavel,

On Fri, Mar 26, 2004 at 12:00:37AM +0100, Pavel Machek wrote:
> Hi!
> 
> > so acpi_system_save_state(state) does do nothing at all ?
> > or does return nothing at all ?
> 
> IIRC there was patch that made acpi_system_save_state() do nothing at
> all if !CONFIG_ACPI_SLEEP. I'm arguing thats very wrong.

I 100% agree that it's dirty to call a fake function instead of not
calling it at all. Indeed, one of my proposed patches removed the
dependency on CONFIG_ACPI_SLEEP through a simple replacement of
acpi_system_save_state() with two calls to pm_send_all() which does
not depend on CONFIG_ACPI_SLEEP. I believe it was cleaner, but it's
not what is included in BK right now... Here it is for reference.

Regards,
Willy

--- linux-2.4.26-pre3-acpi/drivers/acpi/system.c	Mon Mar 15 21:54:55 2004
+++ linux-2.4.26-pre3-acpi-pwoff/drivers/acpi/system.c	Mon Mar 15 21:57:32 2004
@@ -46,13 +46,6 @@
 #include <linux/mc146818rtc.h>
 #include <linux/irq.h>
 #include <asm/hw_irq.h>
-
-acpi_status acpi_system_save_state(u32);
-#else
-static inline acpi_status acpi_system_save_state(u32 state)
-{
-	return AE_OK;
-}
 #endif /* !CONFIG_ACPI_SLEEP */
 
 #define _COMPONENT		ACPI_SYSTEM_COMPONENT
@@ -102,7 +95,8 @@
 		BUG();
 	/* Some SMP machines only can poweroff in boot CPU */
 	set_cpus_allowed(current, 1UL << cpu_logical_map(0));
-	acpi_system_save_state(ACPI_STATE_S5);
+	pm_send_all(PM_SAVE_STATE, (void *)3);
+	pm_send_all(PM_SUSPEND, (void *)3);
 	acpi_enter_sleep_state_prep(ACPI_STATE_S5);
 	ACPI_DISABLE_IRQS();
 	acpi_enter_sleep_state(ACPI_STATE_S5);




-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Hartwig Felger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Salut Willy, Pavel and Sérgio,
I have a problem coming through on this list, altough I am subscribed...

Am Freitag, 26. März 2004 06:55 schrieb Willy Tarreau:
> >...
> > IIRC there was patch that made acpi_system_save_state() do nothing at
> > all if !CONFIG_ACPI_SLEEP. I'm arguing thats very wrong.
>
> I 100% agree that it's dirty to call a fake function instead of not
> calling it at all. Indeed, one of my proposed patches removed the
> dependency on CONFIG_ACPI_SLEEP through a simple replacement of
> acpi_system_save_state() with two calls to pm_send_all() which does
> not depend on CONFIG_ACPI_SLEEP. I believe it was cleaner, but it's
> not what is included in BK right now... Here it is for reference.

The regression started with the vanilla-kernels 2.4.25 and 2.6.3, and did not 
happen with the versions 2.4.24 and 2.6.2.
So I started to look at the differences and found out, that reverting this 3 
lines did revert the regression. I did not try to understand, what I did.

Andy Kleen started recently a new thread "Re: [ACPI] [PATCH] Handle disabled 
local apic better" that made me think... I compiled a vanilla 2.6.4 (without 
my patch, and with APIC disabled in config), et voila, everything is happy 
again.

So we need not to look at my patch. I think it would be better to drive 
forward Andy's work on handling APIC-disable in a correct manner.

Thanks for your discussion ;-)

http://bugme.osdl.org/show_bug.cgi?id=2183

Cheers
  hartwig felger

Hartwig Felger informatics

- -- 
1024D/339FD693 Hartwig Felger <hgfelger-9nAOAgdJVo4b1SvskN2V4Q@public.gmane.org>
Key fingerprint = FB2F 3EE9 345A D55B 6FF2  0EC1 F5B0 684F 339F D693
For the pulic keys, please visit my page.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)

iD8DBQFAY++Y9bBoTzOf1pMRAupCAKCSRV0jjisoVYqqn3M17tQgtCMEEwCbBISe
HeQ2UNdvEbmqSkj4v9T/w3o=
=kBdT
-----END PGP SIGNATURE-----



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click

Re: [PATCH] ACPI poweroff fixes for 2.4.26-pre2

[Pavel Machek]

Hi!

On Pá 26-03-04 06:55:47, Willy Tarreau wrote:
> On Fri, Mar 26, 2004 at 12:00:37AM +0100, Pavel Machek wrote:
> > > so acpi_system_save_state(state) does do nothing at all ?
> > > or does return nothing at all ?
> > 
> > IIRC there was patch that made acpi_system_save_state() do nothing at
> > all if !CONFIG_ACPI_SLEEP. I'm arguing thats very wrong.
> 
> I 100% agree that it's dirty to call a fake function instead of not
> calling it at all. Indeed, one of my proposed patches removed the
> dependency on CONFIG_ACPI_SLEEP through a simple replacement of
> acpi_system_save_state() with two calls to pm_send_all() which does
> not depend on CONFIG_ACPI_SLEEP. I believe it was cleaner, but it's
> not what is included in BK right now... Here it is for reference.

Yes, this looks good and needed.
								Pavel


> --- linux-2.4.26-pre3-acpi/drivers/acpi/system.c	Mon Mar 15 21:54:55 2004
> +++ linux-2.4.26-pre3-acpi-pwoff/drivers/acpi/system.c	Mon Mar 15 21:57:32 2004
> @@ -46,13 +46,6 @@
>  #include <linux/mc146818rtc.h>
>  #include <linux/irq.h>
>  #include <asm/hw_irq.h>
> -
> -acpi_status acpi_system_save_state(u32);
> -#else
> -static inline acpi_status acpi_system_save_state(u32 state)
> -{
> -	return AE_OK;
> -}
>  #endif /* !CONFIG_ACPI_SLEEP */
>  
>  #define _COMPONENT		ACPI_SYSTEM_COMPONENT
> @@ -102,7 +95,8 @@
>  		BUG();
>  	/* Some SMP machines only can poweroff in boot CPU */
>  	set_cpus_allowed(current, 1UL << cpu_logical_map(0));
> -	acpi_system_save_state(ACPI_STATE_S5);
> +	pm_send_all(PM_SAVE_STATE, (void *)3);
> +	pm_send_all(PM_SUSPEND, (void *)3);
>  	acpi_enter_sleep_state_prep(ACPI_STATE_S5);
>  	ACPI_DISABLE_IRQS();
>  	acpi_enter_sleep_state(ACPI_STATE_S5);
> 
> 

-- 
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click