Go Down

Topic: [SOLVED] programming ATmega328p - undefined reference to _CLI() error (Read 6673 times) previous topic - next topic

xyanide1986

May 16, 2011, 01:20 pm Last Edit: May 16, 2011, 07:34 pm by xyanide1986 Reason: 1
For some reason AVR studio 4 is not recognizing the _CLI(); command in my code for reading a 16bit register.
Quote
error: undefined reference to `_CLI'


I'm reading the TCNT1 and ICR1 registers in the same manner suggested by the manual.

unsigned int TIM16_ReadTCNT1( void )
{
unsigned char sreg;
unsigned int i;
/* Save global interrupt flag */
sreg = SREG;
/* Disable interrupts */
_CLI();
/* Read TCNT1 into i */
i = TCNT1;
/* Restore global interrupt flag */
SREG = sreg;
return i;
}

All it should do is temporarily disable the interrupts during the execution of this read function right? I followed the manual since it says a 16bit read operation could get screwed up if an interrupt occurs during this process.
The compiler recognizes the ISR vectors that i'm using but it doesn't seem to know what to do with _CLI(), I thought they would be in the same library but it seems they're not.
I'm not sure if it should be in a certain library I don't know about, or that i'm supposed to write this function myself now.

The libraries i'm using:
#include <io.h>
#include <delay.h>
#include <interrupt.h>
#include <inttypes.h>

I'm programming this sort of thing for the first time and i'm pretty new to programming in general, thanks in advance.

graynomad

#1
May 16, 2011, 02:10 pm Last Edit: May 16, 2011, 02:13 pm by Graynomad Reason: 1
Is that an ISR routine?

If so you should need to do that work, interrupts are disabled until you return.

I haven't used Studio for a while, I think I just did cli().

Also in C I don't think you have to save SREG, is that what the manual says?

______
Rob
Rob Gray aka the GRAYnomad www.robgray.com

xyanide1986

That example function there is the one from the manual, yes it's in C by the manual but just a regular function. They also give an assembly example.
This function just handles the reading of the 16bit register.

Quote
It is important to notice that accessing 16-bit registers are atomic operations. If an interrupt
occurs between the two instructions accessing the 16-bit register, and the interrupt code
updates the temporary register by accessing the same or any other of the 16-bit Timer Registers,
then the result of the access outside the interrupt will be corrupted. Therefore, when both
the main code and the interrupt code update the temporary register, the main code must disable
the interrupts during the 16-bit access.


I'm using an external interrupt to do a read of this register and then clear the timer, then there's an ICP interrupt that takes a seperate timer value. That's why I think I should be using this form of reading:
-If the input capture pin is high, the TCNT1 value is automatically written to the input capture register (ICR1), after that the interrupt routine is used to put it in an integer.
-If INT0 to read the regular TCNT1 occurs at this point, it has a chance of corrupting the data going into ICR1 (if I understood the manual right).
Here's my code:

ISR(INT0_vect)
   {
   timer_readTCNT1();
   }

unsigned int timer_readTCNT1(void){      

   unsigned char sreg;
   unsigned int timer0;

   sreg = SREG;
                  
   _CLI();
       
        timer0 = TCNT1;
   TCNT1=0;               
   SREG = sreg;
   return timer0;
        }

   ISR(TIMER1_CAPT_vect)
   {
   timer_readICR1();
   }


unsigned int timer_readICR1(void){

   unsigned char sreg;
   unsigned int timer1;

   sreg = SREG;          
   _CLI();
   
   timer1 = ICR1;    
      
   SREG = sreg;         
   
   return timer1;
   }

graynomad

Your right, you do have to protect 16-bit access, I originally thought the code was already inside an ISR.

You don't have to save SREG though, I normally just wrap the protected code in a cli()/sei() pair.

Anyway did you try cli()?

______
Rob
Rob Gray aka the GRAYnomad www.robgray.com

xyanide1986

One of those A-HA moments here, I tried the line cli(); without capitals or underscore and it works (silly manual). I also need to add sei(); to restore i-bit now right?
My code now looks like this:

unsigned int timer_readTCNT1(void){      
  unsigned int timer0;
     
     cli();
     
  timer0 = TCNT1;
  TCNT1=0;              
 
     sei();  
  return timer0;
       }

Many thanks! It's been bothering me for a week.

Coding Badly

I also need to add sei(); to restore i-bit now right?


It's safer to use the technique in your first post...

unsigned int TIM16_ReadTCNT1( void )
{
unsigned char sreg;
unsigned int i;

/* Save global interrupt flag */
sreg = SREG;

/* Disable interrupts */
cli();


/* PROTECTED CODE GOES HERE */

/* Read TCNT1 into i */
i = TCNT1;

/* Restore global interrupt flag */
SREG = sreg;


return i;
}

xyanide1986

I'll stick to that then since it was suggested by the manual too, thanks for the quick responds guys!

graynomad

Why is that CB? In case the ISR doesn't do the right thing and protect SREG?

______
Rob
Rob Gray aka the GRAYnomad www.robgray.com

Coding Badly

That pattern allows TIM16_ReadTCNT1 to be safely called in any context (interrupts disabled {ISR} or enabled {loop}) without effecting the context.  By explicitly enabling interrupts (calling sei()) TIM16_ReadTCNT1 cannot be used in an interrupt service routine.  In other words, that pattern eliminates an annoying and easily forgotten detail (don't call TIM16_ReadTCNT1 from an ISR).

graynomad

OK, I get that, it's context safe and good "future proofing" in case you forget a year later and call that old useful function from an ISR.

______
Rob
Rob Gray aka the GRAYnomad www.robgray.com

Go Up