[U-Boot] [PATCH 1/5] Tegra2: Add bitfield access macros

[Detlev Zundel <dzu@denx.de>]

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

-- 
It's like manually inflatable airbags -- people will never
think to use it in time to actually get any help from it.
             -- Miles Bader in <20030607122005.GA1086@gnu.org>
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de