[PATCH] bitops: move BITS() macro to the bitops file

[PATCH] bitops: move BITS() macro to the bitops file

[Linus Walleij]

This macro was invented by Mattias Nilsson for the usecase
where you want to set a sequence of bits inside a n-bit
word, while leaving the head and tail of the sequence all
zeroes. For example:

  #include <linux/bitops.h>

  u16 mask = BITS(4, 12);

Yields a mask like this:

  0001111111110000

This patch moves the construct out of the MFD PRCMU driver
and make it available for common use, after noticing in a
review or two that it would be useful for others.

Cc: Akinobu Mita <mita@miraclelinux.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mfd/dbx500-prcmu-regs.h | 2 --
 include/linux/bitops.h          | 1 +
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/mfd/dbx500-prcmu-regs.h b/drivers/mfd/dbx500-prcmu-regs.h
index 4f6f0fa..906e75e 100644
--- a/drivers/mfd/dbx500-prcmu-regs.h
+++ b/drivers/mfd/dbx500-prcmu-regs.h
@@ -13,8 +13,6 @@
 #ifndef __DB8500_PRCMU_REGS_H
 #define __DB8500_PRCMU_REGS_H
 
-#define BITS(_start, _end) ((BIT(_end) - BIT(_start)) + BIT(_end))
-
 #define PRCM_ACLK_MGT		(0x004)
 #define PRCM_SVAMMCSPCLK_MGT	(0x008)
 #define PRCM_SIAMMDSPCLK_MGT	(0x00C)
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index a3b6b82..1f9d78b 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -6,6 +6,7 @@
 #define BIT(nr)			(1UL << (nr))
 #define BIT_MASK(nr)		(1UL << ((nr) % BITS_PER_LONG))
 #define BIT_WORD(nr)		((nr) / BITS_PER_LONG)
+#define BITS(_start, _end)	((BIT(_end) - BIT(_start)) + BIT(_end))
 #define BITS_PER_BYTE		8
 #define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
 #endif
-- 
1.8.1.4

Re: [PATCH] bitops: move BITS() macro to the bitops file

[anish singh]

On Wed, Aug 7, 2013 at 2:36 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> This macro was invented by Mattias Nilsson for the usecase
> where you want to set a sequence of bits inside a n-bit
> word, while leaving the head and tail of the sequence all
> zeroes. For example:
>
>   #include <linux/bitops.h>
>
>   u16 mask = BITS(4, 12);

nice.I don't have anything against this patch but couldn't understand
how this works.So i wrote what i knew as below:

#include <stdio.h>

#define BIT(nr)                        (1UL << (nr))
#define BITS(_start, _end)     ((BIT(_end) - BIT(_start)) + BIT(_end))
#define BITS_MY(_start, _end) (((BIT(_end-_start+1))-1)<<_start)
int main()
{
        printf("%x\n", BITS(4,12));
        printf("%x\n", BITS_MY(4,12));
        return 0;
}

>
> Yields a mask like this:
>
>   0001111111110000
>
> This patch moves the construct out of the MFD PRCMU driver
> and make it available for common use, after noticing in a
> review or two that it would be useful for others.
>
> Cc: Akinobu Mita <mita@miraclelinux.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/mfd/dbx500-prcmu-regs.h | 2 --
>  include/linux/bitops.h          | 1 +
>  2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/mfd/dbx500-prcmu-regs.h b/drivers/mfd/dbx500-prcmu-regs.h
> index 4f6f0fa..906e75e 100644
> --- a/drivers/mfd/dbx500-prcmu-regs.h
> +++ b/drivers/mfd/dbx500-prcmu-regs.h
> @@ -13,8 +13,6 @@
>  #ifndef __DB8500_PRCMU_REGS_H
>  #define __DB8500_PRCMU_REGS_H
>
> -#define BITS(_start, _end) ((BIT(_end) - BIT(_start)) + BIT(_end))
> -
>  #define PRCM_ACLK_MGT          (0x004)
>  #define PRCM_SVAMMCSPCLK_MGT   (0x008)
>  #define PRCM_SIAMMDSPCLK_MGT   (0x00C)
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index a3b6b82..1f9d78b 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -6,6 +6,7 @@
>  #define BIT(nr)                        (1UL << (nr))
>  #define BIT_MASK(nr)           (1UL << ((nr) % BITS_PER_LONG))
>  #define BIT_WORD(nr)           ((nr) / BITS_PER_LONG)
> +#define BITS(_start, _end)     ((BIT(_end) - BIT(_start)) + BIT(_end))

Shouldn't there be compile time check here for checking _end > _start?
BUILD_BUG_ON(_end<_start);

>  #define BITS_PER_BYTE          8
>  #define BITS_TO_LONGS(nr)      DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
>  #endif
> --
> 1.8.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] bitops: move BITS() macro to the bitops file

[Joe Perches]

On Wed, 2013-08-07 at 11:06 +0200, Linus Walleij wrote:
> This macro was invented by Mattias Nilsson for the usecase
> where you want to set a sequence of bits inside a n-bit
> word, while leaving the head and tail of the sequence all
> zeroes. For example:
> 
>   #include <linux/bitops.h>

BITS is a name that's not easily reused because it's
already in use in a few other places.

There are a few conflicts.

$ git grep -E "^\s*#\s*define\s+\bBITS\b"
arch/arc/include/asm/disasm.h:#define BITS(word, s, e)  (((word) >> (s)) & (~((-2) << ((e) - (s)))))
drivers/input/keyboard/tnetv107x-keypad.c:#define BITS(x)                       (BIT(x) - 1)
drivers/mfd/dbx500-prcmu-regs.h:#define BITS(_start, _end) ((BIT(_end) - BIT(_start)) + BIT(_end))
drivers/net/wireless/iwlwifi/mvm/fw-api-bt-coex.h:#define BITS(nb) (BIT(nb) - 1)
fs/select.c:#define BITS(fds, n)        (*FDS_IN(fds, n)|*FDS_OUT(fds, n)|*FDS_EX(fds, n))
lib/zlib_inflate/inflate.c:#define BITS(n) \
sound/core/oss/rate.c:#define BITS      (1<<SHIFT)

>   u16 mask = BITS(4, 12);
> 
> Yields a mask like this:
> 
>   0001111111110000
[]
> diff --git a/drivers/mfd/dbx500-prcmu-regs.h b/drivers/mfd/dbx500-prcmu-regs.h
[]
> @@ -13,8 +13,6 @@
[]
> -#define BITS(_start, _end) ((BIT(_end) - BIT(_start)) + BIT(_end))

> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
[]
> @@ -6,6 +6,7 @@
[]
> +#define BITS(_start, _end)	((BIT(_end) - BIT(_start)) + BIT(_end))

Maybe use a statement expression to make sure
the start and end accesses are done only once.

#define BITS(start, end)				\
({							\
	unsigned long high = BIT(end);			\
	unsigned long low = BIT(start);			\
	unsigned long rtn = high + (high - low);	\
	rtn;						\
})

I suggest | instead of + too.

Maybe add a WARN when low > high too.
Maybe make it a function to consolidate
any WARN instead of inline expansion.

Re: [PATCH] bitops: move BITS() macro to the bitops file

[Borislav Petkov]

On Wed, Aug 07, 2013 at 11:06:19AM +0200, Linus Walleij wrote:
> This macro was invented by Mattias Nilsson for the usecase
> where you want to set a sequence of bits inside a n-bit
> word, while leaving the head and tail of the sequence all
> zeroes. For example:
> 
>   #include <linux/bitops.h>
> 
>   u16 mask = BITS(4, 12);
> 
> Yields a mask like this:
> 
>   0001111111110000
> 
> This patch moves the construct out of the MFD PRCMU driver
> and make it available for common use, after noticing in a
> review or two that it would be useful for others.

Well, we have similar GEN_MASK and GENMASK macros around the tree. Maybe
we could unify them all?

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--