MSP430F5505 and number of MCU cycle for istructions

Thank you very much for your reply!

Indeed my variable was already declared as a global volatile variable.

Since I read it and modify it both inside interrupts and in normal code, before your answear i disabled interrupts before every assignment (plus nop) and re-enable after the assignment. Something like this:

#define HAL_INT_DISABLE()        __disable_interrupt(); asm(" NOP")    
#define HAL_INT_ENABLE()        __enable_interrupt()

volatile uint8 flgTx = 0;   // global variable

// somewhere outside interrupts

HAL_INT_DISABLE();    // atomic code

      flgTx = 0;

HAL_INT_ENABLE();

I'm happy to read that this is not necessary (it causes a lot of overhead in my code) and that i can avoid to disable and enable interrupts!

Is it the same for a read istruction?

if i do

if(flgTx)

 {   ..... }

or

dummy = flgTx;

can i be sure that as long as flgTx  is volatile, the evaluation of "flgTx" it's not interrupted by an interrupt (if flgTx it's an unsigned char or an unsigned int, not if it's a long)?

Or in the case of a read do i need to disable and re-enable interrupts? Something like this:

HAL_INT_DISABLE();    // atomic code

if(flgTx)

{  

     HAL_INT_ENABLE();

.....

}

And what about if the volatile variable it's inside a struct?

union SchedulerFlagUnion
{
    uint8 Byte;           

    struct
    {
        volatile uint8     Trasmetti:1;
        volatile uint8    StartLetturaCentralina:1;
        volatile uint8     CentralinaTrigger:1;
        volatile uint8    dummy:5;
    }flg;
};

union SchedulerFlagUnion SchedulerFlag;

if i want to write or read SchedulerFlag.flg.Trasmetti, is it the same case as flgTx? Can I avoid to disable and re-enalbe interrupt before the write/read ?

Now i think i have covered all my doubt ! :-)

Regards,

Carloalberto

Jens-Michael Gross said:

However, disabling and enabling interrupts around an access is tricky - you may enable interrups after the acces while they had been intentionally disabled already.

do you mean the following?

void function()

{

HAL_INT_DISABLE();

critical code

HAL_INT_ENABLE();

....
}

if somewhere you define the following critical section and you do not know that function() disable and re-enalbe interrupts:

HAL_INT_DISABLE();

assignment1;

function();

assignment2;

HAL_INT_ENABLE();

assignemnt2 it'executed with interrupts enabled even if a quick read would tell that interrupts are disabled.

Is it this that you mean?

Jens-Michael Gross said:

I posted a macro that allows nested usage of 'critical' or 'atomic' sections based on a global counter. Search for 'atomic' in the forum.

I foud your macro:

unsigned int ATOMIC_CNT;
#define ATOMIC(x) do{ATOMIC_CNT++;__disable_interrupts; x; ATOMIC_CNT--; if(!ATOMIC_CNT) __enable_interrupts;}while(0)

In order to make it work, I suppose you have to init ATOMIC_CNT to 0 before everything.

If i undestand correctly, this macro fixes the problem of nested critical section.

And what about the following TI proposal solution (I found it in an CC1101 application note)?

#define HAL_INT_LOCK(x)        do( (x) = __get_interrupt_state(); __disable_interrupt();)while(0)

#define HAL_INT_UNLOCK(x)      do( __set_interrupt_state(x);)while(0)

uint16 key;

HAL_INT_LOCK(key);

     critical code

HAL_INT_LOCK(key);

The only problem that I still can see (every solution we discussed about) is if you disable GIE inside interrupts with an instrinsic like _bic_SR_register_on_exit.

I never do that but I suppose someone may do it.

Anyway I think this is something that should never be done.

Jens-Michael Gross,

I'd like to know how would you handle a case like the following:

....

    TIMEOUT_TIMER_START(); // start timeout

    // Disable global interrupts
    HAL_INT_LOCK(key);
 
    // Wait until tx is done (or timeout is expired).
    while (!TXinfo.complete)
    {
        halMcuSetLowPowerMode(HAL_MCU_LPM_3);
        
        // Disable global interrupts
       __disable_interrupt();
        
        if(0 == flgTimeoutRxTx)    // if 0 timeout is expired
        {
            halDigioIntDisable(&pinGDO0);
            halDigioIntDisable(&pinGDO2);
            
            // Set previous global interrupt state
            HAL_INT_UNLOCK(key);
            
            return TX_STATE_TIMEOUT;
        }
    }

    TIMEOUT_TIMER_STOP();
 
    // Set prevois global interrupt state
    HAL_INT_UNLOCK(key);

....

I can not figure out how to use your atomic macro here. So I would let the code as it is. But i'd like to know what you would do.

Regards,
Carloalberto

I can think something like this:

#define    ATOMIC_UNLOCK    do{ATOMIC_CNT--; if(!ATOMIC_CNT) __enable_interrupts;}while(0)

#define ATOMIC(x)         do{ATOMIC_CNT++;__disable_interrupts; x; ATOMIC_CNT--; if(!ATOMIC_CNT) __enable_interrupts;}while(0)

    TIMEOUT_TIMER_START();

    ATOMIC
    (
        while (!TXinfo.complete)
        {
            halMcuSetLowPowerMode(HAL_MCU_LPM_3);
            
            __disable_interrupts;
            
            if(0 == flgTimeoutRxTx) 
            {
                halDigioIntDisable(&pinGDO0);
                halDigioIntDisable(&pinGDO2);
                
                ATOMIC_UNLOCK;
                
                return TX_STATE_TIMEOUT;
            }
        }

        TIMEOUT_TIMER_STOP();
      
        halDigioIntDisable(&pinGDO2);
    );

But i'd like to know the "right philosophy" :-)

Indeed, this is a problem case. exiting a function from within a code block of course is a problem for any outer code, inlcuding the ATOMIC macro.

I had similar problems when the compiler generated bogus 'code never reached' messages because it optimized the code in a way that I ha dthe same instructions twice, on exit of the inner loop if and on exit of the function. Optimization generated a direct jump to the function end and then generated this ugly warning on the instructions that were obsolete then. Only restructuring the code flow helped:

unsigned int retval = TX_STATE_FINE;
ATOMIC(
 while()
  {
  ...
    if()
    {
      ...
      retval = TX_STATE_TIMEOUT;
      break;
    }
  }
  if(retval==TX_STATE_FINE)
    TIMEOUT_TIMER_STOP();
)
...
return retval;