On Tue, Jun 14, 2011 at 6:53 AM, Wolfgang Denk wd@denx.de 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
#define FIELD_BITS(x) (x << 16) #define FIELD_MASK FIELD_BITS(0xF) #define FIELD_VAL(x) ((x & FIELD_MASK) >> 16)
Hi Wolfgang,
I think you have FIELD_MASK being two meanings: the un-shifted or 'raw' mask, and the shifted mask. So perhaps:
#define FIELD_SHIFT 16 #define FIELD_RAWMASK 0xF
#define FIELD_BITS(x) (x << 16) #define FIELD_MASK FIELD_BITS(FIELD_RAWMASK) #define FIELD_VAL(x) ((x & FIELD_MASK) >> 16)
or with the 16 factored properly, but even harder to read:
#define FIELD_SHIFT 16 #define FIELD_RAWMASK 0xF
#define FIELD_BITS(x) (x << FIELD_SHIFT) #define FIELD_MASK FIELD_BITS(FIELD_RAWMASK) #define FIELD_VAL(x) ((x & FIELD_MASK) >> FIELD_SHIFT)
(note that FIELD_BITS should arguably mask after the shift).
When you have a lot of these definitions in a row you have mentally check the bit width of the mask:
#define FIELD1_RAWMASK 0xF #define FIELD1_SHIFT 16 #define FIELD2_RAWMASK 0x1F #define FIELD2_SHIFT 11 #define FIELD3_RAWMASK 0x1F #define FIELD3_SHIFT 7 #define FIELD4_RAWMASK 0x1F #define FIELD4_SHIFT 1
#define FIELD1_BITS(x) (x << FIELD1_SHIFT) #define FIELD1_MASK FIELD_BITS(FIELD1_RAWMASK) #define FIELD1_VAL(x) ((x & FIELD1_MASK) >> FIELD1_SHIFT) #define FIELD2_BITS(x) (x << FIELD2_SHIFT) #define FIELD2_MASK FIELD_BITS(FIELD2_RAWMASK) #define FIELD2_VAL(x) ((x & FIELD2_MASK) >> FIELD2_SHIFT) ...
Is the above correct, or do fields overlap or not cover fully? (exercise for reader) (see [1] below)
So I think is better to think of the width rather than the mask.
#define FIELD_SHIFT 16
#define FIELD_RAWMASK (1U << FIELD_WIDTH) - 1 #define FIELD_BITS(x) (x << FIELD_WIDTH) #define FIELD_MASK FIELD_BITS(FIELD_RAWMASK) #define FIELD_VAL(x) ((x & FIELD_MASK) >> FIELD_WIDTH)
because then it is more obvious:
#define FIELD1_WIDTH 4 #define FIELD1_SHIFT 16 #define FIELD2_WIDTH 5 #define FIELD2_SHIFT 11 #define FIELD3_WIDTH 5 #define FIELD3_SHIFT 7 #define FIELD4_WIDTH 5 #define FIELD4_SHIFT 1
And now it is a little easier to see that 11+5 = 16, so FIELD2 is ok; 7+5=12 so FIELD3 overlaps, 1+5=6 so FIELD4 isn't big enough. It's still not as good as just numbering your bits on little-endian archs, but I think we have had that discussion.
I think BITS and VAL are not very descriptive which is why I suggested pack and unpack at the time.
But don't get me started talking about bit fields.
Regards, Simon
[1] This is why macros are so nice:
define once: #define bf_mask(field) ((field ## _RAWMASK) << field ## _SHIFT) #define bf_val(field, val) (((val) & bf_mask(field)) >> field ## _SHIFT) #define bf_bits(field, val) (((val) << field ## _SHIFT) & bf_mask(field))
then: #define FIELD1_BITS(x) bf_bits(FIELD1, x) #define FIELD1_MASK bf_mask(FIELD1) #define FIELD1_VAL(x) bf_val(FIELD1, x) #define FIELD2_BITS(x) bf_bits(FIELD2, x) #define FIELD2_MASK bf_mask(FIELD2) #define FIELD2_VAL(x) bf_val(FIELD2, x) ...
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);
...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@denx.de "I dislike companies that have a we-are-the-high-priests-of-hardware- so-you'll-like-what-we-give-you attitude. I like commodity markets in which iron-and-silicon hawkers know that they exist to provide fast toys for software types like me to play with..." - Eric S. Raymond