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