Introduce the for_each_set_clump8 macro
Subsystems (13)
v12 cover Introduce the for_each_set_clump8 macro
Changes in v12:
- Rebase changes (bitmap_set_value8 definition change in wrong commit)
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_clump8 macro and utilizes it in several GPIO drivers. The for_each_set_clump macro8 facilitates a for-loop syntax that iterates over a memory region entire groups of set bits at a time.
For example, suppose you would like to iterate over a 32-bit integer 8 bits at a time, skipping over 8-bit groups with no set bit, where XXXXXXXX represents the current 8-bit group:
Example: 10111110 00000000 11111111 00110011
First loop: 10111110 00000000 11111111 XXXXXXXX
Second loop: 10111110 00000000 XXXXXXXX 00110011
Third loop: XXXXXXXX 00000000 11111111 00110011Each iteration of the loop returns the next 8-bit group that has at least one set bit.
The for_each_set_clump8 macro has four parameters:
* start: set to the bit offset of the current clump
* clump: set to the current clump value
* bits: bitmap to search within
* size: bitmap size in number of bits
In this version of the patchset, the for_each_set_clump macro has been reimplemented and simplified based on the suggestions provided by Rasmus Villemoes and Andy Shevchenko in the version 4 submission.
In particular, the function of the for_each_set_clump macro has been restricted to handle only 8-bit clumps; the drivers that use the for_each_set_clump macro only handle 8-bit ports so a generic for_each_set_clump implementation is not necessary. Thus, a solution for large clumps (i.e. those larger than the width of a bitmap word) can be postponed until a driver appears that actually requires such a generic for_each_set_clump implementation.
For what it's worth, a semi-generic for_each_set_clump (i.e. for clumps smaller than the width of a bitmap word) can be implemented by simply replacing the hardcoded '8' and '0xFF' instances with respective variables. I have not yet had a need for such an implementation, and since it falls short of a true generic for_each_set_clump function, I have decided to forgo such an implementation for now.
In addition, the bitmap_get_value8 and bitmap_set_value8 functions are introduced to get and set 8-bit values respectively. Their use is based on the behavior suggested in the patchset version 4 review.
William Breathitt Gray (11):
bitops: Introduce the for_each_set_clump8 macro lib/test_bitmap.c: Add for_each_set_clump8 test cases
gpio: 104-dio-48e: Utilize for_each_set_clump8 macro
gpio: 104-idi-48: Utilize for_each_set_clump8 macro
gpio: gpio-mm: Utilize for_each_set_clump8 macro
gpio: ws16c48: Utilize for_each_set_clump8 macro
gpio: pci-idio-16: Utilize for_each_set_clump8 macro
gpio: pcie-idio-24: Utilize for_each_set_clump8 macro
gpio: uniphier: Utilize for_each_set_clump8 macro
thermal: intel: intel_soc_dts_iosf: Utilize for_each_set_clump8 macro
gpio: 74x164: Utilize the for_each_set_clump8 macro
drivers/gpio/gpio-104-dio-48e.c | 73 ++++---------- drivers/gpio/gpio-104-idi-48.c | 37 ++----- drivers/gpio/gpio-74x164.c | 20 ++-- drivers/gpio/gpio-gpio-mm.c | 73 ++++---------- drivers/gpio/gpio-pci-idio-16.c | 75 ++++++-------- drivers/gpio/gpio-pcie-idio-24.c | 111 ++++++++------------- drivers/gpio/gpio-uniphier.c | 16 ++- drivers/gpio/gpio-ws16c48.c | 72 ++++--------- drivers/thermal/intel/intel_soc_dts_iosf.c | 29 +++--- drivers/thermal/intel/intel_soc_dts_iosf.h | 2 - include/asm-generic/bitops/find.h | 14 +++ include/linux/bitops.h | 5 + lib/find_bit.c | 81 +++++++++++++++ lib/test_bitmap.c | 65 ++++++++++++ 14 files changed, 340 insertions(+), 333 deletions(-)
Signature · 1 line
2.21.0
PATCH v12 1/11 bitops: Introduce the for_each_set_clump8 macro
This macro iterates for each 8-bit group of bits (clump) with set bits, within a bitmap memory region. For each iteration, "start" is set to the bit offset of the found clump, while the respective clump value is stored to the location pointed by "clump". Additionally, the bitmap_get_value8 and bitmap_set_value8 functions are introduced to respectively get and set an 8-bit value in a bitmap memory region.
--- include/asm-generic/bitops/find.h | 14 ++++++ include/linux/bitops.h | 5 ++ lib/find_bit.c | 81 +++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+)
include/asm-generic/bitops/find.h +14 -0
@@ -80,4 +80,18 @@ extern unsigned long find_first_zero_bit(const unsigned long *addr,#endif /* CONFIG_GENERIC_FIND_FIRST_BIT */ +unsigned long bitmap_get_value8 (const unsigned long *const bitmap, +const unsigned int size, +const unsigned int start); + +void bitmap_set_value8 (unsigned long *const bitmap,const unsigned int size, +const unsigned long value,const unsigned int start); + +unsigned int find_next_clump8 (unsigned long *const clump, +const unsigned long *const addr, +unsigned int offset,const unsigned int size); + +#define find_first_clump8 (clump, bits, size) \ + find_next_clump8((clump), (bits), 0, (size)) +#endif /*_ASM_GENERIC_BITOPS_FIND_H_ */
include/linux/bitops.h +5 -0
@@ -40,6 +40,11 @@ extern unsigned long __sw_hweight64(__u64 w); (bit) < (size); \ (bit) = find_next_zero_bit((addr), (size), (bit) + 1)) +#define for_each_set_clump8 (start, clump, bits, size) \ + for ((start) = find_first_clump8(&(clump), (bits), (size)); \ + (start) < (size); \ + (start) = find_next_clump8(&(clump), (bits), (start) + 8, (size))) +static inline int get_bitmask_order (unsigned int count) {int order;
lib/find_bit.c +81 -0
84 lines not on this page · raw message
Signature · 1 line
2.21.0
Re: [PATCH v12 01/11] bitops: Introduce the for_each_set_clump8 macro
Quoted 15 lines · William Breathitt Gray
lib/find_bit.c +15
-0+/** +* find_next_clump8 - find next 8-bit clump with set bits in a memory region +* @clump: location to store copy of found clump +* @addr: address to base the search on +* @offset: bit offset at which to start searching +* @size: bitmap size in number of bits +* +* Returns the bit offset for the next set clump; the found clump value is +* copied to the location pointed by @clump. If no bits are set, returns @size. +*/ +unsigned int find_next_clump8 (unsigned long *const clump, +const unsigned long *const addr, +unsigned int offset,const unsigned int size) +{ +for (; offset < size; offset +=8 ) {
lib/find_bit.c EXPORT_SYMBOL(find_next_bit_le); · line 301
Quoted 10 lines
lib/find_bit.c +10
-0+ *clump =bitmap_get_value8 (addr, size, offset); +if (!*clump) +continue ; + +return offset; + } + +return size; +} +EXPORT_SYMBOL (find_next_clump8);
Just use find_first_bit() / find_next_bit() to use optimized arch-specific bitops instead of open-coding the iteration over the bitmap.
See max3191x_get_multiple() for an example.
Thanks,
Lukas
Re: [PATCH v12 01/11] bitops: Introduce the for_each_set_clump8 macro
Quoted 30 lines · Lukas Wunner
Quoted 15 lines · William Breathitt Gray
lib/find_bit.c +15
-0+/** +* find_next_clump8 - find next 8-bit clump with set bits in a memory region +* @clump: location to store copy of found clump +* @addr: address to base the search on +* @offset: bit offset at which to start searching +* @size: bitmap size in number of bits +* +* Returns the bit offset for the next set clump; the found clump value is +* copied to the location pointed by @clump. If no bits are set, returns @size. +*/ +unsigned int find_next_clump8 (unsigned long *const clump, +const unsigned long *const addr, +unsigned int offset,const unsigned int size) +{ +for (; offset < size; offset +=8 ) {lib/find_bit.c EXPORT_SYMBOL(find_next_bit_le); · line 301
Quoted 10 lines
lib/find_bit.c +10
-0+ *clump =bitmap_get_value8 (addr, size, offset); +if (!*clump) +continue ; + +return offset; + } + +return size; +} +EXPORT_SYMBOL (find_next_clump8);Just use
find_first_bit()/find_next_bit()to use optimized arch-specific bitops instead of open-coding the iteration over the bitmap.
Quoted 1 line
See
max3191x_get_multiple()for an example.
Actually a good point.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v12 01/11] bitops: Introduce the for_each_set_clump8 macro
In reply to Lukas WunnerQuoted 30 lines · Lukas Wunner
Quoted 15 lines · William Breathitt Gray
lib/find_bit.c +15
-0+/** +* find_next_clump8 - find next 8-bit clump with set bits in a memory region +* @clump: location to store copy of found clump +* @addr: address to base the search on +* @offset: bit offset at which to start searching +* @size: bitmap size in number of bits +* +* Returns the bit offset for the next set clump; the found clump value is +* copied to the location pointed by @clump. If no bits are set, returns @size. +*/ +unsigned int find_next_clump8 (unsigned long *const clump, +const unsigned long *const addr, +unsigned int offset,const unsigned int size) +{ +for (; offset < size; offset +=8 ) {lib/find_bit.c EXPORT_SYMBOL(find_next_bit_le); · line 301
Quoted 10 lines
lib/find_bit.c +10
-0+ *clump =bitmap_get_value8 (addr, size, offset); +if (!*clump) +continue ; + +return offset; + } + +return size; +} +EXPORT_SYMBOL (find_next_clump8);Just use
find_first_bit()/find_next_bit()to use optimized arch-specific bitops instead of open-coding the iteration over the bitmap.
Quoted 5 lines
See
max3191x_get_multiple()for an example.Thanks,
Lukas
Is this the sort of implementation you had in mind:
offset = find_next_bit(addr, size, offset);
if (offset == size)
return size;
offset -= offset % 8;
*clump = bitmap_get_value8(addr, size, offset);
return offset;Yes, this does seem more efficient to leverage the existing find_next_bit function.
Should the offset and size parameters be redefined as unsigned long to match the find_first_bit/find_next_bit function parameters?
William Breathitt Gray
Re: [PATCH v12 01/11] bitops: Introduce the for_each_set_clump8 macro
Quoted 35 lines · William Breathitt Gray
Quoted 30 lines · Lukas Wunner
Quoted 15 lines · William Breathitt Gray
lib/find_bit.c +15
-0+/** +* find_next_clump8 - find next 8-bit clump with set bits in a memory region +* @clump: location to store copy of found clump +* @addr: address to base the search on +* @offset: bit offset at which to start searching +* @size: bitmap size in number of bits +* +* Returns the bit offset for the next set clump; the found clump value is +* copied to the location pointed by @clump. If no bits are set, returns @size. +*/ +unsigned int find_next_clump8 (unsigned long *const clump, +const unsigned long *const addr, +unsigned int offset,const unsigned int size) +{ +for (; offset < size; offset +=8 ) {lib/find_bit.c EXPORT_SYMBOL(find_next_bit_le); · line 301
Quoted 10 lines
lib/find_bit.c +10
-0+ *clump =bitmap_get_value8 (addr, size, offset); +if (!*clump) +continue ; + +return offset; + } + +return size; +} +EXPORT_SYMBOL (find_next_clump8);Just use
find_first_bit()/find_next_bit()to use optimized arch-specific bitops instead of open-coding the iteration over the bitmap.Quoted 1 line
See
max3191x_get_multiple()for an example.Is this the sort of implementation you had in mind:
Quoted 8 lines
offset = find_next_bit(addr, size, offset);
if (offset == size)
return size;
offset -= offset % 8;
*clump = bitmap_get_value8(addr, size, offset);
return offset;Almost. I'd use round_down() instead of "offset -= offset % 8". Then it's just a single cheap logical and operation at runtime.
I'd try to avoid copying around the clump value and use a pointer to u8 instead.
I don't understand the calculations in bitmap_get_value8() at all. Why is it so complicated, does it allow passing in a start value that's not a multiple of 8? Do you really need that? I imagine a simplification is possible if that assumption can be made (and is spelled out in the kerneldoc).
Quoted 2 lines
Should the offset and size parameters be redefined as unsigned long to match the find_first_bit/find_next_bit function parameters?
Yes, probably. It's just the CPU's native length anyway.
Thanks,
Lukas
Re: [PATCH v12 01/11] bitops: Introduce the for_each_set_clump8 macro
Quoted 12 lines · Lukas Wunner
Quoted 11 lines · William Breathitt Gray
On Mon, Mar 25, 2019 at 10:38:54AM +0100, Lukas Wunner wrote:
Is this the sort of implementation you had in mind:
offset = find_next_bit(addr, size, offset); if (offset == size) return size; offset -= offset % 8; *clump = bitmap_get_value8(addr, size, offset);
Quoted 7 lines
Quoted 1 line
return offset;Almost. I'd use
round_down()instead of "offset -= offset % 8". Then it's just a single cheap logical and operation at runtime.I'd try to avoid copying around the clump value and use a pointer to u8 instead.
u8 might be inconvenient in environment where everything else is type of unsigned long.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v12 01/11] bitops: Introduce the for_each_set_clump8 macro
In reply to Lukas WunnerQuoted 40 lines · Lukas Wunner
Quoted 39 lines · William Breathitt Gray
Quoted 30 lines · Lukas Wunner
Quoted 15 lines · William Breathitt Gray
lib/find_bit.c +15
-0+/** +* find_next_clump8 - find next 8-bit clump with set bits in a memory region +* @clump: location to store copy of found clump +* @addr: address to base the search on +* @offset: bit offset at which to start searching +* @size: bitmap size in number of bits +* +* Returns the bit offset for the next set clump; the found clump value is +* copied to the location pointed by @clump. If no bits are set, returns @size. +*/ +unsigned int find_next_clump8 (unsigned long *const clump, +const unsigned long *const addr, +unsigned int offset,const unsigned int size) +{ +for (; offset < size; offset +=8 ) {lib/find_bit.c EXPORT_SYMBOL(find_next_bit_le); · line 301
Quoted 10 lines
lib/find_bit.c +10
-0+ *clump =bitmap_get_value8 (addr, size, offset); +if (!*clump) +continue ; + +return offset; + } + +return size; +} +EXPORT_SYMBOL (find_next_clump8);Just use
find_first_bit()/find_next_bit()to use optimized arch-specific bitops instead of open-coding the iteration over the bitmap.Quoted 1 line
See
max3191x_get_multiple()for an example.Is this the sort of implementation you had in mind:
offset = find_next_bit(addr, size, offset); if (offset == size) return size;
Quoted 7 lines
Quoted 4 lines
offset -= offset % 8; *clump = bitmap_get_value8(addr, size, offset); return offset;Almost. I'd use
round_down()instead of "offset -= offset % 8". Then it's just a single cheap logical and operation at runtime.
All right I'll try this setup using round_down() then.
Quoted 3 lines
I'd try to avoid copying around the clump value and use a pointer to u8 instead.
Although in this case we are handling 8-bit clumps, I anticipate device drivers in the future which may benefit from larger size clumps (e.g. GPIO devices with 24-bit ports). It'll be better to define clumps similar to how we're defining bitmaps now (unsigned long *) so that we can support these sizes if need be in the future without requiring data type changes.
Quoted 6 lines
I don't understand the calculations in
bitmap_get_value8()at all. Why is it so complicated, does it allow passing in a start value that's not a multiple of 8? Do you really need that? I imagine a simplification is possible if that assumption can be made (and is spelled out in the kerneldoc).
That's a good point. Originally, I had envisioned the possibility of calling bitmap_get_value8/bitmap_set_value8 at odd start offsets; this would open up the possibility of a clump landing as a split between 2 words, thus requiring this complicated case handling code. However, I'm not sure how often users would need this case; none of the drivers right now require clumps at odd offsets.
Andy, would you have any objection to restricting the start offset values for bitmap_get_value8/bitmap_set_value8 to multiples of 8? That would prevent the split word case, and thus allow the implementation for those functions to be a lot simpler.
William Breathitt Gray
Quoted 10 lines
Quoted 4 lines
Should the offset and size parameters be redefined as unsigned long to match the find_first_bit/find_next_bit function parameters?
Yes, probably. It's just the CPU's native length anyway.
Thanks,
Lukas
Re: [PATCH v12 01/11] bitops: Introduce the for_each_set_clump8 macro
Quoted 15 lines · William Breathitt Gray
Quoted 6 lines · Lukas Wunner
On Tue, Mar 26, 2019 at 12:14:22PM +0900, William Breathitt Gray wrote:
Why is it so complicated, does it allow passing in a start value that's not a multiple of 8? Do you really need that? I imagine a simplification is possible if that assumption can be made (and is spelled out in the kerneldoc).
That's a good point. Originally, I had envisioned the possibility of calling bitmap_get_value8/bitmap_set_value8 at odd start offsets; this would open up the possibility of a clump landing as a split between 2 words, thus requiring this complicated case handling code. However, I'm not sure how often users would need this case; none of the drivers right now require clumps at odd offsets.
Quoted 4 lines
Andy, would you have any objection to restricting the start offset values for bitmap_get_value8/bitmap_set_value8 to multiples of 8? That would prevent the split word case, and thus allow the implementation for those functions to be a lot simpler.
No, I have no objection.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v12 01/11] bitops: Introduce the for_each_set_clump8 macro
Quoted 16 lines · Andy Shevchenko
Quoted 15 lines · William Breathitt Gray
Quoted 6 lines · Lukas Wunner
On Tue, Mar 26, 2019 at 12:14:22PM +0900, William Breathitt Gray wrote:
Why is it so complicated, does it allow passing in a start value that's not a multiple of 8? Do you really need that? I imagine a simplification is possible if that assumption can be made (and is spelled out in the kerneldoc).
That's a good point. Originally, I had envisioned the possibility of calling bitmap_get_value8/bitmap_set_value8 at odd start offsets; this would open up the possibility of a clump landing as a split between 2 words, thus requiring this complicated case handling code. However, I'm not sure how often users would need this case; none of the drivers right now require clumps at odd offsets.
Quoted 10 lines
Quoted 4 lines
Andy, would you have any objection to restricting the start offset values for bitmap_get_value8/bitmap_set_value8 to multiples of 8? That would prevent the split word case, and thus allow the implementation for those functions to be a lot simpler.
No, I have no objection.
Signature · 2 lines
With Best Regards, Andy Shevchenko
In this case, bitmap_get_value8 could be simplified to something like this:
index = BIT_WORD(start);
offset = start % BITS_PER_LONG;
return (bitmap[index] >> offset) & 0xFF;Or if you prefer a single line:
(bitmap[BIT_WORD(start)] >> (start % BITS_PER_LONG)) & 0xFF;
Would it be better to define bitmap_get_value8 as a macro then?
William Breathitt Gray
Re: [PATCH v12 01/11] bitops: Introduce the for_each_set_clump8 macro
Quoted 10 lines · William Breathitt Gray
Quoted 2 lines · Lukas Wunner
I'd try to avoid copying around the clump value and use a pointer to u8 instead.
Although in this case we are handling 8-bit clumps, I anticipate device drivers in the future which may benefit from larger size clumps (e.g. GPIO devices with 24-bit ports). It'll be better to define clumps similar to how we're defining bitmaps now (unsigned long *) so that we can support these sizes if need be in the future without requiring data type changes.
It's just that copying around data in memory may be more expensive than, say, returning from a function, in which case it's usually stored in a register.
Quoted 6 lines
In this case,
bitmap_get_value8could be simplified to something like this:index = BIT_WORD(start); offset = start % BITS_PER_LONG; return (bitmap[index] >> offset) & 0xFF;
Hm, shouldn't that be "offset = round_down(start, 8)" ?
(I prefer the multi-line version FWIW.)
Quoted 1 line
Would it be better to define
bitmap_get_value8as a macro then?
Or a static inline.
Thanks,
Lukas
Re: [PATCH v12 01/11] bitops: Introduce the for_each_set_clump8 macro
Quoted 6 lines · Lukas Wunner
Quoted 5 lines · William Breathitt Gray
On Tue, Mar 26, 2019 at 10:43:45AM +0100, Lukas Wunner wrote:
In this case,
bitmap_get_value8could be simplified to something like this:
Quoted 6 lines
Quoted 3 lines
index = BIT_WORD(start); offset = start % BITS_PER_LONG; return (bitmap[index] >> offset) & 0xFF;Hm, shouldn't that be "offset =
round_down(start, 8)" ?
No, the index points to the word, the offset points to the offset inside word.
Quoted 1 line
(I prefer the multi-line version FWIW.)
+1 here.
Quoted 3 lines
Quoted 1 line
Would it be better to define
bitmap_get_value8as a macro then?Or a static inline.
Please, no macro. It disadvantages in terms of type checking.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v12 01/11] bitops: Introduce the for_each_set_clump8 macro
In reply to William Breathitt GrayQuoted 6 lines · William Breathitt Gray
This macro iterates for each 8-bit group of bits (clump) with set bits, within a bitmap memory region. For each iteration, "start" is set to the bit offset of the found clump, while the respective clump value is stored to the location pointed by "clump". Additionally, the
bitmap_get_value8andbitmap_set_value8functions are introduced to respectively get and set an 8-bit value in a bitmap memory region.
This seems to miss Randy's (IIRC) comment about too many const specifiers.
Quoted 144 lines
Suggested-by Andy Shevchenko <[email protected]>Suggested-by Rasmus Villemoes <[email protected]>Cc Arnd Bergmann <[email protected]>Acked-by Andrew Morton <[email protected]>Reviewed-by Andy Shevchenko <[email protected]>Reviewed-by Linus Walleij <[email protected]>Signed-off-by William Breathitt Gray <[email protected]>--- include/asm-generic/bitops/find.h | 14 ++++++ include/linux/bitops.h | 5 ++ lib/find_bit.c | 81 +++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+)include/asm-generic/bitops/find.h +14
-0@@ -80,4 +80,18 @@ extern unsigned long find_first_zero_bit(const unsigned long *addr,#endif /* CONFIG_GENERIC_FIND_FIRST_BIT */ +unsigned long bitmap_get_value8 (const unsigned long *const bitmap, +const unsigned int size, +const unsigned int start); + +void bitmap_set_value8 (unsigned long *const bitmap,const unsigned int size, +const unsigned long value,const unsigned int start); + +unsigned int find_next_clump8 (unsigned long *const clump, +const unsigned long *const addr, +unsigned int offset,const unsigned int size); + +#define find_first_clump8 (clump, bits, size) \ + find_next_clump8((clump), (bits), 0, (size)) +#endif /*_ASM_GENERIC_BITOPS_FIND_H_ */ include/linux/bitops.h +5
-0@@ -40,6 +40,11 @@ extern unsigned long __sw_hweight64(__u64 w); (bit) < (size); \ (bit) = find_next_zero_bit((addr), (size), (bit) + 1)) +#define for_each_set_clump8 (start, clump, bits, size) \ + for ((start) = find_first_clump8(&(clump), (bits), (size)); \ + (start) < (size); \ + (start) = find_next_clump8(&(clump), (bits), (start) + 8, (size))) +static inline int get_bitmask_order (unsigned int count) {int order;lib/find_bit.c +81
-0@@ -218,3 +218,84 @@ EXPORT_SYMBOL(find_next_bit_le);#endif #endif /* __BIG_ENDIAN */ + +/** +* bitmap_get_value8 - get an 8-bit value within a memory region +* @bitmap: address to the bitmap memory region +* @size: bitmap size in number of bits +* @start: bit offset of the 8-bit value +* +* Returns the 8-bit value located at the @start bit offset within the @bitmap +* memory region. +*/ +unsigned long bitmap_get_value8 (const unsigned long *const bitmap, +const unsigned int size, +const unsigned int start) +{ +const size_t index =BIT_WORD (start); +const unsigned int offset = start %BITS_PER_LONG ; +const unsigned int low_width = (offset +8 >BITS_PER_LONG ) ? +BITS_PER_LONG - offset :8 ; +const unsigned long low = bitmap[index] >> offset; +const unsigned long high = (low_width <8 && start +8 <= size) ? + bitmap[index +1 ] << low_width :0 ; + +return (low | high) &0xFF ; +} +EXPORT_SYMBOL (bitmap_get_value8); + +/** +* bitmap_set_value8 - set an 8-bit value within a memory region +* @bitmap: address to the bitmap memory region +* @size: bitmap size in number of bits +* @value: the 8-bit value; values wider than 8 bits may clobber bitmap +* @start: bit offset of the 8-bit value +*/ +void bitmap_set_value8 (unsigned long *const bitmap,const unsigned int size, +const unsigned long value,const unsigned int start) +{ +const size_t index =BIT_WORD (start); +const unsigned int offset = start %BITS_PER_LONG ; +const unsigned int low_width = (offset +8 >BITS_PER_LONG ) ? +BITS_PER_LONG - offset :8 ; +const unsigned long low_mask =GENMASK (low_width -1 ,0 ) << offset; +const unsigned int high_width =8 - low_width; +const unsigned long high_mask =GENMASK (high_width -1 ,0 ); + +/* set lower portion */ + bitmap[index] &= ~low_mask; + bitmap[index] |= value << offset; + +/* set higher portion if space available in bitmap */ +if (high_width && start +8 <= size) { + bitmap[index +1 ] &= ~high_mask; + bitmap[index +1 ] |= value >> low_width; + } +} +EXPORT_SYMBOL (bitmap_set_value8); + +/** +* find_next_clump8 - find next 8-bit clump with set bits in a memory region +* @clump: location to store copy of found clump +* @addr: address to base the search on +* @offset: bit offset at which to start searching +* @size: bitmap size in number of bits +* +* Returns the bit offset for the next set clump; the found clump value is +* copied to the location pointed by @clump. If no bits are set, returns @size. +*/ +unsigned int find_next_clump8 (unsigned long *const clump, +const unsigned long *const addr, +unsigned int offset,const unsigned int size) +{ +for (; offset < size; offset +=8 ) { + *clump =bitmap_get_value8 (addr, size, offset); +if (!*clump) +continue ; + +return offset; + } + +return size; +} +EXPORT_SYMBOL (find_next_clump8);Signature · 1 line
2.21.0
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v12 01/11] bitops: Introduce the for_each_set_clump8 macro
Quoted 10 lines · Andy Shevchenko
Quoted 6 lines · William Breathitt Gray
This macro iterates for each 8-bit group of bits (clump) with set bits, within a bitmap memory region. For each iteration, "start" is set to the bit offset of the found clump, while the respective clump value is stored to the location pointed by "clump". Additionally, the
bitmap_get_value8andbitmap_set_value8functions are introduced to respectively get and set an 8-bit value in a bitmap memory region.This seems to miss Randy's (IIRC) comment about too many const specifiers.
I disagree with removing the const qualifiers; I believe they are useful and do not significantly impact the clarity of the code (in fact, I'd argue the opposite). The const qualifiers make it clear these values are constant, allowing readers at a glace to know these values never change within this function. Although I believe GCC is smart enough in this case to deduce implicitly that these are constant values, generally speaking const qualifiers do make it easier for compilers to optimize sections of code (OoO execution, algorithm simplification, etc.), so I believe it's useful in a technical sense as well.
I added the const qualifier to these variables because they really are constant, and I believe there is merit in making it explicit in the code. If the primary reason for removing the const qualifiers is for aesthetics, then I must dissent with that decision.
However, it is difficult to read the definitions that wrap around to a second line. These definitions are long enough that even removing the const qualifiers would not help prevent the wrapping, so perhaps it would make to let these stay on a single line. Do you think it would help to ignore the 80-character maximum line width coding style rule for these cases here?
William Breathitt Gray
Quoted 151 lines
Quoted 145 lines
Suggested-by Andy Shevchenko <[email protected]>Suggested-by Rasmus Villemoes <[email protected]>Cc Arnd Bergmann <[email protected]>Acked-by Andrew Morton <[email protected]>Reviewed-by Andy Shevchenko <[email protected]>Reviewed-by Linus Walleij <[email protected]>Signed-off-by William Breathitt Gray <[email protected]>--- include/asm-generic/bitops/find.h | 14 ++++++ include/linux/bitops.h | 5 ++ lib/find_bit.c | 81 +++++++++++++++++++++++++++++++ 3 files changed, 100 insertions(+)include/asm-generic/bitops/find.h +14
-0@@ -80,4 +80,18 @@ extern unsigned long find_first_zero_bit(const unsigned long *addr,#endif /* CONFIG_GENERIC_FIND_FIRST_BIT */ +unsigned long bitmap_get_value8 (const unsigned long *const bitmap, +const unsigned int size, +const unsigned int start); + +void bitmap_set_value8 (unsigned long *const bitmap,const unsigned int size, +const unsigned long value,const unsigned int start); + +unsigned int find_next_clump8 (unsigned long *const clump, +const unsigned long *const addr, +unsigned int offset,const unsigned int size); + +#define find_first_clump8 (clump, bits, size) \ + find_next_clump8((clump), (bits), 0, (size)) +#endif /*_ASM_GENERIC_BITOPS_FIND_H_ */ include/linux/bitops.h +5
-0@@ -40,6 +40,11 @@ extern unsigned long __sw_hweight64(__u64 w); (bit) < (size); \ (bit) = find_next_zero_bit((addr), (size), (bit) + 1)) +#define for_each_set_clump8 (start, clump, bits, size) \ + for ((start) = find_first_clump8(&(clump), (bits), (size)); \ + (start) < (size); \ + (start) = find_next_clump8(&(clump), (bits), (start) + 8, (size))) +static inline int get_bitmask_order (unsigned int count) {int order;lib/find_bit.c +81
-0@@ -218,3 +218,84 @@ EXPORT_SYMBOL(find_next_bit_le);#endif #endif /* __BIG_ENDIAN */ + +/** +* bitmap_get_value8 - get an 8-bit value within a memory region +* @bitmap: address to the bitmap memory region +* @size: bitmap size in number of bits +* @start: bit offset of the 8-bit value +* +* Returns the 8-bit value located at the @start bit offset within the @bitmap +* memory region. +*/ +unsigned long bitmap_get_value8 (const unsigned long *const bitmap, +const unsigned int size, +const unsigned int start) +{ +const size_t index =BIT_WORD (start); +const unsigned int offset = start %BITS_PER_LONG ; +const unsigned int low_width = (offset +8 >BITS_PER_LONG ) ? +BITS_PER_LONG - offset :8 ; +const unsigned long low = bitmap[index] >> offset; +const unsigned long high = (low_width <8 && start +8 <= size) ? + bitmap[index +1 ] << low_width :0 ; + +return (low | high) &0xFF ; +} +EXPORT_SYMBOL (bitmap_get_value8); + +/** +* bitmap_set_value8 - set an 8-bit value within a memory region +* @bitmap: address to the bitmap memory region +* @size: bitmap size in number of bits +* @value: the 8-bit value; values wider than 8 bits may clobber bitmap +* @start: bit offset of the 8-bit value +*/ +void bitmap_set_value8 (unsigned long *const bitmap,const unsigned int size, +const unsigned long value,const unsigned int start) +{ +const size_t index =BIT_WORD (start); +const unsigned int offset = start %BITS_PER_LONG ; +const unsigned int low_width = (offset +8 >BITS_PER_LONG ) ? +BITS_PER_LONG - offset :8 ; +const unsigned long low_mask =GENMASK (low_width -1 ,0 ) << offset; +const unsigned int high_width =8 - low_width; +const unsigned long high_mask =GENMASK (high_width -1 ,0 ); + +/* set lower portion */ + bitmap[index] &= ~low_mask; + bitmap[index] |= value << offset; + +/* set higher portion if space available in bitmap */ +if (high_width && start +8 <= size) { + bitmap[index +1 ] &= ~high_mask; + bitmap[index +1 ] |= value >> low_width; + } +} +EXPORT_SYMBOL (bitmap_set_value8); + +/** +* find_next_clump8 - find next 8-bit clump with set bits in a memory region +* @clump: location to store copy of found clump +* @addr: address to base the search on +* @offset: bit offset at which to start searching +* @size: bitmap size in number of bits +* +* Returns the bit offset for the next set clump; the found clump value is +* copied to the location pointed by @clump. If no bits are set, returns @size. +*/ +unsigned int find_next_clump8 (unsigned long *const clump, +const unsigned long *const addr, +unsigned int offset,const unsigned int size) +{ +for (; offset < size; offset +=8 ) { + *clump =bitmap_get_value8 (addr, size, offset); +if (!*clump) +continue ; + +return offset; + } + +return size; +} +EXPORT_SYMBOL (find_next_clump8);Signature · 1 line
2.21.0Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH v12 01/11] bitops: Introduce the for_each_set_clump8 macro
Quoted 10 lines · William Breathitt Gray
Quoted 9 lines · Andy Shevchenko
Quoted 6 lines · William Breathitt Gray
This macro iterates for each 8-bit group of bits (clump) with set bits, within a bitmap memory region. For each iteration, "start" is set to the bit offset of the found clump, while the respective clump value is stored to the location pointed by "clump". Additionally, the
bitmap_get_value8andbitmap_set_value8functions are introduced to respectively get and set an 8-bit value in a bitmap memory region.
Quoted 5 lines
Quoted 1 line
This seems to miss Randy's (IIRC) comment about too many const specifiers.
I disagree with removing the const qualifiers; I believe they are useful and do not significantly impact the clarity of the code (in fact, I'd argue the opposite).
Had you checked the assembly? I'm talking about const for values on the stack.
I think if you put less const there compiler can keep something in the registers instead of using direct constants or accessing stack.
I might be mistaken, so, I can't argue without evidence of either.
Quoted 7 lines
The const qualifiers make it clear these values are constant, allowing readers at a glace to know these values never change within this function. Although I believe GCC is smart enough in this case to deduce implicitly that these are constant values, generally speaking const qualifiers do make it easier for compilers to optimize sections of code (OoO execution, algorithm simplification, etc.), so I believe it's useful in a technical sense as well.
Again, what the difference do you see in assembly if any?
Quoted 4 lines
I added the const qualifier to these variables because they really are constant, and I believe there is merit in making it explicit in the code. If the primary reason for removing the const qualifiers is for aesthetics, then I must dissent with that decision.
The point is, if there is no difference, I would prefer one which will be better to read, otherwise check the assembly.
Quoted 6 lines
However, it is difficult to read the definitions that wrap around to a second line. These definitions are long enough that even removing the const qualifiers would not help prevent the wrapping, so perhaps it would make to let these stay on a single line. Do you think it would help to ignore the 80-character maximum line width coding style rule for these cases here?
80-characters rule can be slightly bended depending on the context. Here, I think, we might continue discussing the matter after having an evidence how const qualifiers affect the code.
Signature · 2 lines
With Best Regards, Andy Shevchenko
PATCH v12 2/11 lib/test_bitmap.c: Add for_each_set_clump8 test cases
In reply to William Breathitt GrayThe introduction of the for_each_set_clump8 macro warrants test cases to verify the implementation. This patch adds test case checks for whether an out-of-bounds clump index is returned, a zero clump is returned, or the returned clump value differs from the expected clump value.
--- lib/test_bitmap.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+)
lib/test_bitmap.c +65 -0
@@ -88,6 +88,36 @@ __check_eq_u32_array(const char *srcfile, unsigned int line,return true; } +static bool __init __check_eq_clump8 (const char *srcfile,unsigned int line, +const unsigned int offset, +const unsigned int size, +const unsigned char *const clump_exp, +const unsigned long *const clump) +{ +unsigned long exp; + +if (offset >= size) { +pr_warn ("[%s:%u] bit offset for clump out-of-bounds: expected less than %u, got %u\n" , + srcfile, line, size, offset); +return false; + } + + exp = clump_exp[offset /8 ]; +if (!exp) { +pr_warn ("[%s:%u] bit offset for zero clump: expected nonzero clump, got bit offset %u with clump value 0" , + srcfile, line, offset); +return false; + } + +if (*clump != exp) { +pr_warn ("[%s:%u] expected clump value of 0x%lX, got clump value of 0x%lX" , + srcfile, line, exp, *clump); +return false; + } + +return true; +} +#define __expect_eq (suffix, ...) \ ({ \ int result = 0; \
@@ -104,6 +134,7 @@ __check_eq_u32_array(const char *srcfile, unsigned int line,#define expect_eq_bitmap (...) __expect_eq(bitmap, ##__VA_ARGS__)#define expect_eq_pbl (...) __expect_eq(pbl, ##__VA_ARGS__)#define expect_eq_u32_array (...) __expect_eq(u32_array, ##__VA_ARGS__) +#define expect_eq_clump8 (...) __expect_eq(clump8, ##__VA_ARGS__)static void __init test_zero_clear (void ) {
@@ -361,6 +392,39 @@ static void noinline __init test_mem_optimisations(void) } } +static const unsigned char clump_exp[]__initconst = { +0x01 ,/* 1 bit set */ +0x02 ,/* non-edge 1 bit set */ +0x00 ,/* zero bits set */ +0x38 ,/* 3 bits set across 4-bit boundary */ +0x38 ,/* Repeated clump */ +0x0F ,/* 4 bits set */ +0xFF ,/* all bits set */ +0x05 ,/* non-adjacent 2 bits set */ +}; + +static void __init test_for_each_set_clump8 (void ) +{ +#define CLUMP_EXP_NUMBITS 64 +DECLARE_BITMAP (bits,CLUMP_EXP_NUMBITS ); +unsigned int start; +unsigned long clump; + +/* set bitmap to test case */ +bitmap_zero (bits,CLUMP_EXP_NUMBITS ); +bitmap_set (bits,0 ,1 );/* 0x01 */ +bitmap_set (bits,9 ,1 );/* 0x02 */ +bitmap_set (bits,27 ,3 );/* 0x28 */ +bitmap_set (bits,35 ,3 );/* 0x28 */ +bitmap_set (bits,40 ,4 );/* 0x0F */ +bitmap_set (bits,48 ,8 );/* 0xFF */ +bitmap_set (bits,56 ,1 );/* 0x05 - part 1 */ +bitmap_set (bits,58 ,1 );/* 0x05 - part 2 */ + +for_each_set_clump8 (start, clump, bits,CLUMP_EXP_NUMBITS ) +expect_eq_clump8 (start,CLUMP_EXP_NUMBITS , clump_exp, &clump); +} +static int __init test_bitmap_init (void ) {test_zero_clear ();
@@ -369,6 +433,7 @@ static int __init test_bitmap_init(void)test_bitmap_arr32 ();test_bitmap_parselist ();test_mem_optimisations (); + test_for_each_set_clump8();if (failed_tests ==0 )pr_info ("all %u tests passed\n" , total_tests);
Signature · 1 line
2.21.0
PATCH v12 3/11 gpio: 104-dio-48e: Utilize for_each_set_clump8 macro
In reply to William Breathitt GrayReplace verbose implementation in get_multiple/set_multiple callbacks with for_each_set_clump8 macro to simplify code and improve clarity.
--- drivers/gpio/gpio-104-dio-48e.c | 73 ++++++++++----------------------- 1 file changed, 22 insertions(+), 51 deletions(-)
drivers/gpio/gpio-104-dio-48e.c +22 -51
@@ -183,46 +183,26 @@ 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 ); +unsigned int offset; +unsigned long gpio_mask; +const unsigned int ngpio =ARRAY_SIZE (ports) *8 ; +unsigned int port_addr;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 */ - port_state =inb (dio48egpio->base + ports[i]); +for_each_set_clump8 (offset, gpio_mask, mask, ngpio) { + port_addr = dio48egpio->base + ports[offset /8 ]; + port_state =inb (port_addr) & gpio_mask; -/* store acquired bits at respective bits array offset */ - bits[word_index] |= (port_state << word_offset) & word_mask; +bitmap_set_value8 (bits, ngpio, port_state, offset); }return 0 ;
@@ -252,37 +232,28 @@ 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; +unsigned int offset; +unsigned long gpio_mask; +const unsigned int ngpio =ARRAY_SIZE (ports) *8 ; +size_t index; +unsigned int port_addr;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 ; - } +for_each_set_clump8 (offset, gpio_mask, mask, ngpio) { + index = offset /8 ; + port_addr = dio48egpio->base + ports[index]; - port = i / gpio_reg_size; - out_port = (port >2 ) ? port +1 : port; - bitmask = mask[BIT_WORD (i)] & bits[BIT_WORD (i)]; + bitmask =bitmap_get_value8 (bits, ngpio, offset) & gpio_mask;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[index] &= ~gpio_mask; + dio48egpio->out_state[index] |= bitmask; +outb (dio48egpio->out_state[index], port_addr);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.21.0
PATCH v12 4/11 gpio: 104-idi-48: Utilize for_each_set_clump8 macro
In reply to William Breathitt GrayReplace verbose implementation in get_multiple/set_multiple callbacks with for_each_set_clump8 macro to simplify code and improve clarity.
--- drivers/gpio/gpio-104-idi-48.c | 37 ++++++++-------------------------- 1 file changed, 8 insertions(+), 29 deletions(-)
drivers/gpio/gpio-104-idi-48.c +8 -29
@@ -93,42 +93,21 @@ static int idi_48_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask, unsigned long *bits) {struct idi_48_gpio *const idi48gpio =gpiochip_get_data (chip); -size_t i; +unsigned int offset; +unsigned long gpio_mask;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 ); +const unsigned int ngpio =ARRAY_SIZE (ports) *8 ; +unsigned int port_addr;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; +for_each_set_clump8 (offset, gpio_mask, mask, ngpio) { + port_addr = idi48gpio->base + ports[offset /8 ]; + port_state =inb (port_addr) & gpio_mask; -/* 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 (idi48gpio->base + ports[i]); - -/* store acquired bits at respective bits array offset */ - bits[word_index] |= (port_state << word_offset) & word_mask; +bitmap_set_value8 (bits, ngpio, port_state, offset); }return 0 ;
Signature · 1 line
2.21.0
PATCH v12 5/11 gpio: gpio-mm: Utilize for_each_set_clump8 macro
In reply to William Breathitt GrayReplace verbose implementation in get_multiple/set_multiple callbacks with for_each_set_clump8 macro to simplify code and improve clarity.
--- drivers/gpio/gpio-gpio-mm.c | 73 +++++++++++-------------------------- 1 file changed, 22 insertions(+), 51 deletions(-)
drivers/gpio/gpio-gpio-mm.c +22 -51
@@ -172,46 +172,26 @@ 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 ); +unsigned int offset; +unsigned long gpio_mask; +const unsigned int ngpio =ARRAY_SIZE (ports) *8 ; +unsigned int port_addr;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 */ - port_state =inb (gpiommgpio->base + ports[i]); +for_each_set_clump8 (offset, gpio_mask, mask, ngpio) { + port_addr = gpiommgpio->base + ports[offset /8 ]; + port_state =inb (port_addr) & gpio_mask; -/* store acquired bits at respective bits array offset */ - bits[word_index] |= (port_state << word_offset) & word_mask; +bitmap_set_value8 (bits, ngpio, port_state, offset); }return 0 ;
@@ -242,37 +222,28 @@ 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; +unsigned int offset; +unsigned long gpio_mask; +const unsigned int ngpio =ARRAY_SIZE (ports) *8 ; +size_t index; +unsigned int port_addr;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 ; - } +for_each_set_clump8 (offset, gpio_mask, mask, ngpio) { + index = offset /8 ; + port_addr = gpiommgpio->base + ports[index]; - port = i / gpio_reg_size; - out_port = (port >2 ) ? port +1 : port; - bitmask = mask[BIT_WORD (i)] & bits[BIT_WORD (i)]; + bitmask =bitmap_get_value8 (bits, ngpio, offset) & gpio_mask;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[index] &= ~gpio_mask; + gpiommgpio->out_state[index] |= bitmask; +outb (gpiommgpio->out_state[index], port_addr);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.21.0
PATCH v12 6/11 gpio: ws16c48: Utilize for_each_set_clump8 macro
In reply to William Breathitt GrayReplace verbose implementation in get_multiple/set_multiple callbacks with for_each_set_clump8 macro to simplify code and improve clarity.
--- drivers/gpio/gpio-ws16c48.c | 72 +++++++++++-------------------------- 1 file changed, 20 insertions(+), 52 deletions(-)
drivers/gpio/gpio-ws16c48.c +20 -52
@@ -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 ); +unsigned int offset; +unsigned long gpio_mask; +unsigned int port_addr;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; +for_each_set_clump8 (offset, gpio_mask, mask, chip->ngpio) { + port_addr = ws16c48gpio->base + offset /8 ; + port_state =inb (port_addr) & gpio_mask; -/* 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) & word_mask; +bitmap_set_value8 (bits, chip->ngpio, port_state, offset); }return 0 ;
@@ -203,39 +180,30 @@ 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; -unsigned int iomask; +unsigned int offset; +unsigned long gpio_mask; +size_t index; +unsigned int port_addr;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_clump8 (offset, gpio_mask, mask, chip->ngpio) { + index = offset /8 ; + port_addr = ws16c48gpio->base + index;/* mask out GPIO configured for input */ - iomask = mask[BIT_WORD (i)] & ~ws16c48gpio->io_state[port]; - bitmask = iomask & bits[BIT_WORD (i)]; + gpio_mask &= ~ws16c48gpio->io_state[index]; + bitmask =bitmap_get_value8 (bits, chip->ngpio, offset) & + gpio_mask;raw_spin_lock_irqsave (&ws16c48gpio->lock, flags);/* update output state data and set device gpio register */ - ws16c48gpio->out_state[port] &= ~iomask; - ws16c48gpio->out_state[port] |= bitmask; -outb (ws16c48gpio->out_state[port], ws16c48gpio->base + port); + ws16c48gpio->out_state[index] &= ~gpio_mask; + ws16c48gpio->out_state[index] |= bitmask; +outb (ws16c48gpio->out_state[index], port_addr);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.21.0
PATCH v12 7/11 gpio: pci-idio-16: Utilize for_each_set_clump8 macro
In reply to William Breathitt GrayReplace verbose implementation in get_multiple/set_multiple callbacks with for_each_set_clump8 macro to simplify code and improve clarity.
--- drivers/gpio/gpio-pci-idio-16.c | 75 ++++++++++++--------------------- 1 file changed, 28 insertions(+), 47 deletions(-)
drivers/gpio/gpio-pci-idio-16.c +28 -47
@@ -108,45 +108,24 @@ static int idio_16_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask, unsigned long *bits) { 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; +unsigned int offset; +unsigned long gpio_mask;void __iomem *ports[] = { &idio16gpio->reg->out0_7, &idio16gpio->reg->out8_15, &idio16gpio->reg->in0_7, &idio16gpio->reg->in8_15, }; +const unsigned int ngpio =ARRAY_SIZE (ports) *8 ; +void __iomem *port_addr; +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 ; +for_each_set_clump8 (offset, gpio_mask, mask, ngpio) { + port_addr = ports[offset /8 ]; + port_state =ioread8 (port_addr) & gpio_mask; -/* 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 =ioread8 (ports[i]); - -/* store acquired bits at respective bits array offset */ - bits[word_index] |= (port_state << word_offset) & word_mask; +bitmap_set_value8 (bits, ngpio, port_state, offset); }return 0 ;
@@ -186,30 +165,32 @@ 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); +unsigned int offset; +unsigned long gpio_mask; +void __iomem *ports[] = { + &idio16gpio->reg->out0_7, &idio16gpio->reg->out8_15, + }; +const unsigned int ngpio =ARRAY_SIZE (ports) *8 ; +size_t index; +void __iomem *port_addr; +unsigned int bitmask;unsigned long flags;unsigned int out_state; -raw_spin_lock_irqsave (&idio16gpio->lock, flags); +for_each_set_clump8 (offset, gpio_mask, mask, ngpio) { + index = offset /8 ; + port_addr = ports[index]; -/* 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); - } + bitmask =bitmap_get_value8 (bits, ngpio, offset) & gpio_mask; + +raw_spin_lock_irqsave (&idio16gpio->lock, flags); -/* shift to next output line word */ - *mask >>=8 ; + out_state =ioread8 (port_addr) & ~gpio_mask; + out_state |= bitmask; +iowrite8 (out_state, port_addr); -/* 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.21.0
PATCH v12 8/11 gpio: pcie-idio-24: Utilize for_each_set_clump8 macro
In reply to William Breathitt GrayReplace verbose implementation in get_multiple/set_multiple callbacks with for_each_set_clump8 macro to simplify code and improve clarity.
--- drivers/gpio/gpio-pcie-idio-24.c | 111 ++++++++++++------------------- 1 file changed, 42 insertions(+), 69 deletions(-)
drivers/gpio/gpio-pcie-idio-24.c +42 -69
@@ -198,52 +198,35 @@ static int idio_24_gpio_get_multiple(struct gpio_chip *chip, unsigned long *mask, unsigned long *bits) { 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; +unsigned int offset; +unsigned long gpio_mask;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 unsigned int ngpio =ARRAY_SIZE (ports) *8 ; +size_t index; +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_clump8 (offset, gpio_mask, mask, ngpio) { + index = offset /8 ;/* read bits from current gpio port (port 6 is TTL GPIO) */ -if (i <6 ) - port_state =ioread8 (ports[i]); +if (index <6 ) + port_state =ioread8 (ports[index]);else if (ioread8 (&idio24gpio->reg->ctl) & out_mode_mask) port_state =ioread8 (&idio24gpio->reg->ttl_out0_7);else port_state =ioread8 (&idio24gpio->reg->ttl_in0_7); -/* store acquired bits at respective bits array offset */ - bits[word_index] |= (port_state << word_offset) & word_mask; + port_state &= gpio_mask; + +bitmap_set_value8 (bits, ngpio, port_state, offset); }return 0 ;
@@ -294,59 +277,49 @@ static void idio_24_gpio_set_multiple(struct gpio_chip *chip, unsigned long *mask, unsigned long *bits) { struct idio_24_gpio *const idio24gpio = gpiochip_get_data(chip); -size_t i; -unsigned long bits_offset; +unsigned int 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;void __iomem *ports[] = { &idio24gpio->reg->out0_7, &idio24gpio->reg->out8_15, &idio24gpio->reg->out16_23 }; +const unsigned int ngpio =ARRAY_SIZE (ports) *8 ; +size_t index; +unsigned int bitmask; +unsigned long flags; +unsigned int out_state;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 */ -continue ; - } -raw_spin_lock_irqsave (&idio24gpio->lock, flags); +for_each_set_clump8 (offset, gpio_mask, mask, ngpio) { + index = offset /8 ; -/* process output lines */ - out_state =ioread8 (ports[i]) & ~gpio_mask; - out_state |= (*bits >> bits_offset) & gpio_mask; -iowrite8 (out_state, ports[i]); + bitmask =bitmap_get_value8 (bits, ngpio, offset) & gpio_mask; -raw_spin_unlock_irqrestore (&idio24gpio->lock, flags); - } +raw_spin_lock_irqsave (&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 ; +/* read bits from current gpio port (port 6 is TTL GPIO) */ +if (index <6 ) { + out_state =ioread8 (ports[index]); + }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 ; + } -/* handle TTL output */ -raw_spin_lock_irqsave (&idio24gpio->lock, flags); +/* set requested bit states */ + out_state &= ~gpio_mask; + out_state |= bitmask; -/* 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); +/* write bits for current gpio port (port 6 is TTL GPIO) */ +if (index <6 ) +iowrite8 (out_state, ports[index]); +else +iowrite8 (out_state, &idio24gpio->reg->ttl_out0_7); -raw_spin_unlock_irqrestore (&idio24gpio->lock, flags); +raw_spin_unlock_irqrestore (&idio24gpio->lock, flags); + } }static void idio_24_irq_ack (struct irq_data *data)
Signature · 1 line
2.21.0
PATCH v12 9/11 gpio: uniphier: Utilize for_each_set_clump8 macro
In reply to William Breathitt GrayReplace verbose implementation in set_multiple callback with for_each_set_clump8 macro to simplify code and improve clarity. An improvement in this case is that banks that are not masked will now be skipped.
--- drivers/gpio/gpio-uniphier.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-)
drivers/gpio/gpio-uniphier.c +6 -10
@@ -15,9 +15,6 @@#include <linux/spinlock.h> #include <dt-bindings/gpio/uniphier-gpio.h> -#define UNIPHIER_GPIO_BANK_MASK \ - GENMASK((UNIPHIER_GPIO_LINES_PER_BANK) - 1, 0) -#define UNIPHIER_GPIO_IRQ_MAX_NUM 24#define UNIPHIER_GPIO_PORT_DATA 0x0/* data */
@@ -147,15 +144,14 @@ static void uniphier_gpio_set(struct gpio_chip *chip,static void uniphier_gpio_set_multiple (struct gpio_chip *chip,unsigned long *mask,unsigned long *bits) { -unsigned int bank, shift, bank_mask, bank_bits; -int i; +unsigned int i; +unsigned long bank_mask; +unsigned int bank; +unsigned int bank_bits; -for (i =0 ; i < chip->ngpio; i +=UNIPHIER_GPIO_LINES_PER_BANK ) { +for_each_set_clump8 (i, bank_mask, mask, chip->ngpio) { bank = i /UNIPHIER_GPIO_LINES_PER_BANK ; - shift = i %BITS_PER_LONG ; - bank_mask = (mask[BIT_WORD (i)] >> shift) & -UNIPHIER_GPIO_BANK_MASK ; - bank_bits = bits[BIT_WORD (i)] >> shift; + bank_bits =bitmap_get_value8 (bits, chip->ngpio, i);uniphier_gpio_bank_write (chip, bank,UNIPHIER_GPIO_PORT_DATA , bank_mask, bank_bits);
Signature · 1 line
2.21.0
PATCH v12 10/11 thermal: intel: intel_soc_dts_iosf: Utilize for_each_set_clump8 macro
In reply to William Breathitt GrayUtilize for_each_set_clump8 macro, and the bitmap_set_value8 and bitmap_get_value8 functions, where appropriate. In addition, remove the now unnecessary temp_mask and temp_shift members of the intel_soc_dts_sensor_entry structure.
--- drivers/thermal/intel/intel_soc_dts_iosf.c | 29 +++++++++++++--------- drivers/thermal/intel/intel_soc_dts_iosf.h | 2 -- 2 files changed, 17 insertions(+), 14 deletions(-)
drivers/thermal/intel/intel_soc_dts_iosf.c +17 -12
@@ -15,6 +15,7 @@#define pr_fmt (fmt) KBUILD_MODNAME ": " fmt +#include <linux/bitops.h> #include <linux/module.h> #include <linux/slab.h> #include <linux/interrupt.h>
@@ -111,6 +112,7 @@ static int update_trip_temp(struct intel_soc_dts_sensor_entry *dts, {int status;u32 temp_out; +unsigned long update_ptps;u32 out;u32 store_ptps;u32 store_ptmc;
@@ -129,8 +131,9 @@ static int update_trip_temp(struct intel_soc_dts_sensor_entry *dts,if (status)return status; - out = (store_ptps & ~(0xFF << (thres_index *8 ))); - out |= (temp_out &0xFF ) << (thres_index *8 ); + update_ptps = store_ptps; +bitmap_set_value8 (&update_ptps,32 , temp_out &0xFF , thres_index *8 ); + out = update_ptps; status =iosf_mbi_write (BT_MBI_UNIT_PMC ,MBI_REG_WRITE ,SOC_DTS_OFFSET_PTPS , out);if (status)
@@ -232,6 +235,7 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd,u32 out;struct intel_soc_dts_sensor_entry *dts;struct intel_soc_dts_sensors *sensors; +unsigned long temp_raw; dts = tzd->devdata; sensors = dts->sensors;
@@ -240,7 +244,8 @@ static int sys_get_curr_temp(struct thermal_zone_device *tzd,if (status)return status; - out = (out & dts->temp_mask) >> dts->temp_shift; + temp_raw = out; + out =bitmap_get_value8 (&temp_raw,32 , dts->id *8 ); out -=SOC_DTS_TJMAX_ENCODING ; *temp = sensors->tj_max - out *1000 ;
@@ -290,10 +295,13 @@ static int add_dts_thermal_zone(int id, struct intel_soc_dts_sensor_entry *dts, {char name[10 ];int trip_count =0 ; +int writable_trip_count =0 ;int trip_mask =0 ;u32 store_ptps;int ret; -int i; +unsigned int i; +unsigned long trip; +unsigned long ptps;/* Store status to restor on exit */ ret = iosf_mbi_read(BT_MBI_UNIT_PMC, MBI_REG_READ,
@@ -302,11 +310,10 @@ static int add_dts_thermal_zone(int id, struct intel_soc_dts_sensor_entry *dts, goto err_ret; dts->id = id; - dts->temp_mask =0x00FF << (id *8 ); - dts->temp_shift = id *8 ;if (notification_support) { trip_count =min (SOC_MAX_DTS_TRIPS , trip_cnt); - trip_mask =BIT (trip_count - read_only_trip_cnt) -1 ; + writable_trip_count = trip_count - read_only_trip_cnt; + trip_mask =GENMASK (writable_trip_count -1 ,0 ); }/* Check if the writable trip we provide is not used by BIOS */
@@ -315,11 +322,9 @@ static int add_dts_thermal_zone(int id, struct intel_soc_dts_sensor_entry *dts,if (ret) trip_mask =0 ;else { -for (i =0 ; i < trip_count; ++i) { -if (trip_mask &BIT (i)) -if (store_ptps & (0xff << (i *8 ))) - trip_mask &= ~BIT (i); - } + ptps = store_ptps; +for_each_set_clump8 (i, trip, &ptps, writable_trip_count *8 ) + trip_mask &= ~BIT (i /8 ); } dts->trip_mask = trip_mask; dts->trip_count = trip_count;
drivers/thermal/intel/intel_soc_dts_iosf.h +0 -2
@@ -33,8 +33,6 @@ struct intel_soc_dts_sensors;struct intel_soc_dts_sensor_entry {int id; -u32 temp_mask; -u32 temp_shift;u32 store_status;u32 trip_mask;u32 trip_count;
Signature · 1 line
2.21.0
PATCH v12 11/11 gpio: 74x164: Utilize the for_each_set_clump8 macro
In reply to William Breathitt GrayReplace verbose implementation in set_multiple callback with for_each_set_clump8 macro to simplify code and improve clarity.
--- drivers/gpio/gpio-74x164.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
drivers/gpio/gpio-74x164.c +10 -10
@@ -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,19 @@ 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 int offset; +unsigned long bankmask; +const unsigned int ngpio = chip->registers *8 ; +size_t bank; +unsigned int 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, ngpio) { + bank = (chip->registers -1 ) - (offset /8 ); + bitmask =bitmap_get_value8 (bits, ngpio, 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);
Signature · 1 line
2.21.0
Re: [PATCH v12 11/11] gpio: 74x164: Utilize the for_each_set_clump8 macro
Quoted 2 lines · William Breathitt Gray
Replace verbose implementation in
set_multiplecallback withfor_each_set_clump8macro to simplify code and improve clarity.
I guess it's better to group this with other GPIO related patches.
Signature · 2 lines
With Best Regards, Andy Shevchenko