| Message ID | 20191014161148.10543-1-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] gpio: pca953x: Convert to use bitmap API | expand |
On Mon, Oct 14, 2019 at 07:11:48PM +0300, Andy Shevchenko wrote: > Instead of customized approach convert the driver to use bitmap API. > > Depends-on: 6e9c6674d1bf ("gpio: pca953x: utilize the for_each_set_clump8 macro") > Cc: William Breathitt Gray <vilhelm.gray@gmail.com> > Cc: Geert Uytterhoeven <geert+renesas@glider.be> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > Cc: Marek Vasut <marek.vasut+renesas@gmail.com> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Acked-by: William Breathitt Gray <vilhelm.gray@gmail.com> I agree with the concept of this change, but there is one fix I would like made detailed inline below. > --- > drivers/gpio/gpio-pca953x.c | 170 ++++++++++++++++-------------------- > 1 file changed, 73 insertions(+), 97 deletions(-) > > diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c > index 10b669b8f27d..95c2d6c99f41 100644 > --- a/drivers/gpio/gpio-pca953x.c > +++ b/drivers/gpio/gpio-pca953x.c > @@ -9,8 +9,7 @@ > */ > > #include <linux/acpi.h> > -#include <linux/bits.h> > -#include <linux/bitops.h> > +#include <linux/bitmap.h> > #include <linux/gpio/driver.h> > #include <linux/gpio/consumer.h> > #include <linux/i2c.h> > @@ -116,6 +115,7 @@ MODULE_DEVICE_TABLE(acpi, pca953x_acpi_ids); > > #define MAX_BANK 5 > #define BANK_SZ 8 > +#define MAX_LINE (MAX_BANK * BANK_SZ) > > #define NBANK(chip) DIV_ROUND_UP(chip->gpio_chip.ngpio, BANK_SZ) > > @@ -147,10 +147,10 @@ struct pca953x_chip { > > #ifdef CONFIG_GPIO_PCA953X_IRQ > struct mutex irq_lock; > - u8 irq_mask[MAX_BANK]; > - u8 irq_stat[MAX_BANK]; > - u8 irq_trig_raise[MAX_BANK]; > - u8 irq_trig_fall[MAX_BANK]; > + DECLARE_BITMAP(irq_mask, MAX_LINE); > + DECLARE_BITMAP(irq_stat, MAX_LINE); > + DECLARE_BITMAP(irq_trig_raise, MAX_LINE); > + DECLARE_BITMAP(irq_trig_fall, MAX_LINE); > struct irq_chip irq_chip; > #endif > atomic_t wakeup_path; > @@ -334,12 +334,16 @@ static u8 pca953x_recalc_addr(struct pca953x_chip *chip, int reg, int off, > return regaddr; > } > > -static int pca953x_write_regs(struct pca953x_chip *chip, int reg, u8 *val) > +static int pca953x_write_regs(struct pca953x_chip *chip, int reg, unsigned long *val) > { > u8 regaddr = pca953x_recalc_addr(chip, reg, 0, true, true); > - int ret; > + u8 value[MAX_BANK]; > + int i, ret; > + > + for (i = 0; i < NBANK(chip); i++) > + value[i] = bitmap_get_value8(val, i * BANK_SZ); > > - ret = regmap_bulk_write(chip->regmap, regaddr, val, NBANK(chip)); > + ret = regmap_bulk_write(chip->regmap, regaddr, value, NBANK(chip)); > if (ret < 0) { > dev_err(&chip->client->dev, "failed writing register\n"); > return ret; > @@ -348,17 +352,21 @@ static int pca953x_write_regs(struct pca953x_chip *chip, int reg, u8 *val) > return 0; > } > > -static int pca953x_read_regs(struct pca953x_chip *chip, int reg, u8 *val) > +static int pca953x_read_regs(struct pca953x_chip *chip, int reg, unsigned long *val) > { > u8 regaddr = pca953x_recalc_addr(chip, reg, 0, false, true); > - int ret; > + u8 value[MAX_BANK]; > + int i, ret; > > - ret = regmap_bulk_read(chip->regmap, regaddr, val, NBANK(chip)); > + ret = regmap_bulk_read(chip->regmap, regaddr, value, NBANK(chip)); > if (ret < 0) { > dev_err(&chip->client->dev, "failed reading register\n"); > return ret; > } > > + for (i = 0; i < NBANK(chip); i++) > + bitmap_set_value8(val, value[i], i * BANK_SZ); > + > return 0; > } > > @@ -457,10 +465,7 @@ static void pca953x_gpio_set_multiple(struct gpio_chip *gc, > unsigned long *mask, unsigned long *bits) > { > struct pca953x_chip *chip = gpiochip_get_data(gc); > - unsigned long offset; > - unsigned long bank_mask; > - int bank; > - u8 reg_val[MAX_BANK]; > + DECLARE_BITMAP(reg_val, MAX_LINE); > int ret; > > mutex_lock(&chip->i2c_lock); > @@ -468,11 +473,7 @@ static void pca953x_gpio_set_multiple(struct gpio_chip *gc, > if (ret) > goto exit; > > - for_each_set_clump8(offset, bank_mask, mask, gc->ngpio) { > - bank = offset / 8; > - reg_val[bank] &= ~bank_mask; > - reg_val[bank] |= bitmap_get_value8(bits, offset) & bank_mask; > - } > + bitmap_and(reg_val, bits, mask, gc->ngpio); When using set_multiple, it's expected that only the GPIO lines requested to be set are actually set -- albeit if the hardware is capable of that sort of control. This bitmap_and operation is ignoring the existing state of reg_val and overwriting it with (bits & mask), so existing GPIO states are lost and all bits not masked are set to 0. What you should do instead is something akin to this (but for bitmaps): regval &= ~mask; regval |= bits & mask; That should preserve the existing GPIO states in reg_val, while setting those requested by mask and supplied by bits. William Breathitt Gray > > pca953x_write_regs(chip, chip->regs->output, reg_val); > exit: > @@ -599,10 +600,10 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d) > { > struct gpio_chip *gc = irq_data_get_irq_chip_data(d); > struct pca953x_chip *chip = gpiochip_get_data(gc); > - u8 new_irqs; > - int level, i; > - u8 invert_irq_mask[MAX_BANK]; > - u8 reg_direction[MAX_BANK]; > + DECLARE_BITMAP(irq_mask, MAX_LINE); > + DECLARE_BITMAP(reg_direction, MAX_LINE); > + DECLARE_BITMAP(new_irqs, MAX_LINE); > + int level; > > pca953x_read_regs(chip, chip->regs->direction, reg_direction); > > @@ -610,25 +611,18 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d) > /* Enable latch on interrupt-enabled inputs */ > pca953x_write_regs(chip, PCAL953X_IN_LATCH, chip->irq_mask); > > - for (i = 0; i < NBANK(chip); i++) > - invert_irq_mask[i] = ~chip->irq_mask[i]; > + bitmap_complement(irq_mask, chip->irq_mask, gc->ngpio); > > /* Unmask enabled interrupts */ > - pca953x_write_regs(chip, PCAL953X_INT_MASK, invert_irq_mask); > + pca953x_write_regs(chip, PCAL953X_INT_MASK, irq_mask); > } > > + bitmap_or(new_irqs, chip->irq_trig_fall, chip->irq_trig_raise, gc->ngpio); > + bitmap_and(irq_mask, new_irqs, reg_direction, gc->ngpio); > + > /* Look for any newly setup interrupt */ > - for (i = 0; i < NBANK(chip); i++) { > - new_irqs = chip->irq_trig_fall[i] | chip->irq_trig_raise[i]; > - new_irqs &= reg_direction[i]; > - > - while (new_irqs) { > - level = __ffs(new_irqs); > - pca953x_gpio_direction_input(&chip->gpio_chip, > - level + (BANK_SZ * i)); > - new_irqs &= ~(1 << level); > - } > - } > + for_each_set_bit(level, irq_mask, gc->ngpio) > + pca953x_gpio_direction_input(&chip->gpio_chip, level); > > mutex_unlock(&chip->irq_lock); > } > @@ -669,15 +663,15 @@ static void pca953x_irq_shutdown(struct irq_data *d) > chip->irq_trig_fall[d->hwirq / BANK_SZ] &= ~mask; > } > > -static bool pca953x_irq_pending(struct pca953x_chip *chip, u8 *pending) > +static bool pca953x_irq_pending(struct pca953x_chip *chip, unsigned long *pending) > { > - u8 cur_stat[MAX_BANK]; > - u8 old_stat[MAX_BANK]; > - bool pending_seen = false; > - bool trigger_seen = false; > - u8 trigger[MAX_BANK]; > - u8 reg_direction[MAX_BANK]; > - int ret, i; > + struct gpio_chip *gc = &chip->gpio_chip; > + DECLARE_BITMAP(reg_direction, MAX_LINE); > + DECLARE_BITMAP(old_stat, MAX_LINE); > + DECLARE_BITMAP(cur_stat, MAX_LINE); > + DECLARE_BITMAP(new_stat, MAX_LINE); > + DECLARE_BITMAP(trigger, MAX_LINE); > + int ret; > > if (chip->driver_data & PCA_PCAL) { > /* Read the current interrupt status from the device */ > @@ -690,16 +684,13 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip, u8 *pending) > if (ret) > return false; > > - for (i = 0; i < NBANK(chip); i++) { > - /* Apply filter for rising/falling edge selection */ > - pending[i] = (~cur_stat[i] & chip->irq_trig_fall[i]) | > - (cur_stat[i] & chip->irq_trig_raise[i]); > - pending[i] &= trigger[i]; > - if (pending[i]) > - pending_seen = true; > - } > + /* Apply filter for rising/falling edge selection */ > + bitmap_andnot(new_stat, chip->irq_trig_fall, cur_stat, gc->ngpio); > + bitmap_and(old_stat, chip->irq_trig_raise, cur_stat, gc->ngpio); > + bitmap_or(cur_stat, old_stat, new_stat, gc->ngpio); > + bitmap_and(pending, cur_stat, trigger, gc->ngpio); > > - return pending_seen; > + return !bitmap_empty(pending, gc->ngpio); > } > > ret = pca953x_read_regs(chip, chip->regs->input, cur_stat); > @@ -708,55 +699,40 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip, u8 *pending) > > /* Remove output pins from the equation */ > pca953x_read_regs(chip, chip->regs->direction, reg_direction); > - for (i = 0; i < NBANK(chip); i++) > - cur_stat[i] &= reg_direction[i]; > > - memcpy(old_stat, chip->irq_stat, NBANK(chip)); > + bitmap_copy(old_stat, chip->irq_stat, gc->ngpio); > > - for (i = 0; i < NBANK(chip); i++) { > - trigger[i] = (cur_stat[i] ^ old_stat[i]) & chip->irq_mask[i]; > - if (trigger[i]) > - trigger_seen = true; > - } > + bitmap_and(new_stat, cur_stat, reg_direction, gc->ngpio); > + bitmap_xor(cur_stat, new_stat, old_stat, gc->ngpio); > + bitmap_and(trigger, cur_stat, chip->irq_mask, gc->ngpio); > > - if (!trigger_seen) > + if (bitmap_empty(trigger, gc->ngpio)) > return false; > > - memcpy(chip->irq_stat, cur_stat, NBANK(chip)); > + bitmap_copy(chip->irq_stat, new_stat, gc->ngpio); > > - for (i = 0; i < NBANK(chip); i++) { > - pending[i] = (old_stat[i] & chip->irq_trig_fall[i]) | > - (cur_stat[i] & chip->irq_trig_raise[i]); > - pending[i] &= trigger[i]; > - if (pending[i]) > - pending_seen = true; > - } > + bitmap_and(cur_stat, chip->irq_trig_fall, old_stat, gc->ngpio); > + bitmap_and(old_stat, chip->irq_trig_raise, new_stat, gc->ngpio); > + bitmap_or(new_stat, old_stat, cur_stat, gc->ngpio); > + bitmap_and(pending, new_stat, trigger, gc->ngpio); > > - return pending_seen; > + return !bitmap_empty(pending, gc->ngpio); > } > > static irqreturn_t pca953x_irq_handler(int irq, void *devid) > { > struct pca953x_chip *chip = devid; > - u8 pending[MAX_BANK]; > - u8 level; > - unsigned nhandled = 0; > - int i; > + struct gpio_chip *gc = &chip->gpio_chip; > + DECLARE_BITMAP(pending, MAX_LINE); > + int level; > > if (!pca953x_irq_pending(chip, pending)) > return IRQ_NONE; > > - for (i = 0; i < NBANK(chip); i++) { > - while (pending[i]) { > - level = __ffs(pending[i]); > - handle_nested_irq(irq_find_mapping(chip->gpio_chip.irq.domain, > - level + (BANK_SZ * i))); > - pending[i] &= ~(1 << level); > - nhandled++; > - } > - } > + for_each_set_bit(level, pending, gc->ngpio) > + handle_nested_irq(irq_find_mapping(gc->irq.domain, level)); > > - return (nhandled > 0) ? IRQ_HANDLED : IRQ_NONE; > + return bitmap_empty(pending, gc->ngpio) ? IRQ_NONE : IRQ_HANDLED; > } > > static int pca953x_irq_setup(struct pca953x_chip *chip, > @@ -764,8 +740,9 @@ static int pca953x_irq_setup(struct pca953x_chip *chip, > { > struct i2c_client *client = chip->client; > struct irq_chip *irq_chip = &chip->irq_chip; > - u8 reg_direction[MAX_BANK]; > - int ret, i; > + DECLARE_BITMAP(reg_direction, MAX_LINE); > + DECLARE_BITMAP(irq_stat, MAX_LINE); > + int ret; > > if (!client->irq) > return 0; > @@ -776,7 +753,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip, > if (!(chip->driver_data & PCA_INT)) > return 0; > > - ret = pca953x_read_regs(chip, chip->regs->input, chip->irq_stat); > + ret = pca953x_read_regs(chip, chip->regs->input, irq_stat); > if (ret) > return ret; > > @@ -786,8 +763,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip, > * this purpose. > */ > pca953x_read_regs(chip, chip->regs->direction, reg_direction); > - for (i = 0; i < NBANK(chip); i++) > - chip->irq_stat[i] &= reg_direction[i]; > + bitmap_and(chip->irq_stat, irq_stat, reg_direction, chip->gpio_chip.ngpio); > mutex_init(&chip->irq_lock); > > ret = devm_request_threaded_irq(&client->dev, client->irq, > @@ -839,8 +815,8 @@ static int pca953x_irq_setup(struct pca953x_chip *chip, > > static int device_pca95xx_init(struct pca953x_chip *chip, u32 invert) > { > + DECLARE_BITMAP(val, MAX_LINE); > int ret; > - u8 val[MAX_BANK]; > > ret = regcache_sync_region(chip->regmap, chip->regs->output, > chip->regs->output + NBANK(chip)); > @@ -854,9 +830,9 @@ static int device_pca95xx_init(struct pca953x_chip *chip, u32 invert) > > /* set platform specific polarity inversion */ > if (invert) > - memset(val, 0xFF, NBANK(chip)); > + bitmap_fill(val, MAX_LINE); > else > - memset(val, 0, NBANK(chip)); > + bitmap_zero(val, MAX_LINE); > > ret = pca953x_write_regs(chip, chip->regs->invert, val); > out: > @@ -865,8 +841,8 @@ static int device_pca95xx_init(struct pca953x_chip *chip, u32 invert) > > static int device_pca957x_init(struct pca953x_chip *chip, u32 invert) > { > + DECLARE_BITMAP(val, MAX_LINE); > int ret; > - u8 val[MAX_BANK]; > > ret = device_pca95xx_init(chip, invert); > if (ret) > -- > 2.23.0 >
On Wed, Oct 16, 2019 at 05:06:35PM -0400, William Breathitt Gray wrote: > On Mon, Oct 14, 2019 at 07:11:48PM +0300, Andy Shevchenko wrote: > > Instead of customized approach convert the driver to use bitmap API. > Acked-by: William Breathitt Gray <vilhelm.gray@gmail.com> > > I agree with the concept of this change, but there is one fix I would > like made detailed inline below. Thanks! > > - for_each_set_clump8(offset, bank_mask, mask, gc->ngpio) { > > - bank = offset / 8; > > - reg_val[bank] &= ~bank_mask; > > - reg_val[bank] |= bitmap_get_value8(bits, offset) & bank_mask; > > - } > > + bitmap_and(reg_val, bits, mask, gc->ngpio); > > When using set_multiple, it's expected that only the GPIO lines > requested to be set are actually set -- albeit if the hardware is > capable of that sort of control. > > This bitmap_and operation is ignoring the existing state of reg_val and > overwriting it with (bits & mask), so existing GPIO states are lost and > all bits not masked are set to 0. > > What you should do instead is something akin to this (but for bitmaps): > > regval &= ~mask; > regval |= bits & mask; > > That should preserve the existing GPIO states in reg_val, while setting > those requested by mask and supplied by bits. Good catch! I suspected I did something wrong here, that's why I Cc'ed you, and I wasn't mistaken — you helped me, thanks!
diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c index 10b669b8f27d..95c2d6c99f41 100644 --- a/drivers/gpio/gpio-pca953x.c +++ b/drivers/gpio/gpio-pca953x.c @@ -9,8 +9,7 @@ */ #include <linux/acpi.h> -#include <linux/bits.h> -#include <linux/bitops.h> +#include <linux/bitmap.h> #include <linux/gpio/driver.h> #include <linux/gpio/consumer.h> #include <linux/i2c.h> @@ -116,6 +115,7 @@ MODULE_DEVICE_TABLE(acpi, pca953x_acpi_ids); #define MAX_BANK 5 #define BANK_SZ 8 +#define MAX_LINE (MAX_BANK * BANK_SZ) #define NBANK(chip) DIV_ROUND_UP(chip->gpio_chip.ngpio, BANK_SZ) @@ -147,10 +147,10 @@ struct pca953x_chip { #ifdef CONFIG_GPIO_PCA953X_IRQ struct mutex irq_lock; - u8 irq_mask[MAX_BANK]; - u8 irq_stat[MAX_BANK]; - u8 irq_trig_raise[MAX_BANK]; - u8 irq_trig_fall[MAX_BANK]; + DECLARE_BITMAP(irq_mask, MAX_LINE); + DECLARE_BITMAP(irq_stat, MAX_LINE); + DECLARE_BITMAP(irq_trig_raise, MAX_LINE); + DECLARE_BITMAP(irq_trig_fall, MAX_LINE); struct irq_chip irq_chip; #endif atomic_t wakeup_path; @@ -334,12 +334,16 @@ static u8 pca953x_recalc_addr(struct pca953x_chip *chip, int reg, int off, return regaddr; } -static int pca953x_write_regs(struct pca953x_chip *chip, int reg, u8 *val) +static int pca953x_write_regs(struct pca953x_chip *chip, int reg, unsigned long *val) { u8 regaddr = pca953x_recalc_addr(chip, reg, 0, true, true); - int ret; + u8 value[MAX_BANK]; + int i, ret; + + for (i = 0; i < NBANK(chip); i++) + value[i] = bitmap_get_value8(val, i * BANK_SZ); - ret = regmap_bulk_write(chip->regmap, regaddr, val, NBANK(chip)); + ret = regmap_bulk_write(chip->regmap, regaddr, value, NBANK(chip)); if (ret < 0) { dev_err(&chip->client->dev, "failed writing register\n"); return ret; @@ -348,17 +352,21 @@ static int pca953x_write_regs(struct pca953x_chip *chip, int reg, u8 *val) return 0; } -static int pca953x_read_regs(struct pca953x_chip *chip, int reg, u8 *val) +static int pca953x_read_regs(struct pca953x_chip *chip, int reg, unsigned long *val) { u8 regaddr = pca953x_recalc_addr(chip, reg, 0, false, true); - int ret; + u8 value[MAX_BANK]; + int i, ret; - ret = regmap_bulk_read(chip->regmap, regaddr, val, NBANK(chip)); + ret = regmap_bulk_read(chip->regmap, regaddr, value, NBANK(chip)); if (ret < 0) { dev_err(&chip->client->dev, "failed reading register\n"); return ret; } + for (i = 0; i < NBANK(chip); i++) + bitmap_set_value8(val, value[i], i * BANK_SZ); + return 0; } @@ -457,10 +465,7 @@ static void pca953x_gpio_set_multiple(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits) { struct pca953x_chip *chip = gpiochip_get_data(gc); - unsigned long offset; - unsigned long bank_mask; - int bank; - u8 reg_val[MAX_BANK]; + DECLARE_BITMAP(reg_val, MAX_LINE); int ret; mutex_lock(&chip->i2c_lock); @@ -468,11 +473,7 @@ static void pca953x_gpio_set_multiple(struct gpio_chip *gc, if (ret) goto exit; - for_each_set_clump8(offset, bank_mask, mask, gc->ngpio) { - bank = offset / 8; - reg_val[bank] &= ~bank_mask; - reg_val[bank] |= bitmap_get_value8(bits, offset) & bank_mask; - } + bitmap_and(reg_val, bits, mask, gc->ngpio); pca953x_write_regs(chip, chip->regs->output, reg_val); exit: @@ -599,10 +600,10 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d) { struct gpio_chip *gc = irq_data_get_irq_chip_data(d); struct pca953x_chip *chip = gpiochip_get_data(gc); - u8 new_irqs; - int level, i; - u8 invert_irq_mask[MAX_BANK]; - u8 reg_direction[MAX_BANK]; + DECLARE_BITMAP(irq_mask, MAX_LINE); + DECLARE_BITMAP(reg_direction, MAX_LINE); + DECLARE_BITMAP(new_irqs, MAX_LINE); + int level; pca953x_read_regs(chip, chip->regs->direction, reg_direction); @@ -610,25 +611,18 @@ static void pca953x_irq_bus_sync_unlock(struct irq_data *d) /* Enable latch on interrupt-enabled inputs */ pca953x_write_regs(chip, PCAL953X_IN_LATCH, chip->irq_mask); - for (i = 0; i < NBANK(chip); i++) - invert_irq_mask[i] = ~chip->irq_mask[i]; + bitmap_complement(irq_mask, chip->irq_mask, gc->ngpio); /* Unmask enabled interrupts */ - pca953x_write_regs(chip, PCAL953X_INT_MASK, invert_irq_mask); + pca953x_write_regs(chip, PCAL953X_INT_MASK, irq_mask); } + bitmap_or(new_irqs, chip->irq_trig_fall, chip->irq_trig_raise, gc->ngpio); + bitmap_and(irq_mask, new_irqs, reg_direction, gc->ngpio); + /* Look for any newly setup interrupt */ - for (i = 0; i < NBANK(chip); i++) { - new_irqs = chip->irq_trig_fall[i] | chip->irq_trig_raise[i]; - new_irqs &= reg_direction[i]; - - while (new_irqs) { - level = __ffs(new_irqs); - pca953x_gpio_direction_input(&chip->gpio_chip, - level + (BANK_SZ * i)); - new_irqs &= ~(1 << level); - } - } + for_each_set_bit(level, irq_mask, gc->ngpio) + pca953x_gpio_direction_input(&chip->gpio_chip, level); mutex_unlock(&chip->irq_lock); } @@ -669,15 +663,15 @@ static void pca953x_irq_shutdown(struct irq_data *d) chip->irq_trig_fall[d->hwirq / BANK_SZ] &= ~mask; } -static bool pca953x_irq_pending(struct pca953x_chip *chip, u8 *pending) +static bool pca953x_irq_pending(struct pca953x_chip *chip, unsigned long *pending) { - u8 cur_stat[MAX_BANK]; - u8 old_stat[MAX_BANK]; - bool pending_seen = false; - bool trigger_seen = false; - u8 trigger[MAX_BANK]; - u8 reg_direction[MAX_BANK]; - int ret, i; + struct gpio_chip *gc = &chip->gpio_chip; + DECLARE_BITMAP(reg_direction, MAX_LINE); + DECLARE_BITMAP(old_stat, MAX_LINE); + DECLARE_BITMAP(cur_stat, MAX_LINE); + DECLARE_BITMAP(new_stat, MAX_LINE); + DECLARE_BITMAP(trigger, MAX_LINE); + int ret; if (chip->driver_data & PCA_PCAL) { /* Read the current interrupt status from the device */ @@ -690,16 +684,13 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip, u8 *pending) if (ret) return false; - for (i = 0; i < NBANK(chip); i++) { - /* Apply filter for rising/falling edge selection */ - pending[i] = (~cur_stat[i] & chip->irq_trig_fall[i]) | - (cur_stat[i] & chip->irq_trig_raise[i]); - pending[i] &= trigger[i]; - if (pending[i]) - pending_seen = true; - } + /* Apply filter for rising/falling edge selection */ + bitmap_andnot(new_stat, chip->irq_trig_fall, cur_stat, gc->ngpio); + bitmap_and(old_stat, chip->irq_trig_raise, cur_stat, gc->ngpio); + bitmap_or(cur_stat, old_stat, new_stat, gc->ngpio); + bitmap_and(pending, cur_stat, trigger, gc->ngpio); - return pending_seen; + return !bitmap_empty(pending, gc->ngpio); } ret = pca953x_read_regs(chip, chip->regs->input, cur_stat); @@ -708,55 +699,40 @@ static bool pca953x_irq_pending(struct pca953x_chip *chip, u8 *pending) /* Remove output pins from the equation */ pca953x_read_regs(chip, chip->regs->direction, reg_direction); - for (i = 0; i < NBANK(chip); i++) - cur_stat[i] &= reg_direction[i]; - memcpy(old_stat, chip->irq_stat, NBANK(chip)); + bitmap_copy(old_stat, chip->irq_stat, gc->ngpio); - for (i = 0; i < NBANK(chip); i++) { - trigger[i] = (cur_stat[i] ^ old_stat[i]) & chip->irq_mask[i]; - if (trigger[i]) - trigger_seen = true; - } + bitmap_and(new_stat, cur_stat, reg_direction, gc->ngpio); + bitmap_xor(cur_stat, new_stat, old_stat, gc->ngpio); + bitmap_and(trigger, cur_stat, chip->irq_mask, gc->ngpio); - if (!trigger_seen) + if (bitmap_empty(trigger, gc->ngpio)) return false; - memcpy(chip->irq_stat, cur_stat, NBANK(chip)); + bitmap_copy(chip->irq_stat, new_stat, gc->ngpio); - for (i = 0; i < NBANK(chip); i++) { - pending[i] = (old_stat[i] & chip->irq_trig_fall[i]) | - (cur_stat[i] & chip->irq_trig_raise[i]); - pending[i] &= trigger[i]; - if (pending[i]) - pending_seen = true; - } + bitmap_and(cur_stat, chip->irq_trig_fall, old_stat, gc->ngpio); + bitmap_and(old_stat, chip->irq_trig_raise, new_stat, gc->ngpio); + bitmap_or(new_stat, old_stat, cur_stat, gc->ngpio); + bitmap_and(pending, new_stat, trigger, gc->ngpio); - return pending_seen; + return !bitmap_empty(pending, gc->ngpio); } static irqreturn_t pca953x_irq_handler(int irq, void *devid) { struct pca953x_chip *chip = devid; - u8 pending[MAX_BANK]; - u8 level; - unsigned nhandled = 0; - int i; + struct gpio_chip *gc = &chip->gpio_chip; + DECLARE_BITMAP(pending, MAX_LINE); + int level; if (!pca953x_irq_pending(chip, pending)) return IRQ_NONE; - for (i = 0; i < NBANK(chip); i++) { - while (pending[i]) { - level = __ffs(pending[i]); - handle_nested_irq(irq_find_mapping(chip->gpio_chip.irq.domain, - level + (BANK_SZ * i))); - pending[i] &= ~(1 << level); - nhandled++; - } - } + for_each_set_bit(level, pending, gc->ngpio) + handle_nested_irq(irq_find_mapping(gc->irq.domain, level)); - return (nhandled > 0) ? IRQ_HANDLED : IRQ_NONE; + return bitmap_empty(pending, gc->ngpio) ? IRQ_NONE : IRQ_HANDLED; } static int pca953x_irq_setup(struct pca953x_chip *chip, @@ -764,8 +740,9 @@ static int pca953x_irq_setup(struct pca953x_chip *chip, { struct i2c_client *client = chip->client; struct irq_chip *irq_chip = &chip->irq_chip; - u8 reg_direction[MAX_BANK]; - int ret, i; + DECLARE_BITMAP(reg_direction, MAX_LINE); + DECLARE_BITMAP(irq_stat, MAX_LINE); + int ret; if (!client->irq) return 0; @@ -776,7 +753,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip, if (!(chip->driver_data & PCA_INT)) return 0; - ret = pca953x_read_regs(chip, chip->regs->input, chip->irq_stat); + ret = pca953x_read_regs(chip, chip->regs->input, irq_stat); if (ret) return ret; @@ -786,8 +763,7 @@ static int pca953x_irq_setup(struct pca953x_chip *chip, * this purpose. */ pca953x_read_regs(chip, chip->regs->direction, reg_direction); - for (i = 0; i < NBANK(chip); i++) - chip->irq_stat[i] &= reg_direction[i]; + bitmap_and(chip->irq_stat, irq_stat, reg_direction, chip->gpio_chip.ngpio); mutex_init(&chip->irq_lock); ret = devm_request_threaded_irq(&client->dev, client->irq, @@ -839,8 +815,8 @@ static int pca953x_irq_setup(struct pca953x_chip *chip, static int device_pca95xx_init(struct pca953x_chip *chip, u32 invert) { + DECLARE_BITMAP(val, MAX_LINE); int ret; - u8 val[MAX_BANK]; ret = regcache_sync_region(chip->regmap, chip->regs->output, chip->regs->output + NBANK(chip)); @@ -854,9 +830,9 @@ static int device_pca95xx_init(struct pca953x_chip *chip, u32 invert) /* set platform specific polarity inversion */ if (invert) - memset(val, 0xFF, NBANK(chip)); + bitmap_fill(val, MAX_LINE); else - memset(val, 0, NBANK(chip)); + bitmap_zero(val, MAX_LINE); ret = pca953x_write_regs(chip, chip->regs->invert, val); out: @@ -865,8 +841,8 @@ static int device_pca95xx_init(struct pca953x_chip *chip, u32 invert) static int device_pca957x_init(struct pca953x_chip *chip, u32 invert) { + DECLARE_BITMAP(val, MAX_LINE); int ret; - u8 val[MAX_BANK]; ret = device_pca95xx_init(chip, invert); if (ret)
Instead of customized approach convert the driver to use bitmap API. Depends-on: 6e9c6674d1bf ("gpio: pca953x: utilize the for_each_set_clump8 macro") Cc: William Breathitt Gray <vilhelm.gray@gmail.com> Cc: Geert Uytterhoeven <geert+renesas@glider.be> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> Cc: Marek Vasut <marek.vasut+renesas@gmail.com> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/gpio/gpio-pca953x.c | 170 ++++++++++++++++-------------------- 1 file changed, 73 insertions(+), 97 deletions(-)