diff mbox

ACPI / EC: Add parameter force disable the GPE on suspend

Message ID 12949796.Ix7M0mMrlm@aspire.rjw.lan (mailing list archive)
State Accepted, archived
Delegated to: Rafael Wysocki
Headers show

Commit Message

Rafael J. Wysocki July 18, 2017, 11:04 p.m. UTC
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

After commit 8110dd281e15 (ACPI / sleep: EC-based wakeup from
suspend-to-idle on recent systems) the configuration of GPEs,
including the EC one, is not changed during suspend-to-idle on
recent systems.  That's in order to make system wakeup events
generated by the EC work, in particular.

However, on some of the systems in question (for example on Dell
XPS13 9365), in addition to generating system wakeup events the
EC generates a heartbeat sequence of interrupts that have nothing
to do with wakeup while suspended, and the Low Power Idle S0 _DSM
interface doesn't change that behavior.

The users of those systems may prefer to disable the EC GPE during
system suspend, for the cost of non-functional power button wakeup
or similar, but currently there is no way to do that.

For this reason, add a new module parameter, ec_no_wakeup, for the
EC driver module that, if set, will cause the EC GPE to be disabled
during system suspend and re-enabled during the subsequent system
resume.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=192591#c106
Fixes: 8110dd281e15 (ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems)
Reported-and-tested-by: Patrik Kullman <patrik.kullman@gmail.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/ec.c |   31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)


--
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

Comments

Rafael J. Wysocki July 19, 2017, 8:44 p.m. UTC | #1
On Wed, Jul 19, 2017 at 1:04 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> After commit 8110dd281e15 (ACPI / sleep: EC-based wakeup from
> suspend-to-idle on recent systems) the configuration of GPEs,
> including the EC one, is not changed during suspend-to-idle on
> recent systems.  That's in order to make system wakeup events
> generated by the EC work, in particular.
>
> However, on some of the systems in question (for example on Dell
> XPS13 9365), in addition to generating system wakeup events the
> EC generates a heartbeat sequence of interrupts that have nothing
> to do with wakeup while suspended, and the Low Power Idle S0 _DSM
> interface doesn't change that behavior.
>
> The users of those systems may prefer to disable the EC GPE during
> system suspend, for the cost of non-functional power button wakeup
> or similar, but currently there is no way to do that.
>
> For this reason, add a new module parameter, ec_no_wakeup, for the
> EC driver module that, if set, will cause the EC GPE to be disabled
> during system suspend and re-enabled during the subsequent system
> resume.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=192591#c106
> Fixes: 8110dd281e15 (ACPI / sleep: EC-based wakeup from suspend-to-idle on recent systems)
> Reported-and-tested-by: Patrik Kullman <patrik.kullman@gmail.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Any objections against this?

If not, I will queue it up for 4.13.

> ---
>  drivers/acpi/ec.c |   31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> Index: linux-pm/drivers/acpi/ec.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/ec.c
> +++ linux-pm/drivers/acpi/ec.c
> @@ -151,6 +151,10 @@ static bool ec_freeze_events __read_most
>  module_param(ec_freeze_events, bool, 0644);
>  MODULE_PARM_DESC(ec_freeze_events, "Disabling event handling during suspend/resume");
>
> +static bool ec_no_wakeup __read_mostly;
> +module_param(ec_no_wakeup, bool, 0644);
> +MODULE_PARM_DESC(ec_no_wakeup, "Do not wake up from suspend-to-idle");
> +
>  struct acpi_ec_query_handler {
>         struct list_head node;
>         acpi_ec_query_func func;
> @@ -1880,6 +1884,32 @@ static int acpi_ec_suspend(struct device
>         return 0;
>  }
>
> +static int acpi_ec_suspend_noirq(struct device *dev)
> +{
> +       struct acpi_ec *ec = acpi_driver_data(to_acpi_device(dev));
> +
> +       /*
> +        * The SCI handler doesn't run at this point, so the GPE can be
> +        * masked at the low level without side effects.
> +        */
> +       if (ec_no_wakeup && test_bit(EC_FLAGS_STARTED, &ec->flags) &&
> +           ec->reference_count >= 1)
> +               acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE);
> +
> +       return 0;
> +}
> +
> +static int acpi_ec_resume_noirq(struct device *dev)
> +{
> +       struct acpi_ec *ec = acpi_driver_data(to_acpi_device(dev));
> +
> +       if (ec_no_wakeup && test_bit(EC_FLAGS_STARTED, &ec->flags) &&
> +           ec->reference_count >= 1)
> +               acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_ENABLE);
> +
> +       return 0;
> +}
> +
>  static int acpi_ec_resume(struct device *dev)
>  {
>         struct acpi_ec *ec =
> @@ -1891,6 +1921,7 @@ static int acpi_ec_resume(struct device
>  #endif
>
>  static const struct dev_pm_ops acpi_ec_pm = {
> +       SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(acpi_ec_suspend_noirq, acpi_ec_resume_noirq)
>         SET_SYSTEM_SLEEP_PM_OPS(acpi_ec_suspend, acpi_ec_resume)
>  };
>
>
> --
> 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
--
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
diff mbox

Patch

Index: linux-pm/drivers/acpi/ec.c
===================================================================
--- linux-pm.orig/drivers/acpi/ec.c
+++ linux-pm/drivers/acpi/ec.c
@@ -151,6 +151,10 @@  static bool ec_freeze_events __read_most
 module_param(ec_freeze_events, bool, 0644);
 MODULE_PARM_DESC(ec_freeze_events, "Disabling event handling during suspend/resume");
 
+static bool ec_no_wakeup __read_mostly;
+module_param(ec_no_wakeup, bool, 0644);
+MODULE_PARM_DESC(ec_no_wakeup, "Do not wake up from suspend-to-idle");
+
 struct acpi_ec_query_handler {
 	struct list_head node;
 	acpi_ec_query_func func;
@@ -1880,6 +1884,32 @@  static int acpi_ec_suspend(struct device
 	return 0;
 }
 
+static int acpi_ec_suspend_noirq(struct device *dev)
+{
+	struct acpi_ec *ec = acpi_driver_data(to_acpi_device(dev));
+
+	/*
+	 * The SCI handler doesn't run at this point, so the GPE can be
+	 * masked at the low level without side effects.
+	 */
+	if (ec_no_wakeup && test_bit(EC_FLAGS_STARTED, &ec->flags) &&
+	    ec->reference_count >= 1)
+		acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE);
+
+	return 0;
+}
+
+static int acpi_ec_resume_noirq(struct device *dev)
+{
+	struct acpi_ec *ec = acpi_driver_data(to_acpi_device(dev));
+
+	if (ec_no_wakeup && test_bit(EC_FLAGS_STARTED, &ec->flags) &&
+	    ec->reference_count >= 1)
+		acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_ENABLE);
+
+	return 0;
+}
+
 static int acpi_ec_resume(struct device *dev)
 {
 	struct acpi_ec *ec =
@@ -1891,6 +1921,7 @@  static int acpi_ec_resume(struct device
 #endif
 
 static const struct dev_pm_ops acpi_ec_pm = {
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(acpi_ec_suspend_noirq, acpi_ec_resume_noirq)
 	SET_SYSTEM_SLEEP_PM_OPS(acpi_ec_suspend, acpi_ec_resume)
 };