Dear Matthias Kaehlcke,
In message <20100211200302.ge15...@darwin> you wrote:
> Most code defines constants for bit positions by means of "(1 << n)". The
> Linux
Most code does? I disagree. Only minor parts of the code do, and I
generally tend to consider this bad style.
> kernel defines the BIT macro for this purpose, providing a uniform and more
> readable way to define these constants. This patch adds the BIT macro to
> linux/bitops.h, and removes its local definitions from davinci and ixp code.
I am strongly tempted to reject this patch, as it does not improve
anything. On contrary, it makes things worse. I'll try to explain
why.
> Signed-off-by: Matthias Kaehlcke <matth...@kaehlcke.net>
> ---
> cpu/arm926ejs/davinci/cpu.c | 2 --
> cpu/ixp/npe/include/IxOsalOsIxp400.h | 2 --
> include/asm-arm/arch-ixp/ixp425.h | 2 --
> include/linux/bitops.h | 1 +
> 4 files changed, 1 insertions(+), 6 deletions(-)
With the local definitions in ARM related header files you could at
least make an edicated guess what the code might mean.
By moving the definition into a globally valid and visible location
the inherent problems of sucha definition become obvious.
If you see a line of code like
value |= 0x0020; or value |= (1 << 5)
or
vlaue &= ~0x0020; or vlaue &= ~(1 << 5)
you know immediately and exactly what that means - there is not the
slightest bit of doubt about the meaning of the code.
You probably thing that
value |= BIT(5);
is easier to read. But is this really so?
Why do you prefer to write (or read) "BIT(5)" instead of "1 << 5"?
Because some of your user manuals talk about "bit5" etc.?
Guess why there is not a single use of BIT(x) in PowerPC code in
U-Boot? For the Power Architecture, bit 0 is by definition the most
significant bit. What does this mean for a simple value as "(1 << 5)"?
Size of object (1 << 5) is ...
8 bit bit 2
16 bit bit 10
32 bit bit 26
64 bit bit 58
You see? Even if you are aware that bit 0 is the MSB, the notation of
BIT(x) makes no sense, as it at least also depends on the object size.
Forget BIT(x). It's unportable and misleading at best.
If you want to improve U-Boot code, then please come up with cleanup
patches to remove this construct from U-Boot all together.
Thanks.
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: w...@denx.de
The thing is, as you progress in the Craft, you'll learn there is
another rule... When you break rules, break 'em good and hard.
- Terry Pratchett, _Wyrd Sisters_
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
