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

Detlev Zundel dzu at denx.de
Wed Jul 13 13:28:22 CEST 2011


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.

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


More information about the U-Boot mailing list