[U-Boot] [RESEND PATCH v2 1/5] Tegra2: Add macros to calculate bitfield shifts and masks

[Anton Staaf]

On Wed, Jul 13, 2011 at 4:28 AM, Detlev Zundel <dzu at denx.de> wrote:
> Hi Anton,
>
> [...]
>
>> The only problem with this is that there is one piece of missing
>> information, which is how do you get the value of the field that is
>> masked as if it were a 4-bit or 3-bit number.  That is, if I want the
>> IPS_DIV value, I probably want:
>>
>>    (value & SCFR1_IPS_DIV_MASK) >> 20
>>
>> Likewise, if I want to set the IPS divisor to 5 say, I would have to do:
>>
>>    (value & ~SCFR1_IPS_DIV_MASK) | (5 << 20)
>>
>> In both cases the value 20 needs to come from somewhere.  So you would
>> probably end up having:
>>
>>    #define SCFR1_IPS_DIV_MASK      0x03800000
>>    #define SCFR1_IPS_DIV_SHIFT      20
>>
>> and
>>
>>    (value & SCFR1_IPS_DIV_MASK) >> SCFR1_IPS_DIV_SHIFT
>>    (value & ~SCFR1_IPS_DIV_MASK) | (5 << SCFR1_IPS_DIV_SHIFT)
>>
>> And I think it would be great if these could turn into:
>>
>>    field_value = GET_FIELD(register_value, SCFR1_IPS_DIV)
>>    register_value = SET_FIELD(register_value, SCFR1_IPS_DIV, 5)
>
> Let me take this opportunity to once more explain why I don't like this.
> As a big fan of functional programming, I personally have grown used to
> code as explicit as possible.  So _all_ arguments to a function should
> be explicit in the call - reliance on state or such "hidden arguments"
> violate this principle strongly.  I learnt that code following these
> principles written by other people is much easier for me to understand.

I agree in general that it is preferable to be as explicit as
possible.  But it is also good to be able to express your intent,
instead of implementation when possible.  In other words, I would
rather be explicit about my intent, than the particular
implementation.  One way of attaining both in this case would be to
change the #define to be:

    #define SCFR1_IPS_DIV    {0x03800000, 20}

And use it to initialize a field struct that is accessed in the GET
and SET macros.  Then there are no hidden variables/names being
constructed.  I didn't initially suggest this because it could be seen
as another abuse of macro magic.  Alternatively, you can view the
#define <NAME>_<VALUE> notation as a poor mans structured programming
paradigm.  Effectively, the various #defines make up a single
structured data element that the GET and SET macros are accessing.
The first solution has the advantage that you don't have to repeat
yourself.  Are either of these solutions acceptable (I realize the
second solution is more of a "solution" as it requires a
re-interpretation of the existing proposal).

Thanks,
    Anton

p.s. The above #define could also be changed to:

    #define SCFR1_IPS_DIV    {.mask = 0x03800000, .shift = 20}

Resulting in a verbose, but explicit definition of the field.

> Cheers
>  Detlev
>
> --
> There are two hard things in computer science: cache invalidation,
> naming things, and off-by-one errors.
> --
> 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
>