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

Gerhard Sittig gsi at denx.de
Mon Feb 17 22:33:16 CET 2014


On Mon, Feb 17, 2014 at 22:01 +0100, Wolfgang Denk wrote:
> 
> 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.

That's fine with me.  So this patch can get dropped and nothing
changes for the existing code base.

The MCS7830 driver will then need a respin (after I took in some
more feedback, of course).  There was the question of whether to
use the BIT(5) macro or re-invented (1 << 5) or 0x20 numbers.
The feedback now allows a clear answer. :)


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de


More information about the U-Boot mailing list