| Message ID | 20200125221410.8022-1-pthomas8589@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | GPIO, Fix bug where the wrong GPIO register is written to | expand |
On Sat, Jan 25, 2020 at 05:14:10PM -0500, Paul Thomas wrote: > Care is taken with "index", however with the current version > the actual xgpio_writereg is using index for data but > xgpio_regoffset(chip, i) for the offset. And since i is already > incremented it is incorrect. This patch fixes it so that index > is used for the offset too. > > Signed-off-by: Paul Thomas <pthomas8589@gmail.com> Hi Paul Please put Xilinx into the subject line. I had to actually look at the patch to decide it was not relevant to me. Andrew
On 25. 01. 20 23:14, Paul Thomas wrote: > Care is taken with "index", however with the current version > the actual xgpio_writereg is using index for data but > xgpio_regoffset(chip, i) for the offset. And since i is already > incremented it is incorrect. This patch fixes it so that index > is used for the offset too. > > Signed-off-by: Paul Thomas <pthomas8589@gmail.com> > --- > There are many different ways to correct this, I'd just like it to get > fixed. I've tested this with a 5.2 kernel, but this patch is against > 5.5rc7. > > drivers/gpio/gpio-xilinx.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c > index a9748b5198e6..67f9f82e0db0 100644 > --- a/drivers/gpio/gpio-xilinx.c > +++ b/drivers/gpio/gpio-xilinx.c > @@ -147,9 +147,10 @@ static void xgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask, > for (i = 0; i < gc->ngpio; i++) { > if (*mask == 0) > break; > + /* Once finished with an index write it out to the register */ > if (index != xgpio_index(chip, i)) { > xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET + > - xgpio_regoffset(chip, i), > + index * XGPIO_CHANNEL_OFFSET, > chip->gpio_state[index]); > spin_unlock_irqrestore(&chip->gpio_lock[index], flags); > index = xgpio_index(chip, i); > @@ -165,7 +166,7 @@ static void xgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask, > } > > xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET + > - xgpio_regoffset(chip, i), chip->gpio_state[index]); > + index * XGPIO_CHANNEL_OFFSET, chip->gpio_state[index]); > > spin_unlock_irqrestore(&chip->gpio_lock[index], flags); > } > Shubhrajyoti: Please take a look. Thanks, Michal
On Sat, Jan 25, 2020 at 11:14 PM Paul Thomas <pthomas8589@gmail.com> wrote: > Care is taken with "index", however with the current version > the actual xgpio_writereg is using index for data but > xgpio_regoffset(chip, i) for the offset. And since i is already > incremented it is incorrect. This patch fixes it so that index > is used for the offset too. > > Signed-off-by: Paul Thomas <pthomas8589@gmail.com> > --- > There are many different ways to correct this, I'd just like it to get > fixed. I've tested this with a 5.2 kernel, but this patch is against > 5.5rc7. Fixed up the subject and applied, added a tag for stable. Thanks! Linus Walleij
> Fixed up the subject and applied, added a tag for stable. > > Thanks! > Linus Walleij Thanks everyone, next time I'll do the subject better. -Paul
diff --git a/drivers/gpio/gpio-xilinx.c b/drivers/gpio/gpio-xilinx.c index a9748b5198e6..67f9f82e0db0 100644 --- a/drivers/gpio/gpio-xilinx.c +++ b/drivers/gpio/gpio-xilinx.c @@ -147,9 +147,10 @@ static void xgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask, for (i = 0; i < gc->ngpio; i++) { if (*mask == 0) break; + /* Once finished with an index write it out to the register */ if (index != xgpio_index(chip, i)) { xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET + - xgpio_regoffset(chip, i), + index * XGPIO_CHANNEL_OFFSET, chip->gpio_state[index]); spin_unlock_irqrestore(&chip->gpio_lock[index], flags); index = xgpio_index(chip, i); @@ -165,7 +166,7 @@ static void xgpio_set_multiple(struct gpio_chip *gc, unsigned long *mask, } xgpio_writereg(chip->regs + XGPIO_DATA_OFFSET + - xgpio_regoffset(chip, i), chip->gpio_state[index]); + index * XGPIO_CHANNEL_OFFSET, chip->gpio_state[index]); spin_unlock_irqrestore(&chip->gpio_lock[index], flags); }
Care is taken with "index", however with the current version the actual xgpio_writereg is using index for data but xgpio_regoffset(chip, i) for the offset. And since i is already incremented it is incorrect. This patch fixes it so that index is used for the offset too. Signed-off-by: Paul Thomas <pthomas8589@gmail.com> --- There are many different ways to correct this, I'd just like it to get fixed. I've tested this with a 5.2 kernel, but this patch is against 5.5rc7. drivers/gpio/gpio-xilinx.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)