MSP430 Good C Code Practices

I know very little about c. I cannot answer your questions. Instead, I have a few question to ask you.

Mehdi Rahman said:

...  For these functions, in order to minimize the stack size, I try to avoid using local variables where I  can...

Why? I think the compiler might use CPU register as local variable. If you use global or static variable, the compiler might copy it to a CPU resister to use it locally and store the CPU register back to the global or static variable when finished. Either way, if there is not enough CPU register, one or more of them will be pushed to the stack and popped back later. So you are not saving any stack RAM either way. But using global or static variable will always occupy RAM "permanently" (for the duration of your entire code) in addition to the possible needs of using the stack RAM "temporarily" (for the duration of the function).

Mehdi Rahman said:

Based on others’ experience, is there a recommended level of optimization I should use with the compiler?

There is no any. Use one that's best for your application. If unsure - then you are not experienced enough to optimize, so don't.

Mehdi Rahman said:

On another note, when would one use heap in an MSP430 embedded system?

You shall not. Of course, there are exceptions but not for developers asking such kind of questions.

I would use C heap only when static allocations does not do the job - on msp430 having plenty of memory for "quick and dirty" application which will be run in controller environment - like my laboratory. For "always on" product used by end-users I would __never__ use C standard heap due to fragmentation which is hard to predict. If there's no other way around and heap needed for such type of product, I would create own heap manager (actually buffer manager).

Mehdi Rahman said:

 I already know the maximum size of buffers and such, and create pre-defined array sizes.  From my understanding, this isn’t really dynamic memory management and therefore does not fall under using the heap.

Right. You are doing well.

Mehdi Rahman said:

For these functions, in order to minimize the stack size, I try to avoid using local variables where I  can.

Why? - Don't do this without a grounded reason. Only case when you shall think of doing this - need for seemingly big temporary buffer which can be shared between non-reentrant functions. Even in this case you can size your stack accordingly and have no problems allocating buffer locally.

By moving local variables out of functions and trying to somehow save memory you can run into problems like this: 

int i;
void fn2(void) {
  for (i = 0; i < 10; i++)
    _NOP();
}
void fn1(void) {
  for (i = 0; i< 10; i++)
    fn2();
}

Another essence of unnecessary global variable allocation:

old_cow_yellow said:

But using global or static variable will always occupy RAM "permanently"

Mehdi Rahman said:

especially in regard to stack and heap management for MSP430 C Code.

The only instance where I used heap space so far was for my multitasking scheduler, where the stack and register storage space for the threads is dynamically allocated. Because the scheduler doesn’t know beforehand how many threads with how much stack need to be started.
However, it is allocated once and then used. And I could as well statically allocate the buffer in the thread’s code file and pass it to the scheduler on thread init. A bit less convenient but heap-free then. Hmmm, maybe I’ll change the scheduler to implement this once I work with it again :)

You should not use any local vars in main. All local vars are hidden from the linker. And local vars of main are never released. So put them outside main. Declare them as static to make them invisible to other code modules (local to the source file they reside in).

Also, large buffers should be allocated the same way. You’ll need the space anyway, but declaring them as local vars hides their space usage from the linker. A few INT and CHAR local vars would run in processor registers and the compiler can even re-usa the registers inside the function. But better don’t use local structs or arrays if you can avoid it.

And if you have any arrays that you initialize once but never change (lookup tables etc.), declare them as "const", so they end up in flash, not in ram.

 Your description of how you handle the other things sounds good to me.

Hi Everyone,

Thank you very much for the replies.

I was working with my code and was trying to use IAR Embedded Workbench to get a sense of my stack usage.  Initially IAR told me that the stack overflowed.  I then tried to gradually increase the stack and it still kept overflowing.  I then tried a simple UART echo example code, set the stack size to 500, and for some reason the stack still overflowed:  The stack 'Stack' is filled to 100% (500 bytes used out of 500). The warning threshold is set to 90.%

I must be using IAR incorrectly, as I can't see stack overflow being possible with such a simple example.  I would appreciate any suggestions/feedback.

Here is the code:

---------------------------------------------

//******************************************************************************
// MSP430F5342 Demo - USCI_A1, 115200 UART Echo ISR, DCO SMCLK
//
// Description: Echo a received character, RX ISR used. Normal mode is LPM0.
// USCI_A1 RX interrupt triggers TX Echo.
// Baud rate divider with 1048576hz = 1048576/115200 = ~9.1 (009h|01h)
// ACLK = REFO = ~32768Hz, MCLK = SMCLK = default DCO = 32 x ACLK = 1048576Hz
// See User Guide for baud rate divider table
//
// Built with IAR Embedded Workbench Version: 5.51.6
//******************************************************************************
#include <msp430f5342.h>

void main(void)
{
WDTCTL = WDTPW + WDTHOLD; // Stop WDT

UCSCTL3 = SELREF_2; // Set DCO FLL reference = REFO
UCSCTL4 |= SELA_2; // Set ACLK = REFO
UCSCTL0 = 0x0000; // Set lowest possible DCOx, MODx

// Loop until XT1,XT2 & DCO stabilizes - In this case only DCO has to stabilize
do
{
UCSCTL7 &= ~(XT2OFFG + XT1LFOFFG + DCOFFG);
// Clear XT2,XT1,DCO fault flags
SFRIFG1 &= ~OFIFG; // Clear fault flags
}while (SFRIFG1&OFIFG); // Test oscillator fault flag

__bis_SR_register(SCG0); // Disable the FLL control loop
UCSCTL1 = DCORSEL_5; // Select DCO range 16MHz operation
UCSCTL2 |= 249; // Set DCO Multiplier for 8MHz
// (N + 1) * FLLRef = Fdco
// (249 + 1) * 32768 = 8MHz
__bic_SR_register(SCG0); // Enable the FLL control loop

// Worst-case settling time for the DCO when the DCO range bits have been
// changed is n x 32 x 32 x f_MCLK / f_FLL_reference. See UCS chapter in 5xx
// UG for optimization.
// 32 x 32 x 8 MHz / 32,768 Hz = 250000 = MCLK cycles for DCO to settle
__delay_cycles(250000);

P4SEL |= BIT4+BIT5; // P4.4,5 = USCI_A1 TXD/RXD
UCA1CTL1 |= UCSWRST; // **Put state machine in reset**
UCA1CTL1 |= UCSSEL_2; // SMCLK
UCA1BR0 = 72; // 8MHz 115200 (see User's Guide)
UCA1BR1 = 0; // 8MHz 115200
UCA1MCTL |= UCBRS_7 + UCBRF_0; // Modulation UCBRSx=1, UCBRFx=0
UCA1CTL1 &= ~UCSWRST; // **Initialize USCI state machine**
UCA1IE |= UCRXIE; // Enable USCI_A1 RX interrupt

__bis_SR_register(LPM0_bits + GIE); // Enter LPM0, interrupts enabled
__no_operation(); // For debugger
}

// Echo back RXed character, confirm TX buffer is ready first
#pragma vector=USCI_A1_VECTOR
__interrupt void USCI_A1_ISR(void)
{
switch(__even_in_range(UCA1IV,4))
{
case 0:break; // Vector 0 - no interrupt
case 2: // Vector 2 - RXIFG
while (!(UCA1IFG&UCTXIFG)); // USCI_A1 TX buffer ready?
UCA1TXBUF = UCA1RXBUF; // TX -> RXed character
break;
case 4:break; // Vector 4 - TXIFG
default: break;
}
}

---------------------------------------------

Sincerely,

Mehdi

The semantics is rather strange.

Real stack overflow happens at run-time. IAR Linker does not have the foresight to predict if that will or will not happen. Instead, it allocates some RAM for the stack. It also allocates some RAM for other proposes. And if the total allocated RAM exceeds the RAM size of the chip, it gives you error messages. When that happens, and you tell it to allocated more RAM for stack, you are making the situation worse.

It is difficult to have "Good C Code Practice". In my opinion, using C Code is not good practice to begin with. (Of course, this is just my opinion. And in my opinion, my opinion is always right. Otherwise, I would not have such an opinion.)

Hi OCY,

Thanks for your reply.

I am sorry, I should have been more specific.  I am using IAR's debug tool.  I set an allocated stack size at the beginning, run the code using IAR, and stop the program.  If the stack size is greater than the allocated stack size, IAR debugger gives a warning.

I am still a bit puzzled as to how such a simple program could run into an issue like this.  When I ran the program, I did not even try a key press, so the program should have even entered the UART ISR.  It almost makes me think there is an issue with one of my other IAR settings.

Sincerely,

Mehdi

If you show us the source code of that "simple program", I may be able to find the problem. I use IAR KickStart.

Hi OCY,

You make a good point, as few embedded programs are truly simple.

Anywho, I did post the code in a previous post (look at the fifth post from the top).

Sincerely,

Mehdi

I do not have F5342 and I am not familiar with the UCS. I suggest that you test it as follows:

(1) Take your posted code and add the following lines at the very end. They should have no effect.

#pragma vector=0,2,4,6,8,10,12,14,16,18,20,22,24,26,28,30,32,34,36,38,40,42,44,46
__interrupt void trap_0_46(void)
{
  while (1);
}

#pragma vector=48,50,52,54,56,58,60,62,64,66,68,70,72,74,76,78,80,82,84,86,88,90
__interrupt void trap_48_90(void)
{
  while (1);
}

#pragma vector=94,96,98,100,102,104,106,108,110,112,114,116,118,120,122,124
__interrupt void trap_94_124(void)
{
  while (1);
}

(2) Compile/Link/Load the above to your F5342 as you did before. There should be no big difference.

(3) Enter Debug, but do not pass GO. Set a break-point before your code:

__bis_SR_register(LPM0_bits + GIE);

and also inspect the SP and high address end of RAM I think your stack should be 4 bytes deep because the c-startup code already called your main.

(4) Now tell Debugger "GO". When the break-point is reached, inspect the contents you SP and high address end of RAM again. There should be no change.

(5) Delete the previous break-point, and instead, set one at the beginning of your ISR

switch(__even_in_range(UCA1IV,4)) {...}

Now tell Debugger "GO" again. Do not send any character to the UART of your F5342. Simply wait.

(6a) If it reaches the break-point unexpectedly, find out why. Did the stack blow up? (I think not.)

(6b) If it keeps going, tell Debugger to "break" (the red hand icon). Inspect PC to find out if it is pointing to one of the three "while (1);" traps inside my additional code. Did the stack blow up? (I think not.)

(6c) On the other hand if PC is pointing to the "__no_operation(); " in your main(), this is expected.

Hi OCY,

Thank you for these instructions.

I did as you said, and the results were as you expected: 6a and 6b do not happen, but 6c does.

My problem was that instead of breaking, I stopped debugging while the debugger was running.  In this case, regardless of what I set the stack to, after ending the program, the debugger would tell me the stack overflowed.

Sincerely,

Mehdi

There are some oddities in the code.
The comments show “select DCO range for 16MHz” but you apparently want to run it on 8MHz. For 16MHz, you need to raise core voltage. Well, the default runs the DCO with an FLL input divider of two (so the DCO runs twice as fast) and MCLK is divided by the same factor, using DCOCLKDIV output. OS this might fit.
You do “UCSCTL2 |= 249”. Well, what about the already existing content of UCSCTL2? It’s not zero on startup. So likely, you get a different speed than you want. However, the difference won’t be high. IIRC, you’ll get a factor of 256, as the default setting is 31. Not too much of a difference.

But back to the original problem: In the posted code, I don’t see anything that could cause a stack overflow. No recursions, no nested interrupts, no huge local variable arrays, nothing.

Well, you do a busy-waiting loop inside an ISR (wait for UCTXIFG), which is a no-go (and not required anyway, as incoming and outgoing data has same speed, so TXBUF is always free for the echo). But it doesn’t cause a stack problem. I know that this is TI example code. A bad example in this case.

Now the debugger warning isn’t triggered by a filled stack but rather by the current position of the stack pointer. If using a multitasking OS, the OS may set the stack pointer to separate stack locations for the different threads. The debugger only notices the current position and informs you that it is outside the expected (reserved by the linker) area. But in this case this is not a problem at all and just expected behavior.

In your case, there is no OS, and I don’t see any other reason why there should be more than a few bytes on the stack.

To see whether the stack is really filled, you may fill your ram with a known magic value on start (in the debugger) and then check which parts of hit have been written to when you break later. It may give you a hint. But the posted code is so simple and straight that I doubt it is a code problem.

The only thing that comes in mind is that the stack pointer is initialized to a different location than the debugger expects. Normally, this shouldn’t happen, as the debugger gets the info of the expected stack pointer range from the same source (the linker script).