* [PATCH] ACPI : do not use Lid and Sleep button for S5 wakeup
@ 2012-12-03 8:15 Zhang Rui
2012-12-04 0:02 ` Rafael J. Wysocki
0 siblings, 1 reply; 6+ messages in thread
From: Zhang Rui @ 2012-12-03 8:15 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: ACPI Devel Maling List, anisse, Zhang, Rui
>From 3e7b4da3783d200f35568f72b3b25f16df546ffe Mon Sep 17 00:00:00 2001
From: Zhang Rui <rui.zhang@intel.com>
Date: Fri, 30 Nov 2012 14:35:43 +0800
Subject: [PATCH] ACPI : do not use Lid and Sleep button for S5 wakeup
When system enters power off, the _PSW of Lid device is enabled.
But this may cause the system to reboot instead of power off.
https://bugzilla.kernel.org/show_bug.cgi?id=35262
A proper way to fix this is to always disable lid wakeup capability
for S5.
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/acpi/scan.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index d1ecca2..f20020a 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -807,8 +807,8 @@ acpi_bus_extract_wakeup_device_power_package(acpi_handle handle,
static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
{
struct acpi_device_id button_device_ids[] = {
- {"PNP0C0D", 0},
{"PNP0C0C", 0},
+ {"PNP0C0D", 0},
{"PNP0C0E", 0},
{"", 0},
};
@@ -820,6 +820,11 @@ static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
/* Power button, Lid switch always enable wakeup */
if (!acpi_match_device_ids(device, button_device_ids)) {
device->wakeup.flags.run_wake = 1;
+ if (!acpi_match_device_ids(device, &button_device_ids[1])) {
+ /* Do not use Lid/sleep button for S5 wakeup */
+ if (device->wakeup.gpe_number == 5)
+ device->wakeup.gpe_number = 4;
+ }
device_set_wakeup_capable(&device->dev, true);
return;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI : do not use Lid and Sleep button for S5 wakeup
2012-12-03 8:15 [PATCH] ACPI : do not use Lid and Sleep button for S5 wakeup Zhang Rui
@ 2012-12-04 0:02 ` Rafael J. Wysocki
2012-12-04 16:06 ` Zhang Rui
0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2012-12-04 0:02 UTC (permalink / raw)
To: Zhang Rui; +Cc: linux-acpi, anisse
On Monday, December 03, 2012 04:15:06 PM Zhang Rui wrote:
> From 3e7b4da3783d200f35568f72b3b25f16df546ffe Mon Sep 17 00:00:00 2001
> From: Zhang Rui <rui.zhang@intel.com>
> Date: Fri, 30 Nov 2012 14:35:43 +0800
> Subject: [PATCH] ACPI : do not use Lid and Sleep button for S5 wakeup
>
> When system enters power off, the _PSW of Lid device is enabled.
> But this may cause the system to reboot instead of power off.
>
> https://bugzilla.kernel.org/show_bug.cgi?id=35262
>
> A proper way to fix this is to always disable lid wakeup capability
> for S5.
While I understand the motivation, quite frankly I don't understand the patch. :-)
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
> drivers/acpi/scan.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index d1ecca2..f20020a 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -807,8 +807,8 @@ acpi_bus_extract_wakeup_device_power_package(acpi_handle handle,
> static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
> {
> struct acpi_device_id button_device_ids[] = {
> - {"PNP0C0D", 0},
> {"PNP0C0C", 0},
> + {"PNP0C0D", 0},
> {"PNP0C0E", 0},
> {"", 0},
> };
> @@ -820,6 +820,11 @@ static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
> /* Power button, Lid switch always enable wakeup */
> if (!acpi_match_device_ids(device, button_device_ids)) {
> device->wakeup.flags.run_wake = 1;
> + if (!acpi_match_device_ids(device, &button_device_ids[1])) {
> + /* Do not use Lid/sleep button for S5 wakeup */
> + if (device->wakeup.gpe_number == 5)
> + device->wakeup.gpe_number = 4;
Why do you want to change the wakeup GPE number for those devices? It appears
to be based on some extra knowledge that should be documented.
Moreover, this doesn't look like the right thing to do anyway. Shouldn't we
just change device->wakeup.sleep_state to ACPI_STATE_S4 (if it was S5) instead?
> + }
> device_set_wakeup_capable(&device->dev, true);
> return;
> }
>
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI : do not use Lid and Sleep button for S5 wakeup
2012-12-04 0:02 ` Rafael J. Wysocki
@ 2012-12-04 16:06 ` Zhang Rui
2012-12-10 13:51 ` Anisse Astier
0 siblings, 1 reply; 6+ messages in thread
From: Zhang Rui @ 2012-12-04 16:06 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: linux-acpi, anisse
On Tue, 2012-12-04 at 01:02 +0100, Rafael J. Wysocki wrote:
> On Monday, December 03, 2012 04:15:06 PM Zhang Rui wrote:
> > From 3e7b4da3783d200f35568f72b3b25f16df546ffe Mon Sep 17 00:00:00 2001
> > From: Zhang Rui <rui.zhang@intel.com>
> > Date: Fri, 30 Nov 2012 14:35:43 +0800
> > Subject: [PATCH] ACPI : do not use Lid and Sleep button for S5 wakeup
> >
> > When system enters power off, the _PSW of Lid device is enabled.
> > But this may cause the system to reboot instead of power off.
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=35262
> >
> > A proper way to fix this is to always disable lid wakeup capability
> > for S5.
>
> While I understand the motivation, quite frankly I don't understand the patch. :-)
>
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > ---
> > drivers/acpi/scan.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index d1ecca2..f20020a 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -807,8 +807,8 @@ acpi_bus_extract_wakeup_device_power_package(acpi_handle handle,
> > static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
> > {
> > struct acpi_device_id button_device_ids[] = {
> > - {"PNP0C0D", 0},
> > {"PNP0C0C", 0},
> > + {"PNP0C0D", 0},
> > {"PNP0C0E", 0},
> > {"", 0},
> > };
> > @@ -820,6 +820,11 @@ static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
> > /* Power button, Lid switch always enable wakeup */
> > if (!acpi_match_device_ids(device, button_device_ids)) {
> > device->wakeup.flags.run_wake = 1;
> > + if (!acpi_match_device_ids(device, &button_device_ids[1])) {
> > + /* Do not use Lid/sleep button for S5 wakeup */
> > + if (device->wakeup.gpe_number == 5)
> > + device->wakeup.gpe_number = 4;
>
> Why do you want to change the wakeup GPE number for those devices? It appears
> to be based on some extra knowledge that should be documented.
>
> Moreover, this doesn't look like the right thing to do anyway. Shouldn't we
> just change device->wakeup.sleep_state to ACPI_STATE_S4 (if it was S5) instead?
oops, this is really embarrassing.
I made a stupid mistake in this patch, and you are right that I was
trying to override the device->wakeup.sleep_state to ACPI_STATE_S4.
refreshed patch attached.
Patch has been test by faking a lid device in custom DSDT table.
But I still prefer to get the test result in the bug report,
before pushing it upstream.
>From 64e16e442b7d33801b5c1a41cf5c87d4d8ff1e10 Mon Sep 17 00:00:00 2001
From: Zhang Rui <rui.zhang@intel.com>
Date: Fri, 30 Nov 2012 14:35:43 +0800
Subject: [PATCH 1/2] ACPI : do not use Lid and Sleep button for S5 wakeup
When system enters power off, the _PSW of Lid device is enabled.
But this may cause the system to reboot instead of power off.
A proper way to fix this is to always disable lid wakeup capability for S5.
https://bugzilla.kernel.org/show_bug.cgi?id=35262
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
drivers/acpi/scan.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index d1ecca2..35674c2 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -807,8 +807,8 @@ acpi_bus_extract_wakeup_device_power_package(acpi_handle handle,
static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
{
struct acpi_device_id button_device_ids[] = {
- {"PNP0C0D", 0},
{"PNP0C0C", 0},
+ {"PNP0C0D", 0},
{"PNP0C0E", 0},
{"", 0},
};
@@ -820,6 +820,11 @@ static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
/* Power button, Lid switch always enable wakeup */
if (!acpi_match_device_ids(device, button_device_ids)) {
device->wakeup.flags.run_wake = 1;
+ if (!acpi_match_device_ids(device, &button_device_ids[1])) {
+ /* Do not use Lid/sleep button for S5 wakeup */
+ if (device->wakeup.sleep_state == ACPI_STATE_S5)
+ device->wakeup.sleep_state = ACPI_STATE_S4;
+ }
device_set_wakeup_capable(&device->dev, true);
return;
}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI : do not use Lid and Sleep button for S5 wakeup
2012-12-04 16:06 ` Zhang Rui
@ 2012-12-10 13:51 ` Anisse Astier
2012-12-10 14:44 ` Rafael J. Wysocki
2012-12-11 0:50 ` Zhang Rui
0 siblings, 2 replies; 6+ messages in thread
From: Anisse Astier @ 2012-12-10 13:51 UTC (permalink / raw)
To: Zhang Rui; +Cc: Rafael J. Wysocki, linux-acpi
On Wed, 05 Dec 2012 00:06:46 +0800, Zhang Rui <rui.zhang@intel.com> wrote :
> On Tue, 2012-12-04 at 01:02 +0100, Rafael J. Wysocki wrote:
> > On Monday, December 03, 2012 04:15:06 PM Zhang Rui wrote:
> > > From 3e7b4da3783d200f35568f72b3b25f16df546ffe Mon Sep 17 00:00:00 2001
> > > From: Zhang Rui <rui.zhang@intel.com>
> > > Date: Fri, 30 Nov 2012 14:35:43 +0800
> > > Subject: [PATCH] ACPI : do not use Lid and Sleep button for S5 wakeup
> > >
> > > When system enters power off, the _PSW of Lid device is enabled.
> > > But this may cause the system to reboot instead of power off.
> > >
> > > https://bugzilla.kernel.org/show_bug.cgi?id=35262
> > >
> > > A proper way to fix this is to always disable lid wakeup capability
> > > for S5.
> >
> > While I understand the motivation, quite frankly I don't understand the patch. :-)
> >
> > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > > ---
> > > drivers/acpi/scan.c | 7 ++++++-
> > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > > index d1ecca2..f20020a 100644
> > > --- a/drivers/acpi/scan.c
> > > +++ b/drivers/acpi/scan.c
> > > @@ -807,8 +807,8 @@ acpi_bus_extract_wakeup_device_power_package(acpi_handle handle,
> > > static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
> > > {
> > > struct acpi_device_id button_device_ids[] = {
> > > - {"PNP0C0D", 0},
> > > {"PNP0C0C", 0},
> > > + {"PNP0C0D", 0},
> > > {"PNP0C0E", 0},
> > > {"", 0},
> > > };
> > > @@ -820,6 +820,11 @@ static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
> > > /* Power button, Lid switch always enable wakeup */
> > > if (!acpi_match_device_ids(device, button_device_ids)) {
> > > device->wakeup.flags.run_wake = 1;
> > > + if (!acpi_match_device_ids(device, &button_device_ids[1])) {
> > > + /* Do not use Lid/sleep button for S5 wakeup */
> > > + if (device->wakeup.gpe_number == 5)
> > > + device->wakeup.gpe_number = 4;
> >
> > Why do you want to change the wakeup GPE number for those devices? It appears
> > to be based on some extra knowledge that should be documented.
> >
> > Moreover, this doesn't look like the right thing to do anyway. Shouldn't we
> > just change device->wakeup.sleep_state to ACPI_STATE_S4 (if it was S5) instead?
>
> oops, this is really embarrassing.
> I made a stupid mistake in this patch, and you are right that I was
> trying to override the device->wakeup.sleep_state to ACPI_STATE_S4.
>
> refreshed patch attached.
> Patch has been test by faking a lid device in custom DSDT table.
> But I still prefer to get the test result in the bug report,
> before pushing it upstream.
>
> From 64e16e442b7d33801b5c1a41cf5c87d4d8ff1e10 Mon Sep 17 00:00:00 2001
> From: Zhang Rui <rui.zhang@intel.com>
> Date: Fri, 30 Nov 2012 14:35:43 +0800
> Subject: [PATCH 1/2] ACPI : do not use Lid and Sleep button for S5 wakeup
>
> When system enters power off, the _PSW of Lid device is enabled.
> But this may cause the system to reboot instead of power off.
>
> A proper way to fix this is to always disable lid wakeup capability for S5.
> https://bugzilla.kernel.org/show_bug.cgi?id=35262
>
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
It works ! (On a Toshiba C870-12N, which exhibits the problem).
Tested-by: Anisse Astier <anisse@astier.eu>
> ---
> drivers/acpi/scan.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index d1ecca2..35674c2 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -807,8 +807,8 @@ acpi_bus_extract_wakeup_device_power_package(acpi_handle handle,
> static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
> {
> struct acpi_device_id button_device_ids[] = {
> - {"PNP0C0D", 0},
> {"PNP0C0C", 0},
> + {"PNP0C0D", 0},
> {"PNP0C0E", 0},
> {"", 0},
> };
Why do you need to change the device order ? Is just cosmetic ?
(alphabetical)
> @@ -820,6 +820,11 @@ static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
> /* Power button, Lid switch always enable wakeup */
> if (!acpi_match_device_ids(device, button_device_ids)) {
> device->wakeup.flags.run_wake = 1;
> + if (!acpi_match_device_ids(device, &button_device_ids[1])) {
> + /* Do not use Lid/sleep button for S5 wakeup */
> + if (device->wakeup.sleep_state == ACPI_STATE_S5)
> + device->wakeup.sleep_state = ACPI_STATE_S4;
> + }
> device_set_wakeup_capable(&device->dev, true);
> return;
> }
What if someone wants to use power-on with LID ? Is it an accepted use
case ?
Regards,
Anisse
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI : do not use Lid and Sleep button for S5 wakeup
2012-12-10 13:51 ` Anisse Astier
@ 2012-12-10 14:44 ` Rafael J. Wysocki
2012-12-11 0:50 ` Zhang Rui
1 sibling, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2012-12-10 14:44 UTC (permalink / raw)
To: Anisse Astier; +Cc: Zhang Rui, linux-acpi
On Monday, December 10, 2012 02:51:23 PM Anisse Astier wrote:
> On Wed, 05 Dec 2012 00:06:46 +0800, Zhang Rui <rui.zhang@intel.com> wrote :
>
> > On Tue, 2012-12-04 at 01:02 +0100, Rafael J. Wysocki wrote:
> > > On Monday, December 03, 2012 04:15:06 PM Zhang Rui wrote:
> > > > From 3e7b4da3783d200f35568f72b3b25f16df546ffe Mon Sep 17 00:00:00 2001
> > > > From: Zhang Rui <rui.zhang@intel.com>
> > > > Date: Fri, 30 Nov 2012 14:35:43 +0800
> > > > Subject: [PATCH] ACPI : do not use Lid and Sleep button for S5 wakeup
> > > >
> > > > When system enters power off, the _PSW of Lid device is enabled.
> > > > But this may cause the system to reboot instead of power off.
> > > >
> > > > https://bugzilla.kernel.org/show_bug.cgi?id=35262
> > > >
> > > > A proper way to fix this is to always disable lid wakeup capability
> > > > for S5.
> > >
> > > While I understand the motivation, quite frankly I don't understand the patch. :-)
> > >
> > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > > > ---
> > > > drivers/acpi/scan.c | 7 ++++++-
> > > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > > > index d1ecca2..f20020a 100644
> > > > --- a/drivers/acpi/scan.c
> > > > +++ b/drivers/acpi/scan.c
> > > > @@ -807,8 +807,8 @@ acpi_bus_extract_wakeup_device_power_package(acpi_handle handle,
> > > > static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
> > > > {
> > > > struct acpi_device_id button_device_ids[] = {
> > > > - {"PNP0C0D", 0},
> > > > {"PNP0C0C", 0},
> > > > + {"PNP0C0D", 0},
> > > > {"PNP0C0E", 0},
> > > > {"", 0},
> > > > };
> > > > @@ -820,6 +820,11 @@ static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
> > > > /* Power button, Lid switch always enable wakeup */
> > > > if (!acpi_match_device_ids(device, button_device_ids)) {
> > > > device->wakeup.flags.run_wake = 1;
> > > > + if (!acpi_match_device_ids(device, &button_device_ids[1])) {
> > > > + /* Do not use Lid/sleep button for S5 wakeup */
> > > > + if (device->wakeup.gpe_number == 5)
> > > > + device->wakeup.gpe_number = 4;
> > >
> > > Why do you want to change the wakeup GPE number for those devices? It appears
> > > to be based on some extra knowledge that should be documented.
> > >
> > > Moreover, this doesn't look like the right thing to do anyway. Shouldn't we
> > > just change device->wakeup.sleep_state to ACPI_STATE_S4 (if it was S5) instead?
> >
> > oops, this is really embarrassing.
> > I made a stupid mistake in this patch, and you are right that I was
> > trying to override the device->wakeup.sleep_state to ACPI_STATE_S4.
> >
> > refreshed patch attached.
> > Patch has been test by faking a lid device in custom DSDT table.
> > But I still prefer to get the test result in the bug report,
> > before pushing it upstream.
> >
> > From 64e16e442b7d33801b5c1a41cf5c87d4d8ff1e10 Mon Sep 17 00:00:00 2001
> > From: Zhang Rui <rui.zhang@intel.com>
> > Date: Fri, 30 Nov 2012 14:35:43 +0800
> > Subject: [PATCH 1/2] ACPI : do not use Lid and Sleep button for S5 wakeup
> >
> > When system enters power off, the _PSW of Lid device is enabled.
> > But this may cause the system to reboot instead of power off.
> >
> > A proper way to fix this is to always disable lid wakeup capability for S5.
> > https://bugzilla.kernel.org/show_bug.cgi?id=35262
> >
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
>
> It works ! (On a Toshiba C870-12N, which exhibits the problem).
>
> Tested-by: Anisse Astier <anisse@astier.eu>
>
The patch has been queued up for v3.8 and -stable.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI : do not use Lid and Sleep button for S5 wakeup
2012-12-10 13:51 ` Anisse Astier
2012-12-10 14:44 ` Rafael J. Wysocki
@ 2012-12-11 0:50 ` Zhang Rui
1 sibling, 0 replies; 6+ messages in thread
From: Zhang Rui @ 2012-12-11 0:50 UTC (permalink / raw)
To: Anisse Astier; +Cc: Rafael J. Wysocki, linux-acpi
On Mon, 2012-12-10 at 14:51 +0100, Anisse Astier wrote:
> On Wed, 05 Dec 2012 00:06:46 +0800, Zhang Rui <rui.zhang@intel.com> wrote :
>
> > On Tue, 2012-12-04 at 01:02 +0100, Rafael J. Wysocki wrote:
> > > On Monday, December 03, 2012 04:15:06 PM Zhang Rui wrote:
> > > > From 3e7b4da3783d200f35568f72b3b25f16df546ffe Mon Sep 17 00:00:00 2001
> > > > From: Zhang Rui <rui.zhang@intel.com>
> > > > Date: Fri, 30 Nov 2012 14:35:43 +0800
> > > > Subject: [PATCH] ACPI : do not use Lid and Sleep button for S5 wakeup
> > > >
> > > > When system enters power off, the _PSW of Lid device is enabled.
> > > > But this may cause the system to reboot instead of power off.
> > > >
> > > > https://bugzilla.kernel.org/show_bug.cgi?id=35262
> > > >
> > > > A proper way to fix this is to always disable lid wakeup capability
> > > > for S5.
> > >
> > > While I understand the motivation, quite frankly I don't understand the patch. :-)
> > >
> > > > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> > > > ---
> > > > drivers/acpi/scan.c | 7 ++++++-
> > > > 1 file changed, 6 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > > > index d1ecca2..f20020a 100644
> > > > --- a/drivers/acpi/scan.c
> > > > +++ b/drivers/acpi/scan.c
> > > > @@ -807,8 +807,8 @@ acpi_bus_extract_wakeup_device_power_package(acpi_handle handle,
> > > > static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
> > > > {
> > > > struct acpi_device_id button_device_ids[] = {
> > > > - {"PNP0C0D", 0},
> > > > {"PNP0C0C", 0},
> > > > + {"PNP0C0D", 0},
> > > > {"PNP0C0E", 0},
> > > > {"", 0},
> > > > };
> > > > @@ -820,6 +820,11 @@ static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
> > > > /* Power button, Lid switch always enable wakeup */
> > > > if (!acpi_match_device_ids(device, button_device_ids)) {
> > > > device->wakeup.flags.run_wake = 1;
> > > > + if (!acpi_match_device_ids(device, &button_device_ids[1])) {
> > > > + /* Do not use Lid/sleep button for S5 wakeup */
> > > > + if (device->wakeup.gpe_number == 5)
> > > > + device->wakeup.gpe_number = 4;
> > >
> > > Why do you want to change the wakeup GPE number for those devices? It appears
> > > to be based on some extra knowledge that should be documented.
> > >
> > > Moreover, this doesn't look like the right thing to do anyway. Shouldn't we
> > > just change device->wakeup.sleep_state to ACPI_STATE_S4 (if it was S5) instead?
> >
> > oops, this is really embarrassing.
> > I made a stupid mistake in this patch, and you are right that I was
> > trying to override the device->wakeup.sleep_state to ACPI_STATE_S4.
> >
> > refreshed patch attached.
> > Patch has been test by faking a lid device in custom DSDT table.
> > But I still prefer to get the test result in the bug report,
> > before pushing it upstream.
> >
> > From 64e16e442b7d33801b5c1a41cf5c87d4d8ff1e10 Mon Sep 17 00:00:00 2001
> > From: Zhang Rui <rui.zhang@intel.com>
> > Date: Fri, 30 Nov 2012 14:35:43 +0800
> > Subject: [PATCH 1/2] ACPI : do not use Lid and Sleep button for S5 wakeup
> >
> > When system enters power off, the _PSW of Lid device is enabled.
> > But this may cause the system to reboot instead of power off.
> >
> > A proper way to fix this is to always disable lid wakeup capability for S5.
> > https://bugzilla.kernel.org/show_bug.cgi?id=35262
> >
> > Signed-off-by: Zhang Rui <rui.zhang@intel.com>
>
> It works ! (On a Toshiba C870-12N, which exhibits the problem).
>
> Tested-by: Anisse Astier <anisse@astier.eu>
>
>
> > ---
> > drivers/acpi/scan.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index d1ecca2..35674c2 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -807,8 +807,8 @@ acpi_bus_extract_wakeup_device_power_package(acpi_handle handle,
> > static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
> > {
> > struct acpi_device_id button_device_ids[] = {
> > - {"PNP0C0D", 0},
> > {"PNP0C0C", 0},
> > + {"PNP0C0D", 0},
> > {"PNP0C0E", 0},
> > {"", 0},
> > };
>
> Why do you need to change the device order ? Is just cosmetic ?
> (alphabetical)
>
this makes it easy to match the sleep button and lid devices, of which I
want to disable the wakeup ability.
> > @@ -820,6 +820,11 @@ static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
> > /* Power button, Lid switch always enable wakeup */
> > if (!acpi_match_device_ids(device, button_device_ids)) {
> > device->wakeup.flags.run_wake = 1;
> > + if (!acpi_match_device_ids(device, &button_device_ids[1])) {
> > + /* Do not use Lid/sleep button for S5 wakeup */
> > + if (device->wakeup.sleep_state == ACPI_STATE_S5)
> > + device->wakeup.sleep_state = ACPI_STATE_S4;
> > + }
> > device_set_wakeup_capable(&device->dev, true);
> > return;
> > }
>
> What if someone wants to use power-on with LID ? Is it an accepted use
> case ?
>
I do not think it is reasonable to power on a system with Lid
close/open.
thanks,
rui
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-12-11 0:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-03 8:15 [PATCH] ACPI : do not use Lid and Sleep button for S5 wakeup Zhang Rui
2012-12-04 0:02 ` Rafael J. Wysocki
2012-12-04 16:06 ` Zhang Rui
2012-12-10 13:51 ` Anisse Astier
2012-12-10 14:44 ` Rafael J. Wysocki
2012-12-11 0:50 ` Zhang Rui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).