Skip to contentSkip to conversation

Fix TC35894 gpio interrupt bug

PATCH In review [email protected] 3 participants 6 messages Sep 3, 2020 – Sep 9, 2020

Subsystems (2)
GPIO Multifunction devices (MFD)
lore
Share & tools
lore

[email protected]

author
#0 lore raw

v2 cover Fix TC35894 gpio interrupt bug

From: dillon min <[email protected]>

This patchset intend to fix two bug on tc35894

V2:
1 change V1[1] patch's Fixes tag
2 add DIRECTx register configuration in tc3589x_gpio_irq_sync_unlock()
to active general purpose gpio mode, else can't read correct GPIOMISx to identify which interrupt

V1:

1 offset counting is wrong in tc3589x_gpio_irq_sync_unlock() 2 disable Direct KBD interrupts in gpio-tc3589x's probe(),
at least have to do this on tc35894, if not, after chip reset, IRQST(0x91) will always be 0x20, IRQN always low level, can't be cleared. need more test on other tc3589x.

dillon min (2):
gpio: tc35894: fix up tc35894 interrupt configuration
gpio: tc35894: Disable Direct KBD interrupts to enable gpio irq

 drivers/gpio/gpio-tc3589x.c | 20 +++++++++++++++++---
 include/linux/mfd/tc3589x.h |  6 ++++++
 2 files changed, 23 insertions(+), 3 deletions(-)
author
#1 lore raw

PATCH v2 1/2 gpio: tc35894: fix up tc35894 interrupt configuration

From: dillon min <[email protected]>

The offset of regmap is incorrect, j * 8 is move to the wrong register.

for example:

asume i = 0, j = 1. we want to set KPY5 as interrupt falling edge mode, regmap[0][1] should be TC3589x_GPIOIBE1 0xcd but, regmap[i] + j * 8 = TC3589x_GPIOIBE0 + 8 ,point to 0xd4, this is TC3589x_GPIOIE2 not TC3589x_GPIOIBE1.

Fixes d88b25be3584 ("gpio: Add TC35892 GPIO driver")
Signed-off-by dillon min <[email protected]>

---

Hi, Bartosz

Just change Fixes tag.

Thanks.
Dillon

 drivers/gpio/gpio-tc3589x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpio/gpio-tc3589x.c +1 -1
index 58b0da9eb76f..ea3f68a28fea 100644
@@ -212,7 +212,7 @@ static void tc3589x_gpio_irq_sync_unlock(struct irq_data *d)
 				continue;
 
 			tc3589x_gpio->oldregs[i][j] = new;
-			tc3589x_reg_write(tc3589x, regmap[i] + j * 8, new);
+			tc3589x_reg_write(tc3589x, regmap[i] + j, new);
 		}
 	}
Signature · 1 line
2.7.4
#2 lore raw

Re: [PATCH: V2 1/2] gpio: tc35894: fix up tc35894 interrupt configuration

Quoted 8 lines · On Thu, Sep 3, 2020 at 9:30 AM <[email protected]> wrote:

From: dillon min <[email protected]>

The offset of regmap is incorrect, j * 8 is move to the wrong register.

for example:

Quoted 9 lines

asume i = 0, j = 1. we want to set KPY5 as interrupt falling edge mode, regmap[0][1] should be TC3589x_GPIOIBE1 0xcd but, regmap[i] + j * 8 = TC3589x_GPIOIBE0 + 8 ,point to 0xd4, this is TC3589x_GPIOIE2 not TC3589x_GPIOIBE1.

Fixes d88b25be3584 ("gpio: Add TC35892 GPIO driver")
Signed-off-by dillon min <[email protected]>

---

Queued for fixes.

Thanks!
Bartosz

author
#3 lore raw

PATCH v2 2/2 gpio: tc35894: Disable Direct KBD interrupts to enable gpio irq

In reply to [email protected]

From: dillon min <[email protected]>

On tc35894, have to disable direct keypad interrupts to make it as general purpose interrupts functionality work. if not, after chip reset, IRQST(0x91) will always 0x20, IRQN always low level, can't be clear.

Configure DIRECTx to enable general purpose gpio mode, else read GPIOMISx register always zero in irq routine.

verified on tc35894, need more test on other tc3589x.

Signed-off-by dillon min <[email protected]>

---

V1 -> V2:
Add DIRECTx register configuration to active general purpose gpio mode.

 drivers/gpio/gpio-tc3589x.c | 18 ++++++++++++++++--
 include/linux/mfd/tc3589x.h |  6 ++++++
 2 files changed, 22 insertions(+), 2 deletions(-)
drivers/gpio/gpio-tc3589x.c +16 -2
index ea3f68a28fea..55b8dbd13d11 100644
@@ -19,9 +19,9 @@
  * These registers are modified under the irq bus lock and cached to avoid
  * unnecessary writes in bus_sync_unlock.
  */
-enum { REG_IBE, REG_IEV, REG_IS, REG_IE };
+enum { REG_IBE, REG_IEV, REG_IS, REG_IE, REG_DIRECT };
 
-#define CACHE_NR_REGS	4
+#define CACHE_NR_REGS	5
 #define CACHE_NR_BANKS	3
 
 struct tc3589x_gpio {

@@ -200,6 +200,7 @@ static void tc3589x_gpio_irq_sync_unlock(struct irq_data *d)
 		[REG_IEV]	= TC3589x_GPIOIEV0,
 		[REG_IS]	= TC3589x_GPIOIS0,
 		[REG_IE]	= TC3589x_GPIOIE0,
+		[REG_DIRECT]	= TC3589x_DIRECT0,
 	};
 	int i, j;
 

@@ -228,6 +229,7 @@ static void tc3589x_gpio_irq_mask(struct irq_data *d)
 	int mask = BIT(offset % 8);
 
 	tc3589x_gpio->regs[REG_IE][regoffset] &= ~mask;
+	tc3589x_gpio->regs[REG_DIRECT][regoffset] |= mask;
 }
 
 static void tc3589x_gpio_irq_unmask(struct irq_data *d)

@@ -239,6 +241,7 @@ static void tc3589x_gpio_irq_unmask(struct irq_data *d)
 	int mask = BIT(offset % 8);
 
 	tc3589x_gpio->regs[REG_IE][regoffset] |= mask;
+	tc3589x_gpio->regs[REG_DIRECT][regoffset] &= ~mask;
 }
 
 static struct irq_chip tc3589x_gpio_irq_chip = {

@@ -334,6 +337,17 @@ static int tc3589x_gpio_probe(struct platform_device *pdev)
 	if (ret < 0)
 		return ret;
 
+	 /* For tc35894, have to disable Direct KBD interrupts,
+	  * else IRQST will always be 0x20, IRQN low level, can't
+	  * clear the irq status.
+	  * TODO: need more test on other tc3589x chip.
+	  *
+	  */
+	ret = tc3589x_reg_write(tc3589x, TC3589x_DKBDMSK,
+			TC3589x_DKBDMSK_ELINT | TC3589x_DKBDMSK_EINT);
+	if (ret < 0)
+		return ret;
+
 	ret = devm_request_threaded_irq(&pdev->dev,
 					irq, NULL, tc3589x_gpio_irq,
 					IRQF_ONESHOT, "tc3589x-gpio",
include/linux/mfd/tc3589x.h +6 -0
index bb2b19599761..b84955410e03 100644
@@ -19,6 +19,9 @@ enum tx3589x_block {
 #define TC3589x_RSTCTRL_KBDRST	(1 << 1)
 #define TC3589x_RSTCTRL_GPIRST	(1 << 0)
 
+#define TC3589x_DKBDMSK_ELINT	(1 << 1)
+#define TC3589x_DKBDMSK_EINT	(1 << 0)
+
 /* Keyboard Configuration Registers */
 #define TC3589x_KBDSETTLE_REG   0x01
 #define TC3589x_KBDBOUNCE       0x02

@@ -101,6 +104,9 @@ enum tx3589x_block {
 #define TC3589x_GPIOODM2	0xE4
 #define TC3589x_GPIOODE2	0xE5
 
+#define TC3589x_DIRECT0		0xEC
+#define TC3589x_DKBDMSK		0xF3
+
 #define TC3589x_INT_GPIIRQ	0
 #define TC3589x_INT_TI0IRQ	1
 #define TC3589x_INT_TI1IRQ	2
Signature · 1 line
2.7.4
Acked-by on v2 2/2
#4 lore raw

Re: [PATCH: V2 2/2] gpio: tc35894: Disable Direct KBD interrupts to enable gpio irq

Quoted 13 lines · On Thu, 03 Sep 2020, [email protected] wrote:

From: dillon min <[email protected]>

On tc35894, have to disable direct keypad interrupts to make it as general purpose interrupts functionality work. if not, after chip reset, IRQST(0x91) will always 0x20, IRQN always low level, can't be clear.

Configure DIRECTx to enable general purpose gpio mode, else read GPIOMISx register always zero in irq routine.

verified on tc35894, need more test on other tc3589x.

Signed-off-by dillon min <[email protected]>
Quoted 8 lines

---

V1 -> V2:
Add DIRECTx register configuration to active general purpose gpio mode.

 drivers/gpio/gpio-tc3589x.c | 18 ++++++++++++++++--
 include/linux/mfd/tc3589x.h |  6 ++++++
Acked-by Lee Jones <[email protected]>
Signature · 4 lines
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
#5 lore raw

Re: [PATCH: V2 2/2] gpio: tc35894: Disable Direct KBD interrupts to enable gpio irq

In reply to [email protected]
Quoted 8 lines · On Thu, Sep 3, 2020 at 9:30 AM <[email protected]> wrote:

From: dillon min <[email protected]>

On tc35894, have to disable direct keypad interrupts to make it as general purpose interrupts functionality work. if not, after chip reset, IRQST(0x91) will always 0x20, IRQN always low level, can't be clear.

Quoted 7 lines

Configure DIRECTx to enable general purpose gpio mode, else read GPIOMISx register always zero in irq routine.

verified on tc35894, need more test on other tc3589x.

Signed-off-by dillon min <[email protected]>

---

Queued for v5.10.

Thanks!
Bartosz