[U-Boot] [PATCH v2 0/5] Introduce BIT and GENMASK

[Wolfgang Denk]

Dear Tom,

In message <20150512150237.GB5267 at bill-the-cat> you wrote:
> 
> > > Any comments on this series - early push will have enough time to test and
> > > I have more patches that need to use these macros.
> > 
> > I'm not very fond of this macro, it makes the code more cryptic .
>
> BIT/GENMASK are (growing in usage) kernel macros, so I think it'll help
> us in the long run.

When I was U-Boot maintainer (and for quite some time after) I
consequently tried to block all attempts to add such stuff.  My main
concern is that this stuff is inherently unportable and confusing.

For anyone fluent in programming (and others should not be allowed to
contrinute code to U-Boot ;-) the phrase  "(1 << 6)"  cannot be
misunderstood.  Most programmers I know would actually prefer a direct
mask, i. e.  0x40 , which can be parsed and understood even faster,
but this is probably mostly a matter of taste.

But what exactly is BIT(6) ?  Please keep in mind, that BIT(6) (if
interpreted correctly) can be any of the following numbers:

	             0x02 - in a 8 bit data type on Power Architecture
	            0x200 - in a 16 bit data type on Power Architecture
	        0x2000000 - in a 32 bit data type on Power Architecture
	0x200000000000000 - in a 64 bit data type on Power Architecture
	             0x40 - on many other systems

Keep in mind, that on Power Architecture bit 0 is defined to be the
MSB in the data type!!


My feeling is that we should prefer clarity of code and portability
over temporary fashions; not many people working n more generic parts
of the kernel ever care about (now) exostic architectures like PPC.
Very few actually know that there are systems where bit 0 is the MSB -
don't ask me how many boards I have seen where not even the hardware
designers were aware of this :-(


So, in the past I would have vehemently protested against any such
patches, especially when they change good code to the worse.  I must
admit, that currently I have neither the time nor the nerves to engage
in such a semi-religious war.

But I would like to raise this concern: this code is inherently
unportable and misleading!  It does not help understanding of the
code, but makes it more diffcult.  I consider it not an improvement,
on contrary.

I vote _against_ any such macros.  But this is just a comment.  I do
not intend to formally NAK thiese patches.  Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Above all else -- sky.