public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
* [BUG] acpi_hw_[enable|disable]_non_wakeup_gpe_block
@ 2004-02-14 22:17 Alexander Malysh
       [not found] ` <200402142317.41033.a.malysh-1WJ9BOJEYl0b1SvskN2V4Q@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Alexander Malysh @ 2004-02-14 22:17 UTC (permalink / raw)
  To: acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

Hi all,

I have found a bug? in linux acpi that values within gpe_register_info are 
overwritten. Please see these values (note: only system_io gpe_register were 
dumped):

	1) system go into S1
Feb 14 23:01:08 rose kernel: 
acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef335c8) 
gpe_register_info(0xcefec548) gpe_register_info->wake_enable=0x0 
gpe_register_info->enable_address=0x1022 gpe_register_info->enable=0x11
Feb 14 23:01:08 rose kernel: 
acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef335c8) 
gpe_register_info(0xcefec564) gpe_register_info->wake_enable=0x0 
gpe_register_info->enable_address=0x1023 gpe_register_info->enable=0x5d
Feb 14 23:01:08 rose kernel: 
acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef33568) 
gpe_register_info(0xcefec4c8) gpe_register_info->wake_enable=0x0 
gpe_register_info->enable_address=0x1032 gpe_register_info->enable=0x0
Feb 14 23:01:08 rose kernel: 
acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef33568) 
gpe_register_info(0xcefec4e4) gpe_register_info->wake_enable=0x0 
gpe_register_info->enable_address=0x1033 gpe_register_info->enable=0x0

	2) system wakeup
Feb 14 23:01:08 rose kernel: 
acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef335c8) 
gpe_register_info(0xcefec548) gpe_register_info->wake_enable=0x0 
gpe_register_info->enable_address=0x1022 gpe_register_info->enable=0x0
Feb 14 23:01:08 rose kernel: 
acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef335c8) 
gpe_register_info(0xcefec564) gpe_register_info->wake_enable=0x0 
gpe_register_info->enable_address=0x1023 gpe_register_info->enable=0x0
Feb 14 23:01:08 rose kernel: 
acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef33568) 
gpe_register_info(0xcefec4c8) gpe_register_info->wake_enable=0x0 
gpe_register_info->enable_address=0x1032 gpe_register_info->enable=0x0
Feb 14 23:01:08 rose kernel: 
acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef33568) 
gpe_register_info(0xcefec4e4) gpe_register_info->wake_enable=0x0 
gpe_register_info->enable_address=0x1033 gpe_register_info->enable=0x0

As you can see, io_addr 0x1022 and 0x1023 were restored with wrong values.

any thoughts or patches to try?

-- 
Please avoid sending me Word, Excel or PowerPoint attachments.
See http://www.fsf.org/philosophy/no-word-attachments.html


-------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] Re: [BUG] acpi_hw_[enable|disable]_non_wakeup_gpe_block
       [not found] ` <200402142317.41033.a.malysh-1WJ9BOJEYl0b1SvskN2V4Q@public.gmane.org>
@ 2004-02-15 15:47   ` Alexander Malysh
       [not found]     ` <200402151647.21826.a.malysh-1WJ9BOJEYl0b1SvskN2V4Q@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Alexander Malysh @ 2004-02-15 15:47 UTC (permalink / raw)
  To: acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

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

Hi again,

after some more testing, I have found a cause for this.
We disable non wakeup gpes (and store those values) with interrupts disabled, 
but enable those with interrupts enabled which were already overwritten while 
handling of wakeup interrupt[1]. So we must restore non wakeup gpes as long 
interrupts are disabled. Attached patch should fix it.

P.S. With patch attached sleep button works after S1 resume...

[1] Debug log w/o a patch:
Feb 15 15:49:24 rose kernel: acpi_enter_sleep_state
Feb 15 15:49:24 rose kernel: 
acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef335c8) 
gpe_register_info(0xcefec548) gpe_register_info->wake_enable=0x0 
gpe_register_info->enable_address=0x1022 gpe_register_info->enable=0x11
Feb 15 15:49:24 rose kernel: 
acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef335c8) 
gpe_register_info(0xcefec564) gpe_register_info->wake_enable=0x0 
gpe_register_info->enable_address=0x1023 gpe_register_info->enable=0x5d
Feb 15 15:49:24 rose kernel: 
acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef33568) 
gpe_register_info(0xcefec4c8) gpe_register_info->wake_enable=0x0 
gpe_register_info->enable_address=0x1032 gpe_register_info->enable=0x0
Feb 15 15:49:24 rose kernel: 
acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef33568) 
gpe_register_info(0xcefec4e4) gpe_register_info->wake_enable=0x0 
gpe_register_info->enable_address=0x1033 gpe_register_info->enable=0x0
Feb 15 15:49:24 rose kernel: Back to C!
Feb 15 15:49:24 rose kernel:  evevent-0229 [20] ev_fixed_event_detect : Fixed 
Event Block: Enable 00000120 Status 00008100
Feb 15 15:49:24 rose kernel:    evgpe-0191 [20] ev_gpe_detect         : GPE 
pair: Status 0000000000001020 = 80, Enable 0000000000001022 = 00
Feb 15 15:49:24 rose kernel:    evgpe-0191 [20] ev_gpe_detect         : GPE 
pair: Status 0000000000001021 = 00, Enable 0000000000001023 = 00
Feb 15 15:49:24 rose kernel:    evgpe-0191 [20] ev_gpe_detect         : GPE 
pair: Status 0000000000001030 = F8, Enable 0000000000001032 = 00
Feb 15 15:49:24 rose kernel:    evgpe-0191 [20] ev_gpe_detect         : GPE 
pair: Status 0000000000001031 = A7, Enable 0000000000001033 = 00
Feb 15 15:49:24 rose kernel: acpi_leave_sleep_state
acpi_hw_enable_non_wakeup_gpe_block:gpe_block(0xcef335c8) 
gpe_register_info(0xcefec548) gpe_register_info->wake_enable=0x0 
gpe_register_info->enable_address=0x1022 gpe_register_info->enable=0x0
Feb 15 15:49:24 rose kernel: 
acpi_hw_enable_non_wakeup_gpe_block:gpe_block(0xcef335c8) 
gpe_register_info(0xcefec564) gpe_register_info->wake_enable=0x0 
gpe_register_info->enable_address=0x1023 gpe_register_info->enable=0x0


On Saturday 14 February 2004 23:17, Alexander Malysh wrote:
> Hi all,
>
> I have found a bug? in linux acpi that values within gpe_register_info are
> overwritten. Please see these values (note: only system_io gpe_register
> were dumped):
>
> 	1) system go into S1
> Feb 14 23:01:08 rose kernel:
> acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef335c8)
> gpe_register_info(0xcefec548) gpe_register_info->wake_enable=0x0
> gpe_register_info->enable_address=0x1022 gpe_register_info->enable=0x11
> Feb 14 23:01:08 rose kernel:
> acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef335c8)
> gpe_register_info(0xcefec564) gpe_register_info->wake_enable=0x0
> gpe_register_info->enable_address=0x1023 gpe_register_info->enable=0x5d
> Feb 14 23:01:08 rose kernel:
> acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef33568)
> gpe_register_info(0xcefec4c8) gpe_register_info->wake_enable=0x0
> gpe_register_info->enable_address=0x1032 gpe_register_info->enable=0x0
> Feb 14 23:01:08 rose kernel:
> acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef33568)
> gpe_register_info(0xcefec4e4) gpe_register_info->wake_enable=0x0
> gpe_register_info->enable_address=0x1033 gpe_register_info->enable=0x0
>
> 	2) system wakeup
> Feb 14 23:01:08 rose kernel:
> acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef335c8)
> gpe_register_info(0xcefec548) gpe_register_info->wake_enable=0x0
> gpe_register_info->enable_address=0x1022 gpe_register_info->enable=0x0
> Feb 14 23:01:08 rose kernel:
> acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef335c8)
> gpe_register_info(0xcefec564) gpe_register_info->wake_enable=0x0
> gpe_register_info->enable_address=0x1023 gpe_register_info->enable=0x0
> Feb 14 23:01:08 rose kernel:
> acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef33568)
> gpe_register_info(0xcefec4c8) gpe_register_info->wake_enable=0x0
> gpe_register_info->enable_address=0x1032 gpe_register_info->enable=0x0
> Feb 14 23:01:08 rose kernel:
> acpi_hw_disable_non_wakeup_gpe_block:gpe_block(0xcef33568)
> gpe_register_info(0xcefec4e4) gpe_register_info->wake_enable=0x0
> gpe_register_info->enable_address=0x1033 gpe_register_info->enable=0x0
>
> As you can see, io_addr 0x1022 and 0x1023 were restored with wrong values.
>
> any thoughts or patches to try?

-- 
Please avoid sending me Word, Excel or PowerPoint attachments.
See http://www.fsf.org/philosophy/no-word-attachments.html

[-- Attachment #2: acpi_hw_enable_non_wakeup_gpes.diff --]
[-- Type: text/x-diff, Size: 747 bytes --]

--- linux-2.6.2-linus/drivers/acpi/hardware/hwsleep.c~orig	2004-02-15 16:31:50.928455040 +0100
+++ linux-2.6.2-linus/drivers/acpi/hardware/hwsleep.c	2004-02-15 16:35:30.030146520 +0100
@@ -342,6 +342,12 @@
 
 	} while (!in_value);
 
+	/* Enable non_wakeup_gpes as long interrupts are disabled */
+	status = acpi_hw_enable_non_wakeup_gpes ();
+	if (ACPI_FAILURE (status)) {
+		return_ACPI_STATUS (status);
+	}
+
 	return_ACPI_STATUS (AE_OK);
 }
 
@@ -496,11 +502,6 @@
 
 	/* _WAK returns stuff - do we want to look at it? */
 
-	status = acpi_hw_enable_non_wakeup_gpes ();
-	if (ACPI_FAILURE (status)) {
-		return_ACPI_STATUS (status);
-	}
-
 	/* Enable BM arbitration */
 
 	status = acpi_set_register (ACPI_BITREG_ARB_DISABLE, 0, ACPI_MTX_LOCK);

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Re: [BUG] acpi_hw_[enable|disable]_non_wakeup_gpe_block
       [not found]     ` <200402151647.21826.a.malysh-1WJ9BOJEYl0b1SvskN2V4Q@public.gmane.org>
@ 2004-02-15 19:31       ` Karol Kozimor
  2004-02-15 20:07       ` Bruno Ducrot
  1 sibling, 0 replies; 11+ messages in thread
From: Karol Kozimor @ 2004-02-15 19:31 UTC (permalink / raw)
  To: Alexander Malysh; +Cc: acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

Thus wrote Alexander Malysh:
> Hi again,
> 
> after some more testing, I have found a cause for this.
> We disable non wakeup gpes (and store those values) with interrupts disabled, 
> but enable those with interrupts enabled which were already overwritten while 
> handling of wakeup interrupt[1]. So we must restore non wakeup gpes as long 
> interrupts are disabled. Attached patch should fix it.
> 
> P.S. With patch attached sleep button works after S1 resume...

[...]

How does this relate to bugs #1409 and #1661? I still need to apply the
#1409 patch or else my ACPI interrupts are lost after resume from S1 / S3.
Best regards,

-- 
Karol 'sziwan' Kozimor
sziwan-DETuoxkZsSqrDJvtcaxF/A@public.gmane.org


-------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Re: [BUG] acpi_hw_[enable|disable]_non_wakeup_gpe_block
       [not found]     ` <200402151647.21826.a.malysh-1WJ9BOJEYl0b1SvskN2V4Q@public.gmane.org>
  2004-02-15 19:31       ` Karol Kozimor
@ 2004-02-15 20:07       ` Bruno Ducrot
       [not found]         ` <20040215200724.GN13262-kk6yZipjEM5g9hUCZPvPmw@public.gmane.org>
  1 sibling, 1 reply; 11+ messages in thread
From: Bruno Ducrot @ 2004-02-15 20:07 UTC (permalink / raw)
  To: Alexander Malysh; +Cc: acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Sun, Feb 15, 2004 at 04:47:19PM +0100, Alexander Malysh wrote:
> Hi again,
> 
> after some more testing, I have found a cause for this.
> We disable non wakeup gpes (and store those values) with interrupts disabled, 
> but enable those with interrupts enabled which were already overwritten while 
> handling of wakeup interrupt[1]. So we must restore non wakeup gpes as long 
> interrupts are disabled. Attached patch should fix it.
> 

> --- linux-2.6.2-linus/drivers/acpi/hardware/hwsleep.c~orig	2004-02-15 16:31:50.928455040 +0100
> +++ linux-2.6.2-linus/drivers/acpi/hardware/hwsleep.c	2004-02-15 16:35:30.030146520 +0100
> @@ -342,6 +342,12 @@
>  
>  	} while (!in_value);
>  
> +	/* Enable non_wakeup_gpes as long interrupts are disabled */
> +	status = acpi_hw_enable_non_wakeup_gpes ();
> +	if (ACPI_FAILURE (status)) {
> +		return_ACPI_STATUS (status);
> +	}
> +
>  	return_ACPI_STATUS (AE_OK);

First, this path is *never reached* for S3 and S4, so you break somehow S3
and S4 in fact...

Second, the path for which you remove the enable gpes is supposed to be
run with interrupt *dissabled*.  If that is not the case, then it is the
real bug in fact.

-- 
Bruno Ducrot

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


-------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Re: [BUG] acpi_hw_[enable|disable]_non_wakeup_gpe_block
       [not found]         ` <20040215200724.GN13262-kk6yZipjEM5g9hUCZPvPmw@public.gmane.org>
@ 2004-02-15 21:00           ` Alexander Malysh
       [not found]             ` <200402152200.08010.a.malysh-1WJ9BOJEYl0b1SvskN2V4Q@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Alexander Malysh @ 2004-02-15 21:00 UTC (permalink / raw)
  To: Bruno Ducrot; +Cc: acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Sunday 15 February 2004 21:07, you wrote:
> On Sun, Feb 15, 2004 at 04:47:19PM +0100, Alexander Malysh wrote:
> > Hi again,
> >
> > after some more testing, I have found a cause for this.
> > We disable non wakeup gpes (and store those values) with interrupts
> > disabled, but enable those with interrupts enabled which were already
> > overwritten while handling of wakeup interrupt[1]. So we must restore non
> > wakeup gpes as long interrupts are disabled. Attached patch should fix
> > it.
> >
> >
> > --- linux-2.6.2-linus/drivers/acpi/hardware/hwsleep.c~orig	2004-02-15
> > 16:31:50.928455040 +0100 +++
> > linux-2.6.2-linus/drivers/acpi/hardware/hwsleep.c	2004-02-15
> > 16:35:30.030146520 +0100 @@ -342,6 +342,12 @@
> >
> >  	} while (!in_value);
> >
> > +	/* Enable non_wakeup_gpes as long interrupts are disabled */
> > +	status = acpi_hw_enable_non_wakeup_gpes ();
> > +	if (ACPI_FAILURE (status)) {
> > +		return_ACPI_STATUS (status);
> > +	}
> > +
> >  	return_ACPI_STATUS (AE_OK);
>
> First, this path is *never reached* for S3 and S4, so you break somehow S3
> and S4 in fact...

hmm, where do we jump after wakeup from S3/S4 ?

>
> Second, the path for which you remove the enable gpes is supposed to be
> run with interrupt *dissabled*.  If that is not the case, then it is the
> real bug in fact.

as you can see from log (or in drivers/acpi/sleep/main.c) 
acpi_leave_sleep_state called with interrupts enabled.
and you are right: acpi_leave_sleep_state _must_ be called with interrupts 
disabled, so no wonder why sleep/resume fail on many laptops :(



-------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] acpi_leave_sleep_state (was: [PATCH] Re: [BUG] acpi_hw_[enable|disable]_non_wakeup_gpe_block)
       [not found]             ` <200402152200.08010.a.malysh-1WJ9BOJEYl0b1SvskN2V4Q@public.gmane.org>
@ 2004-02-15 22:12               ` Alexander Malysh
       [not found]                 ` <200402152312.53837.a.malysh-1WJ9BOJEYl0b1SvskN2V4Q@public.gmane.org>
  2004-02-17 10:02               ` [PATCH] Re: [BUG] acpi_hw_[enable|disable]_non_wakeup_gpe_block Bruno Ducrot
  1 sibling, 1 reply; 11+ messages in thread
From: Alexander Malysh @ 2004-02-15 22:12 UTC (permalink / raw)
  To: acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f; +Cc: Bruno Ducrot, Karol Kozimor

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

Hi again,

what are you think about attached patch?

Note: at least on my laptop gpes must be enabled before '_WAK' method 
execution, otherwise after '_WAK' was executed interrupts are enabled and gpe 
register were restored with wrong values...

@Karol: can you please get this patch a try?

On Sunday 15 February 2004 22:00, Alexander Malysh wrote:
> On Sunday 15 February 2004 21:07, you wrote:
> > On Sun, Feb 15, 2004 at 04:47:19PM +0100, Alexander Malysh wrote:
> > > Hi again,
> > >
> > > after some more testing, I have found a cause for this.
> > > We disable non wakeup gpes (and store those values) with interrupts
> > > disabled, but enable those with interrupts enabled which were already
> > > overwritten while handling of wakeup interrupt[1]. So we must restore
> > > non wakeup gpes as long interrupts are disabled. Attached patch should
> > > fix it.
> > >
> > >
> > > --- linux-2.6.2-linus/drivers/acpi/hardware/hwsleep.c~orig	2004-02-15
> > > 16:31:50.928455040 +0100 +++
> > > linux-2.6.2-linus/drivers/acpi/hardware/hwsleep.c	2004-02-15
> > > 16:35:30.030146520 +0100 @@ -342,6 +342,12 @@
> > >
> > >  	} while (!in_value);
> > >
> > > +	/* Enable non_wakeup_gpes as long interrupts are disabled */
> > > +	status = acpi_hw_enable_non_wakeup_gpes ();
> > > +	if (ACPI_FAILURE (status)) {
> > > +		return_ACPI_STATUS (status);
> > > +	}
> > > +
> > >  	return_ACPI_STATUS (AE_OK);
> >
> > First, this path is *never reached* for S3 and S4, so you break somehow
> > S3 and S4 in fact...
>
> hmm, where do we jump after wakeup from S3/S4 ?
>
> > Second, the path for which you remove the enable gpes is supposed to be
> > run with interrupt *dissabled*.  If that is not the case, then it is the
> > real bug in fact.
>
> as you can see from log (or in drivers/acpi/sleep/main.c)
> acpi_leave_sleep_state called with interrupts enabled.
> and you are right: acpi_leave_sleep_state _must_ be called with interrupts
> disabled, so no wonder why sleep/resume fail on many laptops :(
>
>
>
> -------------------------------------------------------
> 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
> _______________________________________________
> Acpi-devel mailing list
> Acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/acpi-devel


[-- Attachment #2: acpi_leave_sleep_state.diff --]
[-- Type: text/x-diff, Size: 1608 bytes --]

--- linux-2.6.2-linus/drivers/acpi/hardware/hwsleep.c~orig	2004-02-15 16:31:50.000000000 +0100
+++ linux-2.6.2-linus/drivers/acpi/hardware/hwsleep.c	2004-02-15 23:03:41.522301504 +0100
@@ -477,6 +477,11 @@
 
 	/* Ignore any errors from these methods */
 
+	status = acpi_hw_enable_non_wakeup_gpes ();
+	if (ACPI_FAILURE (status)) {
+		ACPI_REPORT_ERROR(("Could not enable non wakeup gpes, %s\n", acpi_format_exception (status)));
+	}
+
 	arg.integer.value = 0;
 	status = acpi_evaluate_object (NULL, "\\_SI._SST", &arg_list, NULL);
 	if (ACPI_FAILURE (status) && status != AE_NOT_FOUND) {
@@ -496,11 +501,6 @@
 
 	/* _WAK returns stuff - do we want to look at it? */
 
-	status = acpi_hw_enable_non_wakeup_gpes ();
-	if (ACPI_FAILURE (status)) {
-		return_ACPI_STATUS (status);
-	}
-
 	/* Enable BM arbitration */
 
 	status = acpi_set_register (ACPI_BITREG_ARB_DISABLE, 0, ACPI_MTX_LOCK);
--- linux-2.6.2-linus/drivers/acpi/sleep/main.c~orig	2004-02-15 23:03:49.543082160 +0100
+++ linux-2.6.2-linus/drivers/acpi/sleep/main.c	2004-02-15 23:04:14.258324872 +0100
@@ -107,7 +107,6 @@
 	default:
 		return -EINVAL;
 	}
-	local_irq_restore(flags);
 	printk(KERN_DEBUG "Back to C!\n");
 
 	/* restore processor state
@@ -118,6 +117,8 @@
 	if (state > PM_SUSPEND_STANDBY)
 		acpi_restore_state_mem();
 
+	acpi_leave_sleep_state(state);
+	local_irq_restore(flags);
 
 	return ACPI_SUCCESS(status) ? 0 : -EFAULT;
 }
@@ -133,8 +134,6 @@
 
 static int acpi_pm_finish(u32 state)
 {
-	acpi_leave_sleep_state(state);
-
 	/* reset firmware waking vector */
 	acpi_set_firmware_waking_vector((acpi_physical_address) 0);
 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] acpi_leave_sleep_state (was: [PATCH] Re: [BUG] acpi_hw_[enable|disable]_non_wakeup_gpe_block)
       [not found]                 ` <200402152312.53837.a.malysh-1WJ9BOJEYl0b1SvskN2V4Q@public.gmane.org>
@ 2004-02-17  0:11                   ` Karol Kozimor
  2004-02-17  1:31                   ` Some results from Alexander's acpi_leave_sleep_state patch Huw Rogers
  1 sibling, 0 replies; 11+ messages in thread
From: Karol Kozimor @ 2004-02-17  0:11 UTC (permalink / raw)
  To: Alexander Malysh
  Cc: acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Bruno Ducrot

Thus wrote Alexander Malysh:
> Hi again,
> 
> what are you think about attached patch?
> 
> Note: at least on my laptop gpes must be enabled before '_WAK' method 
> execution, otherwise after '_WAK' was executed interrupts are enabled and gpe 
> register were restored with wrong values...
> 
> @Karol: can you please get this patch a try?

It does help, ACPI interrupts are fine after resume. Overall it works the 
same as the one for bug#1409, but the approach is better.

Len, please apply, possibly even for 2.6.3?

Best regards,

-- 
Karol 'sziwan' Kozimor
sziwan-DETuoxkZsSqrDJvtcaxF/A@public.gmane.org


-------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Some results from Alexander's acpi_leave_sleep_state patch
       [not found]                 ` <200402152312.53837.a.malysh-1WJ9BOJEYl0b1SvskN2V4Q@public.gmane.org>
  2004-02-17  0:11                   ` Karol Kozimor
@ 2004-02-17  1:31                   ` Huw Rogers
       [not found]                     ` <20040216182451.1BC5.COUNT0-tC47gz4GrgtWk0Htik3J/w@public.gmane.org>
  1 sibling, 1 reply; 11+ messages in thread
From: Huw Rogers @ 2004-02-17  1:31 UTC (permalink / raw)
  To: Alexander Malysh, acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Bruno Ducrot

Alexander's patch had some interesting effects on my S3 resume problems
with my SiS 648FX/963 / P4 / Radeon 9600 laptop
(http://bugme.osdl.org/show_bug.cgi?id=2048). Although the system was
still locked up, the display was no longer inoperable to the point that
the battery and AC had to be removed to get it back, and I could use a
regular power cycle to recover. A mild improvement.

S4 (CONFIG_PM_DISK) resume booted (wrongly) via POST -> boot loader, and
Linux attempted to repeat the suspend during it's boot! Previously the
S4 resume did go straight to Linux resume as it should. From the console
log (unfortunately not written to disk for posterity for either S3 or S4),
S4 seems confused about whether it's suspending or resuming during both
suspension and resume (I see lines mentioning resume activity during
suspend and vice versa).

This is a SiS 648FX/963 chipset, HT P4, Radeon M-10 (9600 Mobile) laptop.

-Huw

Alexander Malysh wrote:
> what are you think about attached patch?
> 
> Note: at least on my laptop gpes must be enabled before '_WAK' method 
> execution, otherwise after '_WAK' was executed interrupts are enabled and gpe 
> register were restored with wrong values...
> 
> @Karol: can you please get this patch a try?
> 
> On Sunday 15 February 2004 22:00, Alexander Malysh wrote:
> > On Sunday 15 February 2004 21:07, you wrote:
> > > On Sun, Feb 15, 2004 at 04:47:19PM +0100, Alexander Malysh wrote:
> > > > Hi again,
> > > >
> > > > after some more testing, I have found a cause for this.
> > > > We disable non wakeup gpes (and store those values) with interrupts
> > > > disabled, but enable those with interrupts enabled which were already
> > > > overwritten while handling of wakeup interrupt[1]. So we must restore
> > > > non wakeup gpes as long interrupts are disabled. Attached patch should
> > > > fix it.
> > > >
> > > >
> > > > --- linux-2.6.2-linus/drivers/acpi/hardware/hwsleep.c~orig	2004-02-15
> > > > 16:31:50.928455040 +0100 +++
> > > > linux-2.6.2-linus/drivers/acpi/hardware/hwsleep.c	2004-02-15
> > > > 16:35:30.030146520 +0100 @@ -342,6 +342,12 @@
> > > >
> > > >  	} while (!in_value);
> > > >
> > > > +	/* Enable non_wakeup_gpes as long interrupts are disabled */
> > > > +	status = acpi_hw_enable_non_wakeup_gpes ();
> > > > +	if (ACPI_FAILURE (status)) {
> > > > +		return_ACPI_STATUS (status);
> > > > +	}
> > > > +
> > > >  	return_ACPI_STATUS (AE_OK);
> > >
> > > First, this path is *never reached* for S3 and S4, so you break somehow
> > > S3 and S4 in fact...
> >
> > hmm, where do we jump after wakeup from S3/S4 ?
> >
> > > Second, the path for which you remove the enable gpes is supposed to be
> > > run with interrupt *dissabled*.  If that is not the case, then it is the
> > > real bug in fact.
> >
> > as you can see from log (or in drivers/acpi/sleep/main.c)
> > acpi_leave_sleep_state called with interrupts enabled.
> > and you are right: acpi_leave_sleep_state _must_ be called with interrupts
> > disabled, so no wonder why sleep/resume fail on many laptops :(


-- 
Huw Rogers <count0-tC47gz4GrgtWk0Htik3J/w@public.gmane.org>



-------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Some results from Alexander's acpi_leave_sleep_state patch
       [not found]                     ` <20040216182451.1BC5.COUNT0-tC47gz4GrgtWk0Htik3J/w@public.gmane.org>
@ 2004-02-17  8:06                       ` Karol Kozimor
  2004-02-25 17:34                       ` Pavel Machek
  1 sibling, 0 replies; 11+ messages in thread
From: Karol Kozimor @ 2004-02-17  8:06 UTC (permalink / raw)
  To: Huw Rogers
  Cc: Alexander Malysh, acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Bruno Ducrot

Thus wrote Huw Rogers:
> Alexander's patch had some interesting effects on my S3 resume problems
> with my SiS 648FX/963 / P4 / Radeon 9600 laptop
> (http://bugme.osdl.org/show_bug.cgi?id=2048). Although the system was
> still locked up, the display was no longer inoperable to the point that
> the battery and AC had to be removed to get it back, and I could use a
> regular power cycle to recover. A mild improvement.

You might want to try 2.6.3-rc4 with new radeonfb for better suspend /
resume handling.
Best regards,

-- 
Karol 'sziwan' Kozimor
sziwan-DETuoxkZsSqrDJvtcaxF/A@public.gmane.org


-------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] Re: [BUG] acpi_hw_[enable|disable]_non_wakeup_gpe_block
       [not found]             ` <200402152200.08010.a.malysh-1WJ9BOJEYl0b1SvskN2V4Q@public.gmane.org>
  2004-02-15 22:12               ` [PATCH] acpi_leave_sleep_state (was: [PATCH] Re: [BUG] acpi_hw_[enable|disable]_non_wakeup_gpe_block) Alexander Malysh
@ 2004-02-17 10:02               ` Bruno Ducrot
  1 sibling, 0 replies; 11+ messages in thread
From: Bruno Ducrot @ 2004-02-17 10:02 UTC (permalink / raw)
  To: Alexander Malysh; +Cc: acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Sun, Feb 15, 2004 at 10:00:06PM +0100, Alexander Malysh wrote:
> On Sunday 15 February 2004 21:07, you wrote:
> > On Sun, Feb 15, 2004 at 04:47:19PM +0100, Alexander Malysh wrote:
> > > Hi again,
> > >
> > > after some more testing, I have found a cause for this.
> > > We disable non wakeup gpes (and store those values) with interrupts
> > > disabled, but enable those with interrupts enabled which were already
> > > overwritten while handling of wakeup interrupt[1]. So we must restore non
> > > wakeup gpes as long interrupts are disabled. Attached patch should fix
> > > it.
> > >
> > >
> > > --- linux-2.6.2-linus/drivers/acpi/hardware/hwsleep.c~orig	2004-02-15
> > > 16:31:50.928455040 +0100 +++
> > > linux-2.6.2-linus/drivers/acpi/hardware/hwsleep.c	2004-02-15
> > > 16:35:30.030146520 +0100 @@ -342,6 +342,12 @@
> > >
> > >  	} while (!in_value);
> > >
> > > +	/* Enable non_wakeup_gpes as long interrupts are disabled */
> > > +	status = acpi_hw_enable_non_wakeup_gpes ();
> > > +	if (ACPI_FAILURE (status)) {
> > > +		return_ACPI_STATUS (status);
> > > +	}
> > > +
> > >  	return_ACPI_STATUS (AE_OK);
> >
> > First, this path is *never reached* for S3 and S4, so you break somehow S3
> > and S4 in fact...
> 
> hmm, where do we jump after wakeup from S3/S4 ?
> 
> >
> > Second, the path for which you remove the enable gpes is supposed to be
> > run with interrupt *dissabled*.  If that is not the case, then it is the
> > real bug in fact.
> 
> as you can see from log (or in drivers/acpi/sleep/main.c) 
> acpi_leave_sleep_state called with interrupts enabled.
> and you are right: acpi_leave_sleep_state _must_ be called with interrupts 
> disabled, so no wonder why sleep/resume fail on many laptops :(

Actually, I don't know why it worked.  Unfortunately, someone decided to
create a sleep directory, just because he was thinking that it is
'better', renamed things (like main.c), removed copyright holders to
some file, and break at the same time at least S4Bios, and letting
others to fix that mess.

-- 
Bruno Ducrot

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


-------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Some results from Alexander's acpi_leave_sleep_state patch
       [not found]                     ` <20040216182451.1BC5.COUNT0-tC47gz4GrgtWk0Htik3J/w@public.gmane.org>
  2004-02-17  8:06                       ` Karol Kozimor
@ 2004-02-25 17:34                       ` Pavel Machek
  1 sibling, 0 replies; 11+ messages in thread
From: Pavel Machek @ 2004-02-25 17:34 UTC (permalink / raw)
  To: Huw Rogers
  Cc: Alexander Malysh, acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Bruno Ducrot

Hi!

> S4 (CONFIG_PM_DISK) resume booted (wrongly) via POST -> boot loader, and
> Linux attempted to repeat the suspend during it's boot! Previously
> the

Can you try with swsusp, instead?

> S4 resume did go straight to Linux resume as it should. From the console
> log (unfortunately not written to disk for posterity for either S3 or S4),
> S4 seems confused about whether it's suspending or resuming during both
> suspension and resume (I see lines mentioning resume activity during
> suspend and vice versa).

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


-------------------------------------------------------
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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2004-02-25 17:34 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-02-14 22:17 [BUG] acpi_hw_[enable|disable]_non_wakeup_gpe_block Alexander Malysh
     [not found] ` <200402142317.41033.a.malysh-1WJ9BOJEYl0b1SvskN2V4Q@public.gmane.org>
2004-02-15 15:47   ` [PATCH] " Alexander Malysh
     [not found]     ` <200402151647.21826.a.malysh-1WJ9BOJEYl0b1SvskN2V4Q@public.gmane.org>
2004-02-15 19:31       ` Karol Kozimor
2004-02-15 20:07       ` Bruno Ducrot
     [not found]         ` <20040215200724.GN13262-kk6yZipjEM5g9hUCZPvPmw@public.gmane.org>
2004-02-15 21:00           ` Alexander Malysh
     [not found]             ` <200402152200.08010.a.malysh-1WJ9BOJEYl0b1SvskN2V4Q@public.gmane.org>
2004-02-15 22:12               ` [PATCH] acpi_leave_sleep_state (was: [PATCH] Re: [BUG] acpi_hw_[enable|disable]_non_wakeup_gpe_block) Alexander Malysh
     [not found]                 ` <200402152312.53837.a.malysh-1WJ9BOJEYl0b1SvskN2V4Q@public.gmane.org>
2004-02-17  0:11                   ` Karol Kozimor
2004-02-17  1:31                   ` Some results from Alexander's acpi_leave_sleep_state patch Huw Rogers
     [not found]                     ` <20040216182451.1BC5.COUNT0-tC47gz4GrgtWk0Htik3J/w@public.gmane.org>
2004-02-17  8:06                       ` Karol Kozimor
2004-02-25 17:34                       ` Pavel Machek
2004-02-17 10:02               ` [PATCH] Re: [BUG] acpi_hw_[enable|disable]_non_wakeup_gpe_block Bruno Ducrot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox