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

[Wolfgang Denk <wd@denx.de>]

Dear Simon Glass,

In message <BANLkTi=+pqk=UQY_=NoXcMabBDm845xcEw@mail.gmail.com> you wrote:
> 
> >> #define clrsetfield_le32(bitfield, addr, value) =A0...
> >>
> >> Then caller can define these in a header file:
> >>
> >> #define FIELD_MASK 0xf0000
> >> #define FIELD_SHIFT 16
> >>
> >> And use this macro to set the bitfield to 6, for example:
> >>
> >> clrsetfield_le32(FIELD, &my_device->ctrl, 6)
> >>
> >> (this will simply shift the value left 16 bits and apply the supplied ma=
> sk)
> >>
> >> This captures the essence of bitfields, in that we are abstracting the
> >> field under a single name. The change would just add this macro to the
> >> io.h header file.
> >
> > Sorry, I fail to understand how you envision to use this, and how it
> > would be different from =A0clrsetbits*() ?
> 
> For example this allows us to replace:
> 
> clrsetbits_le(&my_device->ctrl, 0xf << 16, 6 << 16)
> 
> with:
> 
> clrsetfield_le32(FIELD, &my_device->ctrl, 6)
> 
> So the two identical shifts are avoided, and the forming of the mask
> is done once in the define.

If you really insist:

	#define FIELD_VAL(x)	(x << 16)
	#define FIELD_MASK	FIELD_VAL(0xF)

	clrsetbits_le32(&my_device->ctrl, FIELD_MASK, FIELD_VAL(6));

In practical use cases, you will probablu not use magg numbers like
'6' anyway, and define symbolic names for this anyway, so this would
be:

	clrsetbits_le32(&my_device->ctrl, FIELD_MASK, FIELD_VAL_FOO);

or similar.

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
News is what a chap who doesn't care much  about  anything  wants  to
read. And it's only news until he's read it. After that it's dead.
                           - Evelyn Waugh _Scoop_ (1938) bk. 1, ch. 5