pinctrl: amd: Honor IRQ trigger type requested by the caller

Message ID 20200626211026.513520-1-furquan@google.com
State Accepted
Commit 5f4962dd55d86d6a3ba5ddbfaf2d793e3b676a20
Headers show
Series
  • pinctrl: amd: Honor IRQ trigger type requested by the caller
Related show

Commit Message

Furquan Shaikh June 26, 2020, 9:10 p.m.
This change drops the override in `amd_gpio_irq_set_type()` that
ignores the IRQ trigger type settings from the caller. The device
driver (caller) is in a better position to identify the right trigger
type for the device based on the usage as well as the information
exposed by the BIOS. There are instances where the device driver might
want to configure the trigger type differently in different modes. An
example of this is gpio-keys driver which configures IRQ type as
trigger on both edges (to identify assert and deassert events) when in
S0 and reconfigures the trigger type using the information provided by
the BIOS when going into suspend to ensure that the wake happens on
the required edge.

This override in `amd_gpio_irq_set_type()` prevents the caller from
being able to reconfigure trigger type once it is set either based on
ACPI information or the type used by the first caller for IRQ on a
given GPIO line.

Without this change, pen-insert gpio key (used by garaged stylus on a
Chromebook) works fine in S0 (i.e. insert and eject events are
correctly identified), however, BIOS configuration for wake on only
pen eject i.e. only-rising edge or only-falling edge is not honored.

With this change, it was verified that pen-insert gpio key behavior is
correct in both S0 and for wakeup from S3.

Signed-off-by: Furquan Shaikh <furquan@google.com>
---
 drivers/pinctrl/pinctrl-amd.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

Comments

Linus Walleij July 7, 2020, 12:50 p.m. | #1
On Fri, Jun 26, 2020 at 11:10 PM Furquan Shaikh <furquan@google.com> wrote:

> This change drops the override in `amd_gpio_irq_set_type()` that

> ignores the IRQ trigger type settings from the caller. The device

> driver (caller) is in a better position to identify the right trigger

> type for the device based on the usage as well as the information

> exposed by the BIOS. There are instances where the device driver might

> want to configure the trigger type differently in different modes. An

> example of this is gpio-keys driver which configures IRQ type as

> trigger on both edges (to identify assert and deassert events) when in

> S0 and reconfigures the trigger type using the information provided by

> the BIOS when going into suspend to ensure that the wake happens on

> the required edge.

>

> This override in `amd_gpio_irq_set_type()` prevents the caller from

> being able to reconfigure trigger type once it is set either based on

> ACPI information or the type used by the first caller for IRQ on a

> given GPIO line.

>

> Without this change, pen-insert gpio key (used by garaged stylus on a

> Chromebook) works fine in S0 (i.e. insert and eject events are

> correctly identified), however, BIOS configuration for wake on only

> pen eject i.e. only-rising edge or only-falling edge is not honored.

>

> With this change, it was verified that pen-insert gpio key behavior is

> correct in both S0 and for wakeup from S3.

>

> Signed-off-by: Furquan Shaikh <furquan@google.com>


Patch applied.

Yours,
Linus Walleij

Patch

diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
index 1fe62a35bb12..c34e6a950b3f 100644
--- a/drivers/pinctrl/pinctrl-amd.c
+++ b/drivers/pinctrl/pinctrl-amd.c
@@ -417,22 +417,13 @@  static int amd_gpio_irq_set_type(struct irq_data *d, unsigned int type)
 {
 	int ret = 0;
 	u32 pin_reg, pin_reg_irq_en, mask;
-	unsigned long flags, irq_flags;
+	unsigned long flags;
 	struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
 	struct amd_gpio *gpio_dev = gpiochip_get_data(gc);
 
 	raw_spin_lock_irqsave(&gpio_dev->lock, flags);
 	pin_reg = readl(gpio_dev->base + (d->hwirq)*4);
 
-	/* Ignore the settings coming from the client and
-	 * read the values from the ACPI tables
-	 * while setting the trigger type
-	 */
-
-	irq_flags = irq_get_trigger_type(d->irq);
-	if (irq_flags != IRQ_TYPE_NONE)
-		type = irq_flags;
-
 	switch (type & IRQ_TYPE_SENSE_MASK) {
 	case IRQ_TYPE_EDGE_RISING:
 		pin_reg &= ~BIT(LEVEL_TRIG_OFF);