Hi Wolfgang,
El Thu, Feb 11, 2010 at 09:46:22PM +0100 Wolfgang Denk ha dit:
> 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 for your explications. i now understand that the BIT macro
rather can obscure what is going on, though at the first glance it
seems to improve readability.
--
Matthias Kaehlcke
Embedded Linux Developer
Barcelona
They that can give up essential liberty to obtain a little
temporary safety deserve neither liberty nor safety
(Benjamin Franklin)
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
