| Message ID | c919f6c2ff54a29114bd5e566997f11e5ebf29c3.1525785532.git.vilhelm.gray@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Introduce the for_each_set_port_word macro | expand |
On Tue, May 8, 2018 at 4:26 PM, William Breathitt Gray <vilhelm.gray@gmail.com> wrote: > 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. Isn't that idea we discussed some time ago? In any case, part "port" is too specific for a generic function like this. Please, get rid of it completely. No-one knows what port means here. Just makes a lot of confusion. > --- a/lib/find_bit.c > +++ b/lib/find_bit.c > @@ -20,6 +20,7 @@ > #include <linux/bitmap.h> > #include <linux/export.h> > #include <linux/kernel.h> > +#include <linux/types.h> No need. It's included by bitmap.h IIRC.
On Sun, May 13, 2018 at 06:06:42PM +0300, Andy Shevchenko wrote: >On Tue, May 8, 2018 at 4:26 PM, William Breathitt Gray ><vilhelm.gray@gmail.com> wrote: >> 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. > >Isn't that idea we discussed some time ago? That's right, I found the time to implement the macro suggestion you made during the get_multiple/set_multiple patchset for the PC104 GPIO drivers a while ago. This macro greatly simplifies the callback function implementations in those drivers and reduces the repeated code that kept appearing among those drivers. Hopefully it can be useful for other drivers as well. > >In any case, part "port" is too specific for a generic function like >this. Please, get rid of it completely. No-one knows what port means >here. Just makes a lot of confusion. Okay, I'll come up with a better name and submit a version 3 of this patchset. > >> --- a/lib/find_bit.c >> +++ b/lib/find_bit.c >> @@ -20,6 +20,7 @@ >> #include <linux/bitmap.h> >> #include <linux/export.h> >> #include <linux/kernel.h> > >> +#include <linux/types.h> > >No need. It's included by bitmap.h IIRC. Ah, you are correct, I'll remove this line then. William Breathitt Gray > > >-- >With Best Regards, >Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, May 14, 2018 at 4:04 PM, William Breathitt Gray <vilhelm.gray@gmail.com> wrote: > On Sun, May 13, 2018 at 06:06:42PM +0300, Andy Shevchenko wrote: >>On Tue, May 8, 2018 at 4:26 PM, William Breathitt Gray >><vilhelm.gray@gmail.com> wrote: >>> 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. >> >>Isn't that idea we discussed some time ago? > > That's right, I found the time to implement the macro suggestion you > made during the get_multiple/set_multiple patchset for the PC104 GPIO > drivers a while ago. So, if you find it appropriate, please add Suggested-by. > This macro greatly simplifies the callback function implementations in > those drivers and reduces the repeated code that kept appearing among > those drivers. Hopefully it can be useful for other drivers as well. Yes, I like the idea! >>In any case, part "port" is too specific for a generic function like >>this. Please, get rid of it completely. No-one knows what port means >>here. Just makes a lot of confusion. > > Okay, I'll come up with a better name and submit a version 3 of this > patchset. I also forgot to mention that kernel-doc should accompany function definition (in *.c), and not a declaration (in *.h).
diff --git a/include/asm-generic/bitops/find.h b/include/asm-generic/bitops/find.h index 8a1ee10014de..c59514547e63 100644 --- a/include/asm-generic/bitops/find.h +++ b/include/asm-generic/bitops/find.h @@ -2,6 +2,8 @@ #ifndef _ASM_GENERIC_BITOPS_FIND_H_ #define _ASM_GENERIC_BITOPS_FIND_H_ +#include <linux/types.h> + #ifndef find_next_bit /** * find_next_bit - find the next set bit in a memory region @@ -80,4 +82,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_ */ diff --git a/include/linux/bitops.h b/include/linux/bitops.h index 4cac4e1a72ff..c3b7caf4ad2d 100644 --- a/include/linux/bitops.h +++ b/include/linux/bitops.h @@ -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; diff --git a/lib/find_bit.c b/lib/find_bit.c index ee3df93ba69a..628c94d63d6b 100644 --- a/lib/find_bit.c +++ b/lib/find_bit.c @@ -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);
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. Cc: Arnd Bergmann <arnd@arndb.de> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> --- include/asm-generic/bitops/find.h | 26 ++++++++++++++++++++++++++ include/linux/bitops.h | 9 +++++++++ lib/find_bit.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+)