
Dear Tom,
In message 20150512150237.GB5267@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