On Wed, 2019-07-10 at 10:43 +0100, Russell King - ARM Linux admin wrote:
On Wed, Jul 10, 2019 at 11:17:31AM +0200, Johannes Berg wrote:
On Tue, 2019-07-09 at 22:04 -0700, Joe Perches wrote:
These GENMASK uses are inverted argument order and the actual masks produced are incorrect. Fix them.
Add checkpatch tests to help avoid more misuses too.
Joe Perches (12): checkpatch: Add GENMASK tests
IMHO this doesn't make a lot of sense as a checkpatch test - just throw in a BUILD_BUG_ON()?
I tried that.
It'd can't be done as it's used in declarations and included in asm files and it uses the UL() macro.
I also tried just making it do the right thing whatever the argument order.
Oh well.
My personal take on this is that GENMASK() is really not useful, it's just pure obfuscation and leads to exactly these kinds of mistakes.
Yes, I fully understand the argument that you can just specify the start and end bits, and it _in theory_ makes the code more readable.
However, the problem is when writing code. GENMASK(a, b). Is a the starting bit or ending bit? Is b the number of bits? It's confusing and causes mistakes resulting in incorrect code. A BUILD_BUG_ON() can catch some of the cases, but not all of them.
It's a horrid little macro and I agree with Russell.
I also think if it existed at all it should have been GENMASK(low, high) not GENMASK(high, low).
I