On Mon, Nov 09, 2020 at 07:22:20PM +0200, Andy Shevchenko wrote:

On Mon, Nov 09, 2020 at 12:11:40PM -0500, William Breathitt Gray wrote:

quoted

On Mon, Nov 09, 2020 at 10:15:29PM +0530, Syed Nayyar Waris wrote:

quoted

On Mon, Nov 09, 2020 at 03:41:53PM +0100, Arnd Bergmann wrote:

...

quoted

quoted

 static inline void bitmap_set_value(unsigned long *map,
-                                    unsigned long value,
+                                    unsigned long value, const size_t length,
                                     unsigned long start, unsigned long nbits)
 {
         const size_t index = BIT_WORD(start);

@@ -15,6 +15,10 @@ static inline void bitmap_set_value(unsigned long *map,
         } else {
                 map[index + 0] &= ~BITMAP_FIRST_WORD_MASK(start);
                 map[index + 0] |= value << offset;
+
+               if (index + 1 >= length)
+                       __builtin_unreachable();
+
                 map[index + 1] &= ~BITMAP_LAST_WORD_MASK(start + nbits);
                 map[index + 1] |= value >> space;
         }

Hi Syed,

Let's rename 'length' to 'nbits' as Arnd suggested, and rename 'nbits'
to value_width.

length here is in longs. I guess this is the point of entire patch.

Ah yes, this should become 'const unsigned long nbits' and represent the
length of the bitmap in bits and not longs.

But to me sounds like it would be better to have simply bitmap_set_value64() /
bitmap_set_value32() with proper optimization done and forget about variadic
ones for now.

The gpio-xilinx driver can have arbitrary sizes for width[0] and
width[1], so unfortunately that means we don't know the start position
nor the width of the value beforehand.

William Breathitt Gray