[PATCH v6 0/1] platform/x86: Add driver for ACPI INT0002 Virtual GPIO

[PATCH v6 0/1] platform/x86: Add driver for ACPI INT0002 Virtual GPIO

[Hans de Goede]

Hi All,

Here is v6 of my patch adding a driver for the ACPI INT0002 Virtual GPIO
device. When I started working on this driver using 4.11 as kernel this
fixed an irq-storm on irq 9, with some of the recent suspend-to-idle wakeup
changes which have landed in 4.12-rc1 this "driver" is necessary to fix
affected systems getting stuck in suspend when the user tries to wake
them through a peripheral which signals PME to the PMC to wakeup the system,
such as the xHCI controller.

As such ideally this driver should get added to a later 4.12-rc# as a
bugfix. Also see this linux-acpi mailinglist-thread:
https://www.spinics.net/lists/linux-acpi/msg74614.html

Regards,

Hans
	

[PATCH v6] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device

[Hans de Goede]

Some peripherals on Bay Trail and Cherry Trail platforms signal a
Power Management Event (PME) to the Power Management Controller (PMC)
to wakeup the system. When this happens software needs to explicitly
clear the PME bus 0 status bit in the GPE0a_STS register to avoid an
IRQ storm on IRQ 9.

This is modelled in ACPI through the INT0002 ACPI device, which is
called a "Virtual GPIO controller" in ACPI because it defines the
event handler to call when the PME triggers through _AEI and _L02
methods as would be done for a real GPIO interrupt in ACPI.

This commit adds a driver which registers the Virtual GPIOs expected
by the DSDT on these devices, letting gpiolib-acpi claim the
virtual GPIO and install a GPIO-interrupt handler which call the _L02
handler as it would for a real GPIO controller.

Cc: joeyli <jlee@suse.com>
Cc: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
Changes in v2:
-Remove dev_err after malloc failure
-Remove unused empty runtime pm callbacks
-s/GPE0A_PME_/GPE0A_PME_B0_/
-Fixed some checkpatch warnings (I forgot to run checkpatch on v1)
Changes in v3:
-Rewrite as gpiochip driver letting gpiolib-acpi deal with claiming the pin
 0x0002 and calling the _L02 event handler when the virtual gpio-irq triggers
-Rebase on 4.12-rc1
Changes in v4:
-Drop device_init_wakeup() from _probe(), use pm_system_wakeup() instead
 of pm_wakeup_hard_event(chip->parent)
-Improve commit message
Changes in v5:
-Use BIT() macro for FOO_BIT defines
-Drop unneeded ACPI_PTR macro usage
Changes in v6:
-Move back to drivers/platform/x86
-Expand certain acronyms (PME, PMC)
-Use linux/gpio/driver.h include instead of linux/gpio.h
-Document why the get / set / direction_output functions are dummys
-No functional changes
---
 drivers/platform/x86/Kconfig               |  19 +++
 drivers/platform/x86/Makefile              |   1 +
 drivers/platform/x86/intel_int0002_vgpio.c | 218 +++++++++++++++++++++++++++++
 3 files changed, 238 insertions(+)
 create mode 100644 drivers/platform/x86/intel_int0002_vgpio.c

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 8489020..a3ccc3c 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -794,6 +794,25 @@ config INTEL_CHT_INT33FE
 	  This driver instantiates i2c-clients for these, so that standard
 	  i2c drivers for these chips can bind to the them.
 
+config INTEL_INT0002_VGPIO
+	tristate "Intel ACPI INT0002 Virtual GPIO driver"
+	depends on GPIOLIB && ACPI
+	select GPIOLIB_IRQCHIP
+	---help---
+	  Some peripherals on Bay Trail and Cherry Trail platforms signal a
+	  Power Management Event (PME) to the Power Management Controller (PMC)
+	  to wakeup the system. When this happens software needs to explicitly
+	  clear the PME bus 0 status bit in the GPE0a_STS register to avoid an
+	  IRQ storm on IRQ 9.
+
+	  This is modelled in ACPI through the INT0002 ACPI device, which is
+	  called a "Virtual GPIO controller" in ACPI because it defines the
+	  event handler to call when the PME triggers through _AEI and _L02
+	  methods as would be done for a real GPIO interrupt in ACPI.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called intel_int0002_vgpio.
+
 config INTEL_HID_EVENT
 	tristate "INTEL HID Event"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 182a3ed..ab22ce7 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_TOSHIBA_BT_RFKILL)	+= toshiba_bluetooth.o
 obj-$(CONFIG_TOSHIBA_HAPS)	+= toshiba_haps.o
 obj-$(CONFIG_TOSHIBA_WMI)	+= toshiba-wmi.o
 obj-$(CONFIG_INTEL_CHT_INT33FE)	+= intel_cht_int33fe.o
+obj-$(CONFIG_INTEL_INT0002_VGPIO) += intel_int0002_vgpio.o
 obj-$(CONFIG_INTEL_HID_EVENT)	+= intel-hid.o
 obj-$(CONFIG_INTEL_VBTN)	+= intel-vbtn.o
 obj-$(CONFIG_INTEL_SCU_IPC)	+= intel_scu_ipc.o
diff --git a/drivers/platform/x86/intel_int0002_vgpio.c b/drivers/platform/x86/intel_int0002_vgpio.c
new file mode 100644
index 0000000..e524b49
--- /dev/null
+++ b/drivers/platform/x86/intel_int0002_vgpio.c
@@ -0,0 +1,218 @@
+/*
+ * Intel INT0002 "Virtual GPIO" driver
+ *
+ * Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
+ *
+ * Loosely based on android x86 kernel code which is:
+ *
+ * Copyright (c) 2014, Intel Corporation.
+ *
+ * Author: Dyut Kumar Sil <dyut.k.sil@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Some peripherals on Bay Trail and Cherry Trail platforms signal a Power
+ * Management Event (PME) to the Power Management Controller (PMC) to wakeup
+ * the system. When this happens software needs to clear the PME bus 0 status
+ * bit in the GPE0a_STS register to avoid an IRQ storm on IRQ 9.
+ *
+ * This is modelled in ACPI through the INT0002 ACPI device, which is
+ * called a "Virtual GPIO controller" in ACPI because it defines the event
+ * handler to call when the PME triggers through _AEI and _L02 / _E02
+ * methods as would be done for a real GPIO interrupt in ACPI. Note this
+ * is a hack to define an AML event handler for the PME while using existing
+ * ACPI mechanisms, this is not a real GPIO at all.
+ *
+ * This driver will bind to the INT0002 device, and register as a GPIO
+ * controller, letting gpiolib-acpi.c call the _L02 handler as it would
+ * for a real GPIO controller.
+ */
+
+#include <asm/cpu_device_id.h>
+#include <asm/intel-family.h>
+#include <linux/acpi.h>
+#include <linux/gpio/driver.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/suspend.h>
+
+#define DRV_NAME			"INT0002 Virtual GPIO"
+
+/* For some reason the virtual GPIO pin tied to the GPE is numbered pin 2 */
+#define GPE0A_PME_B0_VIRT_GPIO_PIN	2
+
+#define GPE0A_PME_B0_STS_BIT		BIT(13)
+#define GPE0A_PME_B0_EN_BIT		BIT(13)
+#define GPE0A_STS_PORT			0x420
+#define GPE0A_EN_PORT			0x428
+
+#define ICPU(model)	{ X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, }
+
+static const struct x86_cpu_id int0002_cpu_ids[] = {
+/*
+ * Limit ourselves to Cherry Trail for now, until testing shows we
+ * need to handle the INT0002 device on Baytrail too.
+ *	ICPU(INTEL_FAM6_ATOM_SILVERMONT1),	 * Valleyview, Bay Trail *
+ */
+	ICPU(INTEL_FAM6_ATOM_AIRMONT),		/* Braswell, Cherry Trail */
+	{}
+};
+
+/*
+ * As this is not a real GPIO at all, but just a hack to model an event in
+ * APCI the get / set functions are dummy functions.
+ */
+
+static int int0002_gpio_get(struct gpio_chip *chip, unsigned int offset)
+{
+	return 0;
+}
+
+static void int0002_gpio_set(struct gpio_chip *chip, unsigned int offset,
+			     int value)
+{
+}
+
+static int int0002_gpio_direction_output(struct gpio_chip *chip,
+					 unsigned int offset, int value)
+{
+	return 0;
+}
+
+static void int0002_irq_ack(struct irq_data *data)
+{
+	outl(GPE0A_PME_B0_STS_BIT, GPE0A_STS_PORT);
+}
+
+static void int0002_irq_unmask(struct irq_data *data)
+{
+	u32 gpe_en_reg;
+
+	gpe_en_reg = inl(GPE0A_EN_PORT);
+	gpe_en_reg |= GPE0A_PME_B0_EN_BIT;
+	outl(gpe_en_reg, GPE0A_EN_PORT);
+}
+
+static void int0002_irq_mask(struct irq_data *data)
+{
+	u32 gpe_en_reg;
+
+	gpe_en_reg = inl(GPE0A_EN_PORT);
+	gpe_en_reg &= ~GPE0A_PME_B0_EN_BIT;
+	outl(gpe_en_reg, GPE0A_EN_PORT);
+}
+
+static irqreturn_t int0002_irq(int irq, void *data)
+{
+	struct gpio_chip *chip = data;
+	u32 gpe_sts_reg;
+
+	gpe_sts_reg = inl(GPE0A_STS_PORT);
+	if (!(gpe_sts_reg & GPE0A_PME_B0_STS_BIT))
+		return IRQ_NONE;
+
+	generic_handle_irq(irq_find_mapping(chip->irqdomain,
+					    GPE0A_PME_B0_VIRT_GPIO_PIN));
+
+	pm_system_wakeup();
+
+	return IRQ_HANDLED;
+}
+
+static struct irq_chip int0002_irqchip = {
+	.name			= DRV_NAME,
+	.irq_ack		= int0002_irq_ack,
+	.irq_mask		= int0002_irq_mask,
+	.irq_unmask		= int0002_irq_unmask,
+};
+
+static int int0002_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	const struct x86_cpu_id *cpu_id;
+	struct gpio_chip *chip;
+	int i, irq, ret;
+
+	/* Menlow has a different INT0002 device? <sigh> */
+	cpu_id = x86_match_cpu(int0002_cpu_ids);
+	if (!cpu_id)
+		return -ENODEV;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(dev, "Error getting IRQ: %d\n", irq);
+		return irq;
+	}
+
+	chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	chip->label = DRV_NAME;
+	chip->parent = dev;
+	chip->owner = THIS_MODULE;
+	chip->get = int0002_gpio_get;
+	chip->set = int0002_gpio_set;
+	chip->direction_input = int0002_gpio_get;
+	chip->direction_output = int0002_gpio_direction_output;
+	chip->base = -1;
+	chip->ngpio = GPE0A_PME_B0_VIRT_GPIO_PIN + 1;
+	chip->irq_need_valid_mask = true;
+
+	ret = devm_gpiochip_add_data(&pdev->dev, chip, NULL);
+	if (ret) {
+		dev_err(dev, "Error adding gpio chip: %d\n", ret);
+		return ret;
+	}
+
+	for (i = 0; i < GPE0A_PME_B0_VIRT_GPIO_PIN; i++)
+		clear_bit(i, chip->irq_valid_mask);
+
+	/*
+	 * We manually request the irq here instead of passing a flow-handler
+	 * to gpiochip_set_chained_irqchip, because the irq is shared.
+	 */
+	ret = devm_request_irq(dev, irq, int0002_irq,
+			       IRQF_SHARED | IRQF_NO_THREAD, "INT0002", chip);
+	if (ret) {
+		dev_err(dev, "Error requesting IRQ %d: %d\n", irq, ret);
+		return ret;
+	}
+
+	ret = gpiochip_irqchip_add(chip, &int0002_irqchip, 0, handle_edge_irq,
+				   IRQ_TYPE_NONE);
+	if (ret) {
+		dev_err(dev, "Error adding irqchip: %d\n", ret);
+		return ret;
+	}
+
+	gpiochip_set_chained_irqchip(chip, &int0002_irqchip, irq, NULL);
+
+	return 0;
+}
+
+static const struct acpi_device_id int0002_acpi_ids[] = {
+	{ "INT0002", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(acpi, int0002_acpi_ids);
+
+static struct platform_driver int0002_driver = {
+	.driver = {
+		.name			= DRV_NAME,
+		.acpi_match_table	= int0002_acpi_ids,
+	},
+	.probe	= int0002_probe,
+};
+
+module_platform_driver(int0002_driver);
+
+MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
+MODULE_DESCRIPTION("Intel INT0002 Virtual GPIO driver");
+MODULE_LICENSE("GPL");
-- 
2.9.4

Re: [PATCH v6] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device

[Andy Shevchenko]

On Fri, Jun 2, 2017 at 6:15 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Some peripherals on Bay Trail and Cherry Trail platforms signal a
> Power Management Event (PME) to the Power Management Controller (PMC)
> to wakeup the system. When this happens software needs to explicitly
> clear the PME bus 0 status bit in the GPE0a_STS register to avoid an
> IRQ storm on IRQ 9.
>
> This is modelled in ACPI through the INT0002 ACPI device, which is
> called a "Virtual GPIO controller" in ACPI because it defines the
> event handler to call when the PME triggers through _AEI and _L02
> methods as would be done for a real GPIO interrupt in ACPI.
>
> This commit adds a driver which registers the Virtual GPIOs expected
> by the DSDT on these devices, letting gpiolib-acpi claim the
> virtual GPIO and install a GPIO-interrupt handler which call the _L02
> handler as it would for a real GPIO controller.
>

Alexandre is not anymore in GPIO boat.

Linus, I would like to have your formal tag on this (since linux/gpio/driver.h).

> +#include <asm/cpu_device_id.h>
> +#include <asm/intel-family.h>

I would move this after linux/* section.

> +#include <linux/acpi.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/suspend.h>

> +/*
> + * As this is not a real GPIO at all, but just a hack to model an event in

> + * APCI the get / set functions are dummy functions.

ACPI ?

> + */

> +static int int0002_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       const struct x86_cpu_id *cpu_id;
> +       struct gpio_chip *chip;
> +       int i, irq, ret;
> +

> +       /* Menlow has a different INT0002 device? <sigh> */
> +       cpu_id = x86_match_cpu(int0002_cpu_ids);
> +       if (!cpu_id)
> +               return -ENODEV;

Consider this in my TODO list to check if we can get rid of ancient
Menlow code for good.

> +       for (i = 0; i < GPE0A_PME_B0_VIRT_GPIO_PIN; i++)
> +               clear_bit(i, chip->irq_valid_mask);

bitmap_clear();

I would fix all above when applying, so, just waiting for Linus' tag.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device

[Andy Shevchenko]

On Fri, Jun 2, 2017 at 6:15 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Some peripherals on Bay Trail and Cherry Trail platforms signal a
> Power Management Event (PME) to the Power Management Controller (PMC)
> to wakeup the system. When this happens software needs to explicitly
> clear the PME bus 0 status bit in the GPE0a_STS register to avoid an
> IRQ storm on IRQ 9.
>
> This is modelled in ACPI through the INT0002 ACPI device, which is
> called a "Virtual GPIO controller" in ACPI because it defines the
> event handler to call when the PME triggers through _AEI and _L02
> methods as would be done for a real GPIO interrupt in ACPI.
>
> This commit adds a driver which registers the Virtual GPIOs expected
> by the DSDT on these devices, letting gpiolib-acpi claim the
> virtual GPIO and install a GPIO-interrupt handler which call the _L02
> handler as it would for a real GPIO controller.
>

Pushed to testing w/o Linus' tag (there is no one yet)

Linus, if you have objections, tell me.

> Cc: joeyli <jlee@suse.com>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> Changes in v2:
> -Remove dev_err after malloc failure
> -Remove unused empty runtime pm callbacks
> -s/GPE0A_PME_/GPE0A_PME_B0_/
> -Fixed some checkpatch warnings (I forgot to run checkpatch on v1)
> Changes in v3:
> -Rewrite as gpiochip driver letting gpiolib-acpi deal with claiming the pin
>  0x0002 and calling the _L02 event handler when the virtual gpio-irq triggers
> -Rebase on 4.12-rc1
> Changes in v4:
> -Drop device_init_wakeup() from _probe(), use pm_system_wakeup() instead
>  of pm_wakeup_hard_event(chip->parent)
> -Improve commit message
> Changes in v5:
> -Use BIT() macro for FOO_BIT defines
> -Drop unneeded ACPI_PTR macro usage
> Changes in v6:
> -Move back to drivers/platform/x86
> -Expand certain acronyms (PME, PMC)
> -Use linux/gpio/driver.h include instead of linux/gpio.h
> -Document why the get / set / direction_output functions are dummys
> -No functional changes
> ---
>  drivers/platform/x86/Kconfig               |  19 +++
>  drivers/platform/x86/Makefile              |   1 +
>  drivers/platform/x86/intel_int0002_vgpio.c | 218 +++++++++++++++++++++++++++++
>  3 files changed, 238 insertions(+)
>  create mode 100644 drivers/platform/x86/intel_int0002_vgpio.c
>
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 8489020..a3ccc3c 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -794,6 +794,25 @@ config INTEL_CHT_INT33FE
>           This driver instantiates i2c-clients for these, so that standard
>           i2c drivers for these chips can bind to the them.
>
> +config INTEL_INT0002_VGPIO
> +       tristate "Intel ACPI INT0002 Virtual GPIO driver"
> +       depends on GPIOLIB && ACPI
> +       select GPIOLIB_IRQCHIP
> +       ---help---
> +         Some peripherals on Bay Trail and Cherry Trail platforms signal a
> +         Power Management Event (PME) to the Power Management Controller (PMC)
> +         to wakeup the system. When this happens software needs to explicitly
> +         clear the PME bus 0 status bit in the GPE0a_STS register to avoid an
> +         IRQ storm on IRQ 9.
> +
> +         This is modelled in ACPI through the INT0002 ACPI device, which is
> +         called a "Virtual GPIO controller" in ACPI because it defines the
> +         event handler to call when the PME triggers through _AEI and _L02
> +         methods as would be done for a real GPIO interrupt in ACPI.
> +
> +         To compile this driver as a module, choose M here: the module will
> +         be called intel_int0002_vgpio.
> +
>  config INTEL_HID_EVENT
>         tristate "INTEL HID Event"
>         depends on ACPI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 182a3ed..ab22ce7 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -46,6 +46,7 @@ obj-$(CONFIG_TOSHIBA_BT_RFKILL)       += toshiba_bluetooth.o
>  obj-$(CONFIG_TOSHIBA_HAPS)     += toshiba_haps.o
>  obj-$(CONFIG_TOSHIBA_WMI)      += toshiba-wmi.o
>  obj-$(CONFIG_INTEL_CHT_INT33FE)        += intel_cht_int33fe.o
> +obj-$(CONFIG_INTEL_INT0002_VGPIO) += intel_int0002_vgpio.o
>  obj-$(CONFIG_INTEL_HID_EVENT)  += intel-hid.o
>  obj-$(CONFIG_INTEL_VBTN)       += intel-vbtn.o
>  obj-$(CONFIG_INTEL_SCU_IPC)    += intel_scu_ipc.o
> diff --git a/drivers/platform/x86/intel_int0002_vgpio.c b/drivers/platform/x86/intel_int0002_vgpio.c
> new file mode 100644
> index 0000000..e524b49
> --- /dev/null
> +++ b/drivers/platform/x86/intel_int0002_vgpio.c
> @@ -0,0 +1,218 @@
> +/*
> + * Intel INT0002 "Virtual GPIO" driver
> + *
> + * Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com>
> + *
> + * Loosely based on android x86 kernel code which is:
> + *
> + * Copyright (c) 2014, Intel Corporation.
> + *
> + * Author: Dyut Kumar Sil <dyut.k.sil@intel.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * Some peripherals on Bay Trail and Cherry Trail platforms signal a Power
> + * Management Event (PME) to the Power Management Controller (PMC) to wakeup
> + * the system. When this happens software needs to clear the PME bus 0 status
> + * bit in the GPE0a_STS register to avoid an IRQ storm on IRQ 9.
> + *
> + * This is modelled in ACPI through the INT0002 ACPI device, which is
> + * called a "Virtual GPIO controller" in ACPI because it defines the event
> + * handler to call when the PME triggers through _AEI and _L02 / _E02
> + * methods as would be done for a real GPIO interrupt in ACPI. Note this
> + * is a hack to define an AML event handler for the PME while using existing
> + * ACPI mechanisms, this is not a real GPIO at all.
> + *
> + * This driver will bind to the INT0002 device, and register as a GPIO
> + * controller, letting gpiolib-acpi.c call the _L02 handler as it would
> + * for a real GPIO controller.
> + */
> +
> +#include <asm/cpu_device_id.h>
> +#include <asm/intel-family.h>
> +#include <linux/acpi.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/suspend.h>
> +
> +#define DRV_NAME                       "INT0002 Virtual GPIO"
> +
> +/* For some reason the virtual GPIO pin tied to the GPE is numbered pin 2 */
> +#define GPE0A_PME_B0_VIRT_GPIO_PIN     2
> +
> +#define GPE0A_PME_B0_STS_BIT           BIT(13)
> +#define GPE0A_PME_B0_EN_BIT            BIT(13)
> +#define GPE0A_STS_PORT                 0x420
> +#define GPE0A_EN_PORT                  0x428
> +
> +#define ICPU(model)    { X86_VENDOR_INTEL, 6, model, X86_FEATURE_ANY, }
> +
> +static const struct x86_cpu_id int0002_cpu_ids[] = {
> +/*
> + * Limit ourselves to Cherry Trail for now, until testing shows we
> + * need to handle the INT0002 device on Baytrail too.
> + *     ICPU(INTEL_FAM6_ATOM_SILVERMONT1),       * Valleyview, Bay Trail *
> + */
> +       ICPU(INTEL_FAM6_ATOM_AIRMONT),          /* Braswell, Cherry Trail */
> +       {}
> +};
> +
> +/*
> + * As this is not a real GPIO at all, but just a hack to model an event in
> + * APCI the get / set functions are dummy functions.
> + */
> +
> +static int int0002_gpio_get(struct gpio_chip *chip, unsigned int offset)
> +{
> +       return 0;
> +}
> +
> +static void int0002_gpio_set(struct gpio_chip *chip, unsigned int offset,
> +                            int value)
> +{
> +}
> +
> +static int int0002_gpio_direction_output(struct gpio_chip *chip,
> +                                        unsigned int offset, int value)
> +{
> +       return 0;
> +}
> +
> +static void int0002_irq_ack(struct irq_data *data)
> +{
> +       outl(GPE0A_PME_B0_STS_BIT, GPE0A_STS_PORT);
> +}
> +
> +static void int0002_irq_unmask(struct irq_data *data)
> +{
> +       u32 gpe_en_reg;
> +
> +       gpe_en_reg = inl(GPE0A_EN_PORT);
> +       gpe_en_reg |= GPE0A_PME_B0_EN_BIT;
> +       outl(gpe_en_reg, GPE0A_EN_PORT);
> +}
> +
> +static void int0002_irq_mask(struct irq_data *data)
> +{
> +       u32 gpe_en_reg;
> +
> +       gpe_en_reg = inl(GPE0A_EN_PORT);
> +       gpe_en_reg &= ~GPE0A_PME_B0_EN_BIT;
> +       outl(gpe_en_reg, GPE0A_EN_PORT);
> +}
> +
> +static irqreturn_t int0002_irq(int irq, void *data)
> +{
> +       struct gpio_chip *chip = data;
> +       u32 gpe_sts_reg;
> +
> +       gpe_sts_reg = inl(GPE0A_STS_PORT);
> +       if (!(gpe_sts_reg & GPE0A_PME_B0_STS_BIT))
> +               return IRQ_NONE;
> +
> +       generic_handle_irq(irq_find_mapping(chip->irqdomain,
> +                                           GPE0A_PME_B0_VIRT_GPIO_PIN));
> +
> +       pm_system_wakeup();
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static struct irq_chip int0002_irqchip = {
> +       .name                   = DRV_NAME,
> +       .irq_ack                = int0002_irq_ack,
> +       .irq_mask               = int0002_irq_mask,
> +       .irq_unmask             = int0002_irq_unmask,
> +};
> +
> +static int int0002_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       const struct x86_cpu_id *cpu_id;
> +       struct gpio_chip *chip;
> +       int i, irq, ret;
> +
> +       /* Menlow has a different INT0002 device? <sigh> */
> +       cpu_id = x86_match_cpu(int0002_cpu_ids);
> +       if (!cpu_id)
> +               return -ENODEV;
> +
> +       irq = platform_get_irq(pdev, 0);
> +       if (irq < 0) {
> +               dev_err(dev, "Error getting IRQ: %d\n", irq);
> +               return irq;
> +       }
> +
> +       chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> +       if (!chip)
> +               return -ENOMEM;
> +
> +       chip->label = DRV_NAME;
> +       chip->parent = dev;
> +       chip->owner = THIS_MODULE;
> +       chip->get = int0002_gpio_get;
> +       chip->set = int0002_gpio_set;
> +       chip->direction_input = int0002_gpio_get;
> +       chip->direction_output = int0002_gpio_direction_output;
> +       chip->base = -1;
> +       chip->ngpio = GPE0A_PME_B0_VIRT_GPIO_PIN + 1;
> +       chip->irq_need_valid_mask = true;
> +
> +       ret = devm_gpiochip_add_data(&pdev->dev, chip, NULL);
> +       if (ret) {
> +               dev_err(dev, "Error adding gpio chip: %d\n", ret);
> +               return ret;
> +       }
> +
> +       for (i = 0; i < GPE0A_PME_B0_VIRT_GPIO_PIN; i++)
> +               clear_bit(i, chip->irq_valid_mask);
> +
> +       /*
> +        * We manually request the irq here instead of passing a flow-handler
> +        * to gpiochip_set_chained_irqchip, because the irq is shared.
> +        */
> +       ret = devm_request_irq(dev, irq, int0002_irq,
> +                              IRQF_SHARED | IRQF_NO_THREAD, "INT0002", chip);
> +       if (ret) {
> +               dev_err(dev, "Error requesting IRQ %d: %d\n", irq, ret);
> +               return ret;
> +       }
> +
> +       ret = gpiochip_irqchip_add(chip, &int0002_irqchip, 0, handle_edge_irq,
> +                                  IRQ_TYPE_NONE);
> +       if (ret) {
> +               dev_err(dev, "Error adding irqchip: %d\n", ret);
> +               return ret;
> +       }
> +
> +       gpiochip_set_chained_irqchip(chip, &int0002_irqchip, irq, NULL);
> +
> +       return 0;
> +}
> +
> +static const struct acpi_device_id int0002_acpi_ids[] = {
> +       { "INT0002", 0 },
> +       { },
> +};
> +MODULE_DEVICE_TABLE(acpi, int0002_acpi_ids);
> +
> +static struct platform_driver int0002_driver = {
> +       .driver = {
> +               .name                   = DRV_NAME,
> +               .acpi_match_table       = int0002_acpi_ids,
> +       },
> +       .probe  = int0002_probe,
> +};
> +
> +module_platform_driver(int0002_driver);
> +
> +MODULE_AUTHOR("Hans de Goede <hdegoede@redhat.com>");
> +MODULE_DESCRIPTION("Intel INT0002 Virtual GPIO driver");
> +MODULE_LICENSE("GPL");
> --
> 2.9.4
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device

[Darren Hart]

On Wed, Jun 07, 2017 at 05:53:38PM +0300, Andy Shevchenko wrote:
> On Fri, Jun 2, 2017 at 6:15 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> > Some peripherals on Bay Trail and Cherry Trail platforms signal a
> > Power Management Event (PME) to the Power Management Controller (PMC)
> > to wakeup the system. When this happens software needs to explicitly
> > clear the PME bus 0 status bit in the GPE0a_STS register to avoid an
> > IRQ storm on IRQ 9.
> >
> > This is modelled in ACPI through the INT0002 ACPI device, which is
> > called a "Virtual GPIO controller" in ACPI because it defines the
> > event handler to call when the PME triggers through _AEI and _L02
> > methods as would be done for a real GPIO interrupt in ACPI.
> >
> > This commit adds a driver which registers the Virtual GPIOs expected
> > by the DSDT on these devices, letting gpiolib-acpi claim the
> > virtual GPIO and install a GPIO-interrupt handler which call the _L02
> > handler as it would for a real GPIO controller.
> >
> 
> Pushed to testing w/o Linus' tag (there is no one yet)

Will you be taking this through fixes Andy? For a 4.12-rc* target per Hans'
response to the cover letter?

-- 
Darren Hart
VMware Open Source Technology Center

Re: [PATCH v6] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device

[Hans de Goede]

Hi,

On 06/08/2017 05:45 PM, Darren Hart wrote:
> On Wed, Jun 07, 2017 at 05:53:38PM +0300, Andy Shevchenko wrote:
>> On Fri, Jun 2, 2017 at 6:15 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Some peripherals on Bay Trail and Cherry Trail platforms signal a
>>> Power Management Event (PME) to the Power Management Controller (PMC)
>>> to wakeup the system. When this happens software needs to explicitly
>>> clear the PME bus 0 status bit in the GPE0a_STS register to avoid an
>>> IRQ storm on IRQ 9.
>>>
>>> This is modelled in ACPI through the INT0002 ACPI device, which is
>>> called a "Virtual GPIO controller" in ACPI because it defines the
>>> event handler to call when the PME triggers through _AEI and _L02
>>> methods as would be done for a real GPIO interrupt in ACPI.
>>>
>>> This commit adds a driver which registers the Virtual GPIOs expected
>>> by the DSDT on these devices, letting gpiolib-acpi claim the
>>> virtual GPIO and install a GPIO-interrupt handler which call the _L02
>>> handler as it would for a real GPIO controller.
>>>
>>
>> Pushed to testing w/o Linus' tag (there is no one yet)
> 
> Will you be taking this through fixes Andy? For a 4.12-rc* target per Hans'
> response to the cover letter?

Note that Rafael is planning to drop (revert) the commit which makes
it (extra) desirable for this driver to get into 4.12, so we should
perhaps reconsider that. I'm fine either way, but the primary reason
to push this as a fix for 4.12 is no longer valid AFAICT.

Regards,

Hans

Re: [PATCH v6] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device

[Rafael J. Wysocki]

On Thu, Jun 8, 2017 at 6:45 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 06/08/2017 05:45 PM, Darren Hart wrote:
>>
>> On Wed, Jun 07, 2017 at 05:53:38PM +0300, Andy Shevchenko wrote:
>>>
>>> On Fri, Jun 2, 2017 at 6:15 PM, Hans de Goede <hdegoede@redhat.com>
>>> wrote:
>>>>
>>>> Some peripherals on Bay Trail and Cherry Trail platforms signal a
>>>> Power Management Event (PME) to the Power Management Controller (PMC)
>>>> to wakeup the system. When this happens software needs to explicitly
>>>> clear the PME bus 0 status bit in the GPE0a_STS register to avoid an
>>>> IRQ storm on IRQ 9.
>>>>
>>>> This is modelled in ACPI through the INT0002 ACPI device, which is
>>>> called a "Virtual GPIO controller" in ACPI because it defines the
>>>> event handler to call when the PME triggers through _AEI and _L02
>>>> methods as would be done for a real GPIO interrupt in ACPI.
>>>>
>>>> This commit adds a driver which registers the Virtual GPIOs expected
>>>> by the DSDT on these devices, letting gpiolib-acpi claim the
>>>> virtual GPIO and install a GPIO-interrupt handler which call the _L02
>>>> handler as it would for a real GPIO controller.
>>>>
>>>
>>> Pushed to testing w/o Linus' tag (there is no one yet)
>>
>>
>> Will you be taking this through fixes Andy? For a 4.12-rc* target per
>> Hans'
>> response to the cover letter?
>
>
> Note that Rafael is planning to drop (revert) the commit which makes
> it (extra) desirable for this driver to get into 4.12, so we should
> perhaps reconsider that. I'm fine either way, but the primary reason
> to push this as a fix for 4.12 is no longer valid AFAICT.

Maybe I will just queue it up along with this series:
http://marc.info/?l=linux-kernel&m=149688087904433&w=2
if Andy and Darren agree?

Thanks,
Rafael

Re: [PATCH v6] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device

[Andy Shevchenko]

On Thu, Jun 8, 2017 at 8:15 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Thu, Jun 8, 2017 at 6:45 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>> On 06/08/2017 05:45 PM, Darren Hart wrote:
>>> On Wed, Jun 07, 2017 at 05:53:38PM +0300, Andy Shevchenko wrote:
>>>> On Fri, Jun 2, 2017 at 6:15 PM, Hans de Goede <hdegoede@redhat.com>
>>>> wrote:

>>> Will you be taking this through fixes Andy? For a 4.12-rc* target per
>>> Hans'
>>> response to the cover letter?

>> Note that Rafael is planning to drop (revert) the commit which makes
>> it (extra) desirable for this driver to get into 4.12, so we should
>> perhaps reconsider that. I'm fine either way, but the primary reason
>> to push this as a fix for 4.12 is no longer valid AFAICT.
>
> Maybe I will just queue it up along with this series:
> http://marc.info/?l=linux-kernel&m=149688087904433&w=2
> if Andy and Darren agree?

Looks like it has ties to your series, so, I see no obstacles to go via PM tree.
Darren?


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device

[Rafael J. Wysocki]

On Thu, Jun 8, 2017 at 7:40 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Thu, Jun 8, 2017 at 8:15 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Thu, Jun 8, 2017 at 6:45 PM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> On 06/08/2017 05:45 PM, Darren Hart wrote:
>>>> On Wed, Jun 07, 2017 at 05:53:38PM +0300, Andy Shevchenko wrote:
>>>>> On Fri, Jun 2, 2017 at 6:15 PM, Hans de Goede <hdegoede@redhat.com>
>>>>> wrote:
>
>>>> Will you be taking this through fixes Andy? For a 4.12-rc* target per
>>>> Hans'
>>>> response to the cover letter?
>
>>> Note that Rafael is planning to drop (revert) the commit which makes
>>> it (extra) desirable for this driver to get into 4.12, so we should
>>> perhaps reconsider that. I'm fine either way, but the primary reason
>>> to push this as a fix for 4.12 is no longer valid AFAICT.
>>
>> Maybe I will just queue it up along with this series:
>> http://marc.info/?l=linux-kernel&m=149688087904433&w=2
>> if Andy and Darren agree?
>
> Looks like it has ties to your series, so, I see no obstacles to go via PM tree.

OK, thanks!

> Darren?

An ACK or equivalent from Linus W would be good to have too ...

Thanks,
Rafael

Re: [PATCH v6] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device

[Andy Shevchenko]

On Fri, 2017-06-09 at 02:30 +0200, Rafael J. Wysocki wrote:
> On Thu, Jun 8, 2017 at 7:40 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Thu, Jun 8, 2017 at 8:15 PM, Rafael J. Wysocki <rafael@kernel.org
> > > wrote:
> > > On Thu, Jun 8, 2017 at 6:45 PM, Hans de Goede <hdegoede@redhat.com
> > > > wrote:
> > > > On 06/08/2017 05:45 PM, Darren Hart wrote:
> > > > > On Wed, Jun 07, 2017 at 05:53:38PM +0300, Andy Shevchenko
> > > > > wrote:
> > > > > > On Fri, Jun 2, 2017 at 6:15 PM, Hans de Goede <hdegoede@redh
> > > > > > at.com>
> > > > > > wrote:
> > > > > Will you be taking this through fixes Andy? For a 4.12-rc*
> > > > > target per
> > > > > Hans'
> > > > > response to the cover letter?
> > > > Note that Rafael is planning to drop (revert) the commit which
> > > > makes
> > > > it (extra) desirable for this driver to get into 4.12, so we
> > > > should
> > > > perhaps reconsider that. I'm fine either way, but the primary
> > > > reason
> > > > to push this as a fix for 4.12 is no longer valid AFAICT.
> > > 
> > > Maybe I will just queue it up along with this series:
> > > http://marc.info/?l=linux-kernel&m=149688087904433&w=2
> > > if Andy and Darren agree?
> > 
> > Looks like it has ties to your series, so, I see no obstacles to go
> > via PM tree.
> 
> OK, thanks!

I removed it from our testing branch, feel free to apply any time.

> > Darren?
> 
> An ACK or equivalent from Linus W would be good to have too ...
> 

I was waiting for it about week and decided to proceed w.o. one.
Linus is overloaded by something, though he appears from time to time in
mailing lists.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v6] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device

[Linus Walleij]

On Fri, Jun 9, 2017 at 10:22 AM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> I was waiting for it about week and decided to proceed w.o. one.
> Linus is overloaded by something, though he appears from time to time in
> mailing lists.

Yeah I get overloaded from time to time, it is because overall kernel
activity is increasing and managing the subsystems take more
time, also I suck at prioritizing and planning. :(

It would be nice to have a new co-maintainer, maybe one of the Intel
people could step up, hm? ;)

Yours,
Linus Walleij

Re: [PATCH v6] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device

[Linus Walleij]

On Fri, Jun 2, 2017 at 5:15 PM, Hans de Goede <hdegoede@redhat.com> wrote:

> Some peripherals on Bay Trail and Cherry Trail platforms signal a
> Power Management Event (PME) to the Power Management Controller (PMC)
> to wakeup the system. When this happens software needs to explicitly
> clear the PME bus 0 status bit in the GPE0a_STS register to avoid an
> IRQ storm on IRQ 9.
>
> This is modelled in ACPI through the INT0002 ACPI device, which is
> called a "Virtual GPIO controller" in ACPI because it defines the
> event handler to call when the PME triggers through _AEI and _L02
> methods as would be done for a real GPIO interrupt in ACPI.
>
> This commit adds a driver which registers the Virtual GPIOs expected
> by the DSDT on these devices, letting gpiolib-acpi claim the
> virtual GPIO and install a GPIO-interrupt handler which call the _L02
> handler as it would for a real GPIO controller.
>
> Cc: joeyli <jlee@suse.com>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> Changes in v2:
> -Remove dev_err after malloc failure
> -Remove unused empty runtime pm callbacks
> -s/GPE0A_PME_/GPE0A_PME_B0_/
> -Fixed some checkpatch warnings (I forgot to run checkpatch on v1)
> Changes in v3:
> -Rewrite as gpiochip driver letting gpiolib-acpi deal with claiming the pin
>  0x0002 and calling the _L02 event handler when the virtual gpio-irq triggers
> -Rebase on 4.12-rc1
> Changes in v4:
> -Drop device_init_wakeup() from _probe(), use pm_system_wakeup() instead
>  of pm_wakeup_hard_event(chip->parent)
> -Improve commit message
> Changes in v5:
> -Use BIT() macro for FOO_BIT defines
> -Drop unneeded ACPI_PTR macro usage
> Changes in v6:
> -Move back to drivers/platform/x86
> -Expand certain acronyms (PME, PMC)
> -Use linux/gpio/driver.h include instead of linux/gpio.h
> -Document why the get / set / direction_output functions are dummys
> -No functional changes

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

With Andy's changes.

Please feel free to push this upstream through the platform tree
or similar.

Yours,
Linus Walleij

Re: [PATCH v6] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device

[Andy Shevchenko]

On Fri, 2017-06-09 at 11:02 +0200, Linus Walleij wrote:
> On Fri, Jun 2, 2017 at 5:15 PM, Hans de Goede <hdegoede@redhat.com>
> wrote:
> 
> > Some peripherals on Bay Trail and Cherry Trail platforms signal a
> > Power Management Event (PME) to the Power Management Controller
> > (PMC)
> > to wakeup the system. When this happens software needs to explicitly
> > clear the PME bus 0 status bit in the GPE0a_STS register to avoid an
> > IRQ storm on IRQ 9.
> > 
> > This is modelled in ACPI through the INT0002 ACPI device, which is
> > called a "Virtual GPIO controller" in ACPI because it defines the
> > event handler to call when the PME triggers through _AEI and _L02
> > methods as would be done for a real GPIO interrupt in ACPI.
> > 
> > This commit adds a driver which registers the Virtual GPIOs expected
> > by the DSDT on these devices, letting gpiolib-acpi claim the
> > virtual GPIO and install a GPIO-interrupt handler which call the
> > _L02
> > handler as it would for a real GPIO controller.
> > 
> > Cc: joeyli <jlee@suse.com>
> > Cc: Takashi Iwai <tiwai@suse.de>
> > Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> > Changes in v2:
> > -Remove dev_err after malloc failure
> > -Remove unused empty runtime pm callbacks
> > -s/GPE0A_PME_/GPE0A_PME_B0_/
> > -Fixed some checkpatch warnings (I forgot to run checkpatch on v1)
> > Changes in v3:
> > -Rewrite as gpiochip driver letting gpiolib-acpi deal with claiming
> > the pin
> >  0x0002 and calling the _L02 event handler when the virtual gpio-irq 
> > triggers
> > -Rebase on 4.12-rc1
> > Changes in v4:
> > -Drop device_init_wakeup() from _probe(), use pm_system_wakeup()
> > instead
> >  of pm_wakeup_hard_event(chip->parent)
> > -Improve commit message
> > Changes in v5:
> > -Use BIT() macro for FOO_BIT defines
> > -Drop unneeded ACPI_PTR macro usage
> > Changes in v6:
> > -Move back to drivers/platform/x86
> > -Expand certain acronyms (PME, PMC)
> > -Use linux/gpio/driver.h include instead of linux/gpio.h
> > -Document why the get / set / direction_output functions are dummys
> > -No functional changes
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> With Andy's changes.

Oops, when I removed this from PDx86 testing branch I removed my changes
together.

Here they are (Hans, perhaps you may resend with them included)
--- a/drivers/platform/x86/intel_int0002_vgpio.c
+++ b/drivers/platform/x86/intel_int0002_vgpio.c
@@ -30,9 +30,8 @@
  * for a real GPIO controller.
  */

-#include <asm/cpu_device_id.h>
-#include <asm/intel-family.h>
 #include <linux/acpi.h>
+#include <linux/bitmap.h>
 #include <linux/gpio/driver.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -42,6 +41,9 @@
 #include <linux/slab.h>
 #include <linux/suspend.h>  

+#include <asm/cpu_device_id.h>
+#include <asm/intel-family.h>
+
 #define DRV_NAME                       "INT0002 Virtual GPIO"

 /* For some reason the virtual GPIO pin tied to the GPE is numbered pin
2 */
@@ -66,7 +68,7 @@ static const struct x86_cpu_id int0002_cpu_ids[] = {

 /*
  * As this is not a real GPIO at all, but just a hack to model an event
in
- * APCI the get / set functions are dummy functions.
+ * ACPI the get / set functions are dummy functions.
  */

 static int int0002_gpio_get(struct gpio_chip *chip, unsigned int
offset)
@@ -137,7 +139,7 @@ static int int0002_probe(struct platform_device
*pdev)
        struct device *dev = &pdev->dev;
        const struct x86_cpu_id *cpu_id;
        struct gpio_chip *chip;
-       int i, irq, ret;
+       int irq, ret;

        /* Menlow has a different INT0002 device? <sigh> */
        cpu_id = x86_match_cpu(int0002_cpu_ids);
@@ -171,8 +173,7 @@ static int int0002_probe(struct platform_device
*pdev)
                return ret;
        }

-       for (i = 0; i < GPE0A_PME_B0_VIRT_GPIO_PIN; i++)
-               clear_bit(i, chip->irq_valid_mask);
+       bitmap_clear(chip->irq_valid_mask, 0,
GPE0A_PME_B0_VIRT_GPIO_PIN);

        /*
         * We manually request the irq here instead of passing a flow-
handler

> 
> Please feel free to push this upstream through the platform tree
> or similar.
> 
> Yours,
> Linus Walleij

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v6] platform/x86: Add driver for ACPI INT0002 Virtual GPIO device

[Hans de Goede]

Hi,

On 09-06-17 11:18, Andy Shevchenko wrote:
> On Fri, 2017-06-09 at 11:02 +0200, Linus Walleij wrote:
>> On Fri, Jun 2, 2017 at 5:15 PM, Hans de Goede <hdegoede@redhat.com>
>> wrote:
>>
>>> Some peripherals on Bay Trail and Cherry Trail platforms signal a
>>> Power Management Event (PME) to the Power Management Controller
>>> (PMC)
>>> to wakeup the system. When this happens software needs to explicitly
>>> clear the PME bus 0 status bit in the GPE0a_STS register to avoid an
>>> IRQ storm on IRQ 9.
>>>
>>> This is modelled in ACPI through the INT0002 ACPI device, which is
>>> called a "Virtual GPIO controller" in ACPI because it defines the
>>> event handler to call when the PME triggers through _AEI and _L02
>>> methods as would be done for a real GPIO interrupt in ACPI.
>>>
>>> This commit adds a driver which registers the Virtual GPIOs expected
>>> by the DSDT on these devices, letting gpiolib-acpi claim the
>>> virtual GPIO and install a GPIO-interrupt handler which call the
>>> _L02
>>> handler as it would for a real GPIO controller.
>>>
>>> Cc: joeyli <jlee@suse.com>
>>> Cc: Takashi Iwai <tiwai@suse.de>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>> ---
>>> Changes in v2:
>>> -Remove dev_err after malloc failure
>>> -Remove unused empty runtime pm callbacks
>>> -s/GPE0A_PME_/GPE0A_PME_B0_/
>>> -Fixed some checkpatch warnings (I forgot to run checkpatch on v1)
>>> Changes in v3:
>>> -Rewrite as gpiochip driver letting gpiolib-acpi deal with claiming
>>> the pin
>>>   0x0002 and calling the _L02 event handler when the virtual gpio-irq
>>> triggers
>>> -Rebase on 4.12-rc1
>>> Changes in v4:
>>> -Drop device_init_wakeup() from _probe(), use pm_system_wakeup()
>>> instead
>>>   of pm_wakeup_hard_event(chip->parent)
>>> -Improve commit message
>>> Changes in v5:
>>> -Use BIT() macro for FOO_BIT defines
>>> -Drop unneeded ACPI_PTR macro usage
>>> Changes in v6:
>>> -Move back to drivers/platform/x86
>>> -Expand certain acronyms (PME, PMC)
>>> -Use linux/gpio/driver.h include instead of linux/gpio.h
>>> -Document why the get / set / direction_output functions are dummys
>>> -No functional changes
>>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>
>> With Andy's changes.
> 
> Oops, when I removed this from PDx86 testing branch I removed my changes
> together.
> 
> Here they are (Hans, perhaps you may resend with them included)

Ok, v7 with these and Linus' Reviewed-by added coming up.

Regards,

Hans



> --- a/drivers/platform/x86/intel_int0002_vgpio.c
> +++ b/drivers/platform/x86/intel_int0002_vgpio.c
> @@ -30,9 +30,8 @@
>    * for a real GPIO controller.
>    */
> 
> -#include <asm/cpu_device_id.h>
> -#include <asm/intel-family.h>
>   #include <linux/acpi.h>
> +#include <linux/bitmap.h>
>   #include <linux/gpio/driver.h>
>   #include <linux/interrupt.h>
>   #include <linux/io.h>
> @@ -42,6 +41,9 @@
>   #include <linux/slab.h>
>   #include <linux/suspend.h>
> 
> +#include <asm/cpu_device_id.h>
> +#include <asm/intel-family.h>
> +
>   #define DRV_NAME                       "INT0002 Virtual GPIO"
> 
>   /* For some reason the virtual GPIO pin tied to the GPE is numbered pin
> 2 */
> @@ -66,7 +68,7 @@ static const struct x86_cpu_id int0002_cpu_ids[] = {
> 
>   /*
>    * As this is not a real GPIO at all, but just a hack to model an event
> in
> - * APCI the get / set functions are dummy functions.
> + * ACPI the get / set functions are dummy functions.
>    */
> 
>   static int int0002_gpio_get(struct gpio_chip *chip, unsigned int
> offset)
> @@ -137,7 +139,7 @@ static int int0002_probe(struct platform_device
> *pdev)
>          struct device *dev = &pdev->dev;
>          const struct x86_cpu_id *cpu_id;
>          struct gpio_chip *chip;
> -       int i, irq, ret;
> +       int irq, ret;
> 
>          /* Menlow has a different INT0002 device? <sigh> */
>          cpu_id = x86_match_cpu(int0002_cpu_ids);
> @@ -171,8 +173,7 @@ static int int0002_probe(struct platform_device
> *pdev)
>                  return ret;
>          }
> 
> -       for (i = 0; i < GPE0A_PME_B0_VIRT_GPIO_PIN; i++)
> -               clear_bit(i, chip->irq_valid_mask);
> +       bitmap_clear(chip->irq_valid_mask, 0,
> GPE0A_PME_B0_VIRT_GPIO_PIN);
> 
>          /*
>           * We manually request the irq here instead of passing a flow-
> handler
> 
>>
>> Please feel free to push this upstream through the platform tree
>> or similar.
>>
>> Yours,
>> Linus Walleij
>