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

[Aneesh V]

Dear Wolfgang,

On Tuesday 14 June 2011 07:23 PM, Wolfgang Denk wrote:
> Dear Aneesh V,
>
> In message<4DF7488A.6000909 at ti.com>  you wrote:
>>
>> Yes. I have seen those macros. But more often than not the bit field is
>> more than 1 bit wide and the value to be set is not necessarily all 0's
>> or all 1's. That's why I have to use clrsetbits_*()
>
> I see.  In such a case (and only then) clrsetbits_*() is indeed the
> right choice.
>
>> The problem I have to deal with is different. get_bit_field() was
>> intended to extract bit fields from an integer. So, the target usage
>> will be something like this(where a, b, and c are bit fields in
>> register my_reg)
>>
>> u32 my_reg, a_val, b_val, c_val;
>>
>> u32 my_reg = readl(my_reg_addr);
>>
>> a_val = get_bit_field(my_reg, a_mask);
>> b_val = get_bit_field(my_reg, b_mask);
>> c_val = get_bit_field(my_reg, c_mask);
>>
>> Do you see an alternative method for doing this using the standard
>> macros?
>
> Please see the example given here:
>
> http://article.gmane.org/gmane.comp.boot-loaders.u-boot/101146
>
> Looking closer, the "FIELD_VAL" macro alone will probably not suffice,
> as you need both shift directions, like that:
>
> 	#define FIELD_SHIFT	16
> 	#define FIELD_MASK	0xF

When I said mask I meant the 'shifted mask' like:

	#define FIELD_MASK	(0xF << 16)

So, the shift information is embedded in this mask and can be extracted
by finding the first set bit. But in reality my get_bit_field()
function indeed takes both arguments. So it's something like this:


#define get_bit_field(x, shift, mask)\
	(((x) & (mask)) >> (shift))

#define A_SHIFT		16
#define A_MASK		(0xF << 16)

and then use it like:

a_val = get_bit_field(my_reg, A_SHIFT, A_MASK);

>
> 	#define FIELD_BITS(x) (x<<  16)
> 	#define FIELD_MASK FIELD_BITS(0xF)
> 	#define FIELD_VAL(x) ((x&  FIELD_MASK)>>  16)
>
> The code would then look something like this:
>
> 	my_reg = readl(my_reg_addr);
>
> 	a_val = A_VAL(my_reg);
> 	b_val = B_VAL(my_reg);
> 	c_val = C_VAL(my_reg);

If I have to do something like this I will have to now generate macros
like A_VAL(x) for hundreds of fields. Is it really necessary when the
following will do the job for me with the existing infrastructure I
have:

a_val = get_bit_field(my_reg, A_SHIFT, A_MASK);
b_val = get_bit_field(my_reg, B_SHIFT, B_MASK);

Please note that in this case I don't have an alternative with standard
macros. If this is not possible I would rather prefer to keep it simple
like this:

a_val = (my_reg & A_MASK) >> A_SHIFT;

best regards,
Aneesh