commit 9630bdd9 changes behavior of the poweroff - bug?
in [Kernel]
|
Prev: [PATCH] proc: Report file name on detected read_proc overflow
Next: [PATCH] hwmon: sht15: Fix sht15_calc_temp interpolation function
From: Rafael J. Wysocki on 9 Apr 2010 16:40 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);*/ } } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Tony Vroon on 9 Apr 2010 17:30 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: tony(a)linx.net
From: Rafael J. Wysocki on 10 Apr 2010 15:40 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Rafael J. Wysocki on 12 Apr 2010 18:10 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); } } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Michal Hocko on 13 Apr 2010 04:30
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |