Introduce the for_each_set_port_word macro
Subsystems (11)
cover Introduce the for_each_set_port_word macro
While adding GPIO get_multiple/set_multiple callback support for various drivers, I noticed a pattern of looping manifesting that would be useful standardized as a macro.
This patchset introduces the for_each_set_port_word macro and utilizes it in several GPIO drivers. The for_each_set_port_word macro facilitates a for-loop syntax that iterates over entire groups of set bits at a time.
For example, suppose you would like to iterate over a 16-bit integer 4 bits at a time, skipping over 4-bit groups with no set bit, where XXXX represents the current 4-bit group:
Example: 1011 1110 0000 1111
First loop: 1011 1110 0000 XXXX
Second loop: 1011 XXXX 0000 1111
Third loop: XXXX 1110 0000 1111Each iteration of the loop returns the next 4-bit group that has at least one set bit.
The for_each_set_port_word macro has six parameters:
* port_word: set to current port word index for the iteration
* word_index: set to current bitmap word index for the iteration
* word_offset: bits offset of the found port word in the bitmap word
* bits: bitmap to search within
* size: bitmap size in number of port words
* port_size: port word size in number of bits
The port_size argument can be an arbitrary number of bits and is not required to be a multiple of 2.
I've called the group of bits a "port word" which may be a confusing naming convention; I was afraid calling that them a "group" may be too vague. Should a different name be chosen; what would you suggest?
This patchset was rebased on top of the following three commits:
* aaf96e51de11 ("gpio: pci-idio-16: Fix port memory offset for get_multiple callback")
* 304440aa96c6 ("gpio: pcie-idio-24: Fix port memory offset for get_multiple/set_multiple callbacks")
* e026646c178d ("gpio: pcie-idio-24: Fix off-by-one error in get_multiple loop")
William Breathitt Gray
William Breathitt Gray (7):
bitops: Introduce the for_each_set_port_word macro
gpio: 104-dio-48e: Utilize for_each_set_port_word macro
gpio: 104-idi-48: Utilize for_each_set_port_word macro
gpio: gpio-mm: Utilize for_each_set_port_word macro
gpio: ws16c48: Utilize for_each_set_port_word macro
gpio: pci-idio-16: Utilize for_each_set_port_word macro
gpio: pcie-idio-24: Utilize for_each_set_port_word macro
drivers/gpio/gpio-104-dio-48e.c | 67 +++++--------------- drivers/gpio/gpio-104-idi-48.c | 32 ++-------- drivers/gpio/gpio-gpio-mm.c | 67 +++++--------------- drivers/gpio/gpio-pci-idio-16.c | 67 ++++++-------------- drivers/gpio/gpio-pcie-idio-24.c | 102 +++++++++++------------------- drivers/gpio/gpio-ws16c48.c | 66 +++++-------------- include/asm-generic/bitops/find.h | 24 +++++++ include/linux/bitops.h | 9 +++ lib/find_bit.c | 28 ++++++++ 9 files changed, 170 insertions(+), 292 deletions(-)
Signature · 1 line
2.17.0
PATCH 1/7 bitops: Introduce the for_each_set_port_word macro
This macro iterates for each group of bits (port word) with set bits, within a bitmap memory region. For each iteration, "port_word" is set to the found port word index, "word_index" is set to the word index of the bitmap containing the found port word, and "word_offset" is set to the bit offset of the found port word within the respective bitmap word.
--- include/asm-generic/bitops/find.h | 24 ++++++++++++++++++++++++ include/linux/bitops.h | 9 +++++++++ lib/find_bit.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+)
include/asm-generic/bitops/find.h +24 -0
@@ -80,4 +80,28 @@ extern unsigned long find_first_zero_bit(const unsigned long *addr,#endif /* CONFIG_GENERIC_FIND_FIRST_BIT */ +/** +* find_next_port_word - find next port word with set bits in a memory region +* @word_index: location to store bitmap word index of found port word +* @word_offset: bits offset of the found port word in respective bitmap word +* @bits: address to base the search on +* @size: bitmap size in number of ports +* @offset: port word index to start searching at +* @port_size: port word size in bits +* +* Returns the port index for the next port word with set bits; the respective +* bitmap word index is stored at the location pointed by @word_index, and the +* bits offset of the found port word within the respective bitmap word is +* stored at the location pointed by @word_offset. If no bits are set, returns +* @size. +*/ +size_t find_next_port_word (size_t *const word_index, +unsigned int *const word_offset, +const unsigned long *const bits,const size_t size, +const size_t offset,const unsigned int port_size); + +#define find_first_port_word (word_index, word_offset, bits, size, port_size) \ + find_next_port_word((word_index), (word_offset), (bits), (size), 0, \ + (port_size)) +#endif /*_ASM_GENERIC_BITOPS_FIND_H_ */
include/linux/bitops.h +9 -0
@@ -59,6 +59,15 @@ extern unsigned long __sw_hweight64(__u64 w); (bit) < (size); \ (bit) = find_next_zero_bit((addr), (size), (bit) + 1)) +#define for_each_set_port_word (port_word, word_index, word_offset, bits, size, \ + port_size) \ + for ((port_word) = find_first_port_word(&(word_index), &(word_offset), \ + (bits), (size), (port_size)); \ + (port_word) < (size); \ + (port_word) = find_next_port_word(&(word_index), &(word_offset), \ + (bits), (size), (port_word) + 1, \ + (port_size))) +static inline int get_bitmask_order (unsigned int count) {int order;
lib/find_bit.c +28 -0
@@ -20,6 +20,7 @@#include <linux/bitmap.h> #include <linux/export.h> #include <linux/kernel.h> +#include <linux/types.h> #if !defined(find_next_bit) || !defined(find_next_zero_bit) || \ !defined(find_next_and_bit)
@@ -218,3 +219,30 @@ EXPORT_SYMBOL(find_next_bit_le);#endif #endif /* __BIG_ENDIAN */ + +size_t find_next_port_word (size_t *const word_index, +unsigned int *const word_offset, +const unsigned long *const bits,const size_t size, +const size_t offset,const unsigned int port_size) +{ +size_t i; +unsigned int bits_offset; +unsigned long word_mask; +const unsigned long port_mask =GENMASK (port_size -1 ,0 ); + +for (i = offset; i < size; i++) { + bits_offset = i * port_size; + + *word_index =BIT_WORD (bits_offset); + *word_offset = bits_offset %BITS_PER_LONG ; + + word_mask = bits[*word_index] & (port_mask << *word_offset); +if (!word_mask) +continue ; + +return i; + } + +return size; +} +EXPORT_SYMBOL (find_next_port_word);
Signature · 1 line
2.17.0
kbuild test robot
build botRe: [PATCH 1/7] bitops: Introduce the for_each_set_port_word macro
I love your patch! Yet something to improve:
Hi William,
I love your patch! Yet something to improve:
[auto build test ERROR on gpio/for-next]
[also build test ERROR on v4.17-rc4 next-20180507]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/William-Breathitt-Gray/Introduce-the-for_each_set_port_word-macro/20180508-034417 base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next config: openrisc-or1ksim_defconfig (attached as .config) compiler: or1k-linux-gcc (GCC) 6.0.0 20160327 (experimental)
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=openrisc
All errors (new ones prefixed by >>):
In file included from arch/openrisc/include/asm/bitops.h:37:0,
from include/linux/bitops.h:38, from include/linux/radix-tree.h:24, from kernel/memremap.c:13:
>> include/asm-generic/bitops/find.h:98:1: error: unknown type name 'size_t'
size_t find_next_port_word(size_t *const word_index,
^~~~~~
include/asm-generic/bitops/find.h:98:28: error: unknown type name 'size_t'
size_t find_next_port_word(size_t *const word_index,
^~~~~~
include/asm-generic/bitops/find.h:100:46: error: unknown type name 'size_t'
const unsigned long *const bits, const size_t size,
^~~~~~
include/asm-generic/bitops/find.h:101:13: error: unknown type name 'size_t'
const size_t offset, const unsigned int port_size);
^~~~~~vim +/size_t +98 include/asm-generic/bitops/find.h
82
83 /**
84 * find_next_port_word - find next port word with set bits in a memory region
85 * @word_index: location to store bitmap word index of found port word
86 * @word_offset: bits offset of the found port word in respective bitmap word
87 * @bits: address to base the search on
88 * @size: bitmap size in number of ports
89 * @offset: port word index to start searching at
90 * @port_size: port word size in bits
91 *
92 * Returns the port index for the next port word with set bits; the respective
93 * bitmap word index is stored at the location pointed by @word_index, and the
94 * bits offset of the found port word within the respective bitmap word is
95 * stored at the location pointed by @word_offset. If no bits are set, returns
96 * @size.
97 */
> 98 size_t find_next_port_word(size_t *const word_index,
99 unsigned int *const word_offset,
100 const unsigned long *const bits, const size_t size,
101 const size_t offset, const unsigned int port_size);102
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
PATCH 2/7 gpio: 104-dio-48e: Utilize for_each_set_port_word macro
In reply to William Breathitt GrayReplace boilerplate code in get_multiple/set_multiple callbacks with for_each_set_port_word macro to simplify code and improve clarity.
--- drivers/gpio/gpio-104-dio-48e.c | 67 ++++++++------------------------- 1 file changed, 16 insertions(+), 51 deletions(-)
drivers/gpio/gpio-104-dio-48e.c +16 -51
@@ -183,46 +183,23 @@ static int dio48e_gpio_get(struct gpio_chip *chip, unsigned offset)return !!(port_state & mask); } +static const size_t ports[] = {0 ,1 ,2 ,4 ,5 ,6 }; +static int dio48e_gpio_get_multiple (struct gpio_chip *chip,unsigned long *mask,unsigned long *bits) {struct dio48e_gpio *const dio48egpio =gpiochip_get_data (chip);size_t i; -static const size_t ports[] = {0 ,1 ,2 ,4 ,5 ,6 }; -const unsigned int gpio_reg_size =8 ; -unsigned int bits_offset; -size_t word_index; -unsigned int word_offset; -unsigned long word_mask; -const unsigned long port_mask =GENMASK (gpio_reg_size -1 ,0 ); +size_t word; +unsigned int offset;unsigned long port_state;/* clear bits array to a clean slate */ bitmap_zero (bits, chip->ngpio); -/* get bits are evaluated a gpio port register at a time */ -for (i =0 ; i <ARRAY_SIZE (ports); i++) { -/* gpio offset in bits array */ - bits_offset = i * gpio_reg_size; - -/* word index for bits array */ - word_index =BIT_WORD (bits_offset); - -/* gpio offset within current word of bits array */ - word_offset = bits_offset %BITS_PER_LONG ; - -/* mask of get bits for current gpio within current word */ - word_mask = mask[word_index] & (port_mask << word_offset); -if (!word_mask) { -/* no get bits in this port so skip to next one */ -continue ; - } - -/* read bits from current gpio port */ +for_each_set_port_word (i, word, offset, mask,ARRAY_SIZE (ports),8 ) { port_state =inb (dio48egpio->base + ports[i]); - -/* store acquired bits at respective bits array offset */ - bits[word_index] |= port_state << word_offset; + bits[word] |= port_state << offset; }return 0 ;
@@ -252,37 +229,25 @@ static void dio48e_gpio_set_multiple(struct gpio_chip *chip, unsigned long *mask, unsigned long *bits) { struct dio48e_gpio *const dio48egpio = gpiochip_get_data(chip); -unsigned int i; -const unsigned int gpio_reg_size =8 ; -unsigned int port; -unsigned int out_port; +size_t i; +size_t word; +unsigned int offset; +unsigned int iomask;unsigned int bitmask;unsigned long flags; -/* set bits are evaluated a gpio register size at a time */ -for (i =0 ; i < chip->ngpio; i += gpio_reg_size) { -/* no more set bits in this mask word; skip to the next word */ -if (!mask[BIT_WORD (i)]) { - i = (BIT_WORD (i) +1 ) *BITS_PER_LONG - gpio_reg_size; -continue ; - } - - port = i / gpio_reg_size; - out_port = (port >2 ) ? port +1 : port; - bitmask = mask[BIT_WORD (i)] & bits[BIT_WORD (i)]; +for_each_set_port_word (i, word, offset, mask,ARRAY_SIZE (ports),8 ) { + iomask = mask[word] >> offset; + bitmask = iomask & (bits[word] >> offset);raw_spin_lock_irqsave (&dio48egpio->lock, flags);/* update output state data and set device gpio register */ - dio48egpio->out_state[port] &= ~mask[BIT_WORD (i)]; - dio48egpio->out_state[port] |= bitmask; -outb (dio48egpio->out_state[port], dio48egpio->base + out_port); + dio48egpio->out_state[i] &= ~iomask; + dio48egpio->out_state[i] |= bitmask; +outb (dio48egpio->out_state[i], dio48egpio->base + ports[i]);raw_spin_unlock_irqrestore (&dio48egpio->lock, flags); - -/* prepare for next gpio register set */ - mask[BIT_WORD (i)] >>= gpio_reg_size; - bits[BIT_WORD (i)] >>= gpio_reg_size; } }
Signature · 1 line
2.17.0
PATCH 3/7 gpio: 104-idi-48: Utilize for_each_set_port_word macro
In reply to William Breathitt GrayReplace boilerplate code in the get_multiple callback with the for_each_set_port_word macro in order to simplify and improve clarity.
--- drivers/gpio/gpio-104-idi-48.c | 32 ++++---------------------------- 1 file changed, 4 insertions(+), 28 deletions(-)
drivers/gpio/gpio-104-idi-48.c +4 -28
@@ -94,41 +94,17 @@ static int idi_48_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask, {struct idi_48_gpio *const idi48gpio =gpiochip_get_data (chip);size_t i; +size_t word; +unsigned int offset;static const size_t ports[] = {0 ,1 ,2 ,4 ,5 ,6 }; -const unsigned int gpio_reg_size =8 ; -unsigned int bits_offset; -size_t word_index; -unsigned int word_offset; -unsigned long word_mask; -const unsigned long port_mask =GENMASK (gpio_reg_size -1 ,0 );unsigned long port_state;/* clear bits array to a clean slate */ bitmap_zero (bits, chip->ngpio); -/* get bits are evaluated a gpio port register at a time */ -for (i =0 ; i <ARRAY_SIZE (ports); i++) { -/* gpio offset in bits array */ - bits_offset = i * gpio_reg_size; - -/* word index for bits array */ - word_index =BIT_WORD (bits_offset); - -/* gpio offset within current word of bits array */ - word_offset = bits_offset %BITS_PER_LONG ; - -/* mask of get bits for current gpio within current word */ - word_mask = mask[word_index] & (port_mask << word_offset); -if (!word_mask) { -/* no get bits in this port so skip to next one */ -continue ; - } - -/* read bits from current gpio port */ +for_each_set_port_word (i, word, offset, mask,ARRAY_SIZE (ports),8 ) { port_state =inb (idi48gpio->base + ports[i]); - -/* store acquired bits at respective bits array offset */ - bits[word_index] |= port_state << word_offset; + bits[word] |= port_state << offset; }return 0 ;
Signature · 1 line
2.17.0
PATCH 4/7 gpio: gpio-mm: Utilize for_each_set_port_word macro
In reply to William Breathitt GrayReplace boilerplate code in get_multiple/set_multiple callbacks with for_each_set_port_word macro to simplify code and improve clarity.
--- drivers/gpio/gpio-gpio-mm.c | 67 +++++++++---------------------------- 1 file changed, 16 insertions(+), 51 deletions(-)
drivers/gpio/gpio-gpio-mm.c +16 -51
@@ -172,46 +172,23 @@ static int gpiomm_gpio_get(struct gpio_chip *chip, unsigned int offset)return !!(port_state & mask); } +static const size_t ports[] = {0 ,1 ,2 ,4 ,5 ,6 }; +static int gpiomm_gpio_get_multiple (struct gpio_chip *chip,unsigned long *mask,unsigned long *bits) {struct gpiomm_gpio *const gpiommgpio =gpiochip_get_data (chip);size_t i; -static const size_t ports[] = {0 ,1 ,2 ,4 ,5 ,6 }; -const unsigned int gpio_reg_size =8 ; -unsigned int bits_offset; -size_t word_index; -unsigned int word_offset; -unsigned long word_mask; -const unsigned long port_mask =GENMASK (gpio_reg_size -1 ,0 ); +size_t word; +unsigned int offset;unsigned long port_state;/* clear bits array to a clean slate */ bitmap_zero (bits, chip->ngpio); -/* get bits are evaluated a gpio port register at a time */ -for (i =0 ; i <ARRAY_SIZE (ports); i++) { -/* gpio offset in bits array */ - bits_offset = i * gpio_reg_size; - -/* word index for bits array */ - word_index =BIT_WORD (bits_offset); - -/* gpio offset within current word of bits array */ - word_offset = bits_offset %BITS_PER_LONG ; - -/* mask of get bits for current gpio within current word */ - word_mask = mask[word_index] & (port_mask << word_offset); -if (!word_mask) { -/* no get bits in this port so skip to next one */ -continue ; - } - -/* read bits from current gpio port */ +for_each_set_port_word (i, word, offset, mask,ARRAY_SIZE (ports),8 ) { port_state =inb (gpiommgpio->base + ports[i]); - -/* store acquired bits at respective bits array offset */ - bits[word_index] |= port_state << word_offset; + bits[word] |= port_state << offset; }return 0 ;
@@ -242,37 +219,25 @@ static void gpiomm_gpio_set_multiple(struct gpio_chip *chip, unsigned long *mask, unsigned long *bits) { struct gpiomm_gpio *const gpiommgpio = gpiochip_get_data(chip); -unsigned int i; -const unsigned int gpio_reg_size =8 ; -unsigned int port; -unsigned int out_port; +size_t i; +size_t word; +unsigned int offset; +unsigned int iomask;unsigned int bitmask;unsigned long flags; -/* set bits are evaluated a gpio register size at a time */ -for (i =0 ; i < chip->ngpio; i += gpio_reg_size) { -/* no more set bits in this mask word; skip to the next word */ -if (!mask[BIT_WORD (i)]) { - i = (BIT_WORD (i) +1 ) *BITS_PER_LONG - gpio_reg_size; -continue ; - } - - port = i / gpio_reg_size; - out_port = (port >2 ) ? port +1 : port; - bitmask = mask[BIT_WORD (i)] & bits[BIT_WORD (i)]; +for_each_set_port_word (i, word, offset, mask,ARRAY_SIZE (ports),8 ) { + iomask = mask[word] >> offset; + bitmask = iomask & (bits[word] >> offset);spin_lock_irqsave (&gpiommgpio->lock, flags);/* update output state data and set device gpio register */ - gpiommgpio->out_state[port] &= ~mask[BIT_WORD (i)]; - gpiommgpio->out_state[port] |= bitmask; -outb (gpiommgpio->out_state[port], gpiommgpio->base + out_port); + gpiommgpio->out_state[i] &= ~iomask; + gpiommgpio->out_state[i] |= bitmask; +outb (gpiommgpio->out_state[i], gpiommgpio->base + ports[i]);spin_unlock_irqrestore (&gpiommgpio->lock, flags); - -/* prepare for next gpio register set */ - mask[BIT_WORD (i)] >>= gpio_reg_size; - bits[BIT_WORD (i)] >>= gpio_reg_size; } }
Signature · 1 line
2.17.0
PATCH 5/7 gpio: ws16c48: Utilize for_each_set_port_word macro
In reply to William Breathitt GrayReplace boilerplate code in get_multiple/set_multiple callbacks with for_each_set_port_word macro to simplify code and improve clarity.
--- drivers/gpio/gpio-ws16c48.c | 66 +++++++++---------------------------- 1 file changed, 16 insertions(+), 50 deletions(-)
drivers/gpio/gpio-ws16c48.c +16 -50
@@ -134,42 +134,19 @@ static int ws16c48_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask, unsigned long *bits) { struct ws16c48_gpio *const ws16c48gpio = gpiochip_get_data(chip); -const unsigned int gpio_reg_size =8 ; -size_t i; -const size_t num_ports = chip->ngpio / gpio_reg_size; -unsigned int bits_offset; -size_t word_index; -unsigned int word_offset; -unsigned long word_mask; -const unsigned long port_mask =GENMASK (gpio_reg_size -1 ,0 ); +size_t port; +size_t word; +unsigned int offset; +const unsigned int port_size =8 ; +const size_t num_ports = chip->ngpio / port_size;unsigned long port_state;/* clear bits array to a clean slate */ bitmap_zero (bits, chip->ngpio); -/* get bits are evaluated a gpio port register at a time */ -for (i =0 ; i < num_ports; i++) { -/* gpio offset in bits array */ - bits_offset = i * gpio_reg_size; - -/* word index for bits array */ - word_index =BIT_WORD (bits_offset); - -/* gpio offset within current word of bits array */ - word_offset = bits_offset %BITS_PER_LONG ; - -/* mask of get bits for current gpio within current word */ - word_mask = mask[word_index] & (port_mask << word_offset); -if (!word_mask) { -/* no get bits in this port so skip to next one */ -continue ; - } - -/* read bits from current gpio port */ - port_state =inb (ws16c48gpio->base + i); - -/* store acquired bits at respective bits array offset */ - bits[word_index] |= port_state << word_offset; +for_each_set_port_word (port, word, offset, mask, num_ports, port_size) { + port_state =inb (ws16c48gpio->base + port); + bits[word] |= port_state << offset; }return 0 ;
@@ -203,26 +180,19 @@ static void ws16c48_gpio_set_multiple(struct gpio_chip *chip, unsigned long *mask, unsigned long *bits) { struct ws16c48_gpio *const ws16c48gpio = gpiochip_get_data(chip); -unsigned int i; -const unsigned int gpio_reg_size =8 ; -unsigned int port; +size_t port; +size_t word; +unsigned int offset; +const unsigned int port_size =8 ; +const size_t num_ports = chip->ngpio / port_size;unsigned int iomask;unsigned int bitmask;unsigned long flags; -/* set bits are evaluated a gpio register size at a time */ -for (i =0 ; i < chip->ngpio; i += gpio_reg_size) { -/* no more set bits in this mask word; skip to the next word */ -if (!mask[BIT_WORD (i)]) { - i = (BIT_WORD (i) +1 ) *BITS_PER_LONG - gpio_reg_size; -continue ; - } - - port = i / gpio_reg_size; - +for_each_set_port_word (port, word, offset, mask, num_ports, port_size) {/* mask out GPIO configured for input */ - iomask = mask[BIT_WORD (i)] & ~ws16c48gpio->io_state[port]; - bitmask = iomask & bits[BIT_WORD (i)]; + iomask = (mask[word] >> offset) & ~ws16c48gpio->io_state[port]; + bitmask = iomask & (bits[word] >> offset);raw_spin_lock_irqsave (&ws16c48gpio->lock, flags);
@@ -232,10 +202,6 @@ static void ws16c48_gpio_set_multiple(struct gpio_chip *chip,outb (ws16c48gpio->out_state[port], ws16c48gpio->base + port);raw_spin_unlock_irqrestore (&ws16c48gpio->lock, flags); - -/* prepare for next gpio register set */ - mask[BIT_WORD (i)] >>= gpio_reg_size; - bits[BIT_WORD (i)] >>= gpio_reg_size; } }
Signature · 1 line
2.17.0
PATCH 6/7 gpio: pci-idio-16: Utilize for_each_set_port_word macro
In reply to William Breathitt GrayReplace boilerplate code in get_multiple/set_multiple callbacks with for_each_set_port_word macro to simplify code and improve clarity.
--- drivers/gpio/gpio-pci-idio-16.c | 67 +++++++++++---------------------- 1 file changed, 21 insertions(+), 46 deletions(-)
drivers/gpio/gpio-pci-idio-16.c +21 -46
@@ -109,44 +109,20 @@ static int idio_16_gpio_get_multiple(struct gpio_chip *chip, {struct idio_16_gpio *const idio16gpio =gpiochip_get_data (chip);size_t i; -const unsigned int gpio_reg_size =8 ; -unsigned int bits_offset; -size_t word_index; -unsigned int word_offset; -unsigned long word_mask; -const unsigned long port_mask =GENMASK (gpio_reg_size -1 ,0 ); -unsigned long port_state; +size_t word; +unsigned int offset;void __iomem *ports[] = { &idio16gpio->reg->out0_7, &idio16gpio->reg->out8_15, &idio16gpio->reg->in0_7, &idio16gpio->reg->in8_15, }; +unsigned long port_state;/* clear bits array to a clean slate */ bitmap_zero (bits, chip->ngpio); -/* get bits are evaluated a gpio port register at a time */ -for (i =0 ; i <ARRAY_SIZE (ports); i++) { -/* gpio offset in bits array */ - bits_offset = i * gpio_reg_size; - -/* word index for bits array */ - word_index =BIT_WORD (bits_offset); - -/* gpio offset within current word of bits array */ - word_offset = bits_offset %BITS_PER_LONG ; - -/* mask of get bits for current gpio within current word */ - word_mask = mask[word_index] & (port_mask << word_offset); -if (!word_mask) { -/* no get bits in this port so skip to next one */ -continue ; - } - -/* read bits from current gpio port */ +for_each_set_port_word (i, word, offset, mask,ARRAY_SIZE (ports),8 ) { port_state =ioread8 (ports[i]); - -/* store acquired bits at respective bits array offset */ - bits[word_index] |= port_state << word_offset; + bits[word] |= port_state << offset; }return 0 ;
@@ -186,30 +162,29 @@ static void idio_16_gpio_set_multiple(struct gpio_chip *chip, unsigned long *mask, unsigned long *bits) { struct idio_16_gpio *const idio16gpio = gpiochip_get_data(chip); +size_t i; +size_t word; +unsigned int offset; +void __iomem *ports[] = { + &idio16gpio->reg->out0_7, &idio16gpio->reg->out8_15, + }; +unsigned int iomask; +unsigned int bitmask;unsigned long flags;unsigned int out_state; -raw_spin_lock_irqsave (&idio16gpio->lock, flags); +for_each_set_port_word (i, word, offset, mask,ARRAY_SIZE (ports),8 ) { + iomask = mask[word] >> offset; + bitmask = iomask & (bits[word] >> offset); -/* process output lines 0-7 */ -if (*mask &0xFF ) { - out_state =ioread8 (&idio16gpio->reg->out0_7) & ~*mask; - out_state |= *mask & *bits; -iowrite8 (out_state, &idio16gpio->reg->out0_7); - } +raw_spin_lock_irqsave (&idio16gpio->lock, flags); -/* shift to next output line word */ - *mask >>=8 ; + out_state =ioread8 (ports[i]) & ~iomask; + out_state |= bitmask; +iowrite8 (out_state, ports[i]); -/* process output lines 8-15 */ -if (*mask &0xFF ) { - *bits >>=8 ; - out_state =ioread8 (&idio16gpio->reg->out8_15) & ~*mask; - out_state |= *mask & *bits; -iowrite8 (out_state, &idio16gpio->reg->out8_15); +raw_spin_unlock_irqrestore (&idio16gpio->lock, flags); } - -raw_spin_unlock_irqrestore (&idio16gpio->lock, flags); }static void idio_16_irq_ack (struct irq_data *data)
Signature · 1 line
2.17.0
PATCH 7/7 gpio: pcie-idio-24: Utilize for_each_set_port_word macro
In reply to William Breathitt GrayReplace boilerplate code in get_multiple/set_multiple callbacks with for_each_set_port_word macro to simplify code and improve clarity.
--- drivers/gpio/gpio-pcie-idio-24.c | 102 +++++++++++-------------------- 1 file changed, 36 insertions(+), 66 deletions(-)
drivers/gpio/gpio-pcie-idio-24.c +36 -66
@@ -199,41 +199,21 @@ static int idio_24_gpio_get_multiple(struct gpio_chip *chip, {struct idio_24_gpio *const idio24gpio =gpiochip_get_data (chip);size_t i; -const unsigned int gpio_reg_size =8 ; -unsigned int bits_offset; -size_t word_index; -unsigned int word_offset; -unsigned long word_mask; -const unsigned long port_mask =GENMASK (gpio_reg_size -1 ,0 ); -unsigned long port_state; +size_t word; +unsigned int offset;void __iomem *ports[] = { &idio24gpio->reg->out0_7, &idio24gpio->reg->out8_15, &idio24gpio->reg->out16_23, &idio24gpio->reg->in0_7, &idio24gpio->reg->in8_15, &idio24gpio->reg->in16_23, }; +const size_t num_ports =ARRAY_SIZE (ports) +1 ; +unsigned long port_state;const unsigned long out_mode_mask =BIT (1 );/* clear bits array to a clean slate */ bitmap_zero (bits, chip->ngpio); -/* get bits are evaluated a gpio port register at a time */ -for (i =0 ; i <ARRAY_SIZE (ports) +1 ; i++) { -/* gpio offset in bits array */ - bits_offset = i * gpio_reg_size; - -/* word index for bits array */ - word_index =BIT_WORD (bits_offset); - -/* gpio offset within current word of bits array */ - word_offset = bits_offset %BITS_PER_LONG ; - -/* mask of get bits for current gpio within current word */ - word_mask = mask[word_index] & (port_mask << word_offset); -if (!word_mask) { -/* no get bits in this port so skip to next one */ -continue ; - } - +for_each_set_port_word (i, word, offset, mask, num_ports,8 ) {/* read bits from current gpio port (port 6 is TTL GPIO) */ if (i <6 ) port_state =ioread8 (ports[i]);
@@ -243,7 +223,7 @@ static int idio_24_gpio_get_multiple(struct gpio_chip *chip, port_state =ioread8 (&idio24gpio->reg->ttl_in0_7);/* store acquired bits at respective bits array offset */ - bits[word_index] |= port_state << word_offset; + bits[word] |= port_state << offset; }return 0 ;
@@ -295,58 +275,48 @@ static void idio_24_gpio_set_multiple(struct gpio_chip *chip, {struct idio_24_gpio *const idio24gpio =gpiochip_get_data (chip);size_t i; -unsigned long bits_offset; -unsigned long gpio_mask; -const unsigned int gpio_reg_size =8 ; -const unsigned long port_mask =GENMASK (gpio_reg_size,0 ); -unsigned long flags; -unsigned int out_state; +size_t word; +unsigned int offset;void __iomem *ports[] = { &idio24gpio->reg->out0_7, &idio24gpio->reg->out8_15, &idio24gpio->reg->out16_23 }; +const size_t num_ports =ARRAY_SIZE (ports) +1 ; +unsigned int iomask; +unsigned int bitmask; +unsigned long flags;const unsigned long out_mode_mask =BIT (1 ); -const unsigned int ttl_offset =48 ; -const size_t ttl_i =BIT_WORD (ttl_offset); -const unsigned int word_offset = ttl_offset %BITS_PER_LONG ; -const unsigned long ttl_mask = (mask[ttl_i] >> word_offset) & port_mask; -const unsigned long ttl_bits = (bits[ttl_i] >> word_offset) & ttl_mask; - -/* set bits are processed a gpio port register at a time */ -for (i =0 ; i <ARRAY_SIZE (ports); i++) { -/* gpio offset in bits array */ - bits_offset = i * gpio_reg_size; - -/* check if any set bits for current port */ - gpio_mask = (*mask >> bits_offset) & port_mask; -if (!gpio_mask) { -/* no set bits for this port so move on to next port */ +unsigned int out_state; + +for_each_set_port_word (i, word, offset, mask, num_ports,8 ) { + iomask = mask[word] >> offset; + bitmask = iomask & (bits[word] >> offset); + +raw_spin_lock_irqsave (&idio24gpio->lock, flags); + +/* read bits from current gpio port (port 6 is TTL GPIO) */ +if (i <6 ) { + out_state =ioread8 (ports[i]) & ~iomask; + }else if (ioread8 (&idio24gpio->reg->ctl) & out_mode_mask) { + out_state =ioread8 (&idio24gpio->reg->ttl_out0_7); + }else { +/* skip TTL GPIO if set for input */ +raw_spin_unlock_irqrestore (&idio24gpio->lock, flags);continue ; } -raw_spin_lock_irqsave (&idio24gpio->lock, flags); +/* set requested bit states */ + out_state &= ~iomask; + out_state |= bitmask; -/* process output lines */ - out_state =ioread8 (ports[i]) & ~gpio_mask; - out_state |= (*bits >> bits_offset) & gpio_mask; -iowrite8 (out_state, ports[i]); +/* write bits for current gpio port (port 6 is TTL GPIO) */ +if (i <6 ) +iowrite8 (out_state, ports[i]); +else +iowrite8 (out_state, &idio24gpio->reg->ttl_out0_7);raw_spin_unlock_irqrestore (&idio24gpio->lock, flags); } - -/* check if setting TTL lines and if they are in output mode */ -if (!ttl_mask || !(ioread8 (&idio24gpio->reg->ctl) & out_mode_mask)) -return ; - -/* handle TTL output */ -raw_spin_lock_irqsave (&idio24gpio->lock, flags); - -/* process output lines */ - out_state =ioread8 (&idio24gpio->reg->ttl_out0_7) & ~ttl_mask; - out_state |= ttl_bits; -iowrite8 (out_state, &idio24gpio->reg->ttl_out0_7); - -raw_spin_unlock_irqrestore (&idio24gpio->lock, flags); }static void idio_24_irq_ack (struct irq_data *data)
Signature · 1 line
2.17.0