Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-06 Thread Thomas Hänig



Am 06.07.2018 um 14:27 schrieb Takashi Iwai:
> On Fri, 06 Jul 2018 14:13:04 +0200,
> Rafael J. Wysocki wrote:
>>
>> On Friday, July 6, 2018 1:21:50 PM CEST Rafael J. Wysocki wrote:
>>> On Fri, Jul 6, 2018 at 1:12 PM, Thomas Hänig  wrote:
>>
>> [cut]
>>  
>>> So the latest patch:
>>>
>>> https://patchwork.kernel.org/patch/10511211/
>>>
>>> should work for you (please verify) and the change in
>>> drivers/acpi/sleep.c in it most likely is not necessary.
>>>
>>> If you can confirm that this one works for you, I'll send a smaller
>>> one with the acpi_hw_legacy_sleep() part alone.
>>
>> Well, scratch this, sorry.
>>
>> The power button probably is a fixed event and it won't be effected by
>> that patch.
>>
>> Instead, please test the patch below.
> 
> FWIW, the test kernel on OBS home:tiwai:bsc1099930-3 was refreshed
> with this one.  The release number will be *.g2351e2d.
> 
> 
> Takashi
> 
>>
>> ---
>>  drivers/acpi/acpica/hwsleep.c |   15 +++
>>  1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> Index: linux-pm/drivers/acpi/acpica/hwsleep.c
>> ===
>> --- linux-pm.orig/drivers/acpi/acpica/hwsleep.c
>> +++ linux-pm/drivers/acpi/acpica/hwsleep.c
>> @@ -51,16 +51,23 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
>>  return_ACPI_STATUS(status);
>>  }
>>  
>> -/*
>> - * 1) Disable all GPEs
>> - * 2) Enable all wakeup GPEs
>> - */
>> +/* Disable all GPEs */
>>  status = acpi_hw_disable_all_gpes();
>>  if (ACPI_FAILURE(status)) {
>>  return_ACPI_STATUS(status);
>>  }
>> +/*
>> + * If the target sleep state is S5, clear all GPEs and fixed events too
>> + */
>> +if (sleep_state == ACPI_STATE_S5) {
>> +status = acpi_hw_clear_acpi_status();
>> +if (ACPI_FAILURE(status)) {
>> +return_ACPI_STATUS(status);
>> +}
>> +}
>>  acpi_gbl_system_awake_and_running = FALSE;
>>  
>> + /* Enable all wakeup GPEs */
>>  status = acpi_hw_enable_all_wakeup_gpes();
>>  if (ACPI_FAILURE(status)) {
>>  return_ACPI_STATUS(status);
>>
Hello Takashi,
with the kernel built by you the system shuts down when pressing the
power button and stays off too!  :-)

So the above mentioned patch to hwsleep.c seems to do the trick.

Regards and thanks a lot @all!
Thomas

thomas@tslb:~> uname -a
Linux tslb 4.17.4-2.g2351e2d-default #1 SMP PREEMPT Fri Jul 6 12:24:35
UTC 2018 (2351e2d) x86_64 x86_64 x86_64 GNU/Linux


thomas@tslb:~> rpm -qi kernel-default-4.17.4-2.1.g2351e2d.x86_64
Name: kernel-default
Version : 4.17.4
Release : 2.1.g2351e2d
Architecture: x86_64
Install Date: Fr 06 Jul 2018 19:43:38 CEST
Group   : System/Kernel
Size: 360100489
License : GPL-2.0
Signature   : RSA/SHA256, Fr 06 Jul 2018 16:47:25 CEST, Key ID
4bf05f46f6e74bf5
Source RPM  : kernel-default-4.17.4-2.1.g2351e2d.nosrc.rpm
Build Date  : Fr 06 Jul 2018 16:42:04 CEST
Build Host  : lamb63
Relocations : (not relocatable)
Vendor  : obs://build.opensuse.org/home:tiwai
URL : http://www.kernel.org/
Summary : The Standard Kernel
Description :
The standard kernel for both uniprocessor and multiprocessor systems.

Source Timestamp: 2018-07-06 14:24:35 +0200
GIT Revision: 2351e2d70cbcb963c461d55abeee967ea9a940ea
GIT Branch: users/tiwai/stable/bsc1099930
Distribution: home:tiwai:bsc1099930-3


Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-06 Thread Thomas Hänig



Am 06.07.2018 um 14:27 schrieb Takashi Iwai:
> On Fri, 06 Jul 2018 14:13:04 +0200,
> Rafael J. Wysocki wrote:
>>
>> On Friday, July 6, 2018 1:21:50 PM CEST Rafael J. Wysocki wrote:
>>> On Fri, Jul 6, 2018 at 1:12 PM, Thomas Hänig  wrote:
>>
>> [cut]
>>  
>>> So the latest patch:
>>>
>>> https://patchwork.kernel.org/patch/10511211/
>>>
>>> should work for you (please verify) and the change in
>>> drivers/acpi/sleep.c in it most likely is not necessary.
>>>
>>> If you can confirm that this one works for you, I'll send a smaller
>>> one with the acpi_hw_legacy_sleep() part alone.
>>
>> Well, scratch this, sorry.
>>
>> The power button probably is a fixed event and it won't be effected by
>> that patch.
>>
>> Instead, please test the patch below.
> 
> FWIW, the test kernel on OBS home:tiwai:bsc1099930-3 was refreshed
> with this one.  The release number will be *.g2351e2d.
> 
> 
> Takashi
> 
>>
>> ---
>>  drivers/acpi/acpica/hwsleep.c |   15 +++
>>  1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> Index: linux-pm/drivers/acpi/acpica/hwsleep.c
>> ===
>> --- linux-pm.orig/drivers/acpi/acpica/hwsleep.c
>> +++ linux-pm/drivers/acpi/acpica/hwsleep.c
>> @@ -51,16 +51,23 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
>>  return_ACPI_STATUS(status);
>>  }
>>  
>> -/*
>> - * 1) Disable all GPEs
>> - * 2) Enable all wakeup GPEs
>> - */
>> +/* Disable all GPEs */
>>  status = acpi_hw_disable_all_gpes();
>>  if (ACPI_FAILURE(status)) {
>>  return_ACPI_STATUS(status);
>>  }
>> +/*
>> + * If the target sleep state is S5, clear all GPEs and fixed events too
>> + */
>> +if (sleep_state == ACPI_STATE_S5) {
>> +status = acpi_hw_clear_acpi_status();
>> +if (ACPI_FAILURE(status)) {
>> +return_ACPI_STATUS(status);
>> +}
>> +}
>>  acpi_gbl_system_awake_and_running = FALSE;
>>  
>> + /* Enable all wakeup GPEs */
>>  status = acpi_hw_enable_all_wakeup_gpes();
>>  if (ACPI_FAILURE(status)) {
>>  return_ACPI_STATUS(status);
>>
Hello Takashi,
with the kernel built by you the system shuts down when pressing the
power button and stays off too!  :-)

So the above mentioned patch to hwsleep.c seems to do the trick.

Regards and thanks a lot @all!
Thomas

thomas@tslb:~> uname -a
Linux tslb 4.17.4-2.g2351e2d-default #1 SMP PREEMPT Fri Jul 6 12:24:35
UTC 2018 (2351e2d) x86_64 x86_64 x86_64 GNU/Linux


thomas@tslb:~> rpm -qi kernel-default-4.17.4-2.1.g2351e2d.x86_64
Name: kernel-default
Version : 4.17.4
Release : 2.1.g2351e2d
Architecture: x86_64
Install Date: Fr 06 Jul 2018 19:43:38 CEST
Group   : System/Kernel
Size: 360100489
License : GPL-2.0
Signature   : RSA/SHA256, Fr 06 Jul 2018 16:47:25 CEST, Key ID
4bf05f46f6e74bf5
Source RPM  : kernel-default-4.17.4-2.1.g2351e2d.nosrc.rpm
Build Date  : Fr 06 Jul 2018 16:42:04 CEST
Build Host  : lamb63
Relocations : (not relocatable)
Vendor  : obs://build.opensuse.org/home:tiwai
URL : http://www.kernel.org/
Summary : The Standard Kernel
Description :
The standard kernel for both uniprocessor and multiprocessor systems.

Source Timestamp: 2018-07-06 14:24:35 +0200
GIT Revision: 2351e2d70cbcb963c461d55abeee967ea9a940ea
GIT Branch: users/tiwai/stable/bsc1099930
Distribution: home:tiwai:bsc1099930-3


Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-06 Thread Thomas Hänig
Am 06.07.2018 um 14:27 schrieb Takashi Iwai:
> On Fri, 06 Jul 2018 14:13:04 +0200,
> Rafael J. Wysocki wrote:
>>
>> On Friday, July 6, 2018 1:21:50 PM CEST Rafael J. Wysocki wrote:
>>> On Fri, Jul 6, 2018 at 1:12 PM, Thomas Hänig  wrote:
>>
>> [cut]
>>  
>>> So the latest patch:
>>>
>>> https://patchwork.kernel.org/patch/10511211/
>>>
>>> should work for you (please verify) and the change in
>>> drivers/acpi/sleep.c in it most likely is not necessary.
>>>
>>> If you can confirm that this one works for you, I'll send a smaller
>>> one with the acpi_hw_legacy_sleep() part alone.
>>
>> Well, scratch this, sorry.
>>
>> The power button probably is a fixed event and it won't be effected by
>> that patch.
>>
>> Instead, please test the patch below.
> 
> FWIW, the test kernel on OBS home:tiwai:bsc1099930-3 was refreshed
> with this one.  The release number will be *.g2351e2d.
> 
> 
> Takashi

to make shure I have made no mistake while building my own, I will try
out your kernel as well.

Thomas


Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-06 Thread Thomas Hänig
Am 06.07.2018 um 14:27 schrieb Takashi Iwai:
> On Fri, 06 Jul 2018 14:13:04 +0200,
> Rafael J. Wysocki wrote:
>>
>> On Friday, July 6, 2018 1:21:50 PM CEST Rafael J. Wysocki wrote:
>>> On Fri, Jul 6, 2018 at 1:12 PM, Thomas Hänig  wrote:
>>
>> [cut]
>>  
>>> So the latest patch:
>>>
>>> https://patchwork.kernel.org/patch/10511211/
>>>
>>> should work for you (please verify) and the change in
>>> drivers/acpi/sleep.c in it most likely is not necessary.
>>>
>>> If you can confirm that this one works for you, I'll send a smaller
>>> one with the acpi_hw_legacy_sleep() part alone.
>>
>> Well, scratch this, sorry.
>>
>> The power button probably is a fixed event and it won't be effected by
>> that patch.
>>
>> Instead, please test the patch below.
> 
> FWIW, the test kernel on OBS home:tiwai:bsc1099930-3 was refreshed
> with this one.  The release number will be *.g2351e2d.
> 
> 
> Takashi

to make shure I have made no mistake while building my own, I will try
out your kernel as well.

Thomas


Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-06 Thread Thomas Hänig



Am 06.07.2018 um 14:13 schrieb Rafael J. Wysocki:
> On Friday, July 6, 2018 1:21:50 PM CEST Rafael J. Wysocki wrote:
>> On Fri, Jul 6, 2018 at 1:12 PM, Thomas Hänig  wrote:
> 
> [cut]
>  
>> So the latest patch:
>>
>> https://patchwork.kernel.org/patch/10511211/
>>
>> should work for you (please verify) and the change in
>> drivers/acpi/sleep.c in it most likely is not necessary.
>>
>> If you can confirm that this one works for you, I'll send a smaller
>> one with the acpi_hw_legacy_sleep() part alone.
> 
> Well, scratch this, sorry.
> 
> The power button probably is a fixed event and it won't be effected by
> that patch.
> 
> Instead, please test the patch below.
> 
> ---
>  drivers/acpi/acpica/hwsleep.c |   15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> Index: linux-pm/drivers/acpi/acpica/hwsleep.c
> ===
> --- linux-pm.orig/drivers/acpi/acpica/hwsleep.c
> +++ linux-pm/drivers/acpi/acpica/hwsleep.c
> @@ -51,16 +51,23 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
>   return_ACPI_STATUS(status);
>   }
>  
> - /*
> -  * 1) Disable all GPEs
> -  * 2) Enable all wakeup GPEs
> -  */
> + /* Disable all GPEs */
>   status = acpi_hw_disable_all_gpes();
>   if (ACPI_FAILURE(status)) {
>   return_ACPI_STATUS(status);
>   }
> + /*
> +  * If the target sleep state is S5, clear all GPEs and fixed events too
> +  */
> + if (sleep_state == ACPI_STATE_S5) {
> + status = acpi_hw_clear_acpi_status();
> + if (ACPI_FAILURE(status)) {
> + return_ACPI_STATUS(status);
> + }
> + }
>   acpi_gbl_system_awake_and_running = FALSE;
>  
> +  /* Enable all wakeup GPEs */
>   status = acpi_hw_enable_all_wakeup_gpes();
>   if (ACPI_FAILURE(status)) {
>   return_ACPI_STATUS(status);
> 
after cancelling the build with patch 10511211 and rolling everything
back, I applied the patch above to my kernel 4.17.3-1-default and when
pressing the power button the system shuts down and stays off! :-)

Thanks
Thomas


Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-06 Thread Thomas Hänig



Am 06.07.2018 um 14:13 schrieb Rafael J. Wysocki:
> On Friday, July 6, 2018 1:21:50 PM CEST Rafael J. Wysocki wrote:
>> On Fri, Jul 6, 2018 at 1:12 PM, Thomas Hänig  wrote:
> 
> [cut]
>  
>> So the latest patch:
>>
>> https://patchwork.kernel.org/patch/10511211/
>>
>> should work for you (please verify) and the change in
>> drivers/acpi/sleep.c in it most likely is not necessary.
>>
>> If you can confirm that this one works for you, I'll send a smaller
>> one with the acpi_hw_legacy_sleep() part alone.
> 
> Well, scratch this, sorry.
> 
> The power button probably is a fixed event and it won't be effected by
> that patch.
> 
> Instead, please test the patch below.
> 
> ---
>  drivers/acpi/acpica/hwsleep.c |   15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> Index: linux-pm/drivers/acpi/acpica/hwsleep.c
> ===
> --- linux-pm.orig/drivers/acpi/acpica/hwsleep.c
> +++ linux-pm/drivers/acpi/acpica/hwsleep.c
> @@ -51,16 +51,23 @@ acpi_status acpi_hw_legacy_sleep(u8 slee
>   return_ACPI_STATUS(status);
>   }
>  
> - /*
> -  * 1) Disable all GPEs
> -  * 2) Enable all wakeup GPEs
> -  */
> + /* Disable all GPEs */
>   status = acpi_hw_disable_all_gpes();
>   if (ACPI_FAILURE(status)) {
>   return_ACPI_STATUS(status);
>   }
> + /*
> +  * If the target sleep state is S5, clear all GPEs and fixed events too
> +  */
> + if (sleep_state == ACPI_STATE_S5) {
> + status = acpi_hw_clear_acpi_status();
> + if (ACPI_FAILURE(status)) {
> + return_ACPI_STATUS(status);
> + }
> + }
>   acpi_gbl_system_awake_and_running = FALSE;
>  
> +  /* Enable all wakeup GPEs */
>   status = acpi_hw_enable_all_wakeup_gpes();
>   if (ACPI_FAILURE(status)) {
>   return_ACPI_STATUS(status);
> 
after cancelling the build with patch 10511211 and rolling everything
back, I applied the patch above to my kernel 4.17.3-1-default and when
pressing the power button the system shuts down and stays off! :-)

Thanks
Thomas


Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-06 Thread Thomas Hänig
Am 06.07.2018 um 08:55 schrieb Takashi Iwai:
> On Fri, 06 Jul 2018 07:18:36 +0200,
> Thomas H4nig wrote:
>>
>>
>>
>> Am 05.07.2018 um 18:56 schrieb Takashi Iwai:
>>> On Thu, 05 Jul 2018 18:02:11 +0200,
>>> Rafael J. Wysocki wrote:

 [The Lv's address is not valid any more, so drop it from the CC]

 On Thursday, July 5, 2018 5:10:20 PM CEST Rafael J. Wysocki wrote:
> On Thu, Jul 5, 2018 at 5:09 PM, Takashi Iwai  wrote:
>> On Thu, 05 Jul 2018 16:00:14 +0200,
>> Thomas H4nig wrote:
>>>
>>> Am 05.07.2018 um 14:12 schrieb Takashi Iwai:
 On Thu, 05 Jul 2018 12:41:03 +0200,
 Rafael J. Wysocki wrote:
>
> On Thursday, July 5, 2018 11:50:11 AM CEST Takashi Iwai wrote:
>> On Thu, 05 Jul 2018 11:34:59 +0200,
>> Rafael J. Wysocki wrote:
>>>
>>> Hi,
>>>
>>> On Thu, Jul 5, 2018 at 9:05 AM, Takashi Iwai  wrote:
 Hi,

 we've got a regression report since 4.17 about the behavior of
 power-off with the power button.  When a machine is powered off 
 with
 the power button on desktop, it reboots after a few seconds 
 instead of
 power down.

 The manual power down via "systemctl poweroff" works fine, so it's
 possibly some spurious wakeup by the power button action, and some
 ACPI-related change is suspected.
 The regression still remains in 4.18-rc3.
>>>
>>> There are only a few ACPI commits directly related to power 
>>> management
>>> between 4.16 and 4.17 and none of them looks particularly 
>>> suspicious.
>>
>> OK, interesting.
>>
>>> It looks like the power button state may not be cleared sufficiently
>>> after it's been pressed which is now visible for some reason.
>>
>> Hmm, where can such a state remain?  Since it happens after the
>> machine turned off, some (ACPI) wakeup bits?
>
> Basically, yes.
>
> It looks like a GPE may remain active which then triggers wakeup after
> shutdown.
>
> On a hunch, I'm wondering if reverting commit
>
> 18996f2db918 ACPICA: Events: Stop unconditionally clearing ACPI IRQs 
> during suspend/resume
>
> (may not revert clearly, though) makes any difference.

 OK, I'm building a 4.17.x test kernel with that revert, in OBS
 home:tiwai:bsc1099930 repo.

 Thomas, could you try later the kernel in
   
 http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930/standard/
 ?  It'll take an hour or so until the build finishes.
>>>
>>> With your new built kernel
>>> 4.17.4-1.g6f23755-default
>>>
>>> the power button works again, so the revert solved the problem
>>
>> Thanks, that clarifies the cause.
>> Adding Erik and Lv to Cc.
>>
>> I guess it's the side-effect by removing
>> acpi_ev_walk_gpe_list(acpi_hw_clear_gpe_block, NULL);
>> in acpi_hw_disable_all_gpes().
>>
>> This function is called from acpi_power_off_prepare(), and the machine
>> goes to power off without clearing the GPEs, hence it's woken up later
>> unexpectedly.
>
> That's correct.
>
> We need to fix up that commit.  I'll try to prepare something.
>

 Below is a patch to test that theory and maybe fix things if it is correct.

 What it does is to clear all GPEs after disabling them in
 acpi_power_off_prepare() which should address the issue if our theory
 about the underlying reason is correct.

 Please test.
>>>
>>> OK, building a new test kernel package in OBS home:tiwai:bsc1099930-2
>>> repo.  It'll appear at
>>>   
>>> http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930-2/standard/
>>>
>>> Thomas, please give it a try later.
>>>
>>>
>>> thanks,
>>>
>>> Takashi
>>
>> I am sorry, but with your test kernel 4.17.4-1.g76c6238-default the
>> notebook again gets not properly powered off but restarts
> 
> Interesting.  The package version shows that the tested kernel must be
> the right one.  (Though, it'd be good to double-check -- it's often
> confusing if you have multiple same kernel versions on the system.)
> 
> If Rafael's patch doesn't work, we'd need to identify which change in
> the commit 18996f2db918 has the effect.
> 
> Thomas, could you try to build & modify the kernel in your side?
> Package build on OBS and test takes too long.
> 
> There are three places that remove the GPE-clearing in the patch.
> 
> One is in acpi_ev_enable_gpe():
> 
> --- a/drivers/acpi/acpica/evgpe.c
> +++ b/drivers/acpi/acpica/evgpe.c
> @@ -115,13 +115,6 @@ acpi_status acpi_ev_enable_gpe(struct 
> acpi_gpe_event_info *gpe_event_info)
>  
>   

Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-06 Thread Thomas Hänig
Am 06.07.2018 um 08:55 schrieb Takashi Iwai:
> On Fri, 06 Jul 2018 07:18:36 +0200,
> Thomas H4nig wrote:
>>
>>
>>
>> Am 05.07.2018 um 18:56 schrieb Takashi Iwai:
>>> On Thu, 05 Jul 2018 18:02:11 +0200,
>>> Rafael J. Wysocki wrote:

 [The Lv's address is not valid any more, so drop it from the CC]

 On Thursday, July 5, 2018 5:10:20 PM CEST Rafael J. Wysocki wrote:
> On Thu, Jul 5, 2018 at 5:09 PM, Takashi Iwai  wrote:
>> On Thu, 05 Jul 2018 16:00:14 +0200,
>> Thomas H4nig wrote:
>>>
>>> Am 05.07.2018 um 14:12 schrieb Takashi Iwai:
 On Thu, 05 Jul 2018 12:41:03 +0200,
 Rafael J. Wysocki wrote:
>
> On Thursday, July 5, 2018 11:50:11 AM CEST Takashi Iwai wrote:
>> On Thu, 05 Jul 2018 11:34:59 +0200,
>> Rafael J. Wysocki wrote:
>>>
>>> Hi,
>>>
>>> On Thu, Jul 5, 2018 at 9:05 AM, Takashi Iwai  wrote:
 Hi,

 we've got a regression report since 4.17 about the behavior of
 power-off with the power button.  When a machine is powered off 
 with
 the power button on desktop, it reboots after a few seconds 
 instead of
 power down.

 The manual power down via "systemctl poweroff" works fine, so it's
 possibly some spurious wakeup by the power button action, and some
 ACPI-related change is suspected.
 The regression still remains in 4.18-rc3.
>>>
>>> There are only a few ACPI commits directly related to power 
>>> management
>>> between 4.16 and 4.17 and none of them looks particularly 
>>> suspicious.
>>
>> OK, interesting.
>>
>>> It looks like the power button state may not be cleared sufficiently
>>> after it's been pressed which is now visible for some reason.
>>
>> Hmm, where can such a state remain?  Since it happens after the
>> machine turned off, some (ACPI) wakeup bits?
>
> Basically, yes.
>
> It looks like a GPE may remain active which then triggers wakeup after
> shutdown.
>
> On a hunch, I'm wondering if reverting commit
>
> 18996f2db918 ACPICA: Events: Stop unconditionally clearing ACPI IRQs 
> during suspend/resume
>
> (may not revert clearly, though) makes any difference.

 OK, I'm building a 4.17.x test kernel with that revert, in OBS
 home:tiwai:bsc1099930 repo.

 Thomas, could you try later the kernel in
   
 http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930/standard/
 ?  It'll take an hour or so until the build finishes.
>>>
>>> With your new built kernel
>>> 4.17.4-1.g6f23755-default
>>>
>>> the power button works again, so the revert solved the problem
>>
>> Thanks, that clarifies the cause.
>> Adding Erik and Lv to Cc.
>>
>> I guess it's the side-effect by removing
>> acpi_ev_walk_gpe_list(acpi_hw_clear_gpe_block, NULL);
>> in acpi_hw_disable_all_gpes().
>>
>> This function is called from acpi_power_off_prepare(), and the machine
>> goes to power off without clearing the GPEs, hence it's woken up later
>> unexpectedly.
>
> That's correct.
>
> We need to fix up that commit.  I'll try to prepare something.
>

 Below is a patch to test that theory and maybe fix things if it is correct.

 What it does is to clear all GPEs after disabling them in
 acpi_power_off_prepare() which should address the issue if our theory
 about the underlying reason is correct.

 Please test.
>>>
>>> OK, building a new test kernel package in OBS home:tiwai:bsc1099930-2
>>> repo.  It'll appear at
>>>   
>>> http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930-2/standard/
>>>
>>> Thomas, please give it a try later.
>>>
>>>
>>> thanks,
>>>
>>> Takashi
>>
>> I am sorry, but with your test kernel 4.17.4-1.g76c6238-default the
>> notebook again gets not properly powered off but restarts
> 
> Interesting.  The package version shows that the tested kernel must be
> the right one.  (Though, it'd be good to double-check -- it's often
> confusing if you have multiple same kernel versions on the system.)
> 
> If Rafael's patch doesn't work, we'd need to identify which change in
> the commit 18996f2db918 has the effect.
> 
> Thomas, could you try to build & modify the kernel in your side?
> Package build on OBS and test takes too long.
> 
> There are three places that remove the GPE-clearing in the patch.
> 
> One is in acpi_ev_enable_gpe():
> 
> --- a/drivers/acpi/acpica/evgpe.c
> +++ b/drivers/acpi/acpica/evgpe.c
> @@ -115,13 +115,6 @@ acpi_status acpi_ev_enable_gpe(struct 
> acpi_gpe_event_info *gpe_event_info)
>  
>   

Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-05 Thread Thomas Hänig



Am 05.07.2018 um 18:56 schrieb Takashi Iwai:
> On Thu, 05 Jul 2018 18:02:11 +0200,
> Rafael J. Wysocki wrote:
>>
>> [The Lv's address is not valid any more, so drop it from the CC]
>>
>> On Thursday, July 5, 2018 5:10:20 PM CEST Rafael J. Wysocki wrote:
>>> On Thu, Jul 5, 2018 at 5:09 PM, Takashi Iwai  wrote:
 On Thu, 05 Jul 2018 16:00:14 +0200,
 Thomas H4nig wrote:
>
> Am 05.07.2018 um 14:12 schrieb Takashi Iwai:
>> On Thu, 05 Jul 2018 12:41:03 +0200,
>> Rafael J. Wysocki wrote:
>>>
>>> On Thursday, July 5, 2018 11:50:11 AM CEST Takashi Iwai wrote:
 On Thu, 05 Jul 2018 11:34:59 +0200,
 Rafael J. Wysocki wrote:
>
> Hi,
>
> On Thu, Jul 5, 2018 at 9:05 AM, Takashi Iwai  wrote:
>> Hi,
>>
>> we've got a regression report since 4.17 about the behavior of
>> power-off with the power button.  When a machine is powered off with
>> the power button on desktop, it reboots after a few seconds instead 
>> of
>> power down.
>>
>> The manual power down via "systemctl poweroff" works fine, so it's
>> possibly some spurious wakeup by the power button action, and some
>> ACPI-related change is suspected.
>> The regression still remains in 4.18-rc3.
>
> There are only a few ACPI commits directly related to power management
> between 4.16 and 4.17 and none of them looks particularly suspicious.

 OK, interesting.

> It looks like the power button state may not be cleared sufficiently
> after it's been pressed which is now visible for some reason.

 Hmm, where can such a state remain?  Since it happens after the
 machine turned off, some (ACPI) wakeup bits?
>>>
>>> Basically, yes.
>>>
>>> It looks like a GPE may remain active which then triggers wakeup after
>>> shutdown.
>>>
>>> On a hunch, I'm wondering if reverting commit
>>>
>>> 18996f2db918 ACPICA: Events: Stop unconditionally clearing ACPI IRQs 
>>> during suspend/resume
>>>
>>> (may not revert clearly, though) makes any difference.
>>
>> OK, I'm building a 4.17.x test kernel with that revert, in OBS
>> home:tiwai:bsc1099930 repo.
>>
>> Thomas, could you try later the kernel in
>>   
>> http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930/standard/
>> ?  It'll take an hour or so until the build finishes.
>
> With your new built kernel
> 4.17.4-1.g6f23755-default
>
> the power button works again, so the revert solved the problem

 Thanks, that clarifies the cause.
 Adding Erik and Lv to Cc.

 I guess it's the side-effect by removing
 acpi_ev_walk_gpe_list(acpi_hw_clear_gpe_block, NULL);
 in acpi_hw_disable_all_gpes().

 This function is called from acpi_power_off_prepare(), and the machine
 goes to power off without clearing the GPEs, hence it's woken up later
 unexpectedly.
>>>
>>> That's correct.
>>>
>>> We need to fix up that commit.  I'll try to prepare something.
>>>
>>
>> Below is a patch to test that theory and maybe fix things if it is correct.
>>
>> What it does is to clear all GPEs after disabling them in
>> acpi_power_off_prepare() which should address the issue if our theory
>> about the underlying reason is correct.
>>
>> Please test.
> 
> OK, building a new test kernel package in OBS home:tiwai:bsc1099930-2
> repo.  It'll appear at
>   
> http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930-2/standard/
> 
> Thomas, please give it a try later.
> 
> 
> thanks,
> 
> Takashi

I am sorry, but with your test kernel 4.17.4-1.g76c6238-default the
notebook again gets not properly powered off but restarts


Thomas


Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-05 Thread Thomas Hänig



Am 05.07.2018 um 18:56 schrieb Takashi Iwai:
> On Thu, 05 Jul 2018 18:02:11 +0200,
> Rafael J. Wysocki wrote:
>>
>> [The Lv's address is not valid any more, so drop it from the CC]
>>
>> On Thursday, July 5, 2018 5:10:20 PM CEST Rafael J. Wysocki wrote:
>>> On Thu, Jul 5, 2018 at 5:09 PM, Takashi Iwai  wrote:
 On Thu, 05 Jul 2018 16:00:14 +0200,
 Thomas H4nig wrote:
>
> Am 05.07.2018 um 14:12 schrieb Takashi Iwai:
>> On Thu, 05 Jul 2018 12:41:03 +0200,
>> Rafael J. Wysocki wrote:
>>>
>>> On Thursday, July 5, 2018 11:50:11 AM CEST Takashi Iwai wrote:
 On Thu, 05 Jul 2018 11:34:59 +0200,
 Rafael J. Wysocki wrote:
>
> Hi,
>
> On Thu, Jul 5, 2018 at 9:05 AM, Takashi Iwai  wrote:
>> Hi,
>>
>> we've got a regression report since 4.17 about the behavior of
>> power-off with the power button.  When a machine is powered off with
>> the power button on desktop, it reboots after a few seconds instead 
>> of
>> power down.
>>
>> The manual power down via "systemctl poweroff" works fine, so it's
>> possibly some spurious wakeup by the power button action, and some
>> ACPI-related change is suspected.
>> The regression still remains in 4.18-rc3.
>
> There are only a few ACPI commits directly related to power management
> between 4.16 and 4.17 and none of them looks particularly suspicious.

 OK, interesting.

> It looks like the power button state may not be cleared sufficiently
> after it's been pressed which is now visible for some reason.

 Hmm, where can such a state remain?  Since it happens after the
 machine turned off, some (ACPI) wakeup bits?
>>>
>>> Basically, yes.
>>>
>>> It looks like a GPE may remain active which then triggers wakeup after
>>> shutdown.
>>>
>>> On a hunch, I'm wondering if reverting commit
>>>
>>> 18996f2db918 ACPICA: Events: Stop unconditionally clearing ACPI IRQs 
>>> during suspend/resume
>>>
>>> (may not revert clearly, though) makes any difference.
>>
>> OK, I'm building a 4.17.x test kernel with that revert, in OBS
>> home:tiwai:bsc1099930 repo.
>>
>> Thomas, could you try later the kernel in
>>   
>> http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930/standard/
>> ?  It'll take an hour or so until the build finishes.
>
> With your new built kernel
> 4.17.4-1.g6f23755-default
>
> the power button works again, so the revert solved the problem

 Thanks, that clarifies the cause.
 Adding Erik and Lv to Cc.

 I guess it's the side-effect by removing
 acpi_ev_walk_gpe_list(acpi_hw_clear_gpe_block, NULL);
 in acpi_hw_disable_all_gpes().

 This function is called from acpi_power_off_prepare(), and the machine
 goes to power off without clearing the GPEs, hence it's woken up later
 unexpectedly.
>>>
>>> That's correct.
>>>
>>> We need to fix up that commit.  I'll try to prepare something.
>>>
>>
>> Below is a patch to test that theory and maybe fix things if it is correct.
>>
>> What it does is to clear all GPEs after disabling them in
>> acpi_power_off_prepare() which should address the issue if our theory
>> about the underlying reason is correct.
>>
>> Please test.
> 
> OK, building a new test kernel package in OBS home:tiwai:bsc1099930-2
> repo.  It'll appear at
>   
> http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930-2/standard/
> 
> Thomas, please give it a try later.
> 
> 
> thanks,
> 
> Takashi

I am sorry, but with your test kernel 4.17.4-1.g76c6238-default the
notebook again gets not properly powered off but restarts


Thomas


Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-05 Thread Thomas Hänig

Am 05.07.2018 um 18:56 schrieb Takashi Iwai:

On Thu, 05 Jul 2018 18:02:11 +0200,
Rafael J. Wysocki wrote:


[The Lv's address is not valid any more, so drop it from the CC]

On Thursday, July 5, 2018 5:10:20 PM CEST Rafael J. Wysocki wrote:

On Thu, Jul 5, 2018 at 5:09 PM, Takashi Iwai  wrote:

On Thu, 05 Jul 2018 16:00:14 +0200,
Thomas H4nig wrote:


Am 05.07.2018 um 14:12 schrieb Takashi Iwai:

On Thu, 05 Jul 2018 12:41:03 +0200,
Rafael J. Wysocki wrote:


On Thursday, July 5, 2018 11:50:11 AM CEST Takashi Iwai wrote:

On Thu, 05 Jul 2018 11:34:59 +0200,
Rafael J. Wysocki wrote:


Hi,

On Thu, Jul 5, 2018 at 9:05 AM, Takashi Iwai  wrote:

Hi,

we've got a regression report since 4.17 about the behavior of
power-off with the power button.  When a machine is powered off with
the power button on desktop, it reboots after a few seconds instead of
power down.

The manual power down via "systemctl poweroff" works fine, so it's
possibly some spurious wakeup by the power button action, and some
ACPI-related change is suspected.
The regression still remains in 4.18-rc3.


There are only a few ACPI commits directly related to power management
between 4.16 and 4.17 and none of them looks particularly suspicious.


OK, interesting.


It looks like the power button state may not be cleared sufficiently
after it's been pressed which is now visible for some reason.


Hmm, where can such a state remain?  Since it happens after the
machine turned off, some (ACPI) wakeup bits?


Basically, yes.

It looks like a GPE may remain active which then triggers wakeup after
shutdown.

On a hunch, I'm wondering if reverting commit

18996f2db918 ACPICA: Events: Stop unconditionally clearing ACPI IRQs during 
suspend/resume

(may not revert clearly, though) makes any difference.


OK, I'm building a 4.17.x test kernel with that revert, in OBS
home:tiwai:bsc1099930 repo.

Thomas, could you try later the kernel in
   http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930/standard/
?  It'll take an hour or so until the build finishes.


With your new built kernel
4.17.4-1.g6f23755-default

the power button works again, so the revert solved the problem


Thanks, that clarifies the cause.
Adding Erik and Lv to Cc.

I guess it's the side-effect by removing
 acpi_ev_walk_gpe_list(acpi_hw_clear_gpe_block, NULL);
in acpi_hw_disable_all_gpes().

This function is called from acpi_power_off_prepare(), and the machine
goes to power off without clearing the GPEs, hence it's woken up later
unexpectedly.


That's correct.

We need to fix up that commit.  I'll try to prepare something.



Below is a patch to test that theory and maybe fix things if it is correct.

What it does is to clear all GPEs after disabling them in
acpi_power_off_prepare() which should address the issue if our theory
about the underlying reason is correct.

Please test.


OK, building a new test kernel package in OBS home:tiwai:bsc1099930-2
repo.  It'll appear at
   http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930-2/standard/

Thomas, please give it a try later.


thanks,

Takashi


Later will have to be tomorrow morning (07:00 UTC+2) as until then I 
have no access to the machine in question.



Thanks
Thomas


Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-05 Thread Thomas Hänig

Am 05.07.2018 um 18:56 schrieb Takashi Iwai:

On Thu, 05 Jul 2018 18:02:11 +0200,
Rafael J. Wysocki wrote:


[The Lv's address is not valid any more, so drop it from the CC]

On Thursday, July 5, 2018 5:10:20 PM CEST Rafael J. Wysocki wrote:

On Thu, Jul 5, 2018 at 5:09 PM, Takashi Iwai  wrote:

On Thu, 05 Jul 2018 16:00:14 +0200,
Thomas H4nig wrote:


Am 05.07.2018 um 14:12 schrieb Takashi Iwai:

On Thu, 05 Jul 2018 12:41:03 +0200,
Rafael J. Wysocki wrote:


On Thursday, July 5, 2018 11:50:11 AM CEST Takashi Iwai wrote:

On Thu, 05 Jul 2018 11:34:59 +0200,
Rafael J. Wysocki wrote:


Hi,

On Thu, Jul 5, 2018 at 9:05 AM, Takashi Iwai  wrote:

Hi,

we've got a regression report since 4.17 about the behavior of
power-off with the power button.  When a machine is powered off with
the power button on desktop, it reboots after a few seconds instead of
power down.

The manual power down via "systemctl poweroff" works fine, so it's
possibly some spurious wakeup by the power button action, and some
ACPI-related change is suspected.
The regression still remains in 4.18-rc3.


There are only a few ACPI commits directly related to power management
between 4.16 and 4.17 and none of them looks particularly suspicious.


OK, interesting.


It looks like the power button state may not be cleared sufficiently
after it's been pressed which is now visible for some reason.


Hmm, where can such a state remain?  Since it happens after the
machine turned off, some (ACPI) wakeup bits?


Basically, yes.

It looks like a GPE may remain active which then triggers wakeup after
shutdown.

On a hunch, I'm wondering if reverting commit

18996f2db918 ACPICA: Events: Stop unconditionally clearing ACPI IRQs during 
suspend/resume

(may not revert clearly, though) makes any difference.


OK, I'm building a 4.17.x test kernel with that revert, in OBS
home:tiwai:bsc1099930 repo.

Thomas, could you try later the kernel in
   http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930/standard/
?  It'll take an hour or so until the build finishes.


With your new built kernel
4.17.4-1.g6f23755-default

the power button works again, so the revert solved the problem


Thanks, that clarifies the cause.
Adding Erik and Lv to Cc.

I guess it's the side-effect by removing
 acpi_ev_walk_gpe_list(acpi_hw_clear_gpe_block, NULL);
in acpi_hw_disable_all_gpes().

This function is called from acpi_power_off_prepare(), and the machine
goes to power off without clearing the GPEs, hence it's woken up later
unexpectedly.


That's correct.

We need to fix up that commit.  I'll try to prepare something.



Below is a patch to test that theory and maybe fix things if it is correct.

What it does is to clear all GPEs after disabling them in
acpi_power_off_prepare() which should address the issue if our theory
about the underlying reason is correct.

Please test.


OK, building a new test kernel package in OBS home:tiwai:bsc1099930-2
repo.  It'll appear at
   http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930-2/standard/

Thomas, please give it a try later.


thanks,

Takashi


Later will have to be tomorrow morning (07:00 UTC+2) as until then I 
have no access to the machine in question.



Thanks
Thomas


Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-05 Thread Thomas Hänig
Am 05.07.2018 um 14:12 schrieb Takashi Iwai:
> On Thu, 05 Jul 2018 12:41:03 +0200,
> Rafael J. Wysocki wrote:
>>
>> On Thursday, July 5, 2018 11:50:11 AM CEST Takashi Iwai wrote:
>>> On Thu, 05 Jul 2018 11:34:59 +0200,
>>> Rafael J. Wysocki wrote:

 Hi,

 On Thu, Jul 5, 2018 at 9:05 AM, Takashi Iwai  wrote:
> Hi,
>
> we've got a regression report since 4.17 about the behavior of
> power-off with the power button.  When a machine is powered off with
> the power button on desktop, it reboots after a few seconds instead of
> power down.
>
> The manual power down via "systemctl poweroff" works fine, so it's
> possibly some spurious wakeup by the power button action, and some
> ACPI-related change is suspected.
> The regression still remains in 4.18-rc3.

 There are only a few ACPI commits directly related to power management
 between 4.16 and 4.17 and none of them looks particularly suspicious.
>>>
>>> OK, interesting.
>>>
 It looks like the power button state may not be cleared sufficiently
 after it's been pressed which is now visible for some reason.
>>>
>>> Hmm, where can such a state remain?  Since it happens after the
>>> machine turned off, some (ACPI) wakeup bits?
>>
>> Basically, yes.
>>
>> It looks like a GPE may remain active which then triggers wakeup after
>> shutdown.
>>
>> On a hunch, I'm wondering if reverting commit
>>
>> 18996f2db918 ACPICA: Events: Stop unconditionally clearing ACPI IRQs during 
>> suspend/resume
>>
>> (may not revert clearly, though) makes any difference.
> 
> OK, I'm building a 4.17.x test kernel with that revert, in OBS
> home:tiwai:bsc1099930 repo.
> 
> Thomas, could you try later the kernel in
>   http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930/standard/
> ?  It'll take an hour or so until the build finishes.

With your new built kernel
4.17.4-1.g6f23755-default

the power button works again, so the revert solved the problem

Thanks
Thomas


Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-05 Thread Thomas Hänig
Am 05.07.2018 um 14:12 schrieb Takashi Iwai:
> On Thu, 05 Jul 2018 12:41:03 +0200,
> Rafael J. Wysocki wrote:
>>
>> On Thursday, July 5, 2018 11:50:11 AM CEST Takashi Iwai wrote:
>>> On Thu, 05 Jul 2018 11:34:59 +0200,
>>> Rafael J. Wysocki wrote:

 Hi,

 On Thu, Jul 5, 2018 at 9:05 AM, Takashi Iwai  wrote:
> Hi,
>
> we've got a regression report since 4.17 about the behavior of
> power-off with the power button.  When a machine is powered off with
> the power button on desktop, it reboots after a few seconds instead of
> power down.
>
> The manual power down via "systemctl poweroff" works fine, so it's
> possibly some spurious wakeup by the power button action, and some
> ACPI-related change is suspected.
> The regression still remains in 4.18-rc3.

 There are only a few ACPI commits directly related to power management
 between 4.16 and 4.17 and none of them looks particularly suspicious.
>>>
>>> OK, interesting.
>>>
 It looks like the power button state may not be cleared sufficiently
 after it's been pressed which is now visible for some reason.
>>>
>>> Hmm, where can such a state remain?  Since it happens after the
>>> machine turned off, some (ACPI) wakeup bits?
>>
>> Basically, yes.
>>
>> It looks like a GPE may remain active which then triggers wakeup after
>> shutdown.
>>
>> On a hunch, I'm wondering if reverting commit
>>
>> 18996f2db918 ACPICA: Events: Stop unconditionally clearing ACPI IRQs during 
>> suspend/resume
>>
>> (may not revert clearly, though) makes any difference.
> 
> OK, I'm building a 4.17.x test kernel with that revert, in OBS
> home:tiwai:bsc1099930 repo.
> 
> Thomas, could you try later the kernel in
>   http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930/standard/
> ?  It'll take an hour or so until the build finishes.

With your new built kernel
4.17.4-1.g6f23755-default

the power button works again, so the revert solved the problem

Thanks
Thomas


Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-05 Thread Thomas Hänig
Am 05.07.2018 um 14:12 schrieb Takashi Iwai:
> OK, I'm building a 4.17.x test kernel with that revert, in OBS
> home:tiwai:bsc1099930 repo.
> 
> Thomas, could you try later the kernel in
>   http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930/standard/
> ?  It'll take an hour or so until the build finishes.
> 

Of course I will try it out and keep you informed.

Thanks for your efforts - I may be able to build a kernel myself, but
not able to revert a patch which may not revert clearly ;-)

Thomas


Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-05 Thread Thomas Hänig
Am 05.07.2018 um 14:12 schrieb Takashi Iwai:
> OK, I'm building a 4.17.x test kernel with that revert, in OBS
> home:tiwai:bsc1099930 repo.
> 
> Thomas, could you try later the kernel in
>   http://download.opensuse.org/repositories/home:/tiwai:/bsc1099930/standard/
> ?  It'll take an hour or so until the build finishes.
> 

Of course I will try it out and keep you informed.

Thanks for your efforts - I may be able to build a kernel myself, but
not able to revert a patch which may not revert clearly ;-)

Thomas


Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-05 Thread Thomas Hänig
Am 05.07.2018 um 11:50 schrieb Takashi Iwai:
> Usually X desktop environment receives an input event from the ACPI
> power button input device, and deals the event accordingly depending
> on the setup.  The power-down behavior itself should be equivalent
> with "systemctl poweroff" or such.
The behaviour occurs even when booting into runlevel 3 /
multi-user.target so if no X11 or graphical DE is involved


Thomas


Re: [REGRESSION 4.17] Spurious wakeup / reboot with power button

2018-07-05 Thread Thomas Hänig
Am 05.07.2018 um 11:50 schrieb Takashi Iwai:
> Usually X desktop environment receives an input event from the ACPI
> power button input device, and deals the event accordingly depending
> on the setup.  The power-down behavior itself should be equivalent
> with "systemctl poweroff" or such.
The behaviour occurs even when booting into runlevel 3 /
multi-user.target so if no X11 or graphical DE is involved


Thomas