Linux-PM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] power: Do not clear events_check_enabled in pm_wakeup_pending()
@ 2019-06-07  0:37 Ravi Chandra Sadineni
  2019-06-14 18:23 ` Ravi Chandra Sadineni
  0 siblings, 1 reply; 7+ messages in thread
From: Ravi Chandra Sadineni @ 2019-06-07  0:37 UTC (permalink / raw)
  To: len.brown, pavel, gregkh, linux-pm, linux-kernel, tbroch,
	ravisadineni, rajatja

events_check_enabled bool is set when wakeup_count sysfs attribute
is written. User level daemon is expected to write this attribute
just before suspend.

When this boolean is set, calls to pm_wakeup_event() will result in
increment of per device and global wakeup count that helps in
identifying the wake source. global wakeup count is also used by
pm_wakeup_pending() to identify if there are any pending events that
should result in an suspend abort.

Currently calls to pm_wakeup_pending() also clears events_check_enabled.
This can be a problem when there are multiple wake events or when the
suspend is aborted due to an interrupt on a shared interrupt line.
For example an Mfd device can create several platform devices which
might fetch the state on resume in the driver resume method and increment
the wakeup count if needed. But if events_check_enabled is cleared before
resume methods get to execute, wakeup count will not be incremented. Thus
let us not reset the bool here.

Note that events_check_enabled is also cleared in suspend.c/enter_state()
on every resume at the end.

Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
---
 drivers/base/power/wakeup.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 5b2b6a05a4f3..88aade871589 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -838,7 +838,6 @@ bool pm_wakeup_pending(void)
 
 		split_counters(&cnt, &inpr);
 		ret = (cnt != saved_count || inpr > 0);
-		events_check_enabled = !ret;
 	}
 	raw_spin_unlock_irqrestore(&events_lock, flags);
 
-- 
2.20.1


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

* Re: [PATCH] power: Do not clear events_check_enabled in pm_wakeup_pending()
  2019-06-07  0:37 [PATCH] power: Do not clear events_check_enabled in pm_wakeup_pending() Ravi Chandra Sadineni
@ 2019-06-14 18:23 ` Ravi Chandra Sadineni
  2019-06-19 17:41   ` Greg KH
  0 siblings, 1 reply; 7+ messages in thread
From: Ravi Chandra Sadineni @ 2019-06-14 18:23 UTC (permalink / raw)
  To: len.brown, pavel, gregkh, linux-pm, linux-kernel, Todd Broch,
	Ravi Chandra Sadineni, Rajat Jain

Hi,

Just wanted to check if this o.k.

Thanks,
Ravi

On Thu, Jun 6, 2019 at 5:37 PM Ravi Chandra Sadineni
<ravisadineni@chromium.org> wrote:
>
> events_check_enabled bool is set when wakeup_count sysfs attribute
> is written. User level daemon is expected to write this attribute
> just before suspend.
>
> When this boolean is set, calls to pm_wakeup_event() will result in
> increment of per device and global wakeup count that helps in
> identifying the wake source. global wakeup count is also used by
> pm_wakeup_pending() to identify if there are any pending events that
> should result in an suspend abort.
>
> Currently calls to pm_wakeup_pending() also clears events_check_enabled.
> This can be a problem when there are multiple wake events or when the
> suspend is aborted due to an interrupt on a shared interrupt line.
> For example an Mfd device can create several platform devices which
> might fetch the state on resume in the driver resume method and increment
> the wakeup count if needed. But if events_check_enabled is cleared before
> resume methods get to execute, wakeup count will not be incremented. Thus
> let us not reset the bool here.
>
> Note that events_check_enabled is also cleared in suspend.c/enter_state()
> on every resume at the end.
>
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> ---
>  drivers/base/power/wakeup.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index 5b2b6a05a4f3..88aade871589 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -838,7 +838,6 @@ bool pm_wakeup_pending(void)
>
>                 split_counters(&cnt, &inpr);
>                 ret = (cnt != saved_count || inpr > 0);
> -               events_check_enabled = !ret;
>         }
>         raw_spin_unlock_irqrestore(&events_lock, flags);
>
> --
> 2.20.1
>

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

* Re: [PATCH] power: Do not clear events_check_enabled in pm_wakeup_pending()
  2019-06-14 18:23 ` Ravi Chandra Sadineni
@ 2019-06-19 17:41   ` Greg KH
  0 siblings, 0 replies; 7+ messages in thread
From: Greg KH @ 2019-06-19 17:41 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: len.brown, pavel, linux-pm, linux-kernel, Todd Broch, Rajat Jain

On Fri, Jun 14, 2019 at 11:23:31AM -0700, Ravi Chandra Sadineni wrote:
> Hi,
> 
> Just wanted to check if this o.k.

You seem to have forgoten to send this to the maintainer of this file :(

Please use scripts/get_maintainer.pl and resend it, remembering to send
it to Rafael next time :)

thanks,

greg k-h

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

* Re: [PATCH] power: Do not clear events_check_enabled in pm_wakeup_pending()
  2019-06-19 17:51 Ravi Chandra Sadineni
  2019-06-24 23:25 ` Ravi Chandra Sadineni
@ 2019-06-27 17:49 ` Rafael J. Wysocki
  1 sibling, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2019-06-27 17:49 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: len.brown, pavel, gregkh, linux-pm, linux-kernel, tbroch, rajatja

On Wednesday, June 19, 2019 7:51:42 PM CEST Ravi Chandra Sadineni wrote:
> events_check_enabled bool is set when wakeup_count sysfs attribute
> is written. User level daemon is expected to write this attribute
> just before suspend.
> 
> When this boolean is set, calls to pm_wakeup_event() will result in
> increment of per device and global wakeup count that helps in
> identifying the wake source. global wakeup count is also used by
> pm_wakeup_pending() to identify if there are any pending events that
> should result in an suspend abort.
> 
> Currently calls to pm_wakeup_pending() also clears events_check_enabled.
> This can be a problem when there are multiple wake events or when the
> suspend is aborted due to an interrupt on a shared interrupt line.
> For example an Mfd device can create several platform devices which
> might fetch the state on resume in the driver resume method and increment
> the wakeup count if needed. But if events_check_enabled is cleared before
> resume methods get to execute, wakeup count will not be incremented. Thus
> let us not reset the bool here.
> 
> Note that events_check_enabled is also cleared in suspend.c/enter_state()
> on every resume at the end.
> 
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> ---
>  drivers/base/power/wakeup.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index 5b2b6a05a4f3..88aade871589 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -838,7 +838,6 @@ bool pm_wakeup_pending(void)
>  
>  		split_counters(&cnt, &inpr);
>  		ret = (cnt != saved_count || inpr > 0);
> -		events_check_enabled = !ret;

This effectively changes the meaning of the wakeup_count metric. so it cannot be applied.

>  	}
>  	raw_spin_unlock_irqrestore(&events_lock, flags);
>  
> 





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

* Re: [PATCH] power: Do not clear events_check_enabled in pm_wakeup_pending()
  2019-06-24 23:25 ` Ravi Chandra Sadineni
@ 2019-06-24 23:35   ` Rafael J. Wysocki
  0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2019-06-24 23:35 UTC (permalink / raw)
  To: Ravi Chandra Sadineni
  Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Greg Kroah-Hartman,
	Linux PM, linux-kernel, Todd Broch, Rajat Jain

On Tue, Jun 25, 2019 at 1:25 AM Ravi Chandra Sadineni
<ravisadineni@chromium.org> wrote:
>
> Hi,
>
> Just wanted to check if this is o.k.

It is in my "for review" queue, I'll let you know.

Thanks!

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

* Re: [PATCH] power: Do not clear events_check_enabled in pm_wakeup_pending()
  2019-06-19 17:51 Ravi Chandra Sadineni
@ 2019-06-24 23:25 ` Ravi Chandra Sadineni
  2019-06-24 23:35   ` Rafael J. Wysocki
  2019-06-27 17:49 ` Rafael J. Wysocki
  1 sibling, 1 reply; 7+ messages in thread
From: Ravi Chandra Sadineni @ 2019-06-24 23:25 UTC (permalink / raw)
  To: rjw, len.brown, pavel, gregkh, linux-pm, linux-kernel,
	Todd Broch, Ravi Chandra Sadineni, Rajat Jain

Hi,

Just wanted to check if this is o.k.

Thanks,
Ravi

On Wed, Jun 19, 2019 at 10:52 AM Ravi Chandra Sadineni
<ravisadineni@chromium.org> wrote:
>
> events_check_enabled bool is set when wakeup_count sysfs attribute
> is written. User level daemon is expected to write this attribute
> just before suspend.
>
> When this boolean is set, calls to pm_wakeup_event() will result in
> increment of per device and global wakeup count that helps in
> identifying the wake source. global wakeup count is also used by
> pm_wakeup_pending() to identify if there are any pending events that
> should result in an suspend abort.
>
> Currently calls to pm_wakeup_pending() also clears events_check_enabled.
> This can be a problem when there are multiple wake events or when the
> suspend is aborted due to an interrupt on a shared interrupt line.
> For example an Mfd device can create several platform devices which
> might fetch the state on resume in the driver resume method and increment
> the wakeup count if needed. But if events_check_enabled is cleared before
> resume methods get to execute, wakeup count will not be incremented. Thus
> let us not reset the bool here.
>
> Note that events_check_enabled is also cleared in suspend.c/enter_state()
> on every resume at the end.
>
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> ---
>  drivers/base/power/wakeup.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
> index 5b2b6a05a4f3..88aade871589 100644
> --- a/drivers/base/power/wakeup.c
> +++ b/drivers/base/power/wakeup.c
> @@ -838,7 +838,6 @@ bool pm_wakeup_pending(void)
>
>                 split_counters(&cnt, &inpr);
>                 ret = (cnt != saved_count || inpr > 0);
> -               events_check_enabled = !ret;
>         }
>         raw_spin_unlock_irqrestore(&events_lock, flags);
>
> --
> 2.20.1
>

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

* [PATCH] power: Do not clear events_check_enabled in pm_wakeup_pending()
@ 2019-06-19 17:51 Ravi Chandra Sadineni
  2019-06-24 23:25 ` Ravi Chandra Sadineni
  2019-06-27 17:49 ` Rafael J. Wysocki
  0 siblings, 2 replies; 7+ messages in thread
From: Ravi Chandra Sadineni @ 2019-06-19 17:51 UTC (permalink / raw)
  To: rjw, len.brown, pavel, gregkh, linux-pm, linux-kernel, tbroch,
	ravisadineni, rajatja

events_check_enabled bool is set when wakeup_count sysfs attribute
is written. User level daemon is expected to write this attribute
just before suspend.

When this boolean is set, calls to pm_wakeup_event() will result in
increment of per device and global wakeup count that helps in
identifying the wake source. global wakeup count is also used by
pm_wakeup_pending() to identify if there are any pending events that
should result in an suspend abort.

Currently calls to pm_wakeup_pending() also clears events_check_enabled.
This can be a problem when there are multiple wake events or when the
suspend is aborted due to an interrupt on a shared interrupt line.
For example an Mfd device can create several platform devices which
might fetch the state on resume in the driver resume method and increment
the wakeup count if needed. But if events_check_enabled is cleared before
resume methods get to execute, wakeup count will not be incremented. Thus
let us not reset the bool here.

Note that events_check_enabled is also cleared in suspend.c/enter_state()
on every resume at the end.

Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
---
 drivers/base/power/wakeup.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index 5b2b6a05a4f3..88aade871589 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -838,7 +838,6 @@ bool pm_wakeup_pending(void)
 
 		split_counters(&cnt, &inpr);
 		ret = (cnt != saved_count || inpr > 0);
-		events_check_enabled = !ret;
 	}
 	raw_spin_unlock_irqrestore(&events_lock, flags);
 
-- 
2.20.1


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

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-07  0:37 [PATCH] power: Do not clear events_check_enabled in pm_wakeup_pending() Ravi Chandra Sadineni
2019-06-14 18:23 ` Ravi Chandra Sadineni
2019-06-19 17:41   ` Greg KH
2019-06-19 17:51 Ravi Chandra Sadineni
2019-06-24 23:25 ` Ravi Chandra Sadineni
2019-06-24 23:35   ` Rafael J. Wysocki
2019-06-27 17:49 ` Rafael J. Wysocki

Linux-PM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pm/0 linux-pm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pm linux-pm/ https://lore.kernel.org/linux-pm \
		linux-pm@vger.kernel.org linux-pm@archiver.kernel.org
	public-inbox-index linux-pm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox