Dear Wolfgang, Wolfgang Denk wrote: > Dear [EMAIL PROTECTED], > > In message <[EMAIL PROTECTED]> you wrote: > >>Subject: [PATCH-OMAP3] OMAP3: Remove BITx magic >> >>From: Dirk Behme <[EMAIL PROTECTED]> >> >>Remove bits.h and it's macros usage. Requested by Wolfgang Denk. >> >>Signed-off-by: Dirk Behme <[EMAIL PROTECTED]> > > ... > >> /* device type */ >>-#define DEVICE_MASK (BIT8 | BIT9 | BIT10) >>+#define DEVICE_MASK (0x7 << 8) > > > That's a funny way to make code difficult to read. Why do you prefer > "(0x7 << 8)" instead of "0x700" (which looks more obvious to me) ? > > >>-#define DLL_NO_FILTER_MASK (BIT8 | BIT9) >>+#define DLL_NO_FILTER_MASK ((0x1 << 9) | (0x1 << 8)) > > Ditto here - why not simply 0x300 ?
For my taste the << style makes it easier to create macros from TRM and later to review code against TRM. Maybe 0x700 and 0x300 are easy cases, but for e.g 0x34B03C00 I need a sheet of paper or calculator to get an idea which bits are exactly set in register. And then later re-calculate twice to be sure I'm correct ;) Having a TRM, looking at a register description and then wanting to set Bits 29 & 28 & 26 & 23 & 21 & 20 & 13 & 12 & 11 & 10 using something like (1 << 29) | (1 << 28) | (1 << 26) | (1 << 23) | (1 << 21) | (1 << 20) | (1 << 13) | (1 << 12) | (1 << 11) | (1 << 10) makes it more obvious for me. Then using preprocessor/compiler to create 0x34B03C00 I'm on the safe side from my point of view. >>-#define GPT_EN ((0 << 2) | BIT1 | BIT0) >>+#define GPT_EN ((0x0 << 2) | (0x1 << 1) | (0x1 << 0)) > > > Why not 0x3 ? > > Note: especially the "(0x0 << 2) | " part in the expression i really > bogus. Again, from TRM point of view, above style makes it clear that bit 2 is intentionally set to zero. Best regards Dirk _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
