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

[Wolfgang Denk]

Dear Simon Glass,

In message <BANLkTimN4NVD43B2Le7_ci9Picjd6pmuXA at 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.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
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
If you want strict real-time behavior, run in the real  time  schedu-
ling class.  But there are no seatbelts or airbags;  main(){for(;;);}
can hard hang your system.                          -- Bart Smaalders