* [PATCH: 0/2] Fix TC35894 gpio interrupt bug @ 2020-08-31 7:14 dillon.minfei 2020-08-31 7:14 ` [PATCH: 1/2] gpio: tc35894: fix up tc35894 interrupt configuration dillon.minfei 2020-08-31 7:14 ` [PATCH: 2/2] gpio: tc35894: Disable Direct KBD interrupts to enable gpio irq dillon.minfei 0 siblings, 2 replies; 5+ messages in thread From: dillon.minfei @ 2020-08-31 7:14 UTC (permalink / raw) To: linus.walleij, lee.jones, bgolaszewski Cc: linux-gpio, linux-kernel, dillon min From: dillon min <dillon.minfei@gmail.com> This patchset intend to fix two bug on tc35894 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 | 12 +++++++++++- include/linux/mfd/tc3589x.h | 5 +++++ 2 files changed, 16 insertions(+), 1 deletion(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH: 1/2] gpio: tc35894: fix up tc35894 interrupt configuration 2020-08-31 7:14 [PATCH: 0/2] Fix TC35894 gpio interrupt bug dillon.minfei @ 2020-08-31 7:14 ` dillon.minfei 2020-09-01 15:46 ` Bartosz Golaszewski 2020-08-31 7:14 ` [PATCH: 2/2] gpio: tc35894: Disable Direct KBD interrupts to enable gpio irq dillon.minfei 1 sibling, 1 reply; 5+ messages in thread From: dillon.minfei @ 2020-08-31 7:14 UTC (permalink / raw) To: linus.walleij, lee.jones, bgolaszewski Cc: linux-gpio, linux-kernel, dillon min From: dillon min <dillon.minfei@gmail.com> 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: c103de240439 ("gpio: reorganize drivers") Signed-off-by: dillon min <dillon.minfei@gmail.com> --- drivers/gpio/gpio-tc3589x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpio/gpio-tc3589x.c b/drivers/gpio/gpio-tc3589x.c index 58b0da9eb76f..ea3f68a28fea 100644 --- a/drivers/gpio/gpio-tc3589x.c +++ b/drivers/gpio/gpio-tc3589x.c @@ -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); } } -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH: 1/2] gpio: tc35894: fix up tc35894 interrupt configuration 2020-08-31 7:14 ` [PATCH: 1/2] gpio: tc35894: fix up tc35894 interrupt configuration dillon.minfei @ 2020-09-01 15:46 ` Bartosz Golaszewski 2020-09-03 7:41 ` dillon min 0 siblings, 1 reply; 5+ messages in thread From: Bartosz Golaszewski @ 2020-09-01 15:46 UTC (permalink / raw) To: dillon.minfei; +Cc: Linus Walleij, Lee Jones, linux-gpio, LKML On Mon, Aug 31, 2020 at 9:14 AM <dillon.minfei@gmail.com> wrote: > > From: dillon min <dillon.minfei@gmail.com> > > 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: c103de240439 ("gpio: reorganize drivers") > Signed-off-by: dillon min <dillon.minfei@gmail.com> > --- > drivers/gpio/gpio-tc3589x.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpio/gpio-tc3589x.c b/drivers/gpio/gpio-tc3589x.c > index 58b0da9eb76f..ea3f68a28fea 100644 > --- a/drivers/gpio/gpio-tc3589x.c > +++ b/drivers/gpio/gpio-tc3589x.c > @@ -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); > } > } > > -- > 2.7.4 > I suppose this patch may be correct but I don't see how commit c103de240439 ("gpio: reorganize drivers") could be the culprit. It's been like this since the original driver implementation from commit d88b25be3584 ("gpio: Add TC35892 GPIO driver"). It's been over a decade since this driver was merged and nobody ever reported this. Either nobody ever used the GPIO module with interrupts (unless the bug's impact is not significant) for this chip or this is a quirk of some specific model you're using. Could you double-check this? Bartosz ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH: 1/2] gpio: tc35894: fix up tc35894 interrupt configuration 2020-09-01 15:46 ` Bartosz Golaszewski @ 2020-09-03 7:41 ` dillon min 0 siblings, 0 replies; 5+ messages in thread From: dillon min @ 2020-09-03 7:41 UTC (permalink / raw) To: Bartosz Golaszewski; +Cc: Linus Walleij, Lee Jones, linux-gpio, LKML Hi Bartosz, Thanks for reviewing. On Tue, Sep 1, 2020 at 11:46 PM Bartosz Golaszewski <bgolaszewski@baylibre.com> wrote: > > On Mon, Aug 31, 2020 at 9:14 AM <dillon.minfei@gmail.com> wrote: > > > > From: dillon min <dillon.minfei@gmail.com> > > > > 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: c103de240439 ("gpio: reorganize drivers") > > Signed-off-by: dillon min <dillon.minfei@gmail.com> > > --- > > drivers/gpio/gpio-tc3589x.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpio/gpio-tc3589x.c b/drivers/gpio/gpio-tc3589x.c > > index 58b0da9eb76f..ea3f68a28fea 100644 > > --- a/drivers/gpio/gpio-tc3589x.c > > +++ b/drivers/gpio/gpio-tc3589x.c > > @@ -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); > > } > > } > > > > -- > > 2.7.4 > > > > I suppose this patch may be correct but I don't see how commit > c103de240439 ("gpio: reorganize drivers") could be the culprit. It's > been like this since the original driver implementation from commit > d88b25be3584 ("gpio: Add TC35892 GPIO driver"). Sorry, i didn't check the original code file, yes, I should use this fixes tag. > > It's been over a decade since this driver was merged and nobody ever > reported this. Either nobody ever used the GPIO module with interrupts > (unless the bug's impact is not significant) for this chip or this is > a quirk of some specific model you're using. Could you double-check > this? I used tc35894xbg, and searched https://toshiba.semicon-storage.com/ , but no tc35892 found, it seems was replaced by tc35894x series. from the git history Linus Walleij has some submittes about the interrupt part. maybe he can give us some feedback. post tc35894 datasheet here for you https://toshiba.semicon-storage.com/info/docget.jsp?did=30200&prodName=TC35894XBG > > Bartosz ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH: 2/2] gpio: tc35894: Disable Direct KBD interrupts to enable gpio irq 2020-08-31 7:14 [PATCH: 0/2] Fix TC35894 gpio interrupt bug dillon.minfei 2020-08-31 7:14 ` [PATCH: 1/2] gpio: tc35894: fix up tc35894 interrupt configuration dillon.minfei @ 2020-08-31 7:14 ` dillon.minfei 1 sibling, 0 replies; 5+ messages in thread From: dillon.minfei @ 2020-08-31 7:14 UTC (permalink / raw) To: linus.walleij, lee.jones, bgolaszewski Cc: linux-gpio, linux-kernel, dillon min From: dillon min <dillon.minfei@gmail.com> 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. verified on tc35894, need more test on other tc3589x. Signed-off-by: dillon min <dillon.minfei@gmail.com> --- drivers/gpio/gpio-tc3589x.c | 10 ++++++++++ include/linux/mfd/tc3589x.h | 5 +++++ 2 files changed, 15 insertions(+) diff --git a/drivers/gpio/gpio-tc3589x.c b/drivers/gpio/gpio-tc3589x.c index ea3f68a28fea..760e716468dc 100644 --- a/drivers/gpio/gpio-tc3589x.c +++ b/drivers/gpio/gpio-tc3589x.c @@ -334,6 +334,16 @@ 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", diff --git a/include/linux/mfd/tc3589x.h b/include/linux/mfd/tc3589x.h index bb2b19599761..c18fc7c3c2d6 100644 --- a/include/linux/mfd/tc3589x.h +++ b/include/linux/mfd/tc3589x.h @@ -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,8 @@ enum tx3589x_block { #define TC3589x_GPIOODM2 0xE4 #define TC3589x_GPIOODE2 0xE5 +#define TC3589x_DKBDMSK 0xF3 + #define TC3589x_INT_GPIIRQ 0 #define TC3589x_INT_TI0IRQ 1 #define TC3589x_INT_TI1IRQ 2 -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-03 7:41 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-08-31 7:14 [PATCH: 0/2] Fix TC35894 gpio interrupt bug dillon.minfei 2020-08-31 7:14 ` [PATCH: 1/2] gpio: tc35894: fix up tc35894 interrupt configuration dillon.minfei 2020-09-01 15:46 ` Bartosz Golaszewski 2020-09-03 7:41 ` dillon min 2020-08-31 7:14 ` [PATCH: 2/2] gpio: tc35894: Disable Direct KBD interrupts to enable gpio irq dillon.minfei
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).