On Wed, 7 Dec 2016 12:23:56 +0100, Sebastian Frias wrote: > On 07/12/16 12:05, Jakub Kicinski wrote: > > On Wed, 7 Dec 2016 11:00:57 +0100, Sebastian Frias wrote: > >> On 07/12/16 09:42, Kalle Valo wrote: > >>> Sebastian Frias <s...@laposte.net> writes: > >>> > >>>> Introduce GENVALUE(msb, lsb, value) macro to ease dealing with > >>>> continuous bitfields, just as BIT(x) does for single bits. > >>>> > >>>> GENVALUE_ULL(msb, lsb, value) macro is also added. > >>>> > >>>> This is useful mostly for creating values to be packed together > >>>> via OR operations, ex: > >>>> > >>>> u32 val = 0x11110000; > >>>> val |= GENVALUE(19, 12, 0x5a); > >>>> > >>>> now 'val = 0x1115a000' > >>>> > >>>> > >>>> Signed-off-by: Sebastian Frias <s...@laposte.net> > >>>> Link: https://marc.info/?l=linux-kernel&m=148094498711000&w=2 > >>>> --- > >>>> > >>>> Change in v2: > >>>> - rename the macro to GENVALUE as proposed by Linus > >>>> - longer comment attempts to show use case for the macro as > >>>> proposed by Borislav > >>>> > >>>> Change in v3: > >>>> - use BUILD_BUG_ON_ZERO() to break if some input parameters > >>>> (essentially 'lsb' but also 'msb') are not constants as > >>>> proposed by Linus. > >>>> Indeed, 'lsb' is used twice so it cannot have side-effects; > >>>> 'msb' is subjected to same constraints for consistency. > >>> > >>> (I missed there was v3 already, but I'll repeat what I said in v1.) > >>> > >>> Please check FIELD_PREP() from include/linux/bitfield.h, doesn't it > >>> already do the same? > >> > >> Indeed, it appears to do the same :-) > >> Any reason why "include/linux/bitfield.h" is not included by default in > >> bitops.h? > > > > Hi! > > > > The code is in a separate header because of circular dependencies > > (coming from bug.h including bitops.h, IIRC). You could possibly add an > > include of bitfield.h in bitops.h if you're very careful, I haven't > > tried TBH :) > > > > Well, the following seems to work just fine: > > diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h > index f6505d8..24c7480 100644 > --- a/include/linux/bitfield.h > +++ b/include/linux/bitfield.h > @@ -15,8 +15,6 @@ > #ifndef _LINUX_BITFIELD_H > #define _LINUX_BITFIELD_H > > -#include <linux/bug.h> > -
That would break users who include bitfield.h directly and don't include bug.h, no? > /* > * Bitfield access macros > * > diff --git a/include/linux/bitops.h b/include/linux/bitops.h > index a83c822..7e5fab8 100644 > --- a/include/linux/bitops.h > +++ b/include/linux/bitops.h > @@ -24,6 +24,8 @@ > #define GENMASK_ULL(h, l) \ > (((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h)))) > > +#include "bitfield.h" > + > extern unsigned int __sw_hweight8(unsigned int w); > extern unsigned int __sw_hweight16(unsigned int w); > extern unsigned int __sw_hweight32(unsigned int w); > > > Is there a way to be sure it works in all cases? Otherwise > I could just submit that, right?
