diff mbox series

[1/1,Artful/linux-oem] pinctrl/amd: fix masking of GPIO interrupts

Message ID 20171129092300.25676-2-kai.heng.feng@canonical.com
State New
Headers show
Series [1/1,Artful/linux-oem] pinctrl/amd: fix masking of GPIO interrupts | expand

Commit Message

Kai-Heng Feng Nov. 29, 2017, 9:23 a.m. UTC 
From: Daniel Drake <drake@endlessm.com>

BugLink: https://bugs.launchpad.net/bugs/1732056

On Asus laptop models X505BA, X505BP, X542BA and X542BP, the i2c-hid
touchpad (using a GPIO for interrupts) becomes unresponsive after a
few minutes of usage, or after placing two fingers on the touchpad,
which seems to have the effect of queuing up a large amount of input
data to be transferred.

When the touchpad is in unresponsive state, we observed that the GPIO
level-triggered interrupt is still at it's active level, however the
pinctrl-amd driver is not receiving/dispatching more interrupts at this
point.

After the initial interrupt arrives, amd_gpio_irq_mask() is called
however we then see amd_gpio_irq_handler() being called repeatedly for
the same irq; the interrupt mask is not taking effect because of the
following sequence of events:
 - amd_gpio_irq_handler fires, reads and caches pin reg
 - amd_gpio_irq_handler calls generic_handle_irq()
 - During IRQ handling, amd_gpio_irq_mask() is called and modifies pin reg
 - amd_gpio_irq_handler clears interrupt by writing cached value

The stale cached value written at the final stage undoes the masking.
Fix this by re-reading the register before clearing the interrupt.

I also spotted that the interrupt-clearing code can race against
amd_gpio_irq_mask() / amd_gpio_irq_unmask(), so add locking there.
Presumably this race was leading to the loss of interrupts.

After these changes, the touchpad appears to be working fine.

Signed-off-by: Daniel Drake <drake@endlessm.com>
Acked-by: Shah, Nehal-bakulchandra <Nehal-Bakulchandra.shah@amd.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
(cherry picked from commit 6afb10267c1692ada3a2903e31ea339917ad3ac0)
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
 drivers/pinctrl/pinctrl-amd.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/drivers/pinctrl/pinctrl-amd.c b/drivers/pinctrl/pinctrl-amd.c
index 7c30fd986560..156e41e3dde2 100644
--- a/drivers/pinctrl/pinctrl-amd.c
+++ b/drivers/pinctrl/pinctrl-amd.c
@@ -534,8 +534,16 @@  static irqreturn_t amd_gpio_irq_handler(int irq, void *dev_id)
 				continue;
 			irq = irq_find_mapping(gc->irqdomain, irqnr + i);
 			generic_handle_irq(irq);
-			/* Clear interrupt */
+
+			/* Clear interrupt.
+			 * We must read the pin register again, in case the
+			 * value was changed while executing
+			 * generic_handle_irq() above.
+			 */
+			raw_spin_lock_irqsave(&gpio_dev->lock, flags);
+			regval = readl(regs + i);
 			writel(regval, regs + i);
+			raw_spin_unlock_irqrestore(&gpio_dev->lock, flags);
 			ret = IRQ_HANDLED;
 		}
 	}