Hi Rafael, Matthew,
my laptop changed behavior during poweroff recently (after upgrading
from .33 to .34-rc1). The system seems to be powered off (status display
is off) at first glance but when I close the lid then I can hear a noise
which sounds like HDD parking and when I open lid again it starts
booting without poweroff button (same like when I suspend to RAM).
I have bisected that down to the following commit:
commit 9630bdd9b15d2f489c646d8bc04b60e53eb5ec78
Author: Rafael J. Wysocki <[email protected]>
Date: Wed Feb 17 23:41:07 2010 +0100
ACPI: Use GPE reference counting to support shared GPEs
ACPI GPEs may map to multiple devices. The current GPE interface
only provides a mechanism for enabling and disabling GPEs, making
it difficult to change the state of GPEs at runtime without extensive
cooperation between devices.
Add an API to allow devices to indicate whether or not they want
their device's GPE to be enabled for both runtime and wakeup events.
Remove the old GPE type handling entirely, which gets rid of various
quirks, like the implicit disabling with GPE type setting. This
requires a small amount of rework in order to ensure that non-wake
GPEs are enabled by default to preserve existing behaviour.
Based on patches from Matthew Garrett <[email protected]>.
Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
Signed-off-by: Jesse Barnes <[email protected]>
When I revert this commit, the system behaves like before. I am not sure
which kind of information might be useful so I have attached only config
file for now. The laptop is Futjitsu Siemens Lifebook S Series.
Let me know what else might help for further investigation.
Thanks
and
best regards
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
On Thu, Apr 01, 2010 at 03:39:23PM +0200, Michal Hocko wrote:
> Hi Rafael, Matthew,
> my laptop changed behavior during poweroff recently (after upgrading
> from .33 to .34-rc1). The system seems to be powered off (status display
> is off) at first glance but when I close the lid then I can hear a noise
> which sounds like HDD parking and when I open lid again it starts
> booting without poweroff button (same like when I suspend to RAM).
Hm. At a guess, we're somehow managing to leave the lid GPE enabled at
poweroff. Can you send the output of the acpidump command?
--
Matthew Garrett | [email protected]
On Thu 01-04-10 14:45:35, Matthew Garrett wrote:
> On Thu, Apr 01, 2010 at 03:39:23PM +0200, Michal Hocko wrote:
> > Hi Rafael, Matthew,
> > my laptop changed behavior during poweroff recently (after upgrading
> > from .33 to .34-rc1). The system seems to be powered off (status display
> > is off) at first glance but when I close the lid then I can hear a noise
> > which sounds like HDD parking and when I open lid again it starts
> > booting without poweroff button (same like when I suspend to RAM).
>
> Hm. At a guess, we're somehow managing to leave the lid GPE enabled at
> poweroff. Can you send the output of the acpidump command?
Sure, see attached.
Thanks!
>
> --
> Matthew Garrett | [email protected]
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
On Thursday 01 April 2010, Michal Hocko wrote:
> On Thu 01-04-10 14:45:35, Matthew Garrett wrote:
> > On Thu, Apr 01, 2010 at 03:39:23PM +0200, Michal Hocko wrote:
> > > Hi Rafael, Matthew,
> > > my laptop changed behavior during poweroff recently (after upgrading
> > > from .33 to .34-rc1). The system seems to be powered off (status display
> > > is off) at first glance but when I close the lid then I can hear a noise
> > > which sounds like HDD parking and when I open lid again it starts
> > > booting without poweroff button (same like when I suspend to RAM).
> >
> > Hm. At a guess, we're somehow managing to leave the lid GPE enabled at
> > poweroff. Can you send the output of the acpidump command?
>
> Sure, see attached.
Thanks.
Please also send the contents of /proc/acpi/wakeup.
Rafael
Sorry for late reply, but I was AFK during holiday.
On Fri 02-04-10 21:31:47, Rafael J. Wysocki wrote:
> On Thursday 01 April 2010, Michal Hocko wrote:
> > On Thu 01-04-10 14:45:35, Matthew Garrett wrote:
> > > On Thu, Apr 01, 2010 at 03:39:23PM +0200, Michal Hocko wrote:
> > > > Hi Rafael, Matthew,
> > > > my laptop changed behavior during poweroff recently (after upgrading
> > > > from .33 to .34-rc1). The system seems to be powered off (status display
> > > > is off) at first glance but when I close the lid then I can hear a noise
> > > > which sounds like HDD parking and when I open lid again it starts
> > > > booting without poweroff button (same like when I suspend to RAM).
> > >
> > > Hm. At a guess, we're somehow managing to leave the lid GPE enabled at
> > > poweroff. Can you send the output of the acpidump command?
> >
> > Sure, see attached.
>
> Thanks.
>
> Please also send the contents of /proc/acpi/wakeup.
See attached. I have checked both kernels (with and without reverted
9630bdd) and the output looks pretty much same. 2.6.34-rc3, however,
looks different a bit, so I am attaching it as well.
>
> Rafael
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
On Wednesday 07 April 2010, Michal Hocko wrote:
> Sorry for late reply, but I was AFK during holiday.
>
> On Fri 02-04-10 21:31:47, Rafael J. Wysocki wrote:
> > On Thursday 01 April 2010, Michal Hocko wrote:
> > > On Thu 01-04-10 14:45:35, Matthew Garrett wrote:
> > > > On Thu, Apr 01, 2010 at 03:39:23PM +0200, Michal Hocko wrote:
> > > > > Hi Rafael, Matthew,
> > > > > my laptop changed behavior during poweroff recently (after upgrading
> > > > > from .33 to .34-rc1). The system seems to be powered off (status display
> > > > > is off) at first glance but when I close the lid then I can hear a noise
> > > > > which sounds like HDD parking and when I open lid again it starts
> > > > > booting without poweroff button (same like when I suspend to RAM).
> > > >
> > > > Hm. At a guess, we're somehow managing to leave the lid GPE enabled at
> > > > poweroff. Can you send the output of the acpidump command?
> > >
> > > Sure, see attached.
> >
> > Thanks.
> >
> > Please also send the contents of /proc/acpi/wakeup.
>
> See attached. I have checked both kernels (with and without reverted
> 9630bdd) and the output looks pretty much same. 2.6.34-rc3, however,
> looks different a bit, so I am attaching it as well.
Well, I fail to see the possible failure scenario, so let's first try a blind
shot.
Please check if this patch helps:
https://patchwork.kernel.org/patch/86238/
Rafael
On Wed 07-04-10 21:49:23, Rafael J. Wysocki wrote:
> On Wednesday 07 April 2010, Michal Hocko wrote:
> > Sorry for late reply, but I was AFK during holiday.
> >
> > On Fri 02-04-10 21:31:47, Rafael J. Wysocki wrote:
> > > On Thursday 01 April 2010, Michal Hocko wrote:
> > > > On Thu 01-04-10 14:45:35, Matthew Garrett wrote:
> > > > > On Thu, Apr 01, 2010 at 03:39:23PM +0200, Michal Hocko wrote:
> > > > > > Hi Rafael, Matthew,
> > > > > > my laptop changed behavior during poweroff recently (after upgrading
> > > > > > from .33 to .34-rc1). The system seems to be powered off (status display
> > > > > > is off) at first glance but when I close the lid then I can hear a noise
> > > > > > which sounds like HDD parking and when I open lid again it starts
> > > > > > booting without poweroff button (same like when I suspend to RAM).
> > > > >
> > > > > Hm. At a guess, we're somehow managing to leave the lid GPE enabled at
> > > > > poweroff. Can you send the output of the acpidump command?
> > > >
> > > > Sure, see attached.
> > >
> > > Thanks.
> > >
> > > Please also send the contents of /proc/acpi/wakeup.
> >
> > See attached. I have checked both kernels (with and without reverted
> > 9630bdd) and the output looks pretty much same. 2.6.34-rc3, however,
> > looks different a bit, so I am attaching it as well.
>
> Well, I fail to see the possible failure scenario, so let's first try a blind
> shot.
>
> Please check if this patch helps:
> https://patchwork.kernel.org/patch/86238/
No change, sorry. I have tried that on top of 0fdf867 which is not the
most recent one (does it make sense to retest on the most recent one
f5284e7?)
>
> Rafael
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
On Thursday 08 April 2010, Michal Hocko wrote:
> On Wed 07-04-10 21:49:23, Rafael J. Wysocki wrote:
> > On Wednesday 07 April 2010, Michal Hocko wrote:
> > > Sorry for late reply, but I was AFK during holiday.
> > >
> > > On Fri 02-04-10 21:31:47, Rafael J. Wysocki wrote:
> > > > On Thursday 01 April 2010, Michal Hocko wrote:
> > > > > On Thu 01-04-10 14:45:35, Matthew Garrett wrote:
> > > > > > On Thu, Apr 01, 2010 at 03:39:23PM +0200, Michal Hocko wrote:
> > > > > > > Hi Rafael, Matthew,
> > > > > > > my laptop changed behavior during poweroff recently (after upgrading
> > > > > > > from .33 to .34-rc1). The system seems to be powered off (status display
> > > > > > > is off) at first glance but when I close the lid then I can hear a noise
> > > > > > > which sounds like HDD parking and when I open lid again it starts
> > > > > > > booting without poweroff button (same like when I suspend to RAM).
> > > > > >
> > > > > > Hm. At a guess, we're somehow managing to leave the lid GPE enabled at
> > > > > > poweroff. Can you send the output of the acpidump command?
> > > > >
> > > > > Sure, see attached.
> > > >
> > > > Thanks.
> > > >
> > > > Please also send the contents of /proc/acpi/wakeup.
> > >
> > > See attached. I have checked both kernels (with and without reverted
> > > 9630bdd) and the output looks pretty much same. 2.6.34-rc3, however,
> > > looks different a bit, so I am attaching it as well.
> >
> > Well, I fail to see the possible failure scenario, so let's first try a blind
> > shot.
> >
> > Please check if this patch helps:
> > https://patchwork.kernel.org/patch/86238/
>
> No change, sorry. I have tried that on top of 0fdf867 which is not the
> most recent one (does it make sense to retest on the most recent one
> f5284e7?)
There were no other changes in that area I know of, but the current git
contains the above patch, so I'd recommend to use it for further testing.
I'll try to prepare a debug patch for this issue.
Rafael
Rafael,
I have the exact same "lid close = powering back on" regression on a
Fujitsu-Siemens Lifebook S6420. Totally missed this thread before, but
I'm glad that I'm not alone with this one.
Happy to test any patches you may come up with; will an acpidump do for
you or do you need additional data?
Regards,
--
Tony Vroon
UNIX systems administrator
London Internet Exchange Ltd, Trinity Court, Trinity Street,
Peterborough, PE1 1DA
Registered in England number 3137929
E-Mail: [email protected]
On Friday 09 April 2010, Tony Vroon wrote:
> Rafael,
>
> I have the exact same "lid close = powering back on" regression on a
> Fujitsu-Siemens Lifebook S6420. Totally missed this thread before, but
> I'm glad that I'm not alone with this one.
> Happy to test any patches you may come up with; will an acpidump do for
> you or do you need additional data?
I need to figure out what actually happens and I'll need you to try a debug
patch, but it's not ready yet.
Rafael
On Thursday 08 April 2010, Rafael J. Wysocki wrote:
> On Thursday 08 April 2010, Michal Hocko wrote:
> > On Wed 07-04-10 21:49:23, Rafael J. Wysocki wrote:
> > > On Wednesday 07 April 2010, Michal Hocko wrote:
> > > > Sorry for late reply, but I was AFK during holiday.
> > > >
> > > > On Fri 02-04-10 21:31:47, Rafael J. Wysocki wrote:
> > > > > On Thursday 01 April 2010, Michal Hocko wrote:
> > > > > > On Thu 01-04-10 14:45:35, Matthew Garrett wrote:
> > > > > > > On Thu, Apr 01, 2010 at 03:39:23PM +0200, Michal Hocko wrote:
> > > > > > > > Hi Rafael, Matthew,
> > > > > > > > my laptop changed behavior during poweroff recently (after upgrading
> > > > > > > > from .33 to .34-rc1). The system seems to be powered off (status display
> > > > > > > > is off) at first glance but when I close the lid then I can hear a noise
> > > > > > > > which sounds like HDD parking and when I open lid again it starts
> > > > > > > > booting without poweroff button (same like when I suspend to RAM).
> > > > > > >
> > > > > > > Hm. At a guess, we're somehow managing to leave the lid GPE enabled at
> > > > > > > poweroff. Can you send the output of the acpidump command?
> > > > > >
> > > > > > Sure, see attached.
> > > > >
> > > > > Thanks.
> > > > >
> > > > > Please also send the contents of /proc/acpi/wakeup.
> > > >
> > > > See attached. I have checked both kernels (with and without reverted
> > > > 9630bdd) and the output looks pretty much same. 2.6.34-rc3, however,
> > > > looks different a bit, so I am attaching it as well.
> > >
> > > Well, I fail to see the possible failure scenario, so let's first try a blind
> > > shot.
> > >
> > > Please check if this patch helps:
> > > https://patchwork.kernel.org/patch/86238/
> >
> > No change, sorry. I have tried that on top of 0fdf867 which is not the
> > most recent one (does it make sense to retest on the most recent one
> > f5284e7?)
>
> There were no other changes in that area I know of, but the current git
> contains the above patch, so I'd recommend to use it for further testing.
>
> I'll try to prepare a debug patch for this issue.
Please check if the patch below changes anything.
Rafael
---
drivers/acpi/wakeup.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Index: linux-2.6/drivers/acpi/wakeup.c
===================================================================
--- linux-2.6.orig/drivers/acpi/wakeup.c
+++ linux-2.6/drivers/acpi/wakeup.c
@@ -71,9 +71,9 @@ void acpi_enable_wakeup_device(u8 sleep_
|| sleep_state > (u32) dev->wakeup.sleep_state)
continue;
- /* The wake-up power should have been enabled already. */
+ /* The wake-up power should have been enabled already.
acpi_set_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
- ACPI_GPE_ENABLE);
+ ACPI_GPE_ENABLE);*/
}
}
On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> Please check if the patch below changes anything.
> drivers/acpi/wakeup.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
That didn't change the behaviour for me, sorry.
(I made sure to go through a full power down session before trying the
patched kernel)
Regards,
--
Tony Vroon
UNIX systems administrator
London Internet Exchange Ltd, Trinity Court, Trinity Street,
Peterborough, PE1 1DA
Registered in England number 3137929
E-Mail: [email protected]
On Friday 09 April 2010, Tony Vroon wrote:
> On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> > Please check if the patch below changes anything.
> > drivers/acpi/wakeup.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
>
> That didn't change the behaviour for me, sorry.
Well, I would be sorry if it did, because the patch removed some useful code. :-)
> (I made sure to go through a full power down session before trying the
> patched kernel)
Thanks for testing. So it looks like we don't disable the GPE during power off.
I'll try to figure out what's going on, please stay tuned.
Rafael
On Saturday 10 April 2010, Rafael J. Wysocki wrote:
> On Friday 09 April 2010, Tony Vroon wrote:
> > On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> > > Please check if the patch below changes anything.
> > > drivers/acpi/wakeup.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > That didn't change the behaviour for me, sorry.
>
> Well, I would be sorry if it did, because the patch removed some useful code. :-)
>
> > (I made sure to go through a full power down session before trying the
> > patched kernel)
>
> Thanks for testing. So it looks like we don't disable the GPE during power off.
>
> I'll try to figure out what's going on, please stay tuned.
Can you please check if the patch below changes the behavior?
Rafael
---
drivers/acpi/wakeup.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/acpi/wakeup.c
===================================================================
--- linux-2.6.orig/drivers/acpi/wakeup.c
+++ linux-2.6/drivers/acpi/wakeup.c
@@ -63,17 +63,17 @@ void acpi_enable_wakeup_device(u8 sleep_
list_for_each_safe(node, next, &acpi_wakeup_device_list) {
struct acpi_device *dev =
container_of(node, struct acpi_device, wakeup_list);
+ u8 action = ACPI_GPE_ENABLE;
if (!dev->wakeup.flags.valid)
continue;
if ((!dev->wakeup.state.enabled && !dev->wakeup.prepare_count)
|| sleep_state > (u32) dev->wakeup.sleep_state)
- continue;
+ action = ACPI_GPE_DISABLE;
- /* The wake-up power should have been enabled already. */
acpi_set_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
- ACPI_GPE_ENABLE);
+ action);
}
}
On Tue 13-04-10 01:01:54, Rafael J. Wysocki wrote:
> On Saturday 10 April 2010, Rafael J. Wysocki wrote:
> > On Friday 09 April 2010, Tony Vroon wrote:
> > > On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> > > > Please check if the patch below changes anything.
> > > > drivers/acpi/wakeup.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > That didn't change the behaviour for me, sorry.
> >
> > Well, I would be sorry if it did, because the patch removed some useful code. :-)
> >
> > > (I made sure to go through a full power down session before trying the
> > > patched kernel)
> >
> > Thanks for testing. So it looks like we don't disable the GPE during power off.
> >
> > I'll try to figure out what's going on, please stay tuned.
>
> Can you please check if the patch below changes the behavior?
Unfortunately, it didn't help either (I have tried on top of the fresh
rc4).
>
> Rafael
>
> ---
> drivers/acpi/wakeup.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> Index: linux-2.6/drivers/acpi/wakeup.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/wakeup.c
> +++ linux-2.6/drivers/acpi/wakeup.c
> @@ -63,17 +63,17 @@ void acpi_enable_wakeup_device(u8 sleep_
> list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> struct acpi_device *dev =
> container_of(node, struct acpi_device, wakeup_list);
> + u8 action = ACPI_GPE_ENABLE;
>
> if (!dev->wakeup.flags.valid)
> continue;
>
> if ((!dev->wakeup.state.enabled && !dev->wakeup.prepare_count)
> || sleep_state > (u32) dev->wakeup.sleep_state)
> - continue;
> + action = ACPI_GPE_DISABLE;
>
> - /* The wake-up power should have been enabled already. */
> acpi_set_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
> - ACPI_GPE_ENABLE);
> + action);
> }
> }
>
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
On Tuesday 13 April 2010, Michal Hocko wrote:
> On Tue 13-04-10 01:01:54, Rafael J. Wysocki wrote:
> > On Saturday 10 April 2010, Rafael J. Wysocki wrote:
> > > On Friday 09 April 2010, Tony Vroon wrote:
> > > > On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> > > > > Please check if the patch below changes anything.
> > > > > drivers/acpi/wakeup.c | 4 ++--
> > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > That didn't change the behaviour for me, sorry.
> > >
> > > Well, I would be sorry if it did, because the patch removed some useful code. :-)
> > >
> > > > (I made sure to go through a full power down session before trying the
> > > > patched kernel)
> > >
> > > Thanks for testing. So it looks like we don't disable the GPE during power off.
> > >
> > > I'll try to figure out what's going on, please stay tuned.
> >
> > Can you please check if the patch below changes the behavior?
>
> Unfortunately, it didn't help either (I have tried on top of the fresh
> rc4).
That gets really weird.
> > ---
> > drivers/acpi/wakeup.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > Index: linux-2.6/drivers/acpi/wakeup.c
> > ===================================================================
> > --- linux-2.6.orig/drivers/acpi/wakeup.c
> > +++ linux-2.6/drivers/acpi/wakeup.c
> > @@ -63,17 +63,17 @@ void acpi_enable_wakeup_device(u8 sleep_
> > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > struct acpi_device *dev =
> > container_of(node, struct acpi_device, wakeup_list);
> > + u8 action = ACPI_GPE_ENABLE;
Can you try to change the above to ACPI_GPE_DISABLE and retest, please?
> > if (!dev->wakeup.flags.valid)
> > continue;
> >
> > if ((!dev->wakeup.state.enabled && !dev->wakeup.prepare_count)
> > || sleep_state > (u32) dev->wakeup.sleep_state)
> > - continue;
> > + action = ACPI_GPE_DISABLE;
> >
> > - /* The wake-up power should have been enabled already. */
> > acpi_set_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
> > - ACPI_GPE_ENABLE);
> > + action);
> > }
> > }
Rafael
On Tue 13-04-10 22:53:37, Rafael J. Wysocki wrote:
> On Tuesday 13 April 2010, Michal Hocko wrote:
> > On Tue 13-04-10 01:01:54, Rafael J. Wysocki wrote:
> > > On Saturday 10 April 2010, Rafael J. Wysocki wrote:
> > > > On Friday 09 April 2010, Tony Vroon wrote:
> > > > > On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> > > > > > Please check if the patch below changes anything.
> > > > > > drivers/acpi/wakeup.c | 4 ++--
> > > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > That didn't change the behaviour for me, sorry.
> > > >
> > > > Well, I would be sorry if it did, because the patch removed some useful code. :-)
> > > >
> > > > > (I made sure to go through a full power down session before trying the
> > > > > patched kernel)
> > > >
> > > > Thanks for testing. So it looks like we don't disable the GPE during power off.
> > > >
> > > > I'll try to figure out what's going on, please stay tuned.
> > >
> > > Can you please check if the patch below changes the behavior?
> >
> > Unfortunately, it didn't help either (I have tried on top of the fresh
> > rc4).
>
> That gets really weird.
>
> > > ---
> > > drivers/acpi/wakeup.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > Index: linux-2.6/drivers/acpi/wakeup.c
> > > ===================================================================
> > > --- linux-2.6.orig/drivers/acpi/wakeup.c
> > > +++ linux-2.6/drivers/acpi/wakeup.c
> > > @@ -63,17 +63,17 @@ void acpi_enable_wakeup_device(u8 sleep_
> > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > struct acpi_device *dev =
> > > container_of(node, struct acpi_device, wakeup_list);
> > > + u8 action = ACPI_GPE_ENABLE;
>
> Can you try to change the above to ACPI_GPE_DISABLE and retest, please?
Unfortunately didn't help as well...
Just for reference:
diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
index 248b473..f23c08f 100644
--- a/drivers/acpi/wakeup.c
+++ b/drivers/acpi/wakeup.c
@@ -63,7 +63,7 @@ void acpi_enable_wakeup_device(u8 sleep_state)
list_for_each_safe(node, next, &acpi_wakeup_device_list) {
struct acpi_device *dev =
container_of(node, struct acpi_device, wakeup_list);
- u8 action = ACPI_GPE_ENABLE;
+ u8 action = ACPI_GPE_DISABLE;
if (!dev->wakeup.flags.valid)
continue;
>
> > > if (!dev->wakeup.flags.valid)
> > > continue;
> > >
> > > if ((!dev->wakeup.state.enabled && !dev->wakeup.prepare_count)
> > > || sleep_state > (u32) dev->wakeup.sleep_state)
> > > - continue;
> > > + action = ACPI_GPE_DISABLE;
> > >
> > > - /* The wake-up power should have been enabled already. */
> > > acpi_set_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
> > > - ACPI_GPE_ENABLE);
> > > + action);
> > > }
> > > }
>
> Rafael
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
On Wednesday 14 April 2010, Michal Hocko wrote:
> On Tue 13-04-10 22:53:37, Rafael J. Wysocki wrote:
> > On Tuesday 13 April 2010, Michal Hocko wrote:
> > > On Tue 13-04-10 01:01:54, Rafael J. Wysocki wrote:
> > > > On Saturday 10 April 2010, Rafael J. Wysocki wrote:
> > > > > On Friday 09 April 2010, Tony Vroon wrote:
> > > > > > On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> > > > > > > Please check if the patch below changes anything.
> > > > > > > drivers/acpi/wakeup.c | 4 ++--
> > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > That didn't change the behaviour for me, sorry.
> > > > >
> > > > > Well, I would be sorry if it did, because the patch removed some useful code. :-)
> > > > >
> > > > > > (I made sure to go through a full power down session before trying the
> > > > > > patched kernel)
> > > > >
> > > > > Thanks for testing. So it looks like we don't disable the GPE during power off.
> > > > >
> > > > > I'll try to figure out what's going on, please stay tuned.
> > > >
> > > > Can you please check if the patch below changes the behavior?
> > >
> > > Unfortunately, it didn't help either (I have tried on top of the fresh
> > > rc4).
> >
> > That gets really weird.
> >
> > > > ---
> > > > drivers/acpi/wakeup.c | 6 +++---
> > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > Index: linux-2.6/drivers/acpi/wakeup.c
> > > > ===================================================================
> > > > --- linux-2.6.orig/drivers/acpi/wakeup.c
> > > > +++ linux-2.6/drivers/acpi/wakeup.c
> > > > @@ -63,17 +63,17 @@ void acpi_enable_wakeup_device(u8 sleep_
> > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > struct acpi_device *dev =
> > > > container_of(node, struct acpi_device, wakeup_list);
> > > > + u8 action = ACPI_GPE_ENABLE;
> >
> > Can you try to change the above to ACPI_GPE_DISABLE and retest, please?
>
> Unfortunately didn't help as well...
> Just for reference:
>
> diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
> index 248b473..f23c08f 100644
> --- a/drivers/acpi/wakeup.c
> +++ b/drivers/acpi/wakeup.c
> @@ -63,7 +63,7 @@ void acpi_enable_wakeup_device(u8 sleep_state)
> list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> struct acpi_device *dev =
> container_of(node, struct acpi_device, wakeup_list);
> - u8 action = ACPI_GPE_ENABLE;
> + u8 action = ACPI_GPE_DISABLE;
That probably means the chipset enables the GPEs by itself _after_ we've
disabled them in acpi_enable_wakeup_device().
Unfortunately, I can't reproduce the issue on any of my test boxes and it's
hard to find the source of the problem staring at the code.
Rafael
On Fri 16-04-10 20:00:29, Rafael J. Wysocki wrote:
> On Wednesday 14 April 2010, Michal Hocko wrote:
> > On Tue 13-04-10 22:53:37, Rafael J. Wysocki wrote:
> > > On Tuesday 13 April 2010, Michal Hocko wrote:
> > > > On Tue 13-04-10 01:01:54, Rafael J. Wysocki wrote:
> > > > > On Saturday 10 April 2010, Rafael J. Wysocki wrote:
> > > > > > On Friday 09 April 2010, Tony Vroon wrote:
> > > > > > > On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> > > > > > > > Please check if the patch below changes anything.
> > > > > > > > drivers/acpi/wakeup.c | 4 ++--
> > > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > That didn't change the behaviour for me, sorry.
> > > > > >
> > > > > > Well, I would be sorry if it did, because the patch removed some useful code. :-)
> > > > > >
> > > > > > > (I made sure to go through a full power down session before trying the
> > > > > > > patched kernel)
> > > > > >
> > > > > > Thanks for testing. So it looks like we don't disable the GPE during power off.
> > > > > >
> > > > > > I'll try to figure out what's going on, please stay tuned.
> > > > >
> > > > > Can you please check if the patch below changes the behavior?
> > > >
> > > > Unfortunately, it didn't help either (I have tried on top of the fresh
> > > > rc4).
> > >
> > > That gets really weird.
> > >
> > > > > ---
> > > > > drivers/acpi/wakeup.c | 6 +++---
> > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > >
> > > > > Index: linux-2.6/drivers/acpi/wakeup.c
> > > > > ===================================================================
> > > > > --- linux-2.6.orig/drivers/acpi/wakeup.c
> > > > > +++ linux-2.6/drivers/acpi/wakeup.c
> > > > > @@ -63,17 +63,17 @@ void acpi_enable_wakeup_device(u8 sleep_
> > > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > > struct acpi_device *dev =
> > > > > container_of(node, struct acpi_device, wakeup_list);
> > > > > + u8 action = ACPI_GPE_ENABLE;
> > >
> > > Can you try to change the above to ACPI_GPE_DISABLE and retest, please?
> >
> > Unfortunately didn't help as well...
> > Just for reference:
> >
> > diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
> > index 248b473..f23c08f 100644
> > --- a/drivers/acpi/wakeup.c
> > +++ b/drivers/acpi/wakeup.c
> > @@ -63,7 +63,7 @@ void acpi_enable_wakeup_device(u8 sleep_state)
> > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > struct acpi_device *dev =
> > container_of(node, struct acpi_device, wakeup_list);
> > - u8 action = ACPI_GPE_ENABLE;
> > + u8 action = ACPI_GPE_DISABLE;
>
> That probably means the chipset enables the GPEs by itself _after_ we've
> disabled them in acpi_enable_wakeup_device().
Is this something BIOS specific?
>
> Unfortunately, I can't reproduce the issue on any of my test boxes and it's
> hard to find the source of the problem staring at the code.
Are there any debug options I can turn on to provide some information?
Btw. what exactly does this mean? In what state is the laptop while it
is turned off and GPE is enabled?
Thanks!
>
> Rafael
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
On Monday 19 April 2010, Michal Hocko wrote:
> On Fri 16-04-10 20:00:29, Rafael J. Wysocki wrote:
> > On Wednesday 14 April 2010, Michal Hocko wrote:
> > > On Tue 13-04-10 22:53:37, Rafael J. Wysocki wrote:
> > > > On Tuesday 13 April 2010, Michal Hocko wrote:
> > > > > On Tue 13-04-10 01:01:54, Rafael J. Wysocki wrote:
> > > > > > On Saturday 10 April 2010, Rafael J. Wysocki wrote:
> > > > > > > On Friday 09 April 2010, Tony Vroon wrote:
> > > > > > > > On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> > > > > > > > > Please check if the patch below changes anything.
> > > > > > > > > drivers/acpi/wakeup.c | 4 ++--
> > > > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > >
> > > > > > > > That didn't change the behaviour for me, sorry.
> > > > > > >
> > > > > > > Well, I would be sorry if it did, because the patch removed some useful code. :-)
> > > > > > >
> > > > > > > > (I made sure to go through a full power down session before trying the
> > > > > > > > patched kernel)
> > > > > > >
> > > > > > > Thanks for testing. So it looks like we don't disable the GPE during power off.
> > > > > > >
> > > > > > > I'll try to figure out what's going on, please stay tuned.
> > > > > >
> > > > > > Can you please check if the patch below changes the behavior?
> > > > >
> > > > > Unfortunately, it didn't help either (I have tried on top of the fresh
> > > > > rc4).
> > > >
> > > > That gets really weird.
> > > >
> > > > > > ---
> > > > > > drivers/acpi/wakeup.c | 6 +++---
> > > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > Index: linux-2.6/drivers/acpi/wakeup.c
> > > > > > ===================================================================
> > > > > > --- linux-2.6.orig/drivers/acpi/wakeup.c
> > > > > > +++ linux-2.6/drivers/acpi/wakeup.c
> > > > > > @@ -63,17 +63,17 @@ void acpi_enable_wakeup_device(u8 sleep_
> > > > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > > > struct acpi_device *dev =
> > > > > > container_of(node, struct acpi_device, wakeup_list);
> > > > > > + u8 action = ACPI_GPE_ENABLE;
> > > >
> > > > Can you try to change the above to ACPI_GPE_DISABLE and retest, please?
> > >
> > > Unfortunately didn't help as well...
> > > Just for reference:
> > >
> > > diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
> > > index 248b473..f23c08f 100644
> > > --- a/drivers/acpi/wakeup.c
> > > +++ b/drivers/acpi/wakeup.c
> > > @@ -63,7 +63,7 @@ void acpi_enable_wakeup_device(u8 sleep_state)
> > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > struct acpi_device *dev =
> > > container_of(node, struct acpi_device, wakeup_list);
> > > - u8 action = ACPI_GPE_ENABLE;
> > > + u8 action = ACPI_GPE_DISABLE;
> >
> > That probably means the chipset enables the GPEs by itself _after_ we've
> > disabled them in acpi_enable_wakeup_device().
>
> Is this something BIOS specific?
>
> >
> > Unfortunately, I can't reproduce the issue on any of my test boxes and it's
> > hard to find the source of the problem staring at the code.
>
> Are there any debug options I can turn on to provide some information?
We can only check what the kernel tells us before power off, but all that seems
correct.
> Btw. what exactly does this mean? In what state is the laptop while it
> is turned off and GPE is enabled?
If a GPE is enabled, then some part of the chipset has power provided so that
it can signal wakeup.
I'll look into it a bit more later today.
Thanks,
Rafael
On Monday 19 April 2010, Rafael J. Wysocki wrote:
> On Monday 19 April 2010, Michal Hocko wrote:
> > On Fri 16-04-10 20:00:29, Rafael J. Wysocki wrote:
> > > On Wednesday 14 April 2010, Michal Hocko wrote:
> > > > On Tue 13-04-10 22:53:37, Rafael J. Wysocki wrote:
> > > > > On Tuesday 13 April 2010, Michal Hocko wrote:
> > > > > > On Tue 13-04-10 01:01:54, Rafael J. Wysocki wrote:
> > > > > > > On Saturday 10 April 2010, Rafael J. Wysocki wrote:
> > > > > > > > On Friday 09 April 2010, Tony Vroon wrote:
> > > > > > > > > On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> > > > > > > > > > Please check if the patch below changes anything.
> > > > > > > > > > drivers/acpi/wakeup.c | 4 ++--
> > > > > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > >
> > > > > > > > > That didn't change the behaviour for me, sorry.
> > > > > > > >
> > > > > > > > Well, I would be sorry if it did, because the patch removed some useful code. :-)
> > > > > > > >
> > > > > > > > > (I made sure to go through a full power down session before trying the
> > > > > > > > > patched kernel)
> > > > > > > >
> > > > > > > > Thanks for testing. So it looks like we don't disable the GPE during power off.
> > > > > > > >
> > > > > > > > I'll try to figure out what's going on, please stay tuned.
> > > > > > >
> > > > > > > Can you please check if the patch below changes the behavior?
> > > > > >
> > > > > > Unfortunately, it didn't help either (I have tried on top of the fresh
> > > > > > rc4).
> > > > >
> > > > > That gets really weird.
> > > > >
> > > > > > > ---
> > > > > > > drivers/acpi/wakeup.c | 6 +++---
> > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > > >
> > > > > > > Index: linux-2.6/drivers/acpi/wakeup.c
> > > > > > > ===================================================================
> > > > > > > --- linux-2.6.orig/drivers/acpi/wakeup.c
> > > > > > > +++ linux-2.6/drivers/acpi/wakeup.c
> > > > > > > @@ -63,17 +63,17 @@ void acpi_enable_wakeup_device(u8 sleep_
> > > > > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > > > > struct acpi_device *dev =
> > > > > > > container_of(node, struct acpi_device, wakeup_list);
> > > > > > > + u8 action = ACPI_GPE_ENABLE;
> > > > >
> > > > > Can you try to change the above to ACPI_GPE_DISABLE and retest, please?
> > > >
> > > > Unfortunately didn't help as well...
> > > > Just for reference:
> > > >
> > > > diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
> > > > index 248b473..f23c08f 100644
> > > > --- a/drivers/acpi/wakeup.c
> > > > +++ b/drivers/acpi/wakeup.c
> > > > @@ -63,7 +63,7 @@ void acpi_enable_wakeup_device(u8 sleep_state)
> > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > struct acpi_device *dev =
> > > > container_of(node, struct acpi_device, wakeup_list);
> > > > - u8 action = ACPI_GPE_ENABLE;
> > > > + u8 action = ACPI_GPE_DISABLE;
> > >
> > > That probably means the chipset enables the GPEs by itself _after_ we've
> > > disabled them in acpi_enable_wakeup_device().
> >
> > Is this something BIOS specific?
> >
> > >
> > > Unfortunately, I can't reproduce the issue on any of my test boxes and it's
> > > hard to find the source of the problem staring at the code.
> >
> > Are there any debug options I can turn on to provide some information?
>
> We can only check what the kernel tells us before power off, but all that seems
> correct.
>
> > Btw. what exactly does this mean? In what state is the laptop while it
> > is turned off and GPE is enabled?
>
> If a GPE is enabled, then some part of the chipset has power provided so that
> it can signal wakeup.
>
> I'll look into it a bit more later today.
Please try the patch below. It kind of restores the previous behavior,
let's see if it changes anything.
Rafael
---
drivers/acpi/acpica/evgpeblk.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/acpi/acpica/evgpeblk.c
===================================================================
--- linux-2.6.orig/drivers/acpi/acpica/evgpeblk.c
+++ linux-2.6/drivers/acpi/acpica/evgpeblk.c
@@ -364,7 +364,7 @@ acpi_ev_match_prw_and_gpe(acpi_handle ob
union acpi_operand_object *pkg_desc;
union acpi_operand_object *obj_desc;
u32 gpe_number;
- acpi_status status;
+ acpi_status status = AE_OK;
ACPI_FUNCTION_TRACE(ev_match_prw_and_gpe);
@@ -439,13 +439,15 @@ acpi_ev_match_prw_and_gpe(acpi_handle ob
if (gpe_device == target_gpe_device) {
gpe_event_info = acpi_ev_gpeblk_event_info(gpe_block,
gpe_number);
- if (gpe_event_info)
+ if (gpe_event_info) {
+ status = acpi_ev_disable_gpe(gpe_event_info);
gpe_event_info->flags |= ACPI_GPE_CAN_WAKE;
+ }
}
cleanup:
acpi_ut_remove_reference(pkg_desc);
- return_ACPI_STATUS(AE_OK);
+ return_ACPI_STATUS(status);
}
/*******************************************************************************
On Sunday 25 April 2010, Rafael J. Wysocki wrote:
> On Monday 19 April 2010, Rafael J. Wysocki wrote:
> > On Monday 19 April 2010, Michal Hocko wrote:
> > > On Fri 16-04-10 20:00:29, Rafael J. Wysocki wrote:
> > > > On Wednesday 14 April 2010, Michal Hocko wrote:
> > > > > On Tue 13-04-10 22:53:37, Rafael J. Wysocki wrote:
> > > > > > On Tuesday 13 April 2010, Michal Hocko wrote:
> > > > > > > On Tue 13-04-10 01:01:54, Rafael J. Wysocki wrote:
> > > > > > > > On Saturday 10 April 2010, Rafael J. Wysocki wrote:
> > > > > > > > > On Friday 09 April 2010, Tony Vroon wrote:
> > > > > > > > > > On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> > > > > > > > > > > Please check if the patch below changes anything.
> > > > > > > > > > > drivers/acpi/wakeup.c | 4 ++--
> > > > > > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > > >
> > > > > > > > > > That didn't change the behaviour for me, sorry.
> > > > > > > > >
> > > > > > > > > Well, I would be sorry if it did, because the patch removed some useful code. :-)
> > > > > > > > >
> > > > > > > > > > (I made sure to go through a full power down session before trying the
> > > > > > > > > > patched kernel)
> > > > > > > > >
> > > > > > > > > Thanks for testing. So it looks like we don't disable the GPE during power off.
> > > > > > > > >
> > > > > > > > > I'll try to figure out what's going on, please stay tuned.
> > > > > > > >
> > > > > > > > Can you please check if the patch below changes the behavior?
> > > > > > >
> > > > > > > Unfortunately, it didn't help either (I have tried on top of the fresh
> > > > > > > rc4).
> > > > > >
> > > > > > That gets really weird.
> > > > > >
> > > > > > > > ---
> > > > > > > > drivers/acpi/wakeup.c | 6 +++---
> > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > > > >
> > > > > > > > Index: linux-2.6/drivers/acpi/wakeup.c
> > > > > > > > ===================================================================
> > > > > > > > --- linux-2.6.orig/drivers/acpi/wakeup.c
> > > > > > > > +++ linux-2.6/drivers/acpi/wakeup.c
> > > > > > > > @@ -63,17 +63,17 @@ void acpi_enable_wakeup_device(u8 sleep_
> > > > > > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > > > > > struct acpi_device *dev =
> > > > > > > > container_of(node, struct acpi_device, wakeup_list);
> > > > > > > > + u8 action = ACPI_GPE_ENABLE;
> > > > > >
> > > > > > Can you try to change the above to ACPI_GPE_DISABLE and retest, please?
> > > > >
> > > > > Unfortunately didn't help as well...
> > > > > Just for reference:
> > > > >
> > > > > diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
> > > > > index 248b473..f23c08f 100644
> > > > > --- a/drivers/acpi/wakeup.c
> > > > > +++ b/drivers/acpi/wakeup.c
> > > > > @@ -63,7 +63,7 @@ void acpi_enable_wakeup_device(u8 sleep_state)
> > > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > > struct acpi_device *dev =
> > > > > container_of(node, struct acpi_device, wakeup_list);
> > > > > - u8 action = ACPI_GPE_ENABLE;
> > > > > + u8 action = ACPI_GPE_DISABLE;
> > > >
> > > > That probably means the chipset enables the GPEs by itself _after_ we've
> > > > disabled them in acpi_enable_wakeup_device().
> > >
> > > Is this something BIOS specific?
> > >
> > > >
> > > > Unfortunately, I can't reproduce the issue on any of my test boxes and it's
> > > > hard to find the source of the problem staring at the code.
> > >
> > > Are there any debug options I can turn on to provide some information?
> >
> > We can only check what the kernel tells us before power off, but all that seems
> > correct.
> >
> > > Btw. what exactly does this mean? In what state is the laptop while it
> > > is turned off and GPE is enabled?
> >
> > If a GPE is enabled, then some part of the chipset has power provided so that
> > it can signal wakeup.
> >
> > I'll look into it a bit more later today.
>
> Please try the patch below. It kind of restores the previous behavior,
> let's see if it changes anything.
If it doesn't help, please try to comment out the acpi_enable_gpe() in
drivers/acpi/wakeup.c:acpi_wakeup_device_init() and retest (if you haven't
done that already).
Rafael
On Sun 25-04-10 04:35:42, Rafael J. Wysocki wrote:
> On Monday 19 April 2010, Rafael J. Wysocki wrote:
> > On Monday 19 April 2010, Michal Hocko wrote:
> > > On Fri 16-04-10 20:00:29, Rafael J. Wysocki wrote:
> > > > On Wednesday 14 April 2010, Michal Hocko wrote:
> > > > > On Tue 13-04-10 22:53:37, Rafael J. Wysocki wrote:
> > > > > > On Tuesday 13 April 2010, Michal Hocko wrote:
> > > > > > > On Tue 13-04-10 01:01:54, Rafael J. Wysocki wrote:
> > > > > > > > On Saturday 10 April 2010, Rafael J. Wysocki wrote:
> > > > > > > > > On Friday 09 April 2010, Tony Vroon wrote:
> > > > > > > > > > On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> > > > > > > > > > > Please check if the patch below changes anything.
> > > > > > > > > > > drivers/acpi/wakeup.c | 4 ++--
> > > > > > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > > >
> > > > > > > > > > That didn't change the behaviour for me, sorry.
> > > > > > > > >
> > > > > > > > > Well, I would be sorry if it did, because the patch removed some useful code. :-)
> > > > > > > > >
> > > > > > > > > > (I made sure to go through a full power down session before trying the
> > > > > > > > > > patched kernel)
> > > > > > > > >
> > > > > > > > > Thanks for testing. So it looks like we don't disable the GPE during power off.
> > > > > > > > >
> > > > > > > > > I'll try to figure out what's going on, please stay tuned.
> > > > > > > >
> > > > > > > > Can you please check if the patch below changes the behavior?
> > > > > > >
> > > > > > > Unfortunately, it didn't help either (I have tried on top of the fresh
> > > > > > > rc4).
> > > > > >
> > > > > > That gets really weird.
> > > > > >
> > > > > > > > ---
> > > > > > > > drivers/acpi/wakeup.c | 6 +++---
> > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > > > >
> > > > > > > > Index: linux-2.6/drivers/acpi/wakeup.c
> > > > > > > > ===================================================================
> > > > > > > > --- linux-2.6.orig/drivers/acpi/wakeup.c
> > > > > > > > +++ linux-2.6/drivers/acpi/wakeup.c
> > > > > > > > @@ -63,17 +63,17 @@ void acpi_enable_wakeup_device(u8 sleep_
> > > > > > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > > > > > struct acpi_device *dev =
> > > > > > > > container_of(node, struct acpi_device, wakeup_list);
> > > > > > > > + u8 action = ACPI_GPE_ENABLE;
> > > > > >
> > > > > > Can you try to change the above to ACPI_GPE_DISABLE and retest, please?
> > > > >
> > > > > Unfortunately didn't help as well...
> > > > > Just for reference:
> > > > >
> > > > > diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
> > > > > index 248b473..f23c08f 100644
> > > > > --- a/drivers/acpi/wakeup.c
> > > > > +++ b/drivers/acpi/wakeup.c
> > > > > @@ -63,7 +63,7 @@ void acpi_enable_wakeup_device(u8 sleep_state)
> > > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > > struct acpi_device *dev =
> > > > > container_of(node, struct acpi_device, wakeup_list);
> > > > > - u8 action = ACPI_GPE_ENABLE;
> > > > > + u8 action = ACPI_GPE_DISABLE;
> > > >
> > > > That probably means the chipset enables the GPEs by itself _after_ we've
> > > > disabled them in acpi_enable_wakeup_device().
> > >
> > > Is this something BIOS specific?
> > >
> > > >
> > > > Unfortunately, I can't reproduce the issue on any of my test boxes and it's
> > > > hard to find the source of the problem staring at the code.
> > >
> > > Are there any debug options I can turn on to provide some information?
> >
> > We can only check what the kernel tells us before power off, but all that seems
> > correct.
> >
> > > Btw. what exactly does this mean? In what state is the laptop while it
> > > is turned off and GPE is enabled?
> >
> > If a GPE is enabled, then some part of the chipset has power provided so that
> > it can signal wakeup.
> >
> > I'll look into it a bit more later today.
>
> Please try the patch below. It kind of restores the previous behavior,
> let's see if it changes anything.
Again, no success. Just to make sure that I didn't screw anything. I
have used just the following patch on top of the clean rc5 (your patch
has failed with some rejects but I guess that the following should do
the same):
commit 65eafe4a504e3bb2c13b4feb8590dc52e7439baa
Author: Rafael J. Wysocki <[email protected]>
Date: Sun Apr 25 04:35:42 2010 +0200
diff --git a/drivers/acpi/acpica/evgpeblk.c b/drivers/acpi/acpica/evgpeblk.c
index fef7219..b6e1e0c 100644
--- a/drivers/acpi/acpica/evgpeblk.c
+++ b/drivers/acpi/acpica/evgpeblk.c
@@ -367,7 +367,7 @@ acpi_ev_match_prw_and_gpe(acpi_handle obj_handle,
union acpi_operand_object *pkg_desc;
union acpi_operand_object *obj_desc;
u32 gpe_number;
- acpi_status status;
+ acpi_status status = AE_OK;
ACPI_FUNCTION_TRACE(ev_match_prw_and_gpe);
@@ -447,12 +447,13 @@ acpi_ev_match_prw_and_gpe(acpi_handle obj_handle,
gpe_block->
block_base_number];
+ status = acpi_ev_disable_gpe(gpe_event_info);
gpe_event_info->flags |= ACPI_GPE_CAN_WAKE;
}
cleanup:
acpi_ut_remove_reference(pkg_desc);
- return_ACPI_STATUS(AE_OK);
+ return_ACPI_STATUS(status);
}
/*******************************************************************************
>
> Rafael
>
> ---
> drivers/acpi/acpica/evgpeblk.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> Index: linux-2.6/drivers/acpi/acpica/evgpeblk.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/acpica/evgpeblk.c
> +++ linux-2.6/drivers/acpi/acpica/evgpeblk.c
> @@ -364,7 +364,7 @@ acpi_ev_match_prw_and_gpe(acpi_handle ob
> union acpi_operand_object *pkg_desc;
> union acpi_operand_object *obj_desc;
> u32 gpe_number;
> - acpi_status status;
> + acpi_status status = AE_OK;
>
> ACPI_FUNCTION_TRACE(ev_match_prw_and_gpe);
>
> @@ -439,13 +439,15 @@ acpi_ev_match_prw_and_gpe(acpi_handle ob
> if (gpe_device == target_gpe_device) {
> gpe_event_info = acpi_ev_gpeblk_event_info(gpe_block,
> gpe_number);
> - if (gpe_event_info)
> + if (gpe_event_info) {
> + status = acpi_ev_disable_gpe(gpe_event_info);
> gpe_event_info->flags |= ACPI_GPE_CAN_WAKE;
> + }
> }
>
> cleanup:
> acpi_ut_remove_reference(pkg_desc);
> - return_ACPI_STATUS(AE_OK);
> + return_ACPI_STATUS(status);
> }
>
> /*******************************************************************************
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
Michal,
> my laptop changed behavior during poweroff recently (after upgrading
> from .33 to .34-rc1). The system seems to be powered off (status display
> is off) at first glance but when I close the lid then I can hear a noise
> which sounds like HDD parking and when I open lid again it starts
> booting without poweroff button (same like when I suspend to RAM).
Tony,
> I have the exact same "lid close = powering back on" regression on a
> Fujitsu-Siemens Lifebook S6420.
Perhaps you can review this description and help me
better understand the symptoms?
Michal,
Re: poweroff
Before the regression you could hear the HDD park as a result of the
"poweroff" command without touching the lid, and after the regression
you don't hear that sound until after you close the lid -- no matter
how long you wait to close the lid?
During this period after poweroff and LCD goes out, but before
lid close, are there any other signs of life? eg. does caps
lock button light the caps LED?
How about during this period if you press the power button and
hold it for 5 seconds, does that cause the HDD parking sound?
If yes, then the regression is that poweroff never completes,
though perhaps a lid event allows it to complete.
When you say 'when I open the lid again i starts booting'.
Is that a cold boot from scratch, or a resume from suspend?
Does the new boot start when the lid is closed, or when it is
subsequently opened?
Thanks for sending your /proc/acpi/wakeup, it shows that
LID S4 *enabled
which means that the LID is not an S5 wakeup device.
So if you really do get down to S5, the LID should
not be able to wake the system. So if it is, then
we are never getting to S5.
Tony,
Please send the /proc/acpi/wakeup file before and after
the regression. Does the system reach a complete poweroff
before the lid is closed? If yes, what happens when
you press the power button w/o closing the lid?
If you reached S5, it should give you a cold boot.
If you did not reach S5, it will have no effect.
thanks,
Len Brown, Intel Open Source Technology Center
On Monday 26 April 2010, Michal Hocko wrote:
> On Sun 25-04-10 04:35:42, Rafael J. Wysocki wrote:
> > On Monday 19 April 2010, Rafael J. Wysocki wrote:
> > > On Monday 19 April 2010, Michal Hocko wrote:
> > > > On Fri 16-04-10 20:00:29, Rafael J. Wysocki wrote:
> > > > > On Wednesday 14 April 2010, Michal Hocko wrote:
> > > > > > On Tue 13-04-10 22:53:37, Rafael J. Wysocki wrote:
> > > > > > > On Tuesday 13 April 2010, Michal Hocko wrote:
> > > > > > > > On Tue 13-04-10 01:01:54, Rafael J. Wysocki wrote:
> > > > > > > > > On Saturday 10 April 2010, Rafael J. Wysocki wrote:
> > > > > > > > > > On Friday 09 April 2010, Tony Vroon wrote:
> > > > > > > > > > > On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> > > > > > > > > > > > Please check if the patch below changes anything.
> > > > > > > > > > > > drivers/acpi/wakeup.c | 4 ++--
> > > > > > > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > > > >
> > > > > > > > > > > That didn't change the behaviour for me, sorry.
> > > > > > > > > >
> > > > > > > > > > Well, I would be sorry if it did, because the patch removed some useful code. :-)
> > > > > > > > > >
> > > > > > > > > > > (I made sure to go through a full power down session before trying the
> > > > > > > > > > > patched kernel)
> > > > > > > > > >
> > > > > > > > > > Thanks for testing. So it looks like we don't disable the GPE during power off.
> > > > > > > > > >
> > > > > > > > > > I'll try to figure out what's going on, please stay tuned.
> > > > > > > > >
> > > > > > > > > Can you please check if the patch below changes the behavior?
> > > > > > > >
> > > > > > > > Unfortunately, it didn't help either (I have tried on top of the fresh
> > > > > > > > rc4).
> > > > > > >
> > > > > > > That gets really weird.
> > > > > > >
> > > > > > > > > ---
> > > > > > > > > drivers/acpi/wakeup.c | 6 +++---
> > > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > > > > >
> > > > > > > > > Index: linux-2.6/drivers/acpi/wakeup.c
> > > > > > > > > ===================================================================
> > > > > > > > > --- linux-2.6.orig/drivers/acpi/wakeup.c
> > > > > > > > > +++ linux-2.6/drivers/acpi/wakeup.c
> > > > > > > > > @@ -63,17 +63,17 @@ void acpi_enable_wakeup_device(u8 sleep_
> > > > > > > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > > > > > > struct acpi_device *dev =
> > > > > > > > > container_of(node, struct acpi_device, wakeup_list);
> > > > > > > > > + u8 action = ACPI_GPE_ENABLE;
> > > > > > >
> > > > > > > Can you try to change the above to ACPI_GPE_DISABLE and retest, please?
> > > > > >
> > > > > > Unfortunately didn't help as well...
> > > > > > Just for reference:
> > > > > >
> > > > > > diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
> > > > > > index 248b473..f23c08f 100644
> > > > > > --- a/drivers/acpi/wakeup.c
> > > > > > +++ b/drivers/acpi/wakeup.c
> > > > > > @@ -63,7 +63,7 @@ void acpi_enable_wakeup_device(u8 sleep_state)
> > > > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > > > struct acpi_device *dev =
> > > > > > container_of(node, struct acpi_device, wakeup_list);
> > > > > > - u8 action = ACPI_GPE_ENABLE;
> > > > > > + u8 action = ACPI_GPE_DISABLE;
> > > > >
> > > > > That probably means the chipset enables the GPEs by itself _after_ we've
> > > > > disabled them in acpi_enable_wakeup_device().
> > > >
> > > > Is this something BIOS specific?
> > > >
> > > > >
> > > > > Unfortunately, I can't reproduce the issue on any of my test boxes and it's
> > > > > hard to find the source of the problem staring at the code.
> > > >
> > > > Are there any debug options I can turn on to provide some information?
> > >
> > > We can only check what the kernel tells us before power off, but all that seems
> > > correct.
> > >
> > > > Btw. what exactly does this mean? In what state is the laptop while it
> > > > is turned off and GPE is enabled?
> > >
> > > If a GPE is enabled, then some part of the chipset has power provided so that
> > > it can signal wakeup.
> > >
> > > I'll look into it a bit more later today.
> >
> > Please try the patch below. It kind of restores the previous behavior,
> > let's see if it changes anything.
>
> Again, no success. Just to make sure that I didn't screw anything. I
> have used just the following patch on top of the clean rc5 (your patch
> has failed with some rejects but I guess that the following should do
> the same):
Thanks for testing. Did you try the second thing, ie. try to comment out the
acpi_enable_gpe() in drivers/acpi/wakeup.c:acpi_wakeup_device_init() and
retest (if you haven't done that already)?
Rafael
On Mon 26-04-10 20:51:09, Rafael J. Wysocki wrote:
> On Monday 26 April 2010, Michal Hocko wrote:
> > On Sun 25-04-10 04:35:42, Rafael J. Wysocki wrote:
> > > On Monday 19 April 2010, Rafael J. Wysocki wrote:
> > > > On Monday 19 April 2010, Michal Hocko wrote:
> > > > > On Fri 16-04-10 20:00:29, Rafael J. Wysocki wrote:
> > > > > > On Wednesday 14 April 2010, Michal Hocko wrote:
> > > > > > > On Tue 13-04-10 22:53:37, Rafael J. Wysocki wrote:
> > > > > > > > On Tuesday 13 April 2010, Michal Hocko wrote:
> > > > > > > > > On Tue 13-04-10 01:01:54, Rafael J. Wysocki wrote:
> > > > > > > > > > On Saturday 10 April 2010, Rafael J. Wysocki wrote:
> > > > > > > > > > > On Friday 09 April 2010, Tony Vroon wrote:
> > > > > > > > > > > > On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> > > > > > > > > > > > > Please check if the patch below changes anything.
> > > > > > > > > > > > > drivers/acpi/wakeup.c | 4 ++--
> > > > > > > > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > > > > >
> > > > > > > > > > > > That didn't change the behaviour for me, sorry.
> > > > > > > > > > >
> > > > > > > > > > > Well, I would be sorry if it did, because the patch removed some useful code. :-)
> > > > > > > > > > >
> > > > > > > > > > > > (I made sure to go through a full power down session before trying the
> > > > > > > > > > > > patched kernel)
> > > > > > > > > > >
> > > > > > > > > > > Thanks for testing. So it looks like we don't disable the GPE during power off.
> > > > > > > > > > >
> > > > > > > > > > > I'll try to figure out what's going on, please stay tuned.
> > > > > > > > > >
> > > > > > > > > > Can you please check if the patch below changes the behavior?
> > > > > > > > >
> > > > > > > > > Unfortunately, it didn't help either (I have tried on top of the fresh
> > > > > > > > > rc4).
> > > > > > > >
> > > > > > > > That gets really weird.
> > > > > > > >
> > > > > > > > > > ---
> > > > > > > > > > drivers/acpi/wakeup.c | 6 +++---
> > > > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > > > > > >
> > > > > > > > > > Index: linux-2.6/drivers/acpi/wakeup.c
> > > > > > > > > > ===================================================================
> > > > > > > > > > --- linux-2.6.orig/drivers/acpi/wakeup.c
> > > > > > > > > > +++ linux-2.6/drivers/acpi/wakeup.c
> > > > > > > > > > @@ -63,17 +63,17 @@ void acpi_enable_wakeup_device(u8 sleep_
> > > > > > > > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > > > > > > > struct acpi_device *dev =
> > > > > > > > > > container_of(node, struct acpi_device, wakeup_list);
> > > > > > > > > > + u8 action = ACPI_GPE_ENABLE;
> > > > > > > >
> > > > > > > > Can you try to change the above to ACPI_GPE_DISABLE and retest, please?
> > > > > > >
> > > > > > > Unfortunately didn't help as well...
> > > > > > > Just for reference:
> > > > > > >
> > > > > > > diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
> > > > > > > index 248b473..f23c08f 100644
> > > > > > > --- a/drivers/acpi/wakeup.c
> > > > > > > +++ b/drivers/acpi/wakeup.c
> > > > > > > @@ -63,7 +63,7 @@ void acpi_enable_wakeup_device(u8 sleep_state)
> > > > > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > > > > struct acpi_device *dev =
> > > > > > > container_of(node, struct acpi_device, wakeup_list);
> > > > > > > - u8 action = ACPI_GPE_ENABLE;
> > > > > > > + u8 action = ACPI_GPE_DISABLE;
> > > > > >
> > > > > > That probably means the chipset enables the GPEs by itself _after_ we've
> > > > > > disabled them in acpi_enable_wakeup_device().
> > > > >
> > > > > Is this something BIOS specific?
> > > > >
> > > > > >
> > > > > > Unfortunately, I can't reproduce the issue on any of my test boxes and it's
> > > > > > hard to find the source of the problem staring at the code.
> > > > >
> > > > > Are there any debug options I can turn on to provide some information?
> > > >
> > > > We can only check what the kernel tells us before power off, but all that seems
> > > > correct.
> > > >
> > > > > Btw. what exactly does this mean? In what state is the laptop while it
> > > > > is turned off and GPE is enabled?
> > > >
> > > > If a GPE is enabled, then some part of the chipset has power provided so that
> > > > it can signal wakeup.
> > > >
> > > > I'll look into it a bit more later today.
> > >
> > > Please try the patch below. It kind of restores the previous behavior,
> > > let's see if it changes anything.
> >
> > Again, no success. Just to make sure that I didn't screw anything. I
> > have used just the following patch on top of the clean rc5 (your patch
> > has failed with some rejects but I guess that the following should do
> > the same):
>
> Thanks for testing. Did you try the second thing, ie. try to comment out the
> acpi_enable_gpe() in drivers/acpi/wakeup.c:acpi_wakeup_device_init() and
> retest (if you haven't done that already)?
I have tried it just now and still no success.
commit 1a7e7c9ac82aa2de185fa2f686417a5eb7765420
Author: Michal Hocko <[email protected]>
Date: Tue Apr 27 08:22:23 2010 +0200
disable acpi_enable_gpe in acpi_wakeup_device_init
as suggested by Rafael.
diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
index 4b9d339..a0f4859 100644
--- a/drivers/acpi/wakeup.c
+++ b/drivers/acpi/wakeup.c
@@ -113,8 +113,9 @@ int __init acpi_wakeup_device_init(void)
if (!dev->wakeup.flags.always_enabled ||
dev->wakeup.state.enabled)
continue;
- acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
+ /*acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
ACPI_GPE_TYPE_WAKE);
+ */
dev->wakeup.state.enabled = 1;
}
mutex_unlock(&acpi_device_lock);
On top of the previous patch. If I understand that correctly then this
is really strange because this patch disables GPE completely, doesn't
it?
>
> Rafael
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
Hi Len,
On Mon 26-04-10 12:22:58, Len Brown wrote:
> Michal,
>
> > my laptop changed behavior during poweroff recently (after upgrading
> > from .33 to .34-rc1). The system seems to be powered off (status display
> > is off) at first glance but when I close the lid then I can hear a noise
> > which sounds like HDD parking and when I open lid again it starts
> > booting without poweroff button (same like when I suspend to RAM).
>
> Tony,
>
> > I have the exact same "lid close = powering back on" regression on a
> > Fujitsu-Siemens Lifebook S6420.
>
> Perhaps you can review this description and help me
> better understand the symptoms?
>
> Michal,
> Re: poweroff
>
> Before the regression you could hear the HDD park as a result of the
It is hard to tell whether this sound comes from HDD and I think that it
is actually something different as I've realized that I've heard this
kind of sound only when system was suspended to RAM and lid was closed.
Never during poweroff.
> "poweroff" command without touching the lid, and after the regression
> you don't hear that sound until after you close the lid -- no matter
> how long you wait to close the lid?
Yes. Also the main difference is that the laptop turns on automatically
after lid is open which did not happen before.
>
> During this period after poweroff and LCD goes out, but before
> lid close, are there any other signs of life? eg. does caps
> lock button light the caps LED?
No signs of life at all. The small display which shows the status
(charging, battery status, caps lock, num lock, etc.) is turned off.
No reaction to any key press.
>
> How about during this period if you press the power button and
> hold it for 5 seconds, does that cause the HDD parking sound?
No sound. Even when I closed the lid. Nevertheless, laptop doesn't
start automatically after lid is open.
>
> If yes, then the regression is that poweroff never completes,
> though perhaps a lid event allows it to complete.
>
> When you say 'when I open the lid again i starts booting'.
> Is that a cold boot from scratch, or a resume from suspend?
Cold boot.
>
> Does the new boot start when the lid is closed, or when it is
> subsequently opened?
I have not seen start during lid being closed. As I already said, the
system starts up automatically when I open the lid.
>
> Thanks for sending your /proc/acpi/wakeup, it shows that
>
> LID S4 *enabled
>
> which means that the LID is not an S5 wakeup device.
> So if you really do get down to S5, the LID should
> not be able to wake the system. So if it is, then
> we are never getting to S5.
>
> Tony,
> Please send the /proc/acpi/wakeup file before and after
> the regression. Does the system reach a complete poweroff
> before the lid is closed? If yes, what happens when
> you press the power button w/o closing the lid?
> If you reached S5, it should give you a cold boot.
> If you did not reach S5, it will have no effect.
>
> thanks,
> Len Brown, Intel Open Source Technology Center
>
Thanks!
--
Michal Hocko
L3 team
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9
Czech Republic
On Tuesday 27 April 2010, Michal Hocko wrote:
> On Mon 26-04-10 20:51:09, Rafael J. Wysocki wrote:
> > On Monday 26 April 2010, Michal Hocko wrote:
> > > On Sun 25-04-10 04:35:42, Rafael J. Wysocki wrote:
> > > > On Monday 19 April 2010, Rafael J. Wysocki wrote:
> > > > > On Monday 19 April 2010, Michal Hocko wrote:
> > > > > > On Fri 16-04-10 20:00:29, Rafael J. Wysocki wrote:
> > > > > > > On Wednesday 14 April 2010, Michal Hocko wrote:
> > > > > > > > On Tue 13-04-10 22:53:37, Rafael J. Wysocki wrote:
> > > > > > > > > On Tuesday 13 April 2010, Michal Hocko wrote:
> > > > > > > > > > On Tue 13-04-10 01:01:54, Rafael J. Wysocki wrote:
> > > > > > > > > > > On Saturday 10 April 2010, Rafael J. Wysocki wrote:
> > > > > > > > > > > > On Friday 09 April 2010, Tony Vroon wrote:
> > > > > > > > > > > > > On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> > > > > > > > > > > > > > Please check if the patch below changes anything.
> > > > > > > > > > > > > > drivers/acpi/wakeup.c | 4 ++--
> > > > > > > > > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > > > > > >
> > > > > > > > > > > > > That didn't change the behaviour for me, sorry.
> > > > > > > > > > > >
> > > > > > > > > > > > Well, I would be sorry if it did, because the patch removed some useful code. :-)
> > > > > > > > > > > >
> > > > > > > > > > > > > (I made sure to go through a full power down session before trying the
> > > > > > > > > > > > > patched kernel)
> > > > > > > > > > > >
> > > > > > > > > > > > Thanks for testing. So it looks like we don't disable the GPE during power off.
> > > > > > > > > > > >
> > > > > > > > > > > > I'll try to figure out what's going on, please stay tuned.
> > > > > > > > > > >
> > > > > > > > > > > Can you please check if the patch below changes the behavior?
> > > > > > > > > >
> > > > > > > > > > Unfortunately, it didn't help either (I have tried on top of the fresh
> > > > > > > > > > rc4).
> > > > > > > > >
> > > > > > > > > That gets really weird.
> > > > > > > > >
> > > > > > > > > > > ---
> > > > > > > > > > > drivers/acpi/wakeup.c | 6 +++---
> > > > > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > > > > > > >
> > > > > > > > > > > Index: linux-2.6/drivers/acpi/wakeup.c
> > > > > > > > > > > ===================================================================
> > > > > > > > > > > --- linux-2.6.orig/drivers/acpi/wakeup.c
> > > > > > > > > > > +++ linux-2.6/drivers/acpi/wakeup.c
> > > > > > > > > > > @@ -63,17 +63,17 @@ void acpi_enable_wakeup_device(u8 sleep_
> > > > > > > > > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > > > > > > > > struct acpi_device *dev =
> > > > > > > > > > > container_of(node, struct acpi_device, wakeup_list);
> > > > > > > > > > > + u8 action = ACPI_GPE_ENABLE;
> > > > > > > > >
> > > > > > > > > Can you try to change the above to ACPI_GPE_DISABLE and retest, please?
> > > > > > > >
> > > > > > > > Unfortunately didn't help as well...
> > > > > > > > Just for reference:
> > > > > > > >
> > > > > > > > diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
> > > > > > > > index 248b473..f23c08f 100644
> > > > > > > > --- a/drivers/acpi/wakeup.c
> > > > > > > > +++ b/drivers/acpi/wakeup.c
> > > > > > > > @@ -63,7 +63,7 @@ void acpi_enable_wakeup_device(u8 sleep_state)
> > > > > > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > > > > > struct acpi_device *dev =
> > > > > > > > container_of(node, struct acpi_device, wakeup_list);
> > > > > > > > - u8 action = ACPI_GPE_ENABLE;
> > > > > > > > + u8 action = ACPI_GPE_DISABLE;
> > > > > > >
> > > > > > > That probably means the chipset enables the GPEs by itself _after_ we've
> > > > > > > disabled them in acpi_enable_wakeup_device().
> > > > > >
> > > > > > Is this something BIOS specific?
> > > > > >
> > > > > > >
> > > > > > > Unfortunately, I can't reproduce the issue on any of my test boxes and it's
> > > > > > > hard to find the source of the problem staring at the code.
> > > > > >
> > > > > > Are there any debug options I can turn on to provide some information?
> > > > >
> > > > > We can only check what the kernel tells us before power off, but all that seems
> > > > > correct.
> > > > >
> > > > > > Btw. what exactly does this mean? In what state is the laptop while it
> > > > > > is turned off and GPE is enabled?
> > > > >
> > > > > If a GPE is enabled, then some part of the chipset has power provided so that
> > > > > it can signal wakeup.
> > > > >
> > > > > I'll look into it a bit more later today.
> > > >
> > > > Please try the patch below. It kind of restores the previous behavior,
> > > > let's see if it changes anything.
> > >
> > > Again, no success. Just to make sure that I didn't screw anything. I
> > > have used just the following patch on top of the clean rc5 (your patch
> > > has failed with some rejects but I guess that the following should do
> > > the same):
> >
> > Thanks for testing. Did you try the second thing, ie. try to comment out the
> > acpi_enable_gpe() in drivers/acpi/wakeup.c:acpi_wakeup_device_init() and
> > retest (if you haven't done that already)?
>
> I have tried it just now and still no success.
>
>
> commit 1a7e7c9ac82aa2de185fa2f686417a5eb7765420
> Author: Michal Hocko <[email protected]>
> Date: Tue Apr 27 08:22:23 2010 +0200
>
> disable acpi_enable_gpe in acpi_wakeup_device_init
>
> as suggested by Rafael.
>
> diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
> index 4b9d339..a0f4859 100644
> --- a/drivers/acpi/wakeup.c
> +++ b/drivers/acpi/wakeup.c
> @@ -113,8 +113,9 @@ int __init acpi_wakeup_device_init(void)
> if (!dev->wakeup.flags.always_enabled ||
> dev->wakeup.state.enabled)
> continue;
> - acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
> + /*acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
> ACPI_GPE_TYPE_WAKE);
> + */
> dev->wakeup.state.enabled = 1;
> }
> mutex_unlock(&acpi_device_lock);
>
> On top of the previous patch. If I understand that correctly then this
> is really strange because this patch disables GPE completely, doesn't
> it?
Kind of, but this only happens for devices that are not handled by the button
driver which now has become our primary suspect.
Rafael
On Wednesday 28 April 2010, Rafael J. Wysocki wrote:
> On Tuesday 27 April 2010, Michal Hocko wrote:
> > On Mon 26-04-10 20:51:09, Rafael J. Wysocki wrote:
> > > On Monday 26 April 2010, Michal Hocko wrote:
> > > > On Sun 25-04-10 04:35:42, Rafael J. Wysocki wrote:
> > > > > On Monday 19 April 2010, Rafael J. Wysocki wrote:
> > > > > > On Monday 19 April 2010, Michal Hocko wrote:
> > > > > > > On Fri 16-04-10 20:00:29, Rafael J. Wysocki wrote:
> > > > > > > > On Wednesday 14 April 2010, Michal Hocko wrote:
> > > > > > > > > On Tue 13-04-10 22:53:37, Rafael J. Wysocki wrote:
> > > > > > > > > > On Tuesday 13 April 2010, Michal Hocko wrote:
> > > > > > > > > > > On Tue 13-04-10 01:01:54, Rafael J. Wysocki wrote:
> > > > > > > > > > > > On Saturday 10 April 2010, Rafael J. Wysocki wrote:
> > > > > > > > > > > > > On Friday 09 April 2010, Tony Vroon wrote:
> > > > > > > > > > > > > > On Fri, 2010-04-09 at 22:42 +0200, Rafael J. Wysocki wrote:
> > > > > > > > > > > > > > > Please check if the patch below changes anything.
> > > > > > > > > > > > > > > drivers/acpi/wakeup.c | 4 ++--
> > > > > > > > > > > > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > That didn't change the behaviour for me, sorry.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Well, I would be sorry if it did, because the patch removed some useful code. :-)
> > > > > > > > > > > > >
> > > > > > > > > > > > > > (I made sure to go through a full power down session before trying the
> > > > > > > > > > > > > > patched kernel)
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks for testing. So it looks like we don't disable the GPE during power off.
> > > > > > > > > > > > >
> > > > > > > > > > > > > I'll try to figure out what's going on, please stay tuned.
> > > > > > > > > > > >
> > > > > > > > > > > > Can you please check if the patch below changes the behavior?
> > > > > > > > > > >
> > > > > > > > > > > Unfortunately, it didn't help either (I have tried on top of the fresh
> > > > > > > > > > > rc4).
> > > > > > > > > >
> > > > > > > > > > That gets really weird.
> > > > > > > > > >
> > > > > > > > > > > > ---
> > > > > > > > > > > > drivers/acpi/wakeup.c | 6 +++---
> > > > > > > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > > > > > > > >
> > > > > > > > > > > > Index: linux-2.6/drivers/acpi/wakeup.c
> > > > > > > > > > > > ===================================================================
> > > > > > > > > > > > --- linux-2.6.orig/drivers/acpi/wakeup.c
> > > > > > > > > > > > +++ linux-2.6/drivers/acpi/wakeup.c
> > > > > > > > > > > > @@ -63,17 +63,17 @@ void acpi_enable_wakeup_device(u8 sleep_
> > > > > > > > > > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > > > > > > > > > struct acpi_device *dev =
> > > > > > > > > > > > container_of(node, struct acpi_device, wakeup_list);
> > > > > > > > > > > > + u8 action = ACPI_GPE_ENABLE;
> > > > > > > > > >
> > > > > > > > > > Can you try to change the above to ACPI_GPE_DISABLE and retest, please?
> > > > > > > > >
> > > > > > > > > Unfortunately didn't help as well...
> > > > > > > > > Just for reference:
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
> > > > > > > > > index 248b473..f23c08f 100644
> > > > > > > > > --- a/drivers/acpi/wakeup.c
> > > > > > > > > +++ b/drivers/acpi/wakeup.c
> > > > > > > > > @@ -63,7 +63,7 @@ void acpi_enable_wakeup_device(u8 sleep_state)
> > > > > > > > > list_for_each_safe(node, next, &acpi_wakeup_device_list) {
> > > > > > > > > struct acpi_device *dev =
> > > > > > > > > container_of(node, struct acpi_device, wakeup_list);
> > > > > > > > > - u8 action = ACPI_GPE_ENABLE;
> > > > > > > > > + u8 action = ACPI_GPE_DISABLE;
> > > > > > > >
> > > > > > > > That probably means the chipset enables the GPEs by itself _after_ we've
> > > > > > > > disabled them in acpi_enable_wakeup_device().
> > > > > > >
> > > > > > > Is this something BIOS specific?
> > > > > > >
> > > > > > > >
> > > > > > > > Unfortunately, I can't reproduce the issue on any of my test boxes and it's
> > > > > > > > hard to find the source of the problem staring at the code.
> > > > > > >
> > > > > > > Are there any debug options I can turn on to provide some information?
> > > > > >
> > > > > > We can only check what the kernel tells us before power off, but all that seems
> > > > > > correct.
> > > > > >
> > > > > > > Btw. what exactly does this mean? In what state is the laptop while it
> > > > > > > is turned off and GPE is enabled?
> > > > > >
> > > > > > If a GPE is enabled, then some part of the chipset has power provided so that
> > > > > > it can signal wakeup.
> > > > > >
> > > > > > I'll look into it a bit more later today.
> > > > >
> > > > > Please try the patch below. It kind of restores the previous behavior,
> > > > > let's see if it changes anything.
> > > >
> > > > Again, no success. Just to make sure that I didn't screw anything. I
> > > > have used just the following patch on top of the clean rc5 (your patch
> > > > has failed with some rejects but I guess that the following should do
> > > > the same):
> > >
> > > Thanks for testing. Did you try the second thing, ie. try to comment out the
> > > acpi_enable_gpe() in drivers/acpi/wakeup.c:acpi_wakeup_device_init() and
> > > retest (if you haven't done that already)?
> >
> > I have tried it just now and still no success.
> >
> >
> > commit 1a7e7c9ac82aa2de185fa2f686417a5eb7765420
> > Author: Michal Hocko <[email protected]>
> > Date: Tue Apr 27 08:22:23 2010 +0200
> >
> > disable acpi_enable_gpe in acpi_wakeup_device_init
> >
> > as suggested by Rafael.
> >
> > diff --git a/drivers/acpi/wakeup.c b/drivers/acpi/wakeup.c
> > index 4b9d339..a0f4859 100644
> > --- a/drivers/acpi/wakeup.c
> > +++ b/drivers/acpi/wakeup.c
> > @@ -113,8 +113,9 @@ int __init acpi_wakeup_device_init(void)
> > if (!dev->wakeup.flags.always_enabled ||
> > dev->wakeup.state.enabled)
> > continue;
> > - acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
> > + /*acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number,
> > ACPI_GPE_TYPE_WAKE);
> > + */
> > dev->wakeup.state.enabled = 1;
> > }
> > mutex_unlock(&acpi_device_lock);
> >
> > On top of the previous patch. If I understand that correctly then this
> > is really strange because this patch disables GPE completely, doesn't
> > it?
>
> Kind of, but this only happens for devices that are not handled by the button
> driver which now has become our primary suspect.
I'm thinking at the moment that we probably enable something at the hardware
level that we didn't before.
Why it is not _disabled_ before power off although there are all of the
necessary things in the code remains to be a mystery to me.
Rafael
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 26/04/10 17:22, Len Brown wrote:
> During this period after poweroff and LCD goes out, but before
> lid close, are there any other signs of life? eg. does caps
> lock button light the caps LED?
The caps lock button does not respond, no.
> How about during this period if you press the power button and
> hold it for 5 seconds, does that cause the HDD parking sound?
Holding down the power button for 5 seconds has no audible or visible
effects, but does indeed properly shut it down. Closing or opening the
lid afterwards does not cause a phantom power-up.
A short push of the power button seems to start the machine up normally
(from a cold boot; just like when the lid is closed/opened after a linux
"shutdown").
> Please send the /proc/acpi/wakeup file before and after
> the regression.
I don't have this file in my /proc filesystem, sorry.
Regards,
- --
Tony Vroon
UNIX systems administrator
London Internet Exchange Ltd, Trinity Court, Trinity Street,
Peterborough, PE1 1DA
Registered in England number 3137929
E-Mail: [email protected]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAkvZmdYACgkQp5vW4rUFj5oUxgCdGPnfQsSE9jhjSzzar/kQbw95
Er4AmgLGStZlrvGZltqsU/REs2srO95A
=6VLH
-----END PGP SIGNATURE-----