From mboxrd@z Thu Jan  1 00:00:00 1970
From: Aneesh V <aneesh@ti.com>
Date: Wed, 15 Jun 2011 14:18:35 +0530
Subject: [U-Boot] [PATCH v3 02/10] armv7: add miscellaneous utility
	macros
In-Reply-To: <20110614135335.A750811EEDAF@gemini.denx.de>
References: <1299589658-30896-1-git-send-email-aneesh@ti.com>
	<1305202276-27784-3-git-send-email-aneesh@ti.com>
	<20110515184421.D7AB91491B06@gemini.denx.de>
	<4DD13DA3.3030505@ti.com> <4DECF8DA.9030806@ti.com>
	<20110606185046.BB8991736815@gemini.denx.de>
	<4DEDE8D9.7030306@ti.com>
	<20110607103923.7E1CC1B993A8@gemini.denx.de>
	<4DEE161B.2050402@ti.com>
	<20110607154028.D6540195494F@gemini.denx.de>
	<4DEF62A6.7060706@ti.com>
	<20110608214124.849821B993A8@gemini.denx.de>
	<4DF71FBF.6030408@ti.com>
	<20110614105115.01D6611EEDAF@gemini.denx.de>
	<4DF7488A.6000909@ti.com>
	<20110614135335.A750811EEDAF@gemini.denx.de>
Message-ID: <4DF871E3.8080307@ti.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Dear Wolfgang,

On Tuesday 14 June 2011 07:23 PM, Wolfgang Denk wrote:
> Dear Aneesh V,
>
> In message<4DF7488A.6000909@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