Re: [PATCH v5] gpio: Add driver for ACPI INT0002 Virtual GPIO device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 06/01/2017 05:23 PM, Linus Walleij wrote:
On Wed, May 24, 2017 at 12:42 PM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:

Some peripherals on Bay Trail and Cherry Trail 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 modeled 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, 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@xxxxxxxx>
Cc: Takashi Iwai <tiwai@xxxxxxx>
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig

Please move this thing to drivers/platform/x86.

+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

Spell out the acronyms.

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

I.e. it is not really GPIO.

Virtual GPIO == not GPIO at all, obviously.

Please write something about this weird newspeak here.

+++ b/drivers/gpio/gpio-int0002.c
(...)
+ * Some peripherals on Bay Trail and Cherry Trail 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.

Spell out acronyms.

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

No <linux/gpio.h>, only:
#include <linux/gpio/driver.h>

in drivers.

The above is all fixed for the next version.

+/* For some reason the virtual GPIO pin tied to the GPE is numbered pin 2 */
+#define GPE0A_PME_B0_VIRT_GPIO_PIN     2

I think it's not weird at all, it is a 32bit word which is not a GPIO
register at all, but
misc IPC register, where bits 0 and 1 does something else.

That is not true the bus 0 status bit we're reacting to and which we're
clearing in this driver is bit 13 ...


+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;
+}

If you're anyways pretending to be a GPIO chip, then you could implement
these. But I guess you're not implementing them, because this is not
a GPIO chip, so let's add a big fat comment above these stating clearly
why they are not implemented proper.

Done for v6.


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

I wonder what happens the day an IRQ start arriving on the other
bits. Oh well, that day we handle that.

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

That's neat.

Yours,
Linus Walleij


Regards,

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



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux