Re: [PATCH] ACPI / EC: Add parameter force disable the GPE on suspend

[Rafael J. Wysocki]

On Wed, Jul 19, 2017 at 1:04 AM, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

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@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html