Re: [PATCH v6 1/8] bitops: Introduce the for_each_set_clump8 macro

2019-01-11 Thread Rasmus Villemoes
On 19/12/2018 07.00, William Breathitt Gray wrote:
> +/**
> + * bitmap_set_value8 - set an 8-bit value within a memory region
> + * @bitmap: address to the bitmap memory region
> + * @value: the 8-bit value
> + * @start: bit offset of the 8-bit value
> + */
> +void bitmap_set_value8(unsigned long *const bitmap,
> +const unsigned long *const value,
> +const unsigned int start)
> +{
> + const size_t index = BIT_WORD(start);
> + const unsigned int offset = start % BITS_PER_LONG;
> + const unsigned long mask = GENMASK(7, offset);
> +
> + bitmap[index] &= ~mask;
> + bitmap[index] |= (*value << offset) & mask;
> +}

errh, why pass the value by reference? That seems rather odd. Also, if
anybody passes a value that doesn't fit in 8 bits, they get what they
deserve; IOW I don't see a reason for the masking in the last line, but
if its convenient for the users, keep it. In any case, please change the
type of value.

Rasmus


Re: [PATCH v6 1/8] bitops: Introduce the for_each_set_clump8 macro

2019-01-11 Thread Linus Walleij
On Wed, Dec 19, 2018 at 7:00 AM William Breathitt Gray
 wrote:

> 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.
>
> Suggested-by: Andy Shevchenko 
> Suggested-by: Rasmus Villemoes 
> Cc: Arnd Bergmann 
> Cc: Andrew Morton 
> Signed-off-by: William Breathitt Gray 

I am still waiting for feedback from the core lib maintainers on this patch.
If I just get an ACK I can apply it to the GPIO tree, if noone else cares
I might just go in and apply it, but it feels slightly outside of my
jurisdiction.

Maybe people were just a bit stressed out around christmas?

What about looking at it now?

Yours,
Linus Walleij


[PATCH v6 1/8] bitops: Introduce the for_each_set_clump8 macro

2018-12-18 Thread 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_value8 and bitmap_set_value8 functions are introduced to
respectively get and set an 8-bit value in a bitmap memory region.

Suggested-by: Andy Shevchenko 
Suggested-by: Rasmus Villemoes 
Cc: Arnd Bergmann 
Cc: Andrew Morton 
Signed-off-by: William Breathitt Gray 
---
 include/asm-generic/bitops/find.h | 14 +++
 include/linux/bitops.h|  5 +++
 lib/find_bit.c| 63 +++
 3 files changed, 82 insertions(+)

diff --git a/include/asm-generic/bitops/find.h 
b/include/asm-generic/bitops/find.h
index 8a1ee10014de..457b93e6f5c9 100644
--- a/include/asm-generic/bitops/find.h
+++ b/include/asm-generic/bitops/find.h
@@ -80,4 +80,18 @@ extern unsigned long find_first_zero_bit(const unsigned long 
*addr,
 
 #endif /* CONFIG_GENERIC_FIND_FIRST_BIT */
 
+unsigned int bitmap_get_value8(const unsigned long *const bitmap,
+  const unsigned int start);
+
+void bitmap_set_value8(unsigned long *const bitmap,
+  const unsigned long *const 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_ */
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index 705f7c442691..61c10f20079e 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -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;
diff --git a/lib/find_bit.c b/lib/find_bit.c
index ee3df93ba69a..2e56d2b907bc 100644
--- a/lib/find_bit.c
+++ b/lib/find_bit.c
@@ -218,3 +218,66 @@ 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
+ * @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 int bitmap_get_value8(const unsigned long *const bitmap,
+  const unsigned int start)
+{
+   const size_t index = BIT_WORD(start);
+   const unsigned int offset = start % BITS_PER_LONG;
+
+   return (bitmap[index] >> offset) & 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
+ * @value: the 8-bit value
+ * @start: bit offset of the 8-bit value
+ */
+void bitmap_set_value8(unsigned long *const bitmap,
+  const unsigned long *const value,
+  const unsigned int start)
+{
+   const size_t index = BIT_WORD(start);
+   const unsigned int offset = start % BITS_PER_LONG;
+   const unsigned long mask = GENMASK(7, offset);
+
+   bitmap[index] &= ~mask;
+   bitmap[index] |= (*value << offset) & mask;
+}
+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, offset);
+   if (!*clump)
+   continue;
+
+   return offset;
+   }
+
+   return size;
+}
+EXPORT_SYMBOL(find_next_clump8);
-- 
2.20.1