[PATCH] bitops: add equivalent of BIT(x) for bitfields

[PATCH] bitops: add equivalent of BIT(x) for bitfields

[Sebastian Frias]

Introduce SETBITFIELD(msb, lsb, value) macro to ease dealing with
continuous bitfields, just as BIT(x) does for single bits.

SETBITFIELD_ULL(msb, lsb, value) macro is also added.

Signed-off-by: Sebastian Frias <sf84@laposte.net>
---

Code protected with "#ifdef __KERNEL__" just as the BIT(x)
macros.

I would have preferred another name, like BITS(x) but it is
already used.

Suggestions for other names welcome.
---
 include/linux/bitops.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index a83c822..4659237 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -24,6 +24,20 @@
 #define GENMASK_ULL(h, l) \
 	(((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
 
+#ifdef	__KERNEL__
+/*
+ * Equivalent of BIT(x) but for contiguous bitfields
+ * SETBITFIELD(1, 0,0xff) = 0x00000003
+ * SETBITFIELD(3, 0,0xff) = 0x0000000f
+ * SETBITFIELD(15,8,0xff) = 0x0000ff00
+ * SETBITFIELD(6, 6,   1) = 0x00000040 == BIT(6)
+ */
+#define SETBITFIELD(msb, lsb, val)     \
+	(((val) << (lsb)) & (GENMASK((msb), (lsb))))
+#define SETBITFIELD_ULL(msb, lsb, val) \
+	(((val) << (lsb)) & (GENMASK_ULL((msb), (lsb))))
+#endif
+
 extern unsigned int __sw_hweight8(unsigned int w);
 extern unsigned int __sw_hweight16(unsigned int w);
 extern unsigned int __sw_hweight32(unsigned int w);
-- 
1.8.3.1

Re: [PATCH] bitops: add equivalent of BIT(x) for bitfields

[Borislav Petkov]

On Mon, Dec 05, 2016 at 02:36:07PM +0100, Sebastian Frias wrote:
> + * Equivalent of BIT(x) but for contiguous bitfields
> + * SETBITFIELD(1, 0,0xff) = 0x00000003
> + * SETBITFIELD(3, 0,0xff) = 0x0000000f
> + * SETBITFIELD(15,8,0xff) = 0x0000ff00
> + * SETBITFIELD(6, 6,   1) = 0x00000040 == BIT(6)
> + */
> +#define SETBITFIELD(msb, lsb, val)     \
> +	(((val) << (lsb)) & (GENMASK((msb), (lsb))))
> +#define SETBITFIELD_ULL(msb, lsb, val) \
> +	(((val) << (lsb)) & (GENMASK_ULL((msb), (lsb))))

What is the use case for these macros?

I'm thinking it would be better if they were really generic so that I
can be able to hand in a value and to say, set bits from [lsb, msb] to
this bit pattern. Because then you'd have to punch a hole in the value
with a mask and then OR-in the actual mask you want to have.

I.e., something like this:

#define SET_BITMASK(val, msb, lsb, mask)        \
        (val = (val & ~(GENMASK(msb, lsb))) | (mask << lsb))

So that you can be able to do:

unsigned int val = 0x11110000;

SET_BITMASK(val, 19, 12, 0x5a)
val = 0x1115a000;

I'm not sure, though, whether this won't make the actual use too cryptic
and people having to go look it up each time they use it instead of
actually doing the ORing in by hand which everyone can understand from
staring at it instead of seeing a funny macro SET_BITMASK().

Actually, you could simply add another macro which is called

GENMASK_MASK(msb, lsb, mask)

or so (yeah, yucky name) which gives you that arbitrary mask which you
then can use anywhere.

Because then it becomes more readable:

        val &= ~GENMASK(19, 12);
        val |= GENMASK_MASK(19, 12, 0xa5);

Now you know exactly what happens: first line punches the hole, second
ORs in the new mask.

Hmmm...

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

Re: [PATCH] bitops: add equivalent of BIT(x) for bitfields

[Sebastian Frias]

On 05/12/16 16:34, Borislav Petkov wrote:
> On Mon, Dec 05, 2016 at 02:36:07PM +0100, Sebastian Frias wrote:
>> + * Equivalent of BIT(x) but for contiguous bitfields
>> + * SETBITFIELD(1, 0,0xff) = 0x00000003
>> + * SETBITFIELD(3, 0,0xff) = 0x0000000f
>> + * SETBITFIELD(15,8,0xff) = 0x0000ff00
>> + * SETBITFIELD(6, 6,   1) = 0x00000040 == BIT(6)
>> + */
>> +#define SETBITFIELD(msb, lsb, val)     \
>> +	(((val) << (lsb)) & (GENMASK((msb), (lsb))))
>> +#define SETBITFIELD_ULL(msb, lsb, val) \
>> +	(((val) << (lsb)) & (GENMASK_ULL((msb), (lsb))))
> 
> What is the use case for these macros?
> 

Well, right now I'm using them for stuff like:

#define TIMEOUT_CLK_UNIT_MHZ       BIT(6)
#define BUS_CLK_FREQ_FOR_SD_CLK(x) SETBITFIELD(14,7,x)
...
    val = 0;
    val |= TIMEOUT_CLK_UNIT_MHZ;         /* unit: MHz */
    val |= BUS_CLK_FREQ_FOR_SD_CLK(200); /* SDIO clock: 200MHz */
...

which makes it very practical for writing macros for associated HW
documentation.

It is true though that the name SETBITFIELD may not be great, since the
macro is more about taking a value and massaging it to fit the given
bitfield so that it can be ORed to place said bits.
I wouldn't call it a mask, because IMHO it does not really mask (i.e.:
hides something).

> I'm thinking it would be better if they were really generic so that I
> can be able to hand in a value and to say, set bits from [lsb, msb] to
> this bit pattern. Because then you'd have to punch a hole in the value
> with a mask and then OR-in the actual mask you want to have.
> 
> I.e., something like this:
> 
> #define SET_BITMASK(val, msb, lsb, mask)        \
>         (val = (val & ~(GENMASK(msb, lsb))) | (mask << lsb))
> 
> So that you can be able to do:
> 
> unsigned int val = 0x11110000;
> 
> SET_BITMASK(val, 19, 12, 0x5a)
> val = 0x1115a000;
> 
> I'm not sure, though, whether this won't make the actual use too cryptic
> and people having to go look it up each time they use it instead of
> actually doing the ORing in by hand which everyone can understand from
> staring at it instead of seeing a funny macro SET_BITMASK().
> 

Well, we could have both, with your proposed SET_BITMASK() building on top
of the proposed SETBITFIELD:

#define SET_BITMASK(target, msb, lsb, val) \
   (target = (target | SETBITFIELD(msb, lsb, val)))

> Actually, you could simply add another macro which is called
> 
> GENMASK_MASK(msb, lsb, mask)
> 
> or so (yeah, yucky name) which gives you that arbitrary mask which you
> then can use anywhere.
> 
> Because then it becomes more readable:
> 
>         val &= ~GENMASK(19, 12);
>         val |= GENMASK_MASK(19, 12, 0xa5);
> 
> Now you know exactly what happens: first line punches the hole, second
> ORs in the new mask.
> 
> Hmmm...
> 

Ok, so you are basically proposing to keep the macro, but with a different
name (SETBITFIELD to GENMASK_MASK).

I think GENMASK_MASK may not be much better than SETBITFIELD.

Re: [PATCH] bitops: add equivalent of BIT(x) for bitfields

[Linus Torvalds]

On Mon, Dec 5, 2016 at 5:36 AM, Sebastian Frias <sf84@laposte.net> wrote:
> Introduce SETBITFIELD(msb, lsb, value) macro to ease dealing with
> continuous bitfields, just as BIT(x) does for single bits.
>
> SETBITFIELD_ULL(msb, lsb, value) macro is also added.

No. No, no, no.

Didn't we have this discussion already? Or was that for one of the
other silly naming things?

That thing doesn't "SET" anything at all. It generates a value, nothing more.

So the name is completely unacceptable. It follows the convention of
GENMASK, so maybe GENVALUE?

I also absolutely hate the stupid "big bit first" idiocy, but we did
that for GENMASK too, so I guess we're stuck with that retarded model.

Yes, I understand why it happened - people look at register definition
graphics, and the high bits are to the left.

But when you then read the documentation, it will still say things
like "bits 9 through 12 contain the value XYZ", because while
individual numbers are written MSB first, we actuall _read_ left to
right. You'd never give a range as "12 to 5", you'd say "5 to 12".

           Linus

Re: [PATCH] bitops: add equivalent of BIT(x) for bitfields

[Sebastian Frias]

On 05/12/16 18:13, Linus Torvalds wrote:
> On Mon, Dec 5, 2016 at 5:36 AM, Sebastian Frias <sf84@laposte.net> wrote:
>> Introduce SETBITFIELD(msb, lsb, value) macro to ease dealing with
>> continuous bitfields, just as BIT(x) does for single bits.
>>
>> SETBITFIELD_ULL(msb, lsb, value) macro is also added.
> 
> No. No, no, no.
> 
> Didn't we have this discussion already? Or was that for one of the
> other silly naming things?
> 
> That thing doesn't "SET" anything at all. It generates a value, nothing more.
> 
> So the name is completely unacceptable. It follows the convention of
> GENMASK, so maybe GENVALUE?
> 

Thanks for your input.
I was looking for suggestions on the name, thanks for yours, I will submit
a v2 with the name changed as you proposed.

> I also absolutely hate the stupid "big bit first" idiocy, but we did
> that for GENMASK too, so I guess we're stuck with that retarded model.
> 

Yes, I followed the same convention.

> Yes, I understand why it happened - people look at register definition
> graphics, and the high bits are to the left.
> 
> But when you then read the documentation, it will still say things
> like "bits 9 through 12 contain the value XYZ", because while
> individual numbers are written MSB first, we actuall _read_ left to
> right. You'd never give a range as "12 to 5", you'd say "5 to 12".
> 
>            Linus
> 

Re: [PATCH] bitops: add equivalent of BIT(x) for bitfields

[Geert Uytterhoeven]

On Mon, Dec 5, 2016 at 2:36 PM, Sebastian Frias <sf84@laposte.net> wrote:
> Introduce SETBITFIELD(msb, lsb, value) macro to ease dealing with
> continuous bitfields, just as BIT(x) does for single bits.

If it's a bitfield, why not calling it that way?

So what about BITFIELD(start ,size), like arch/tile/kernel/tile-desc_32.c has?

> SETBITFIELD_ULL(msb, lsb, value) macro is also added.

Confused by the need for a "value" parameter...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] bitops: add equivalent of BIT(x) for bitfields

[Sebastian Frias]

On 05/12/16 18:48, Geert Uytterhoeven wrote:
> On Mon, Dec 5, 2016 at 2:36 PM, Sebastian Frias <sf84@laposte.net> wrote:
>> Introduce SETBITFIELD(msb, lsb, value) macro to ease dealing with
>> continuous bitfields, just as BIT(x) does for single bits.
> 
> If it's a bitfield, why not calling it that way?
> 

I don't know if you saw v2 (or v3 for that matter), but the name was changed
to GENVALUE.
Also a small use case was added to the commit message:

"Introduce GENVALUE(msb, lsb, value) macro..."
"...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'"

> So what about BITFIELD(start ,size), like arch/tile/kernel/tile-desc_32.c has?
> 
>> SETBITFIELD_ULL(msb, lsb, value) macro is also added.
> 
> Confused by the need for a "value" parameter...

"value" is the value to be massaged (shifted, masked) into a [msb:lsb] bitfield.

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 

Re: [PATCH] bitops: add equivalent of BIT(x) for bitfields

[Geert Uytterhoeven]

On Tue, Dec 6, 2016 at 11:31 AM, Sebastian Frias <sf84@laposte.net> wrote:
> On 05/12/16 18:48, Geert Uytterhoeven wrote:
>> On Mon, Dec 5, 2016 at 2:36 PM, Sebastian Frias <sf84@laposte.net> wrote:
>>> Introduce SETBITFIELD(msb, lsb, value) macro to ease dealing with
>>> continuous bitfields, just as BIT(x) does for single bits.
>>
>> If it's a bitfield, why not calling it that way?
>
> I don't know if you saw v2 (or v3 for that matter), but the name was changed
> to GENVALUE.

... which means "generate a value"??

> Also a small use case was added to the commit message:
>
> "Introduce GENVALUE(msb, lsb, value) macro..."
> "...This is useful mostly for creating values to be packed together
> via OR operations, ex:
>
>    u32 val = 0x11110000;
>    val |= GENVALUE(19, 12, 0x5a);

"val |= 0x5a << 12;" looks much more readable to me...

> now 'val = 0x1115a000'"
>
>> So what about BITFIELD(start ,size), like arch/tile/kernel/tile-desc_32.c has?
>>
>>> SETBITFIELD_ULL(msb, lsb, value) macro is also added.
>>
>> Confused by the need for a "value" parameter...
>
> "value" is the value to be massaged (shifted, masked) into a [msb:lsb] bitfield.

OK. So it inserts a value into a bitfield.

Yes, that can be useful. Now let's find a sensible name for this.
Perhaps inspired by a PowerPC mnemonic? At least that would be more
obvious than "GENVALUE", IMHO...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] bitops: add equivalent of BIT(x) for bitfields

[Sebastian Frias]

On 06/12/16 11:42, Geert Uytterhoeven wrote:
> On Tue, Dec 6, 2016 at 11:31 AM, Sebastian Frias <sf84@laposte.net> wrote:
>> On 05/12/16 18:48, Geert Uytterhoeven wrote:
>>> On Mon, Dec 5, 2016 at 2:36 PM, Sebastian Frias <sf84@laposte.net> wrote:
>>>> Introduce SETBITFIELD(msb, lsb, value) macro to ease dealing with
>>>> continuous bitfields, just as BIT(x) does for single bits.
>>>
>>> If it's a bitfield, why not calling it that way?
>>
>> I don't know if you saw v2 (or v3 for that matter), but the name was changed
>> to GENVALUE.
> 
> ... which means "generate a value"??
> 

Yes.
Although I'm not sure if I understood the essence of your point.
Are you suggesting that the name should be GENERATE_A_VALUE?

There's already GENMASK, which "generates a mask".

>> Also a small use case was added to the commit message:
>>
>> "Introduce GENVALUE(msb, lsb, value) macro..."
>> "...This is useful mostly for creating values to be packed together
>> via OR operations, ex:
>>
>>    u32 val = 0x11110000;
>>    val |= GENVALUE(19, 12, 0x5a);
> 
> "val |= 0x5a << 12;" looks much more readable to me...
> 

Well, the idea behind this is that one can use it like:

(see https://marc.info/?l=linux-kernel&m=148095872915717&w=2)

...
#define TIMEOUT_CLK_UNIT_MHZ       BIT(6)
#define BUS_CLK_FREQ_FOR_SD_CLK(x) GENVALUE(14,7,x)
...
    val = 0;
    val |= TIMEOUT_CLK_UNIT_MHZ;         /* unit: MHz */
    val |= BUS_CLK_FREQ_FOR_SD_CLK(200); /* SDIO clock: 200MHz */
...

which makes it very practical for writing macros for associated HW
documentation.

>> now 'val = 0x1115a000'"
>>
>>> So what about BITFIELD(start ,size), like arch/tile/kernel/tile-desc_32.c has?
>>>
>>>> SETBITFIELD_ULL(msb, lsb, value) macro is also added.
>>>
>>> Confused by the need for a "value" parameter...
>>
>> "value" is the value to be massaged (shifted, masked) into a [msb:lsb] bitfield.
> 
> OK. So it inserts a value into a bitfield.
> 
> Yes, that can be useful. Now let's find a sensible name for this.
> Perhaps inspired by a PowerPC mnemonic? At least that would be more
> obvious than "GENVALUE", IMHO...

I'm open to suggestions.

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 

Re: [PATCH] bitops: add equivalent of BIT(x) for bitfields

[Geert Uytterhoeven]

Hi Sebastian,

On Tue, Dec 6, 2016 at 12:03 PM, Sebastian Frias <sf84@laposte.net> wrote:
> On 06/12/16 11:42, Geert Uytterhoeven wrote:
>> On Tue, Dec 6, 2016 at 11:31 AM, Sebastian Frias <sf84@laposte.net> wrote:
>>> On 05/12/16 18:48, Geert Uytterhoeven wrote:
>>>> On Mon, Dec 5, 2016 at 2:36 PM, Sebastian Frias <sf84@laposte.net> wrote:
>>>>> Introduce SETBITFIELD(msb, lsb, value) macro to ease dealing with
>>>>> continuous bitfields, just as BIT(x) does for single bits.
>>>>
>>>> If it's a bitfield, why not calling it that way?
>>>
>>> I don't know if you saw v2 (or v3 for that matter), but the name was changed
>>> to GENVALUE.
>>
>> ... which means "generate a value"??
>>
>
> Yes.
> Although I'm not sure if I understood the essence of your point.
> Are you suggesting that the name should be GENERATE_A_VALUE?

No. I mean that "value" is a way too generic name.
Hence "GENVALUE" may be suitable for a macro local to a driver, but is way
too generic and fuzzy for a global function.

> There's already GENMASK, which "generates a mask".

Yes. And it generates a (bit)mask, which is clear from its name.
But a "value" is just too generic for a global function, and make me think of
a pseudo-random number generator ;-)

>>> Also a small use case was added to the commit message:
>>>
>>> "Introduce GENVALUE(msb, lsb, value) macro..."
>>> "...This is useful mostly for creating values to be packed together
>>> via OR operations, ex:
>>>
>>>    u32 val = 0x11110000;
>>>    val |= GENVALUE(19, 12, 0x5a);
>>
>> "val |= 0x5a << 12;" looks much more readable to me...
>>
>
> Well, the idea behind this is that one can use it like:
>
> (see https://marc.info/?l=linux-kernel&m=148095872915717&w=2)
>
> ...
> #define TIMEOUT_CLK_UNIT_MHZ       BIT(6)
> #define BUS_CLK_FREQ_FOR_SD_CLK(x) GENVALUE(14,7,x)
> ...
>     val = 0;
>     val |= TIMEOUT_CLK_UNIT_MHZ;         /* unit: MHz */
>     val |= BUS_CLK_FREQ_FOR_SD_CLK(200); /* SDIO clock: 200MHz */
> ...
>
> which makes it very practical for writing macros for associated HW
> documentation.

Actually I more like the SETBITFIELD name...

>>> now 'val = 0x1115a000'"
>>>
>>>> So what about BITFIELD(start ,size), like arch/tile/kernel/tile-desc_32.c has?
>>>>
>>>>> SETBITFIELD_ULL(msb, lsb, value) macro is also added.
>>>>
>>>> Confused by the need for a "value" parameter...
>>>
>>> "value" is the value to be massaged (shifted, masked) into a [msb:lsb] bitfield.
>>
>> OK. So it inserts a value into a bitfield.
>>
>> Yes, that can be useful. Now let's find a sensible name for this.
>> Perhaps inspired by a PowerPC mnemonic? At least that would be more
>> obvious than "GENVALUE", IMHO...
>
> I'm open to suggestions.

BITFIELD_INSERT()?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] bitops: add equivalent of BIT(x) for bitfields

[Sebastian Frias]

Hi Geert,

On 06/12/16 12:12, Geert Uytterhoeven wrote:
>>> ... which means "generate a value"??
>>>
>>
>> Yes.
>> Although I'm not sure if I understood the essence of your point.
>> Are you suggesting that the name should be GENERATE_A_VALUE?
> 
> No. I mean that "value" is a way too generic name.
> Hence "GENVALUE" may be suitable for a macro local to a driver, but is way
> too generic and fuzzy for a global function.

IMHO GENMASK presents the same situation, but actually I don't really mind
about the name.

> 
>> There's already GENMASK, which "generates a mask".
> 
> Yes. And it generates a (bit)mask, which is clear from its name.
> But a "value" is just too generic for a global function, and make me think of
> a pseudo-random number generator ;-)

:-)

>>> "val |= 0x5a << 12;" looks much more readable to me...
>>>
>>
>> Well, the idea behind this is that one can use it like:
>>
>> (see https://marc.info/?l=linux-kernel&m=148095872915717&w=2)
>>
>> ...
>> #define TIMEOUT_CLK_UNIT_MHZ       BIT(6)
>> #define BUS_CLK_FREQ_FOR_SD_CLK(x) GENVALUE(14,7,x)
>> ...
>>     val = 0;
>>     val |= TIMEOUT_CLK_UNIT_MHZ;         /* unit: MHz */
>>     val |= BUS_CLK_FREQ_FOR_SD_CLK(200); /* SDIO clock: 200MHz */
>> ...
>>
>> which makes it very practical for writing macros for associated HW
>> documentation.
> 
> Actually I more like the SETBITFIELD name...
> 

Well, Linus did not like it for example.
Since the start I was open to suggestions because I felt the name
was not great either.

https://marc.info/?l=linux-kernel&m=148095805515487&w=2


>>> OK. So it inserts a value into a bitfield.
>>>
>>> Yes, that can be useful. Now let's find a sensible name for this.
>>> Perhaps inspired by a PowerPC mnemonic? At least that would be more
>>> obvious than "GENVALUE", IMHO...
>>
>> I'm open to suggestions.
> 
> BITFIELD_INSERT()?

The problem is that right now it does not inserts into anything,
it just generates a value. The user can then OR that with something else
essentially "inserting" the value.

(see my reply to Borislav here:
https://marc.info/?l=linux-kernel&m=148095872915717&w=2 )

This allows a use case where BIT() and GENVALUE() can be used in a
similar way:

#define TIMEOUT_CLK_UNIT_MHZ       BIT(6)
#define BUS_CLK_FREQ_FOR_SD_CLK(x) GENVALUE(14,7,x)
...
    val = 0;
    val |= TIMEOUT_CLK_UNIT_MHZ;         /* unit: MHz */
    val |= BUS_CLK_FREQ_FOR_SD_CLK(200); /* SDIO clock: 200MHz */
...

I can write BITFIELD_INSERT as well, but I would not want to have
to choose between BITFIELD_INSERT and GENVALUE, because that would
mean that the snippet above would become:

#define BITFIELD_INSERT(target, msb, lsb, val)				\
	(target = ((target & ~GENMASK(msb, lsb)) | GENVALUE(msb, lsb, val)))
...
#define TIMEOUT_CLK_UNIT_MHZ          BIT(6)
#define BUS_CLK_FREQ_FOR_SD_CLK(y, x) BITFIELD_INSERT(y,14,7,x)
...
    val = 0;
    val |= TIMEOUT_CLK_UNIT_MHZ;         /* unit: MHz */
    BUS_CLK_FREQ_FOR_SD_CLK(val, 200);   /* SDIO clock: 200MHz */
...


NOTES:
- Going for BITFIELD_INSERT() means that we probably
need to decide its behaviour w.r.t existing bits, does it OR
(thus preserving bits set) or does it overwrite? (most likely
the later option)
- Going for BITFIELD_INSERT() calls for its counter-part
BITFIELD_EXTRACT(), so that we can do:

...
   val = 0x1115a000;
   if (BITFIELD_EXTRACT(val, 19, 12) == 0x5a)
...


Best regards,

Sebastian

Re: [PATCH] bitops: add equivalent of BIT(x) for bitfields

[Kalle Valo]

Sebastian Frias <sf84@laposte.net> writes:

> Introduce SETBITFIELD(msb, lsb, value) macro to ease dealing with
> continuous bitfields, just as BIT(x) does for single bits.
>
> SETBITFIELD_ULL(msb, lsb, value) macro is also added.
>
> Signed-off-by: Sebastian Frias <sf84@laposte.net>
> ---
>
> Code protected with "#ifdef __KERNEL__" just as the BIT(x)
> macros.
>
> I would have preferred another name, like BITS(x) but it is
> already used.
>
> Suggestions for other names welcome.
> ---
>  include/linux/bitops.h | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index a83c822..4659237 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -24,6 +24,20 @@
>  #define GENMASK_ULL(h, l) \
>  	(((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
>  
> +#ifdef	__KERNEL__
> +/*
> + * Equivalent of BIT(x) but for contiguous bitfields
> + * SETBITFIELD(1, 0,0xff) = 0x00000003
> + * SETBITFIELD(3, 0,0xff) = 0x0000000f
> + * SETBITFIELD(15,8,0xff) = 0x0000ff00
> + * SETBITFIELD(6, 6,   1) = 0x00000040 == BIT(6)
> + */
> +#define SETBITFIELD(msb, lsb, val)     \
> +	(((val) << (lsb)) & (GENMASK((msb), (lsb))))
> +#define SETBITFIELD_ULL(msb, lsb, val) \
> +	(((val) << (lsb)) & (GENMASK_ULL((msb), (lsb))))
> +#endif

Pleaes check FIELD_PREP() from include/linux/bitfield.h, doesn't it
already do the same?

Adding Jakub to comment more.

-- 
Kalle Valo