[U-Boot] [PATCH v2 1/7] include: move the BIT() macro into the common.h header file

Wolfgang Denk
2014-02-17T21:01:56Z

Dear Gerhard Sittig,

In message <1392665727-5734-2-git-send-email-gsi@denx.de> you wrote:
> several compile units and local header files re-invented the
> BIT() macro, move BIT() and BITMASK() declarations into the
> common.h header file and adjust call sites
...
> diff --git a/include/common.h b/include/common.h
> index 221b7768c5be..49885f3c01bf 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -967,6 +967,9 @@ static inline phys_addr_t map_to_sysmem(const void *ptr)
> #define ALIGN(x,a) __ALIGN_MASK((x),(typeof(x))(a)-1)
> #define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask))
>
> +#define BIT(n) (1U << (n))
> +#define BITMASK(bits) (BIT(bits) - 1)
> +

I'm sorry, but these macros are ugly and make the code harder to read.
Also, they are inherently non-portable. I guess you are not aware
that "bit 0" is the MSB on Power architecture, not the LSB as you
expect in your definition.

I strongly reommend NOT to use any such macros. Adding these to
common.h could be considered as an invitation to use them, which is
the opposite of what I want.

So sorry, but this is a clear NAK.

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Anything free is worth what you pay for it.