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

Wolfgang Denk wd at denx.de
Mon Feb 17 22:01:56 CET 2014


Dear Gerhard Sittig,

In message <1392665727-5734-2-git-send-email-gsi at 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.


More information about the U-Boot mailing list