| Message ID | 20200327172244.30816-3-u.kleine-koenig@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: pca953x: fix handling of automatic address incrementing | expand |
> Am 27.03.2020 um 18:22 schrieb Uwe Kleine-König <u.kleine-koenig@pengutronix.de>: > > After the previous patch the two last parameters of > pca953x_recalc_addr() are unused and so can be dropped. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/gpio/gpio-pca953x.c | 31 +++++++++++-------------------- > 1 file changed, 11 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c > index 8168558299c2..f4943cad5d0e 100644 > --- a/drivers/gpio/gpio-pca953x.c > +++ b/drivers/gpio/gpio-pca953x.c > @@ -325,8 +325,7 @@ static const struct regmap_config pca953x_ai_i2c_regmap = { > .max_register = 0x7f, > }; > > -static u8 pca953x_recalc_addr(struct pca953x_chip *chip, int reg, int off, > - bool write, bool addrinc) > +static u8 pca953x_recalc_addr(struct pca953x_chip *chip, int reg, int off) > { > int bank_shift = pca953x_bank_shift(chip); > int addr = (reg & PCAL_GPIO_MASK) << bank_shift; > @@ -338,7 +337,7 @@ static u8 pca953x_recalc_addr(struct pca953x_chip *chip, int reg, int off, > > static int pca953x_write_regs(struct pca953x_chip *chip, int reg, unsigned long *val) > { > - u8 regaddr = pca953x_recalc_addr(chip, reg, 0, true, true); > + u8 regaddr = pca953x_recalc_addr(chip, reg, 0; here is some ")" missing. After fix&compile it still works for the pcal6524 of the OMAP5/Pyra (LPAE build). > u8 value[MAX_BANK]; > int i, ret; > > @@ -356,7 +355,7 @@ static int pca953x_write_regs(struct pca953x_chip *chip, int reg, unsigned long > > static int pca953x_read_regs(struct pca953x_chip *chip, int reg, unsigned long *val) > { > - u8 regaddr = pca953x_recalc_addr(chip, reg, 0, false, true); > + u8 regaddr = pca953x_recalc_addr(chip, reg, 0); > u8 value[MAX_BANK]; > int i, ret; > > @@ -375,8 +374,7 @@ static int pca953x_read_regs(struct pca953x_chip *chip, int reg, unsigned long * > static int pca953x_gpio_direction_input(struct gpio_chip *gc, unsigned off) > { > struct pca953x_chip *chip = gpiochip_get_data(gc); > - u8 dirreg = pca953x_recalc_addr(chip, chip->regs->direction, off, > - true, false); > + u8 dirreg = pca953x_recalc_addr(chip, chip->regs->direction, off); > u8 bit = BIT(off % BANK_SZ); > int ret; > > @@ -390,10 +388,8 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc, > unsigned off, int val) > { > struct pca953x_chip *chip = gpiochip_get_data(gc); > - u8 dirreg = pca953x_recalc_addr(chip, chip->regs->direction, off, > - true, false); > - u8 outreg = pca953x_recalc_addr(chip, chip->regs->output, off, > - true, false); > + u8 dirreg = pca953x_recalc_addr(chip, chip->regs->direction, off); > + u8 outreg = pca953x_recalc_addr(chip, chip->regs->output, off); > u8 bit = BIT(off % BANK_SZ); > int ret; > > @@ -413,8 +409,7 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc, > static int pca953x_gpio_get_value(struct gpio_chip *gc, unsigned off) > { > struct pca953x_chip *chip = gpiochip_get_data(gc); > - u8 inreg = pca953x_recalc_addr(chip, chip->regs->input, off, > - true, false); > + u8 inreg = pca953x_recalc_addr(chip, chip->regs->input, off); > u8 bit = BIT(off % BANK_SZ); > u32 reg_val; > int ret; > @@ -438,8 +433,7 @@ static int pca953x_gpio_get_value(struct gpio_chip *gc, unsigned off) > static void pca953x_gpio_set_value(struct gpio_chip *gc, unsigned off, int val) > { > struct pca953x_chip *chip = gpiochip_get_data(gc); > - u8 outreg = pca953x_recalc_addr(chip, chip->regs->output, off, > - true, false); > + u8 outreg = pca953x_recalc_addr(chip, chip->regs->output, off); > u8 bit = BIT(off % BANK_SZ); > > mutex_lock(&chip->i2c_lock); > @@ -450,8 +444,7 @@ static void pca953x_gpio_set_value(struct gpio_chip *gc, unsigned off, int val) > static int pca953x_gpio_get_direction(struct gpio_chip *gc, unsigned off) > { > struct pca953x_chip *chip = gpiochip_get_data(gc); > - u8 dirreg = pca953x_recalc_addr(chip, chip->regs->direction, off, > - true, false); > + u8 dirreg = pca953x_recalc_addr(chip, chip->regs->direction, off); > u8 bit = BIT(off % BANK_SZ); > u32 reg_val; > int ret; > @@ -491,10 +484,8 @@ static int pca953x_gpio_set_pull_up_down(struct pca953x_chip *chip, > unsigned int offset, > unsigned long config) > { > - u8 pull_en_reg = pca953x_recalc_addr(chip, PCAL953X_PULL_EN, offset, > - true, false); > - u8 pull_sel_reg = pca953x_recalc_addr(chip, PCAL953X_PULL_SEL, offset, > - true, false); > + u8 pull_en_reg = pca953x_recalc_addr(chip, PCAL953X_PULL_EN, offset); > + u8 pull_sel_reg = pca953x_recalc_addr(chip, PCAL953X_PULL_SEL, offset); > u8 bit = BIT(offset % BANK_SZ); > int ret; > > -- > 2.26.0.rc2 > BR and thanks, Nikolaus Schaller
On Fri, Mar 27, 2020 at 11:06:43PM +0100, H. Nikolaus Schaller wrote: > > > Am 27.03.2020 um 18:22 schrieb Uwe Kleine-König <u.kleine-koenig@pengutronix.de>: > > > > After the previous patch the two last parameters of > > pca953x_recalc_addr() are unused and so can be dropped. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > drivers/gpio/gpio-pca953x.c | 31 +++++++++++-------------------- > > 1 file changed, 11 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c > > index 8168558299c2..f4943cad5d0e 100644 > > --- a/drivers/gpio/gpio-pca953x.c > > +++ b/drivers/gpio/gpio-pca953x.c > > @@ -325,8 +325,7 @@ static const struct regmap_config pca953x_ai_i2c_regmap = { > > .max_register = 0x7f, > > }; > > > > -static u8 pca953x_recalc_addr(struct pca953x_chip *chip, int reg, int off, > > - bool write, bool addrinc) > > +static u8 pca953x_recalc_addr(struct pca953x_chip *chip, int reg, int off) > > { > > int bank_shift = pca953x_bank_shift(chip); > > int addr = (reg & PCAL_GPIO_MASK) << bank_shift; > > @@ -338,7 +337,7 @@ static u8 pca953x_recalc_addr(struct pca953x_chip *chip, int reg, int off, > > > > static int pca953x_write_regs(struct pca953x_chip *chip, int reg, unsigned long *val) > > { > > - u8 regaddr = pca953x_recalc_addr(chip, reg, 0, true, true); > > + u8 regaddr = pca953x_recalc_addr(chip, reg, 0; > > here is some ")" missing. > > After fix&compile it still works for the pcal6524 of the OMAP5/Pyra > (LPAE build). Argh, it seems I didn't try to recompile after rebasing :-| I'm still waiting for more feedback and then will send out a v2. Thanks for your test, Uwe
On Sat, Mar 28, 2020 at 10:27:30AM +0100, Uwe Kleine-König wrote: > On Fri, Mar 27, 2020 at 11:06:43PM +0100, H. Nikolaus Schaller wrote: > I'm still waiting for more feedback and then will send out a v2. From code perspective looks good, Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> If you give me some time (couple of days) I'll test it on PCA9555 (for no regression).
On Sat, Mar 28, 2020 at 09:41:23PM +0200, Andy Shevchenko wrote: > On Sat, Mar 28, 2020 at 10:27:30AM +0100, Uwe Kleine-König wrote: > > On Fri, Mar 27, 2020 at 11:06:43PM +0100, H. Nikolaus Schaller wrote: > > > I'm still waiting for more feedback and then will send out a v2. > > From code perspective looks good, > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > If you give me some time (couple of days) I'll test it on PCA9555 (for no > regression). No regression found in my case (PCA9555 + IRQ test) Tested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c index 8168558299c2..f4943cad5d0e 100644 --- a/drivers/gpio/gpio-pca953x.c +++ b/drivers/gpio/gpio-pca953x.c @@ -325,8 +325,7 @@ static const struct regmap_config pca953x_ai_i2c_regmap = { .max_register = 0x7f, }; -static u8 pca953x_recalc_addr(struct pca953x_chip *chip, int reg, int off, - bool write, bool addrinc) +static u8 pca953x_recalc_addr(struct pca953x_chip *chip, int reg, int off) { int bank_shift = pca953x_bank_shift(chip); int addr = (reg & PCAL_GPIO_MASK) << bank_shift; @@ -338,7 +337,7 @@ static u8 pca953x_recalc_addr(struct pca953x_chip *chip, int reg, int off, static int pca953x_write_regs(struct pca953x_chip *chip, int reg, unsigned long *val) { - u8 regaddr = pca953x_recalc_addr(chip, reg, 0, true, true); + u8 regaddr = pca953x_recalc_addr(chip, reg, 0; u8 value[MAX_BANK]; int i, ret; @@ -356,7 +355,7 @@ static int pca953x_write_regs(struct pca953x_chip *chip, int reg, unsigned long static int pca953x_read_regs(struct pca953x_chip *chip, int reg, unsigned long *val) { - u8 regaddr = pca953x_recalc_addr(chip, reg, 0, false, true); + u8 regaddr = pca953x_recalc_addr(chip, reg, 0); u8 value[MAX_BANK]; int i, ret; @@ -375,8 +374,7 @@ static int pca953x_read_regs(struct pca953x_chip *chip, int reg, unsigned long * static int pca953x_gpio_direction_input(struct gpio_chip *gc, unsigned off) { struct pca953x_chip *chip = gpiochip_get_data(gc); - u8 dirreg = pca953x_recalc_addr(chip, chip->regs->direction, off, - true, false); + u8 dirreg = pca953x_recalc_addr(chip, chip->regs->direction, off); u8 bit = BIT(off % BANK_SZ); int ret; @@ -390,10 +388,8 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc, unsigned off, int val) { struct pca953x_chip *chip = gpiochip_get_data(gc); - u8 dirreg = pca953x_recalc_addr(chip, chip->regs->direction, off, - true, false); - u8 outreg = pca953x_recalc_addr(chip, chip->regs->output, off, - true, false); + u8 dirreg = pca953x_recalc_addr(chip, chip->regs->direction, off); + u8 outreg = pca953x_recalc_addr(chip, chip->regs->output, off); u8 bit = BIT(off % BANK_SZ); int ret; @@ -413,8 +409,7 @@ static int pca953x_gpio_direction_output(struct gpio_chip *gc, static int pca953x_gpio_get_value(struct gpio_chip *gc, unsigned off) { struct pca953x_chip *chip = gpiochip_get_data(gc); - u8 inreg = pca953x_recalc_addr(chip, chip->regs->input, off, - true, false); + u8 inreg = pca953x_recalc_addr(chip, chip->regs->input, off); u8 bit = BIT(off % BANK_SZ); u32 reg_val; int ret; @@ -438,8 +433,7 @@ static int pca953x_gpio_get_value(struct gpio_chip *gc, unsigned off) static void pca953x_gpio_set_value(struct gpio_chip *gc, unsigned off, int val) { struct pca953x_chip *chip = gpiochip_get_data(gc); - u8 outreg = pca953x_recalc_addr(chip, chip->regs->output, off, - true, false); + u8 outreg = pca953x_recalc_addr(chip, chip->regs->output, off); u8 bit = BIT(off % BANK_SZ); mutex_lock(&chip->i2c_lock); @@ -450,8 +444,7 @@ static void pca953x_gpio_set_value(struct gpio_chip *gc, unsigned off, int val) static int pca953x_gpio_get_direction(struct gpio_chip *gc, unsigned off) { struct pca953x_chip *chip = gpiochip_get_data(gc); - u8 dirreg = pca953x_recalc_addr(chip, chip->regs->direction, off, - true, false); + u8 dirreg = pca953x_recalc_addr(chip, chip->regs->direction, off); u8 bit = BIT(off % BANK_SZ); u32 reg_val; int ret; @@ -491,10 +484,8 @@ static int pca953x_gpio_set_pull_up_down(struct pca953x_chip *chip, unsigned int offset, unsigned long config) { - u8 pull_en_reg = pca953x_recalc_addr(chip, PCAL953X_PULL_EN, offset, - true, false); - u8 pull_sel_reg = pca953x_recalc_addr(chip, PCAL953X_PULL_SEL, offset, - true, false); + u8 pull_en_reg = pca953x_recalc_addr(chip, PCAL953X_PULL_EN, offset); + u8 pull_sel_reg = pca953x_recalc_addr(chip, PCAL953X_PULL_SEL, offset); u8 bit = BIT(offset % BANK_SZ); int ret;
After the previous patch the two last parameters of pca953x_recalc_addr() are unused and so can be dropped. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/gpio/gpio-pca953x.c | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-)