gpiolib: acpi: Rework honor_wakeup option into an ignore_wake option

Commit aa23ca3d98f7 ("gpiolib: acpi: Add honor_wakeup module-option +
quirk mechanism") was added to deal with spurious wakeups on one specific
model of the HP x2 10 series.

The approach taken there was to add a bool controlling wakeup support for
all ACPI GPIO events. This was sufficient for the specific HP x2 10 model
the commit was trying to fix, but in the mean time other models have
turned up which need a similar workaround to avoid spurious wakeups from
suspend, but only for one of the pins on which the ACPI tables request
ACPI GPIO events.

Since the honor_wakeup option was added to be able to ignore wake events,
the name was perhaps not the best, this commit renames it to ignore_wake
and changes it to a string with the following format:
gpiolib_acpi.ignore_wake=controller@pin[,controller@pin[,...]]

This allows working around spurious wakeup issues on a per pin basis.

This commit also reworks the existing quirk for the HP x2 10 so that
it functions as before.

Note:
-This removes the honor_wakeup parameter. This has only been upstream for
 a short time and to the best of my knowledge there are no users using
 this module parameter.

-The controller@pin[,controller@pin[,...]] syntax is based on an existing
 kernel module parameter using the same controller@pin format. That version
 uses ';' as separator, but in practice that is problematic because grub2
 cannot handle this without taking special care to escape the ';', so here
 we are using a ',' as separator instead which does not have this issue.

Fixes: aa23ca3d98f7 ("gpiolib: acpi: Add honor_wakeup module-option + quirk mechanism")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20200302111225.6641-2-hdegoede@redhat.com
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index a77edd31dd600ff0001626a64326a3a61baeb7cb..87bb0d3c69da406e668ca79f494a1bc8cd0dc10b 100644 (file)
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -21,18 +21,21 @@
 #include "gpiolib.h"
 #include "gpiolib-acpi.h"
 
-#define QUIRK_NO_EDGE_EVENTS_ON_BOOT           0x01l
-#define QUIRK_NO_WAKEUP                                0x02l
-
 static int run_edge_events_on_boot = -1;
 module_param(run_edge_events_on_boot, int, 0444);
 MODULE_PARM_DESC(run_edge_events_on_boot,
                 "Run edge _AEI event-handlers at boot: 0=no, 1=yes, -1=auto");
 
-static int honor_wakeup = -1;
-module_param(honor_wakeup, int, 0444);
-MODULE_PARM_DESC(honor_wakeup,
-                "Honor the ACPI wake-capable flag: 0=no, 1=yes, -1=auto");
+static char *ignore_wake;
+module_param(ignore_wake, charp, 0444);
+MODULE_PARM_DESC(ignore_wake,
+                "controller@pin combos on which to ignore the ACPI wake flag "
+                "ignore_wake=controller@pin[,controller@pin[,...]]");
+
+struct acpi_gpiolib_dmi_quirk {
+       bool no_edge_events_on_boot;
+       char *ignore_wake;
+};
 
 /**
  * struct acpi_gpio_event - ACPI GPIO event handler data
@@ -202,6 +205,57 @@ static void acpi_gpiochip_request_irqs(struct acpi_gpio_chip *acpi_gpio)
                acpi_gpiochip_request_irq(acpi_gpio, event);
 }
 
+static bool acpi_gpio_in_ignore_list(const char *controller_in, int pin_in)
+{
+       const char *controller, *pin_str;
+       int len, pin;
+       char *endp;
+
+       controller = ignore_wake;
+       while (controller) {
+               pin_str = strchr(controller, '@');
+               if (!pin_str)
+                       goto err;
+
+               len = pin_str - controller;
+               if (len == strlen(controller_in) &&
+                   strncmp(controller, controller_in, len) == 0) {
+                       pin = simple_strtoul(pin_str + 1, &endp, 10);
+                       if (*endp != 0 && *endp != ',')
+                               goto err;
+
+                       if (pin == pin_in)
+                               return true;
+               }
+
+               controller = strchr(controller, ',');
+               if (controller)
+                       controller++;
+       }
+
+       return false;
+err:
+       pr_err_once("Error invalid value for gpiolib_acpi.ignore_wake: %s\n",
+                   ignore_wake);
+       return false;
+}
+
+static bool acpi_gpio_irq_is_wake(struct device *parent,
+                                 struct acpi_resource_gpio *agpio)
+{
+       int pin = agpio->pin_table[0];
+
+       if (agpio->wake_capable != ACPI_WAKE_CAPABLE)
+               return false;
+
+       if (acpi_gpio_in_ignore_list(dev_name(parent), pin)) {
+               dev_info(parent, "Ignoring wakeup on pin %d\n", pin);
+               return false;
+       }
+
+       return true;
+}
+
 /* Always returns AE_OK so that we keep looping over the resources */
 static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
                                             void *context)
@@ -289,7 +343,7 @@ static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
        event->handle = evt_handle;
        event->handler = handler;
        event->irq = irq;
-       event->irq_is_wake = honor_wakeup && agpio->wake_capable == ACPI_WAKE_CAPABLE;
+       event->irq_is_wake = acpi_gpio_irq_is_wake(chip->parent, agpio);
        event->pin = pin;
        event->desc = desc;
 
@@ -1328,7 +1382,9 @@ static const struct dmi_system_id gpiolib_acpi_quirks[] = {
                        DMI_MATCH(DMI_SYS_VENDOR, "MINIX"),
                        DMI_MATCH(DMI_PRODUCT_NAME, "Z83-4"),
                },
-               .driver_data = (void *)QUIRK_NO_EDGE_EVENTS_ON_BOOT,
+               .driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+                       .no_edge_events_on_boot = true,
+               },
        },
        {
                /*
@@ -1341,7 +1397,9 @@ static const struct dmi_system_id gpiolib_acpi_quirks[] = {
                        DMI_MATCH(DMI_SYS_VENDOR, "Wortmann_AG"),
                        DMI_MATCH(DMI_PRODUCT_NAME, "TERRA_PAD_1061"),
                },
-               .driver_data = (void *)QUIRK_NO_EDGE_EVENTS_ON_BOOT,
+               .driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+                       .no_edge_events_on_boot = true,
+               },
        },
        {
                /*
@@ -1360,33 +1418,31 @@ static const struct dmi_system_id gpiolib_acpi_quirks[] = {
                        DMI_MATCH(DMI_SYS_VENDOR, "HP"),
                        DMI_MATCH(DMI_PRODUCT_NAME, "HP x2 Detachable 10-p0XX"),
                },
-               .driver_data = (void *)QUIRK_NO_WAKEUP,
+               .driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+                       .ignore_wake = "INT33FF:01@0,INT0002:00@2",
+               },
        },
        {} /* Terminating entry */
 };
 
 static int acpi_gpio_setup_params(void)
 {
+       const struct acpi_gpiolib_dmi_quirk *quirk = NULL;
        const struct dmi_system_id *id;
-       long quirks = 0;
 
        id = dmi_first_match(gpiolib_acpi_quirks);
        if (id)
-               quirks = (long)id->driver_data;
+               quirk = id->driver_data;
 
        if (run_edge_events_on_boot < 0) {
-               if (quirks & QUIRK_NO_EDGE_EVENTS_ON_BOOT)
+               if (quirk && quirk->no_edge_events_on_boot)
                        run_edge_events_on_boot = 0;
                else
                        run_edge_events_on_boot = 1;
        }
 
-       if (honor_wakeup < 0) {
-               if (quirks & QUIRK_NO_WAKEUP)
-                       honor_wakeup = 0;
-               else
-                       honor_wakeup = 1;
-       }
+       if (ignore_wake == NULL && quirk && quirk->ignore_wake)
+               ignore_wake = quirk->ignore_wake;
 
        return 0;
 }