[U-Boot] [PATCH-OMAP3] OMAP3: Remove BITx magic
Dirk Behme
dirk.behme at googlemail.com
Mon Nov 10 20:43:16 CET 2008
Dear Wolfgang,
Wolfgang Denk wrote:
> Dear dirk.behme at googlemail.com,
>
> In message <49172e4a.0b38560a.42bc.ffffb794 at mx.google.com> you wrote:
>
>>Subject: [PATCH-OMAP3] OMAP3: Remove BITx magic
>>
>>From: Dirk Behme <dirk.behme at gmail.com>
>>
>>Remove bits.h and it's macros usage. Requested by Wolfgang Denk.
>>
>>Signed-off-by: Dirk Behme <dirk.behme at gmail.com>
>
> ...
>
>> /* 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
More information about the U-Boot
mailing list