Best way to check if interrupt is enablet

Go To Last Post
19 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I see I just made a mistake in a program, and I can easy resolve it, but what is the correct way, I ask after all the fuss about cli and sei being moved.

Problem my eeprom write routine disable interrupt and at the end it enables it again!

Under init I don't have interrupt enabled yet, but I have added some code that count the number of times the micro have booted and now I enable interrupt as a side effect.

my code look like this:

void EEPROM_write(unsigned int uiAddress, unsigned char ucData){
/* Wait for completion of previous write */
  while(EECR & (1<<EEPE))
      ;
  cli();
/* Setup address and data registers */
    EEAR = uiAddress+10;
    EEDR = ucData;
/* Write logical one to EEMWE */
    EECR |= (1<<EEMPE);
/* Start eeprom write by setting EEWE */
    EECR |= (1<<EEPE);
  sei();
}

As I remember it's a 100% cut and paste from the datasheet.
I use WinAVR-20100110 if it make a difference

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Shirley, you should save the current state of SREG. Disable IRQs. And restore at the end.

Only you know whether interrupts were enabled originally.

i.e. what the ATOMIC macros do.

AFIK, the EEPROM only needs the atomic sequence for the magic 4-cycles. You can have interrupts running after the write has started.

David.

Last Edited: Tue. Mar 25, 2014 - 11:11 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
#include 

No RSTDISBL, no fun!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Shirley, you should save the current state of SREG. Disable IRQs. And restore at the end.

As a ASM programmer it's so wrong that it's legal to change the flags on return from a function.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

As a ASM programmer it's so wrong that it's legal to change the flags on return from a function.

Eh?

  cli
main:
  call change
loop:
  rjmp loop

change:
  sei
  ret

How is that not "legal" Asm code?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Rubbish. You are simply restoring the original (valid) state of SREG.

You could always just read the original I bit. And only SEI if required.

As an ASM programmer, I am sure that you have altered RET addresses, altered flags, ...
and many other 'risky' operations.

You have complete control with ASM. You just need to think carefully first !

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The Asm version of what David is talking about might be something like:

protected:
  push r16
  in r16, SREG
  cli
  ; do stuff
  out SREG, r16
  pop r16
  ret 

In C this would be:

#include 
void protected(void) {
  ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
    // do stuff
  }
}

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

yes but normally a ASM function (at least for me) will return with true/false in C or Z !
so in ASM it would be something like this:

test:
IN R0,SREG
CLI
..
..
SBIC R0,7
SEI
RET


;main code
rcall test
brne problems
..

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

BRIX wanted.
cshdt

No RSTDISBL, no fun!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have have used BRIC in some ASM code (but it could easily have been solved an other way).
The program had to play sounds so ISR was kind of the main program,(A bit like jespers DDS, but the other way around), and instead of disable ISR while updating from main (you could hear that), I set a flag in main and waited until ISR cleared it, that way I knew that the next n clk cycles updates could be done without problems, but that code I needed to branch over if ISR was disabled (in mute).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

david.prentice wrote:
You could always just read the original I bit. And only SEI if required.
How is that different from just doing an SEI?

Iluvatar is the better part of Valar.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

sparrow2 wrote:
yes but normally a ASM function (at least for me) will return with true/false in C or Z ! [...]

That looks like a much better way. I've always thought something wasn't quite right with the code that saved/restored sreg. When I first learned assembler (6502) the code I saw always used cli/sei.

What I don't understand is why not just use cli/sei? If interrupts are already turned off, then you don't need to use an atomic block, and therefore no reason to have to check for interrupts enabled or not.

I have no special talents.  I am only passionately curious. - Albert Einstein

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ralphd wrote:
sparrow2 wrote:
yes but normally a ASM function (at least for me) will return with true/false in C or Z ! [...]
That looks like a much better way.
Not in C. C has no single-bit type. A true/false return value must be returned via one of the defined types.

And, the language definition doesn't include the concept of a carry flag, zero flag, or any other condition code flag. These are machine-dependent concepts.

In fact, none of the condition codes are preserved (explicitly) across function calls (or often across simple C statements, although the optimiser in AVR GCC has a few tricks up its sleeves... again, implementation-dependent).

Quote:
I've always thought something wasn't quite right with the code that saved/restored sreg.
Huh?
asm                 C
--------------------------------
in r0,SREG    |     in r0, SREG
cli           |     cli
..            |     ..
..            |     ..
sbic r0,7     |     ..
sei           |     out SREG, r0
ret           |     ret

What's not right about it? If anything, the way it's generally done by a C compiler is 1 cycle faster than the 'much better' asm way. The fact that other condition codes are lost is of no consequence in C.

Quote:
If interrupts are already turned off, then you don't need to use an atomic block, and therefore no reason to have to check for interrupts enabled or not.
That's up to the programmer.

There is also:

ATOMIC_BLOCK(ATOMIC_FORCEON)

In the case of library code (think wdt.h), it can't make the assumption that interrupts aren't enabled.

If you need full control over condition codes or other tight control of machine-dependent behaviour, C isn't a good choice. Thankfully you can mix C and asm when you need it.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ralphd wrote:
sparrow2 wrote:
yes but normally a ASM function (at least for me) will return with true/false in C or Z ! [...]

That looks like a much better way. I've always thought something wasn't quite right with the code that saved/restored sreg. When I first learned assembler (6502) the code I saw always used cli/sei.

What I don't understand is why not just use cli/sei? If interrupts are already turned off, then you don't need to use an atomic block, and therefore no reason to have to check for interrupts enabled or not.

It is the same with any CPU. If interrupts are not enabled, there is no need to disable them.

You certainly do not want them to be enabled at the end of your function.

Of course, if you know they are enabled, you can happily use CLI, atomic_sequence, SEI.

In 6502, you examine the I status by PHP, PLA, AND #$02, BNE was_enabled

I really don't see the difference with AVR:
if (SREG & 0x80) goto was_enabled

It took me a while to understand the idea of RETI being SEI,RET. You have to preserve all the other bits in SREG manualy.
With the 6502, RTI is PLP,RTS. i.e. all your flags are restored.

OK, the AVR interrupt response expects you to do the PHP manually. If you have a trivial ISR(), you can avoid saving SREG.
The 6502 affects flags with almost every opcode. So I can't imagine any trivial ISR().

David.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

joeymorin wrote:
ralphd wrote:
sparrow2 wrote:
yes but normally a ASM function (at least for me) will return with true/false in C or Z ! [...]
That looks like a much better way.
Not in C. C has no single-bit type. A true/false return value must be returned via one of the defined types.

C = carry flag
Z = zero flag

I have no special talents.  I am only passionately curious. - Albert Einstein

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
C = carry flag
Z = zero flag
Not in C.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Koshchi wrote:
Quote:
C = carry flag
Z = zero flag
Not in C.

The reference was to "a ASM function", not a C function.

I have no special talents.  I am only passionately curious. - Albert Einstein

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

ralphd wrote:
C = carry flag
Z = zero flag
No kidding?:
joeymorin wrote:
the language definition doesn't include the concept of a carry flag, zero flag, or any other condition code flag.
Were you not commenting on @sparrow2's post?:
sparrow2 wrote:
yes but normally a ASM function (at least for me) will return with true/false in C or Z !
And was he not making a (implicit) comparison between the C-generated code for an atomic block vs. how he would do it in asm?

It seemed clear the point was that restoring all of SREG would preclude the use of C or Z for a boolean function return code. Since that approach confers no advantage to C, restoring all of SREG is the better choice since it is one word/cycle shorter.

In asm, if speed is the most important thing, and flash is plentiful, duplicate the body of the atomic block:

  brid  I_disabled
  cli
  ..  ; duplicated 
  ..  ; atomic block
  sei
  ..  ; duplcated
  ..  ; code
  ret
I_disabled:
  ..  ; duplicated
  ..  ; atomic block
  
  ..  ; duplicated
  ..  ; code
  ret

Costs 3 cycles if 'I' enabled, 2 if 'I' disabled. No GP register required, so this might save a push/pop if registers are scarce. Flash costs can be high since you must duplicate all code up to the ret.

Alternatively, duplicate only the atomic block by using brie:

  brie  I_enabled
  ..  ; duplicated 
  ..  ; atomic block
  rjmp continue
I_enabled:
  cli
  ..  ; duplicated
  ..  ; atomic block
  sei
continue:

Costs 3 cycles if 'I' disabled, 4 cycles if 'I' enabled. This is actually no better for the 'I disabled' case than @sparrow2's example, but ultimately may be faster if it avoids a register push/pop.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
The reference was to "a ASM function", not a C function.
Wouldn't you want to keep a common ABI for freedom of interworking ?