[PATCH] ACPI / PM / EC: Flush all EC work in acpi_freeze_sync()

[PATCH] ACPI / PM / EC: Flush all EC work in acpi_freeze_sync()

[Rafael J. Wysocki]

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Commit eed4d47efe95 (ACPI / sleep: Ignore spurious SCI wakeups from
suspend-to-idle) introduced acpi_freeze_sync() whose purpose is to
flush all of the processing of possible wakeup events signaled via
the ACPI SCI.  However, it doesn't flush the query workqueue used
by the EC driver, so the events generated by the EC may not be
processed timely which leads to issues (increased overhead at least,
lost events possibly).

To fix that introduce acpi_ec_flush_work() that will flush all of
the outstanding EC work and call it from acpi_freeze_sync().

Fixes: eed4d47efe95 (ACPI / sleep: Ignore spurious SCI wakeups from suspend-to-idle)
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/ec.c       |    8 ++++++++
 drivers/acpi/internal.h |    4 ++++
 drivers/acpi/sleep.c    |    6 +++---
 3 files changed, 15 insertions(+), 3 deletions(-)

Index: linux-pm/drivers/acpi/ec.c
===================================================================
--- linux-pm.orig/drivers/acpi/ec.c
+++ linux-pm/drivers/acpi/ec.c
@@ -539,6 +539,14 @@ static void acpi_ec_disable_event(struct
 	spin_unlock_irqrestore(&ec->lock, flags);
 	__acpi_ec_flush_event(ec);
 }
+
+void acpi_ec_flush_work(void)
+{
+	if (first_ec)
+		__acpi_ec_flush_event(first_ec);
+
+	flush_scheduled_work();
+}
 #endif /* CONFIG_PM_SLEEP */
 
 static bool acpi_ec_guard_event(struct acpi_ec *ec)
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -193,6 +193,10 @@ int acpi_ec_add_query_handler(struct acp
 			      void *data);
 void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
 
+#ifdef CONFIG_PM_SLEEP
+void acpi_ec_flush_work(void);
+#endif
+
 
 /*--------------------------------------------------------------------------
                                   Suspend/Resume
Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -777,11 +777,11 @@ static void acpi_freeze_sync(void)
 	/*
 	 * Process all pending events in case there are any wakeup ones.
 	 *
-	 * The EC driver uses the system workqueue, so that one needs to be
-	 * flushed too.
+	 * The EC driver uses the system workqueue and an additional special
+	 * one, so those need to be flushed too.
 	 */
+	acpi_ec_flush_work();
 	acpi_os_wait_events_complete();
-	flush_scheduled_work();
 	s2idle_wakeup = false;
 }
 

Re: [PATCH] ACPI / PM / EC: Flush all EC work in acpi_freeze_sync()

[Rafael J. Wysocki]

On Thursday, July 20, 2017 03:43:12 AM Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> Commit eed4d47efe95 (ACPI / sleep: Ignore spurious SCI wakeups from
> suspend-to-idle) introduced acpi_freeze_sync() whose purpose is to
> flush all of the processing of possible wakeup events signaled via
> the ACPI SCI.  However, it doesn't flush the query workqueue used
> by the EC driver, so the events generated by the EC may not be
> processed timely which leads to issues (increased overhead at least,
> lost events possibly).
> 
> To fix that introduce acpi_ec_flush_work() that will flush all of
> the outstanding EC work and call it from acpi_freeze_sync().
> 
> Fixes: eed4d47efe95 (ACPI / sleep: Ignore spurious SCI wakeups from suspend-to-idle)
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

If there are any issues related to this, please let me know.

Otherwise, I'll queue it up as a fix for 4.13.

> ---
>  drivers/acpi/ec.c       |    8 ++++++++
>  drivers/acpi/internal.h |    4 ++++
>  drivers/acpi/sleep.c    |    6 +++---
>  3 files changed, 15 insertions(+), 3 deletions(-)
> 
> Index: linux-pm/drivers/acpi/ec.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/ec.c
> +++ linux-pm/drivers/acpi/ec.c
> @@ -539,6 +539,14 @@ static void acpi_ec_disable_event(struct
>  	spin_unlock_irqrestore(&ec->lock, flags);
>  	__acpi_ec_flush_event(ec);
>  }
> +
> +void acpi_ec_flush_work(void)
> +{
> +	if (first_ec)
> +		__acpi_ec_flush_event(first_ec);
> +
> +	flush_scheduled_work();
> +}
>  #endif /* CONFIG_PM_SLEEP */
>  
>  static bool acpi_ec_guard_event(struct acpi_ec *ec)
> Index: linux-pm/drivers/acpi/internal.h
> ===================================================================
> --- linux-pm.orig/drivers/acpi/internal.h
> +++ linux-pm/drivers/acpi/internal.h
> @@ -193,6 +193,10 @@ int acpi_ec_add_query_handler(struct acp
>  			      void *data);
>  void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
>  
> +#ifdef CONFIG_PM_SLEEP
> +void acpi_ec_flush_work(void);
> +#endif
> +
>  
>  /*--------------------------------------------------------------------------
>                                    Suspend/Resume
> Index: linux-pm/drivers/acpi/sleep.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/sleep.c
> +++ linux-pm/drivers/acpi/sleep.c
> @@ -777,11 +777,11 @@ static void acpi_freeze_sync(void)
>  	/*
>  	 * Process all pending events in case there are any wakeup ones.
>  	 *
> -	 * The EC driver uses the system workqueue, so that one needs to be
> -	 * flushed too.
> +	 * The EC driver uses the system workqueue and an additional special
> +	 * one, so those need to be flushed too.
>  	 */
> +	acpi_ec_flush_work();
>  	acpi_os_wait_events_complete();
> -	flush_scheduled_work();
>  	s2idle_wakeup = false;
>  }
>  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html