sei() innerhalb eines ISR()

OP Persönliche Seite #5102479
Lesenswert?

Hi,

ich verwende gerade jede Menge I2C Aufrufe. Eine davon sogar im ISR
1
uint8_t ks;
2
...
3
ISR( TIMER0_OVF_vect) {
4
    ks = I2C_ReadRegister(HT16K33, 0x41);
5
    ...//do a little bit more
6
}

Nun habe ich pauschal alle I2c_* Funktionen mit einem cli()/sei(); 
umschlossen.
1
uint8_t I2C_ReadRegister(uint8_t busAddr, uint8_t deviceRegister) {
2
  cli();
3
  uint8_t data = 0;
4
  i2c_start(busAddr); // send device address
5
  i2c_write(deviceRegister); // set register pointer
6
  i2c_start(busAddr + I2C_READ); // restart as a read operation
7
  data = i2c_readNak(); // read the register data
8
  i2c_stop(); // stop
9
  sei();
10
  return data;
11
}

Früher gab es ja noch SIGNAL und INTERRUPT (<-- heutige ISR)
SIGNAL hat dabei ja Interrupts während der IR-Routine erlaubt.

Meine Frage wäre daher nun, ob ich mir künstlich wieder ein SIGNAL 
gebaut habe, dadurch dass ich in der ISR einen Funktionsaufruf verwende 
der sei() enthält?

Oder wird das I-Bit unter keinen umständen verändert, solange ich mich 
in der ISR befinde und bin somit safe?

Grüße Oekel
#5102486
Lesenswert?

@ D a v i d K. (oekel)

>ich verwende gerade jede Menge I2C Aufrufe. Eine davon sogar im ISR

Naja, hoffentlich ist die ISR-Frequenz nicht zu hoch.

>Nun habe ich pauschal alle I2c_* Funktionen mit einem cli()/sei();
>umschlossen.

Warum? Das ist im Normalfall nicht nötig. I2C ist synchron und du nutzt 
ja das I2C Modul deines AVRs. Da darf so ein I2C Zugriff auch mal durch 
einen ISR unterbrochen werden.

>Früher gab es ja noch SIGNAL und INTERRUPT (<-- heutige ISR)
>SIGNAL hat dabei ja Interrupts während der IR-Routine erlaubt.

Alter Kram. Vergiss das.

>Meine Frage wäre daher nun, ob ich mir künstlich wieder ein SIGNAL
>gebaut habe, dadurch dass ich in der ISR einen Funktionsaufruf verwende
>der sei() enthält?

Nun ja, selbst wenn man eine Funktion atomar ausführen will, sollte man 
im Normalfall die Funktionen aus util/atomic.h nutzen. Denn die können 
am Ende des atomaren Blocks den vorherigen Zustand des I-Bit wieder 
herstellen. Sprioch, wenn es vorher gelöscht war, bleibt es gelöscht!
1
#include <util/atomic.h>
2

3
    ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
4
        // atomarer Funktionsblock, Interrupts sind gesperrt
5
    }

Siehe Doku der libc.

>Oder wird das I-Bit unter keinen umständen verändert, solange ich mich
>in der ISR befinde und bin somit safe?

Nö. Ein sei() setzt das I-Bit. Immer.
Gast #5102489
Lesenswert?

Du kannst sei() natürlich auch innerhalb einer ISR verwenden. Du 
solltest aber auf jeden Fall vor sei() dafür sorgen, dass der aktuelle 
Interrupt (Also bei dir der Timer0 Overflow) deaktiviert ist, weil sonst 
die Gefahr besteht, dass der Stack überläuft.

Also nach dem Schema
1
ISR( TIMER0_OVF_vect) {
2
// whatever
3
TIMSK &= ~(1<<TOIE0);
4
sei();
5
// noch mehr Code
6
cli();
7
TIMSK |= 1<<TOIE0;
8
return;
9
}


Bei dir ist das aber eigentlich gar nicht sinnvoll, weil du ja keine 
Interrupts während deines Timer-Overflows benötigst (Zumindest macht 
dein Codeschnipsel den Anschein). Ich würde an deiner Stelle eher das 
Statusregister speichern und anschließend wiederherstellen.

Aus dem GCC Tutorial:
1
I2C(){
2
uint8_t tmp_sreg = SREG;
3
cli();
4
// mach was
5
SREG = tmp_sreg;
OP Persönliche Seite #5102499
Lesenswert?

Falk B. schrieb:
> @ D a v i d K. (oekel)
>
>>ich verwende gerade jede Menge I2C Aufrufe. Eine davon sogar im ISR
>
> Naja, hoffentlich ist die ISR-Frequenz nicht zu hoch.
Teiler: 1024

> Nun ja, selbst wenn man eine Funktion atomar ausführen will, sollte man
> im Normalfall die Funktionen aus util/atomic.h nutzen. Denn die können
> am Ende des atomaren Blocks den vorherigen Zustand des I-Bit wieder
> herstellen. Sprioch, wenn es vorher gelöscht war, bleibt es gelöscht!
>
>
1
> #include <util/atomic.h>
2
> 
3
>     ATOMIC_BLOCK(ATOMIC_RESTORESTATE) {
4
>         // atomarer Funktionsblock, Interrupts sind gesperrt
5
>     }
6
>


Mein theoretisches Szenario wäre das sich das ganze wie folgt mal michen 
könnte:
1
void I2C_WriteRegister(uint8_t busAddr, uint8_t deviceRegister, uint8_t data) {
2
  cli();
3
  i2c_start(busAddr);
4
  i2c_write(deviceRegister);
5
        //...nun kommt der ISR..
6

7
        //uint8_t I2C_ReadRegister(uint8_t busAddr, uint8_t deviceRegister) {
8
  cli();
9
  uint8_t data = 0;
10
  i2c_start(busAddr); // send device address
11
  i2c_write(deviceRegister); // set register pointer
12
  i2c_start(busAddr + I2C_READ); // restart as a read operation
13
  data = i2c_readNak(); // read the register data
14
  i2c_stop(); // stop
15
  //sei();
16
        //}
17

18
        //...weiter gehts
19
  i2c_write(data);
20
  i2c_stop();
21
  sei();
22
}

Sprich es kommt im besten Fall Müll auf dem Display an (dafür verwende 
ich das "I2C_WriteRegister") im Schlimmsten Fall hängt er sich auf? 
(darartigen Fall habe ich gerade sporadisch in der Praxis und schiebe es 
auf den ISR.
(konkret läuft eine Endlosschleife mit Testausgaben: 40-100x ok. dann 
steht alles...)
Gast #5102505
Lesenswert?

cli und sei in Funktion vermeiden. Maximal im Hauptstrang. oder vorher 
fragen, ob Interrupts gesperrt sind. Falls ja, dann nachher nicht 
freigegeben. Erfordert aber genaue Überlegungen.

Auch braucht man oft getrennte Funktionen für gleiche Aufgaben: eine für 
den Aufruf aus Interrupts.
OP Persönliche Seite #5102507
Lesenswert?

PPS:

folgenden minimal-Wert für "multi" habe ich gerade per TryAndError für 
mich ermittelt:
1
void timer0_init() {
2

3
  TCCR0 = (CS00 || CS02); /* Takt 1024 */
4

5
  //"Timer/Counter Interrupt Mask" (TIMSK)
6
  //"Timer/Counter Interrupt Enable"(TOIE0)
7
  TIMSK |= (1 << TOIE0);
8

9
}
10
uint16_t multi = 0;
11
ISR( TIMER0_OVF_vect) {
12

13
  if (multi == 0) {
14
    multi = 350;
15

16
    if(SW2){
17
      PORTC ^= ( 1 << PC6 );
18
      SW2 = false;
19
    }
20
    updateSwitches();
21
  }
22
  multi--;
23
}
24

25
void updateSwitches() {
26

27
  if (SW1 || SW2 || SW3)
28
    return;
29
  //uint16_t ks = Key_ReadCom(uint8_t ks);
30
  uint8_t ks = I2C_ReadRegister(HT16K33, 0x41);
31
  SW1 = (ks & 0b00010000);
32
  SW2 = (ks & 0b00001000);
33
  SW3 = (ks & 0b00000100);
34
}

Wieso ich allerdings hier schon 1024*350 Takte für brauche erschließt 
sich mir nicht.
#5102511
Lesenswert?

@D a v i d K. (oekel)

>Mein theoretisches Szenario wäre das sich das ganze wie folgt mal michen
>könnte:

Stimmt. Dann braucht man eine atomare Sicherung. Aber in den meisten 
Fällen ist es besser, keinerlei I2C Zugriffe in der ISR zu machen und 
statt dessen alle Zugriffe nur als normale Funktion in der 
Hauptschleife.

https://www.mikrocontroller.net/articles/Interrupt#Steuersignale_zwischen_ISR_und_Hauptprogramm

Das hat auch den Vorteil, daß man sehr langsame Funktionen nutzen kann, 
die in einer ISR nichts zu suchen haben.
#5102610
Lesenswert?

@ D a v i d K. (oekel)

>Hatte ich auch ursprünglich nicht vor. Doch wie die Flanken eines
>Tasters am HT16K33 rechtzeitig erkennen, wenn ich nicht ständig auf dem
>I2C-Bus pulle?

Man kann die periodische Abfrage (vulgo "pulle") innerhalb einer 
Funktion im Hauptprogramm machen. Wie das geht, steht in meinem Link.
OP Persönliche Seite #5103144
Lesenswert?

Falk B. schrieb:
> @ D a v i d K. (oekel)

> Man kann die periodische Abfrage (vulgo "pulle") innerhalb einer
> Funktion im Hauptprogramm machen. Wie das geht, steht in meinem Link.

Danke, hab es jetzt genau so gelöst.
Die Hauptroutine wirkte anfänglich so riesig, dass ich vermutet habe 
"dort muss unbedingt zwischengesprungen werden". Aber tatsächlich wird 
jeder REST-Zweig mit 5-6 if-Abfragen erreicht und die vorgelagerte
1
if(flag)
2
  alter_ISR_Teil();
3
else
4
  REST();

stört den "REST-Ablauf" nicht wirklich.
Endlich wieder Determinismus. Danke!

PS: In der Theorie hatte ich das alles schon gelesen (lese ja viel der 
hiesigen Wiki-Artikel). In der Praxis muss man aber immer wieder drauf 
gestoßen werden, um seinen Code zu strukturieren.

Antwort schreiben

Bitte melde dich an, um einen Beitrag zu schreiben.

oder

Mit Google-Account einloggen

Die Registrierung ist kostenlos und dauert nur eine Minute.

Jetzt registrieren