[U-Boot] [PATCH v3 02/10] armv7: add miscellaneous utility macros

[Aneesh V]

Dear Wolfgang,

On Monday 16 May 2011 08:37 PM, Aneesh V wrote:
>
>
> On Monday 16 May 2011 12:14 AM, Wolfgang Denk wrote:
>> Dear Aneesh V,
>>
>> In message<1305202276-27784-3-git-send-email-aneesh at ti.com> you wrote:
>>> add utility macros for:
>>> * bit field operations
>>> * log2n functions
>> ...
>>
>>> +/* extract a bit field from a bit vector */
>>> +#define get_bit_field(nr, start, mask)\
>>> + (((nr)& (mask))>> (start))
>>> +
>>> +/* Set a field in a bit vector */
>>> +#define set_bit_field(nr, start, mask, val)\
>>> + do { \
>>> + (nr) = ((nr)& ~(mask)) | (((val)<< (start))& (mask));\
>>> + } while (0);
>>
>> I really dislike such "useful" helpers, because they make the code
>> unreadable. Being nonstandrd, nbody knows what they are doing, so you
>> always will have to look up the implementation before you can continue
>> reading the code. It's a waste of time an resources.
>>

What if I change the above to something like this(please note there
isn't any Linux, U-Boot substitute for this):

+/* extract a bit field from a bit vector */
+#define get_bit_field(nr, field)\
+	(((nr) & (field##_MASK)) >> (field##_OFFSET))
+
+/* Set a field in a bit vector */
+#define set_bit_field(nr, field, val)\
+	do { \
+		(nr) = ((nr) & ~(field##_MASK)) | (((val) << (field##_OFFSET))\
+			& (field##_MASK));\
+	} while (0);
+

And use it like this:
assoc  = get_bit_field(ccsidr, CCSIDR_ASSOCIATIVITY);
set_bit_field(ccsidr, CCSIDR_ASSOCIATIVITY, assoc + 1);

Isn't it more intuitive and almost self-explanatory now.

If you still don't like these as standard generic macros, how about
having these macros just for OMAP with these names and use them only
for OMAP code?

omap_get_bit_field(nr, field)
omap_set_bit_field(nr, field, val)

I can live without them for the ARM generic code but I use it
extensively in my SPL series.

Please note that Simon's recent work for bitfield operations doesn't
help me because the field definitions available to me are in shift/mask
format and not in the range format that he uses. I will not be
able to now generate the range format for the hundreds of registers I
use.

best regards,
Aneesh