Register access in IAR and Code Composer Studio (bitfields versus "standard" access)

Hi Flavio,

i have been working with MSP430 since almost two years, and most of the time, i rarely see codes using bitfields, and also it is not really recommended in C:

http://c-faq.com/struct/bitfields.html

So unfortunatelly, i am afraid if you want to migrate to CCS, you have to modify your code by using the bitmasks definitions in the header file. The good thing is that it is also readable as with the bitfield.

Hi Flavio,

i think it should be ok to use the macros above, because you are using it correctly with the brackets. i can't think of any cases that using the macros would lead to an unexpected result (again because you are doing it with the brackets). However the example codes given for MSP430 usually use the second approach of "Code B". But in the end, i think this just a matter of programming taste :)

Use of bitfields is rather ineffective. They are usually use don hardware registers which are declared volatile. So the compiler has to generate one access per bit assignment.
Normal & and | accesss to a register can be grouped and are way more effective.

Also, in many cases it is unimportant whether e.g. P2.6 returns 1 or 0x40. It's either zero or not.

So in most cases, P2IN_Bit.P6 can be translated into (P2IN&BIT6). In some (few) cases, it shoul dbe better translated into ((P2IN&BIT6) >>6). Which is what the compiler generates for the bitfield anyway.
It has the additional advantage that this is an rvalue. You cannot do SWITCH_IN = 1 anymore, which would have been allowed with the bitfield.

For outputs, things are a bit more complex:

#define AUDIO_AMPLIFIER_ENABLE_OUT(x) P2OUT= (P2OUT&~BIT7)|(x?BIT7:0)

This too isn't an lvalue but more or less what the compiler generates for the bitfields. Well, if you do the change, you'll get an error for each usage in teh code, whcih makes it easy to fix :)

For a new project, I'd suggest using
#define AUDIO_AMPLIFIER_ENABLE (P2OUT|=BIT7)
#define AUDIO_AMPLIFIER_DISABLE (P2OUT &= ~BIT7)

Flavio Renga said:

Is there any procedure to use the already developed code "as it is"?

Yes, you can more or less auto-generate bitfield definitions for all used registers. For one single project/MSP type, this is likely more effort than it's worth.

Flavio Renga said:

Should I modify ALL the code using the "standard" syntax?

That's the better approach.

Flavio Renga said:

There is any reason to avoid this the bitfield syntax?

See above. Mainly because of its inefficiency. But in some cases, ther emight be side-effects on registers which cannot be avoided using the bitfields. Or are obfuscated by using bitfields ("I didn't touch this bit, why is it gone?" - because access to an other member of the bitfield caused a read/write operation that in turn cleared other bits)

Flavio Renga said:

(I started years ago using bitfield approach because in my opinion the code is more readable)

In most cases, especially for user-defined bitfields, this may be true. However, as explaind above, when bitfields are more than plain memory access, things can become even less readable. And cause bugs that are difficult to track down then.
Also, embedded programming is first order about efficiency. Beauty of code is far down on the priority list.

Hi Flavio,

Flavio Renga said:

I put down this macro to assign a predefined value to a subset of bits in a register:

#define ASSIGN_BIT_MASK(REG,VAL,MASK)      ((REG)=(VAL)|(REG)&(~(MASK)))

i would do it in this way:

#define ASSIGN_BIT_MASK(REG,VAL,MASK)      do {REG &= ~(MASK); REG |= VAL;} while(0);

it is more readable, and i think your implementation is not 100% correct (CMIIW), it should be maybe like this:

#define ASSIGN_BIT_MASK(REG,VAL,MASK)      ((REG)=(VAL)| ((REG)&(~(MASK)))

the reason is that i think the compiler will read it left to right, so without the brackets, your macro above will works:

- VAL | REG

- result above & (~(MASK))

and i don't think this is what you want. so i would do it my way above, since after compilation it will produce the same code, but much more readable (not too many brackets).

lhend said:

i would do it in this way:
#define ASSIGN_BIT_MASK(REG,VAL,MASK)      do {REG &= ~(MASK); REG |= VAL;} while(0);

Please without the ';' at the end.

lhend said:

without the brackets, your macro above will works

Yes, it would always clear out the masked values at the end rather than clearing the original content and or'ing the new value in.

However, I'd prefer this version (with the proper brackets) of your proposal because your marco does two read-modify-wite acceses to the register. It does not require any intermediate storage in a processor register for doing the math, but two read/write operations may cause unwanted side-effects and leave the hardware register in a potentially dangerous intermediate state for five clock cycles.