* [PATCH v3] gpio: Add driver for ACPI INT0002 Virtual GPIO device
@ 2017-05-23 20:12 Hans de Goede
2017-05-23 23:58 ` Rafael J. Wysocki
0 siblings, 1 reply; 4+ messages in thread
From: Hans de Goede @ 2017-05-23 20:12 UTC (permalink / raw)
To: Rafael J . Wysocki, Linus Walleij, Alexandre Courbot
Cc: linux-acpi, Andy Shevchenko, linux-gpio, Hans de Goede, joeyli,
Takashi Iwai
Some peripherals on Baytrail and Cherrytrail platforms signal PME to the
PMC to wakeup the system. When this happens software needs to clear the
PME_B0_STS bit in the GPE0a_STS register to avoid an IRQ storm on IRQ 9.
This is modelled in ACPI through the INT0002 ACPI Virtual GPIO device.
This commit adds a driver which registers the Virtual GPIOs expected
by the DSDT on these devices, this will make gpiolib-acpi claim the
virtual GPIO and execute _L02 when the PME_B0_STS bit is set, avoiding
the irq storm.
Cc: joeyli <jlee@suse.com>
Cc: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Hans de Goede <hdegoede@redhat.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
---
drivers/gpio/Kconfig | 14 +++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-int0002.c | 211 ++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 226 insertions(+)
create mode 100644 drivers/gpio/gpio-int0002.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 23ca51e..11e7f9b 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -608,6 +608,20 @@ config GPIO_GPIO_MM
The base port addresses for the devices may be configured via the base
array module parameter.
+config GPIO_INT0002
+ tristate "Intel ACPI INT0002 Virtual GPIO"
+ depends on ACPI
+ select GPIOLIB_IRQCHIP
+ ---help---
+ Some peripherals on Baytrail and Cherrytrail platforms signal
+ PME to the PMC to wakeup the system. When this happens software
+ needs to explicitly clear the interrupt source to avoid an IRQ
+ storm on IRQ 9. This is modelled in ACPI through the INT0002
+ Virtual GPIO ACPI device.
+
+ To compile this driver as a module, choose M here: the module will
+ be called gpio_int0002.
+
config GPIO_IT87
tristate "IT87xx GPIO support"
help
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 68b9627..1f76c56 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_GPIO_GPIO_MM) += gpio-gpio-mm.o
obj-$(CONFIG_GPIO_GRGPIO) += gpio-grgpio.o
obj-$(CONFIG_HTC_EGPIO) += gpio-htc-egpio.o
obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
+obj-$(CONFIG_GPIO_INT0002) += gpio-int0002.o
obj-$(CONFIG_GPIO_IOP) += gpio-iop.o
obj-$(CONFIG_GPIO_IT87) += gpio-it87.o
obj-$(CONFIG_GPIO_JANZ_TTL) += gpio-janz-ttl.o
diff --git a/drivers/gpio/gpio-int0002.c b/drivers/gpio/gpio-int0002.c
new file mode 100644
index 0000000..e73664d
--- /dev/null
+++ b/drivers/gpio/gpio-int0002.c
@@ -0,0 +1,211 @@
+/*
+ * 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 Baytrail and Cherrytrail platforms signal PME to the
+ * PMC to wakeup the system. When this happens software needs to clear the
+ * PME_B0_STS 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.
+ *
+ * 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.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>
+
+#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 0x2000
+#define GPE0A_PME_B0_EN_BIT 0x2000
+#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 */
+ {}
+};
+
+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_wakeup_hard_event(chip->parent);
+
+ 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);
+
+ device_init_wakeup(dev, true);
+
+ 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 = ACPI_PTR(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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v3] gpio: Add driver for ACPI INT0002 Virtual GPIO device
2017-05-23 20:12 [PATCH v3] gpio: Add driver for ACPI INT0002 Virtual GPIO device Hans de Goede
@ 2017-05-23 23:58 ` Rafael J. Wysocki
2017-05-24 7:50 ` Hans de Goede
0 siblings, 1 reply; 4+ messages in thread
From: Rafael J. Wysocki @ 2017-05-23 23:58 UTC (permalink / raw)
To: Hans de Goede
Cc: Linus Walleij, Alexandre Courbot, linux-acpi, Andy Shevchenko,
linux-gpio, joeyli, Takashi Iwai
On Tuesday, May 23, 2017 10:12:41 PM Hans de Goede wrote:
> Some peripherals on Baytrail and Cherrytrail platforms signal PME to the
> PMC to wakeup the system. When this happens software needs to clear the
> PME_B0_STS bit in the GPE0a_STS register to avoid an IRQ storm on IRQ 9.
>
> This is modelled in ACPI through the INT0002 ACPI Virtual GPIO device.
>
> This commit adds a driver which registers the Virtual GPIOs expected
> by the DSDT on these devices, this will make gpiolib-acpi claim the
> virtual GPIO and execute _L02 when the PME_B0_STS bit is set, avoiding
> the irq storm.
>
> Cc: joeyli <jlee@suse.com>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Hans de Goede <hdegoede@redhat.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
> ---
> drivers/gpio/Kconfig | 14 +++
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-int0002.c | 211 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 226 insertions(+)
> create mode 100644 drivers/gpio/gpio-int0002.c
>
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 23ca51e..11e7f9b 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -608,6 +608,20 @@ config GPIO_GPIO_MM
> The base port addresses for the devices may be configured via the base
> array module parameter.
>
> +config GPIO_INT0002
> + tristate "Intel ACPI INT0002 Virtual GPIO"
> + depends on ACPI
> + select GPIOLIB_IRQCHIP
> + ---help---
> + Some peripherals on Baytrail and Cherrytrail platforms signal
> + PME to the PMC to wakeup the system. When this happens software
> + needs to explicitly clear the interrupt source to avoid an IRQ
> + storm on IRQ 9. This is modelled in ACPI through the INT0002
> + Virtual GPIO ACPI device.
> +
> + To compile this driver as a module, choose M here: the module will
> + be called gpio_int0002.
> +
> config GPIO_IT87
> tristate "IT87xx GPIO support"
> help
> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
> index 68b9627..1f76c56 100644
> --- a/drivers/gpio/Makefile
> +++ b/drivers/gpio/Makefile
> @@ -55,6 +55,7 @@ obj-$(CONFIG_GPIO_GPIO_MM) += gpio-gpio-mm.o
> obj-$(CONFIG_GPIO_GRGPIO) += gpio-grgpio.o
> obj-$(CONFIG_HTC_EGPIO) += gpio-htc-egpio.o
> obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
> +obj-$(CONFIG_GPIO_INT0002) += gpio-int0002.o
> obj-$(CONFIG_GPIO_IOP) += gpio-iop.o
> obj-$(CONFIG_GPIO_IT87) += gpio-it87.o
> obj-$(CONFIG_GPIO_JANZ_TTL) += gpio-janz-ttl.o
> diff --git a/drivers/gpio/gpio-int0002.c b/drivers/gpio/gpio-int0002.c
> new file mode 100644
> index 0000000..e73664d
> --- /dev/null
> +++ b/drivers/gpio/gpio-int0002.c
> @@ -0,0 +1,211 @@
> +/*
> + * 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 Baytrail and Cherrytrail platforms signal PME to the
> + * PMC to wakeup the system. When this happens software needs to clear the
> + * PME_B0_STS 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.
> + *
> + * 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.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>
> +
> +#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 0x2000
> +#define GPE0A_PME_B0_EN_BIT 0x2000
> +#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 */
> + {}
> +};
> +
> +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_wakeup_hard_event(chip->parent);
It may be better to just do pm_system_wakeup() here and drop the
device_init_wakeup() from _probe().
The reason why is that device_init_wakeup() allows user space to disable
this wakeup source via sysfs which probably never is a good idea, because
this thing is just for clearing PME status which should be done regardless
and the actual wakeup is triggered by something else.
Also in theory pm_system_wakeup() should not really be necessary for
wakeup via USB keyboard to work, because that should be signaled via
acpi_pm_notify_handler(), at least in principle.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v3] gpio: Add driver for ACPI INT0002 Virtual GPIO device
2017-05-23 23:58 ` Rafael J. Wysocki
@ 2017-05-24 7:50 ` Hans de Goede
2017-05-24 23:10 ` Rafael J. Wysocki
0 siblings, 1 reply; 4+ messages in thread
From: Hans de Goede @ 2017-05-24 7:50 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linus Walleij, Alexandre Courbot, linux-acpi, Andy Shevchenko,
linux-gpio, joeyli, Takashi Iwai
Hi,
On 24-05-17 01:58, Rafael J. Wysocki wrote:
> On Tuesday, May 23, 2017 10:12:41 PM Hans de Goede wrote:
>> Some peripherals on Baytrail and Cherrytrail platforms signal PME to the
>> PMC to wakeup the system. When this happens software needs to clear the
>> PME_B0_STS bit in the GPE0a_STS register to avoid an IRQ storm on IRQ 9.
>>
>> This is modelled in ACPI through the INT0002 ACPI Virtual GPIO device.
>>
>> This commit adds a driver which registers the Virtual GPIOs expected
>> by the DSDT on these devices, this will make gpiolib-acpi claim the
>> virtual GPIO and execute _L02 when the PME_B0_STS bit is set, avoiding
>> the irq storm.
>>
>> Cc: joeyli <jlee@suse.com>
>> Cc: Takashi Iwai <tiwai@suse.de>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.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
>> ---
>> drivers/gpio/Kconfig | 14 +++
>> drivers/gpio/Makefile | 1 +
>> drivers/gpio/gpio-int0002.c | 211 ++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 226 insertions(+)
>> create mode 100644 drivers/gpio/gpio-int0002.c
>>
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> index 23ca51e..11e7f9b 100644
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -608,6 +608,20 @@ config GPIO_GPIO_MM
>> The base port addresses for the devices may be configured via the base
>> array module parameter.
>>
>> +config GPIO_INT0002
>> + tristate "Intel ACPI INT0002 Virtual GPIO"
>> + depends on ACPI
>> + select GPIOLIB_IRQCHIP
>> + ---help---
>> + Some peripherals on Baytrail and Cherrytrail platforms signal
>> + PME to the PMC to wakeup the system. When this happens software
>> + needs to explicitly clear the interrupt source to avoid an IRQ
>> + storm on IRQ 9. This is modelled in ACPI through the INT0002
>> + Virtual GPIO ACPI device.
>> +
>> + To compile this driver as a module, choose M here: the module will
>> + be called gpio_int0002.
>> +
>> config GPIO_IT87
>> tristate "IT87xx GPIO support"
>> help
>> diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
>> index 68b9627..1f76c56 100644
>> --- a/drivers/gpio/Makefile
>> +++ b/drivers/gpio/Makefile
>> @@ -55,6 +55,7 @@ obj-$(CONFIG_GPIO_GPIO_MM) += gpio-gpio-mm.o
>> obj-$(CONFIG_GPIO_GRGPIO) += gpio-grgpio.o
>> obj-$(CONFIG_HTC_EGPIO) += gpio-htc-egpio.o
>> obj-$(CONFIG_GPIO_ICH) += gpio-ich.o
>> +obj-$(CONFIG_GPIO_INT0002) += gpio-int0002.o
>> obj-$(CONFIG_GPIO_IOP) += gpio-iop.o
>> obj-$(CONFIG_GPIO_IT87) += gpio-it87.o
>> obj-$(CONFIG_GPIO_JANZ_TTL) += gpio-janz-ttl.o
>> diff --git a/drivers/gpio/gpio-int0002.c b/drivers/gpio/gpio-int0002.c
>> new file mode 100644
>> index 0000000..e73664d
>> --- /dev/null
>> +++ b/drivers/gpio/gpio-int0002.c
>> @@ -0,0 +1,211 @@
>> +/*
>> + * 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 Baytrail and Cherrytrail platforms signal PME to the
>> + * PMC to wakeup the system. When this happens software needs to clear the
>> + * PME_B0_STS 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.
>> + *
>> + * 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.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>
>> +
>> +#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 0x2000
>> +#define GPE0A_PME_B0_EN_BIT 0x2000
>> +#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 */
>> + {}
>> +};
>> +
>> +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_wakeup_hard_event(chip->parent);
>
> It may be better to just do pm_system_wakeup() here and drop the
> device_init_wakeup() from _probe().
Ok, I've given this a try and it works fine, so I will send a v4 with
this changed.
> The reason why is that device_init_wakeup() allows user space to disable
> this wakeup source via sysfs which probably never is a good idea, because
> this thing is just for clearing PME status which should be done regardless
> and the actual wakeup is triggered by something else.
Ack.
> Also in theory pm_system_wakeup() should not really be necessary for
> wakeup via USB keyboard to work, because that should be signaled via
> acpi_pm_notify_handler(), at least in principle.
Earlier versions of the int0002 driver did not call any wakeup() function
at all and with 4.11 that worked fine, my initial testing with 4.12-rc1 also
did not include a wakeup() call but that resulted in USB-keyboard wakeup
not working and the system not responding to other wakeup sources like
the power-button after the attempted USB-keyb wakeup. To be sure I've
retried my current int0002 code with the wakeup call commented out, that
leads to the same result (unwakeable system) so it seems we really need
to do the pm_system_wakeup() call.
v4 coming up.
Regards,
Hans
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v3] gpio: Add driver for ACPI INT0002 Virtual GPIO device
2017-05-24 7:50 ` Hans de Goede
@ 2017-05-24 23:10 ` Rafael J. Wysocki
0 siblings, 0 replies; 4+ messages in thread
From: Rafael J. Wysocki @ 2017-05-24 23:10 UTC (permalink / raw)
To: Hans de Goede
Cc: Rafael J. Wysocki, Linus Walleij, Alexandre Courbot,
ACPI Devel Maling List, Andy Shevchenko, linux-gpio, joeyli,
Takashi Iwai
On Wed, May 24, 2017 at 9:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
[cut]
>>> +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_wakeup_hard_event(chip->parent);
>>
>>
>> It may be better to just do pm_system_wakeup() here and drop the
>> device_init_wakeup() from _probe().
>
>
> Ok, I've given this a try and it works fine, so I will send a v4 with
> this changed.
OK
>> The reason why is that device_init_wakeup() allows user space to disable
>> this wakeup source via sysfs which probably never is a good idea, because
>> this thing is just for clearing PME status which should be done regardless
>> and the actual wakeup is triggered by something else.
>
>
> Ack.
>
>> Also in theory pm_system_wakeup() should not really be necessary for
>> wakeup via USB keyboard to work, because that should be signaled via
>> acpi_pm_notify_handler(), at least in principle.
>
>
> Earlier versions of the int0002 driver did not call any wakeup() function
> at all and with 4.11 that worked fine, my initial testing with 4.12-rc1 also
> did not include a wakeup() call but that resulted in USB-keyboard wakeup
> not working and the system not responding to other wakeup sources like
> the power-button after the attempted USB-keyb wakeup. To be sure I've
> retried my current int0002 code with the wakeup call commented out, that
> leads to the same result (unwakeable system) so it seems we really need
> to do the pm_system_wakeup() call.
That's fine, it doesn't hurt to put it in there ATM anyway.
Thanks,
Rafael
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-05-24 23:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-23 20:12 [PATCH v3] gpio: Add driver for ACPI INT0002 Virtual GPIO device Hans de Goede
2017-05-23 23:58 ` Rafael J. Wysocki
2017-05-24 7:50 ` Hans de Goede
2017-05-24 23:10 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).