On Mon, Aug 31, 2020 at 9:14 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:
>
> 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 <[email protected]>
> ---
> 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
Hi Bartosz,
Thanks for reviewing.
On Tue, Sep 1, 2020 at 11:46 PM Bartosz Golaszewski
<[email protected]> wrote:
>
> On Mon, Aug 31, 2020 at 9:14 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:
> >
> > 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 <[email protected]>
> > ---
> > 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