Hi,
Dear Simon Glass,
In message BANLkTimN4NVD43B2Le7_ci9Picjd6pmuXA@mail.gmail.com you wrote:
clrsetbits_le32(&my_device->ctrl, FIELD_MASK, FIELD_VAL(6));We now have a computed mask which is good, thank you.
But the FIELD is specified twice in the same statement. Can we therefore please take this a step further and write something like this?
clrsetfield_le32(&my_device->ctrl, FIELD, 6);
I think I should provide a little moe explanation why I dislike your suggestion. With "FOO_MASK, FOO_VAL()" it is obvious to the reader that we are dealing ith some (bit) mask and some value, and it is also trivial to locate the respective definitions of these macros, because you know their exact names.
With "FOO, 6" you know nothing. Searching for "FOO" in the source code and header files will probably return a ton of unrelated references, and you will have to read (and understand) the definition of the bitfield macro and follow it's preprocessor trickery with combining some magic name parts until you finally know what to look for.
You consider this macro helpful, I consider it obscure, and that's why I don't want to have it.
For what its worth, I also consider the magic under the hood to be too much. It may look clever, but it actually makes the code harder to read without knowing the definition of the macrot itself. Having to know the _definition_ of a macro to understand how it works violates the usual paradigm of having a fixed API/contract independent of the implementation.
Cheers Detlev