Shadow Register Problem

[Adam Y.]

I have ran into a problem that someone here might be able to answer. The problem is that I am programing a microcontroller using bit fields created in a structure. The problem comes as I am trying to modify a series of registers that only have 32-bit access. I can't read or write to these registers using a conventional bit field because otherwise the registers would be corrupted. The solution I have read makes perfect sense. You create another structure and then load up the entire 32-bit of the same register and then modify and read the bit fields from that shadow register. Unfortunately, this makes the code a lot more convoluted in in that you constantly have to be making sure that both structures are being updated as seen in the following example I have below which is only checking the a flag. Any solutions on how to make this code more readable or is this the best way to program those registers?
Edit: I know I can just write to those registers in one go but that is a step backwards in readability from where I am right now.

 while(ECanaShadow.CANMC.bit.CCR==0){ECanaShadow.CANMC.all=ECanaRegs.CANMC.all;};
}

[The Lucas]

Do you consider macros more or less readable than what you have?

A macro for that statement off the top of my heads is

#define writeShadow(S,R)   while((S).CANMC.bit.CCR==0){(S).CANMC.all=(R).CANMC.all;};

Then you just say:

writeShadow(ECanaShadow,ECanaRegs)

I haven't tested that, but it should be close. I assume those are the structs that are going to change from statement to statement. Anyway it is a start if you like it.

[Adam Y.]

Originally Posted by The Lucas

Do you consider macros more or less readable than what you have?

A macro for that statement off the top of my heads is

#define writeShadow(S,R)   while((S).CANMC.bit.CCR==0){(S).CANMC.all=(R).CANMC.all;};

Then you just say:

writeShadow(ECanaShadow,ECanaRegs)

I haven't tested that, but it should be close. I assume those are the structs that are going to change from statement to statement. Anyway it is a start if you like it.

Yes. This is exactly what Im thinking off. Thank you. It's exponentially much easier to read that than having two structures whose only difference is the name.

I haven't tested that, but it should be close. I assume those are the structs that are going to change from statement to statement. Anyway it is a start if you like it.

The two structures are going to be the same but both structures have multiple registers defined. So my macro is going to have the register defined and then the specific bit of the register.

#define writeShadow(S,R)   ECanaShadow.(S).bit.(R)

Unfortunately, its not working because the compiler is giving me an error that it's expecting a field name.

[Dave K.]

Originally Posted by Adam Y.

I have ran into a problem that someone here might be able to answer. The problem is that I am programing a microcontroller using bit fields created in a structure. The problem comes as I am trying to modify a series of registers that only have 32-bit access. I can't read or write to these registers using a conventional bit field because otherwise the registers would be corrupted. The solution I have read makes perfect sense. You create another structure and then load up the entire 32-bit of the same register and then modify and read the bit fields from that shadow register. Unfortunately, this makes the code a lot more convoluted in in that you constantly have to be making sure that both structures are being updated as seen in the following example I have below which is only checking the a flag. Any solutions on how to make this code more readable or is this the best way to program those registers?
Edit: I know I can just write to those registers in one go but that is a step backwards in readability from where I am right now.

 while(ECanaShadow.CANMC.bit.CCR==0){ECanaShadow.CANMC.all=ECanaRegs.CANMC.all;};
}

How a particular compiler packs bits is something which can vary from compiler to compiler. While many may allocate the bits in order, others may not, or may choose to order them differently.

I realize that some CPU vendor's header files promote this approach, and for CPU specific hardware registers, portablity isn't a big concern. Where I've seen problems like this create a mess is many years later, when you want to impliment a new compiler, from a new vendor.

That said, I'm unclear as to why you are unable to operate directly on the registers with your structures. If the compiler is generating code that utilizes bit set/clear instructions, those should be read/modify/write type of operations and should perform no differently than you manually read/modify/writing the registers as you seem to propose.

The only time I've had to 'shadow' anything is when dealing with 'write only' types of registers, and that just amounts to keeping a local variable which contains the correct data, and then when modified, gets pushed out to the hardware perhipheral... so perhaps this is the scenario you are trying to address.

As long as you are pursuing non-portable solutions, you might also look at defining a union between an unsigned long (I'm assuming your unidentified CPU considers this a 32 bit variable) and your structure. With the union, you could read the hardware register into a u32, operate on the bits within the struct, and then store the u32 back into the register.


My personal preference for working with hardware registers is to create #defines and just use bit and/or operations to modify the registers as desired. Its both portable and readable.

#define CANMC_CCR   0x1000
#define CANMC_XYZ   0x2000
// etc.

u32 ltmp;

    ltmp = hardware_register
    ltmp |= CANMC_XYZ;        // turn on XYZ
    ltmp &= ~CANMC_CCR;     // turn off CCR
    hardware_register = ltmp;

    // and you can combine bits like this, which will generate only one OR instruction rather than multiples for each bit set (or clear) action.

    ltmp |= CANMC_XYZ + CANMC_CCR

[Adam Y.]

That said, I'm unclear as to why you are unable to operate directly on the registers with your structures. If the compiler is generating code that utilizes bit set/clear instructions, those should be read/modify/write type of operations and should perform no differently than you manually read/modify/writing the registers as you seem to propose.

Well. Reading the manual it makes it pretty clear that if Im going to do any operations to certain registers that I either have to do it to all 32-bits or risk corrupting the register which would actually mean your example code wouldn't work unless you added a few extra zeros. Don't ask me why because its not applicable for any of their other peripherals that I have used. Only the CAN bus is like this.

I realize that some CPU vendor's header files promote this approach, and for CPU specific hardware registers, portablity isn't a big concern. Where I've seen problems like this create a mess is many years later, when you want to impliment a new compiler, from a new vendor.

Im not too worried about this happening because there is probably only going to be one compiler for this device for as long as I live.

[The Lucas]

Originally Posted by Adam Y.

Yes. This is exactly what Im thinking off. Thank you. It's exponentially much easier to read that than having two structures whose only difference is the name.

The two structures are going to be the same but both structures have multiple registers defined. So my macro is going to have the register defined and then the specific bit of the register.

#define writeShadow(S,R)   ECanaShadow.(S).bit.(R)

Unfortunately, its not working because the compiler is giving me an error that it's expecting a field name.

Macros are very literal, so try it without the ().

#define writeShadow(STRCT, BITN)   ECanaShadow.STRCT.bit.BITN

Generally you always use () since you are usually doing math with them and the () keep the order of ops correct. But in this case I think they are messing up the variable name (you might want to print out a macro to test it). Also shouldn’t there be more to your statement than

ECanaShadow.STRCT.bit.BITN

like

ECanaShadow.STRCT.bit.BITN = 
ECanaRegs.STRCT.bit.BITN

[Dave K.]

Originally Posted by Adam Y.

Well. Reading the manual it makes it pretty clear that if Im going to do any operations to certain registers that I either have to do it to all 32-bits or risk corrupting the register which would actually mean your example code wouldn't work unless you added a few extra zeros. Don't ask me why because its not applicable for any of their other peripherals that I have used. Only the CAN bus is like this.

Im not too worried about this happening because there is probably only going to be one compiler for this device for as long as I live.

... I just wanted you to be aware that portability issues exist.

The example code would work fine for the bit positions denoted, as there is no need to prepend zero's to a number. Certainly if you wanted to denote a bit position greater than 16, then it would be proper to add something like a 'UL' suffix to the constant.

Since you haven't mentioned which CPU this is for, and curiosity got the better of me, I used Google and turned up some example code for the TMS320 series.

TI's example code is inconsistent, some of it shows direct manipulation, others show using this shadow register workaround.

Finally, one example came clean with the following embedded comment:

/* Create a shadow register structure for the CAN control registers. This is
needed, since, only 32–bit access is allowed to these registers. 16–bit access
to these registers could potentially corrupt the register contents. This is
especially true while writing to a bit (or group of bits) among bits 16 – 31 */

struct ECAN_REGS ECanaShadow;

There's a better treatment in TI document SPRAA85A, if you aren't already aware of it.

... ok, so this is a workaround for the C compiler which may generate a 16 bit read/modify/write operation. It is an understandable optimization for the compiler, but a bit surprising that TI doesn't offer some native functionality to denote memory locations that are 32bit wide access only.

Since optimization appears to be the problem, then my examples of |= and &= would similarly be optimized and create the same problem.


In any event, this seems to work ok:

struct foo {
	unsigned bit1:1;
 	unsigned bit2:1;
	unsigned bit3:1;
	unsigned many:3;
};

struct bar {
	struct foo cat;
	struct foo dog;
};

struct bar x;
struct bar y;

#define eCANWrite(a,b,c) y=x; y.a.b=c; x=y;

void test(void) {

	eCANWrite(cat,many,1);

        // produces the same code as

	y=x;
	y.cat.many = 1;
	x=y;

}

[Adam Y.]

TI's example code is inconsistent, some of it shows direct manipulation, others show using this shadow register workaround.

I am sorry but I was not explicit in the fact that only certain registers need the manipulation others can just be written to directly. I am pretty sure anything from the Mailbox in the name can be modified directly. In addition to that fact their example code is not the best example because I did in fact find errors in their code for their CAN bus. To configure the pins as the CAN transmit and receive they have it being done with and without the shadow registers in two different examples which had me confused yesterday.

... I just wanted you to be aware that portability issues exist.

I understand. The way the chip is designed though I doubt it would be easy to port the code because of how many different TI specific features it has. Also, I have never done anything like this ever in my entire life so I might just rewrite the entire code at a latter time to make it more portable.

Since you haven't mentioned which CPU this is for, and curiosity got the better of me, I used Google and turned up some example code for the TMS320 series.

Its for the TMS320F2801 processor and that example code is probably what I have all ready on my computer.