| Message ID | 645f16f1d1ad3f215e34a82f457e06725fcf8f56.1553661964.git.vilhelm.gray@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Introduce the for_each_set_clump8 macro | expand |
On Wed, Mar 27, 2019 at 02:02:39PM +0900, William Breathitt Gray wrote: > Replace verbose implementation in set_multiple callback with > for_each_set_clump8 macro to simplify code and improve clarity. > + for_each_set_clump8(offset, bankmask, mask, chip->registers * 8) { > + bank = (chip->registers - 1) - (offset / 8); Excessive parens, but it's minor. > + bitmask = bitmap_get_value8(bits, offset) & bankmask; > > chip->buffer[bank] &= ~bankmask; > + chip->buffer[bank] |= bitmask; > }
On Wed, Mar 27, 2019 at 02:33:14PM +0200, Andy Shevchenko wrote: > On Wed, Mar 27, 2019 at 02:02:39PM +0900, William Breathitt Gray wrote: > > Replace verbose implementation in set_multiple callback with > > for_each_set_clump8 macro to simplify code and improve clarity. > > > + for_each_set_clump8(offset, bankmask, mask, chip->registers * 8) { > > > + bank = (chip->registers - 1) - (offset / 8); > > Excessive parens, but it's minor. Fair point, this could do without parens around (chip->registers - 1). Since this and renaming 'idx' to 'index' in find_bit.c are such a minor changes, I'll wait first to see if something else comes up to be fixed in this review. If so, I'll add these changes to the next version of this patchset. William Breathitt Gray > > > + bitmask = bitmap_get_value8(bits, offset) & bankmask; > > > > chip->buffer[bank] &= ~bankmask; > > + chip->buffer[bank] |= bitmask; > > } > > -- > With Best Regards, > Andy Shevchenko > >
On Thu, Mar 28, 2019 at 01:40:04PM +0900, William Breathitt Gray wrote: > On Wed, Mar 27, 2019 at 02:33:14PM +0200, Andy Shevchenko wrote: > > On Wed, Mar 27, 2019 at 02:02:39PM +0900, William Breathitt Gray wrote: > > > Replace verbose implementation in set_multiple callback with > > > for_each_set_clump8 macro to simplify code and improve clarity. > > > > > + for_each_set_clump8(offset, bankmask, mask, chip->registers * 8) { > > > > > + bank = (chip->registers - 1) - (offset / 8); > > > > Excessive parens, but it's minor. > > Fair point, this could do without parens around (chip->registers - 1). And around division too. > Since this and renaming 'idx' to 'index' in find_bit.c are such a minor > changes, I'll wait first to see if something else comes up to be fixed > in this review. If so, I'll add these changes to the next version of > this patchset. It seems inline proposal would going to be implemented.
diff --git a/drivers/gpio/gpio-74x164.c b/drivers/gpio/gpio-74x164.c index fb7b620763a2..0bbf1162bb31 100644 --- a/drivers/gpio/gpio-74x164.c +++ b/drivers/gpio/gpio-74x164.c @@ -9,6 +9,7 @@ * published by the Free Software Foundation. */ +#include <linux/bitops.h> #include <linux/init.h> #include <linux/mutex.h> #include <linux/spi/spi.h> @@ -75,20 +76,18 @@ static void gen_74x164_set_multiple(struct gpio_chip *gc, unsigned long *mask, unsigned long *bits) { struct gen_74x164_chip *chip = gpiochip_get_data(gc); - unsigned int i, idx, shift; - u8 bank, bankmask; + unsigned long offset; + unsigned long bankmask; + size_t bank; + unsigned long bitmask; mutex_lock(&chip->lock); - for (i = 0, bank = chip->registers - 1; i < chip->registers; - i++, bank--) { - idx = i / sizeof(*mask); - shift = i % sizeof(*mask) * BITS_PER_BYTE; - bankmask = mask[idx] >> shift; - if (!bankmask) - continue; + for_each_set_clump8(offset, bankmask, mask, chip->registers * 8) { + bank = (chip->registers - 1) - (offset / 8); + bitmask = bitmap_get_value8(bits, offset) & bankmask; chip->buffer[bank] &= ~bankmask; - chip->buffer[bank] |= bankmask & (bits[idx] >> shift); + chip->buffer[bank] |= bitmask; } __gen_74x164_write_config(chip); mutex_unlock(&chip->lock);
Replace verbose implementation in set_multiple callback with for_each_set_clump8 macro to simplify code and improve clarity. Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Cc: Geert Uytterhoeven <geert+renesas@glider.be> Cc: Phil Reid <preid@electromag.com.au> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> --- drivers/gpio/gpio-74x164.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)