Interrupt unterdrückung mit cli/sei?

[Fabian S. (jacky2k)]

Hallo,
ich stehe hier vor einem sehr seltsamen Problem. Ich habe im Atmega 
einen SPI Slave implementiert, der ca. alle 10µsec nen Byte bekommt. Bei 
Empfang wird der Interrupt ausgelöst und das Byte in einen fifo 
geschrieben.
Es geht mir nun um das auslesen aus dem Fifo von der main aus, speziell 
um die Variable, die den aktuell belegten Speicher im Fifo beinhaltet.
Wenn ich nun in der main() ein fifo.used--; mache ist das ja kein 
atomarer Befehl, sprich er kann unter brochen werden von dem Interrupt, 
welcher dann ein fifo.used++; macht und schon habe ich inkonsistente 
Daten.
Dafür hatte ich nun um das fifo.used--; ein cli()/sei() gemacht, jedoch 
führt das aus mir nicht ganz erklärlichen Gründen dazu, dass nicht mehr 
alle Bytes angekommen.
Ich dachte, wenn ich cli() mache werden keine Interrupts ausgeführt, 
sollte jedoch ein Flag gesetzt werden, wird der Interrupt ausgeführt, 
sobald ich wieder sei() mache. Ist die Annahme falsch?

[Floh (Gast)]

Fabian S. schrieb:
> Dafür hatte ich nun um das fifo.used--; ein cli()/sei() gemacht, jedoch
> führt das aus mir nicht ganz erklärlichen Gründen dazu, dass nicht mehr
> alle Bytes angekommen.

Dann brauchst du zum Auslesen länger als  (10us - Zeitbedarf für einen 
Interrupt).
:-)

[Sam .. (sam1994)]

Wenn aber 2 Bytes in dieser Zeit ankommen, nützt das nichts. Das Flag 
sagt nicht wie viele Byte und welche angekommen sind.

[Fabian S. (jacky2k)]

Es sollten in der Zeit laut meiner Berechnung keine zwei Bytes ankommen.

cli();
fifo->used--;
sei();

Ich würde mal schätzen, das sind 5 Takte. Alle 10µsec kommt im 
schlimmsten Fall ein Byte, bei 16Mhz macht das 160 Takte, die ich Zeit 
habe.
Ich wage mal zu bezweifeln, dass das zwei ankommen während dieser kurzen 
Zeit.

[Fabian S. (jacky2k)]

Habe das mal eben simuliert, die drei Befehle brauchen 7 Takte, also bei 
weitem weniger als ich Zeit habe. Wo liegt dann das Problem?

[Floh (Gast)]

Fabian S. schrieb:
> Wo liegt dann das Problem?

Zeig doch mal deinen Code. Vielleicht ist ja irgendwo sonst der Hund 
drin.
:-)

[Sam .. (sam1994)]

Fabian S. schrieb:
> Habe das mal eben simuliert, die drei Befehle brauchen 7 Takte, also bei
> weitem weniger als ich Zeit habe. Wo liegt dann das Problem?

sry dann hab ich zuerst deine Beschreibung falsch verstanden.

[Fabian S. (jacky2k)]

Also den ganzen Code kann ich hier nicht posten ;) Ich versuche es mal 
auf das wesentliche zu beschränken:

ISR(SIG_SPI)
{
  PORTA |= (1<<5);
  uint8_t data=SPDR;
  fifoPush(&fifoIn, data);
  PORTA &= ~(1<<5);
}

typedef struct _fifo
{
  volatile uint16_t used;
  volatile uint8_t readPos;
  volatile uint8_t writePos;
  volatile uint8_t data[256];
} Fifo;

void main()
{
  while(1)
  {
    char buffer[200];
    for(i=0; i<200; i++)
    {
      while(!fifoIn.used);
      last++;
      current=fifoPop(&fifoIn);
      if(current!=last)
      {
        //uart_putcc("error: \n");
        PORTD |= (1<<7);
        last=current;
      }
      else
        PORTD &= ~(1<<7);
      buffer[i]=current;
    }
}

uint8_t fifoPop(Fifo* fifo)
{
  if(!fifo->used)
    return 0;
  uint8_t data=fifo->data[fifo->readPos];
  fifo->readPos++;
  //uint8_t sreg=SREG;
  cli();
  fifo->used--;
  //SREG=sreg;
  sei();
  return data;
}

inline void fifoPush(Fifo* fifo, uint8_t data)
{
  if(fifo->used==256)
  {
    //PORTD ^= (1<<7);
    return;
  }
  fifo->data[fifo->writePos]=data;
  fifo->used++;
  fifo->writePos++;
}

In dem Struct fifoIn wird beim starten alles auf 0 initialisiert (außer 
den Daten). Senden tuh ich vom Master immer 0,1,2,3,...,254,255,0,1,... 
und das kommt im Interrupt auch genau so an. Jedoch sehe ich am Scope, 
dass PD7 immer fleißig am flimmern ist, und das auch sehr unregelmäßig.

Interessant ist auch, wenn ich das cli/sei komplett raus nehme scheint 
alles zu funktionieren, jedoch habe ich Angst, dass dann mal ein Byte 
flöten geht, was fatal wäre.

[Thomas E. (thomase)]

Warum nimmst du nicht 2 Indexer? FifoRd und FifoWr.

Wenn etwas im Buffer ist, dann ist FifoRd != FifoWr.

Das: if(fifo->used==256) funktioniert nicht. unint8 ist maximal 255.

mfg.

[Fabian S. (jacky2k)]

> Warum nimmst du nicht 2 Indexer? FifoRd und FifoWr.
> Wenn etwas im Buffer ist, dann ist FifoRd != FifoWr.
>
Das ist ne Idee, aber kann ich dann auch die aktuelle Größe bestimmen? 
Das müssten dann ja zwei 16 bit sein, aber auch die laufen irgendwann 
einmal über...

> Das: if(fifo->used==256) funktioniert nicht. unint8 ist maximal 255.

volatile uint16_t used;

[Peter D. (peda)]

160 Takte ist nicht viel, die können schnell durch Interrupthandler 
draufgehen.
Insbesondere Unterfunktionsaufrufe sind sehr teuer (viel Push/Pop).
Ein einziger Befehl zuviel und es kann krachen.

Laß den Master mal langsamer senden, obs dann geht.


Peter

[Thomas E. (thomase)]

Fabian S. schrieb:
> volatile uint16_t used;

Hab' jetzt auch gesehen.

Fabian S. schrieb:
> Das müssten dann ja zwei 16 bit sein, aber auch die laufen irgendwann
> einmal über...

Mußt du alles gar nicht machen.

Dein Eingangsbuffer ist 256 Byte groß. Dann nimmst du 2 Indexer als 
uchar
und wenn die bei 256 angekommen sind, gehen sie wieder auf 0. Weil 256 
können sie ja nicht.

mfg.

[Fabian S. (jacky2k)]

Je langsamer ich den Master senden lasse, desto seltener treten die 
Fehler auf.
Damit ich das mit dem Interrupt-Zeit im Auge habe setze ich beim Start 
ein Pin auf high und am ende Auf low, das Scope sagt mir, dass der nicht 
mal 20% der Zeit benötigt, zugegeben, es kommen noch ein paar Zyklen an 
Aufräumarbeiten dazu, das sollten jedoch nicht mehr als 20 sein. In 
Assembler braucht er 3 Takte für den Sprung in den Interrupt und 5 
wieder raus. Dazu kommen noch ein paar zum sichern der Register, weiß 
nicht wie effizient gcc da ist.
Was die Funktionsaufrufe angeht: Welche Funktionsaufrufe meinst du? Da 
ist nur fifoPush und das ist inline, also kein push/pop.

[Thomas E. (thomase)]

Peter Dannegger schrieb:
> Insbesondere Unterfunktionsaufrufe sind sehr teuer (viel Push/Pop).
>
> Ein einziger Befehl zuviel und es kann krachen.

Du brauchst in der ISR nicht mehr als das:

nInBuf[nIndW++] = SPDR;

Und in der main:

if (nIndW != nIndR)
{
  Lesen und verarbeiten.
}

Fabian S. schrieb:
> aber kann ich dann auch die aktuelle Größe bestimmen?

Ist das wichtig? Du liest Daten solange welche da sind.

mfg.

[Fabian S. (jacky2k)]

Deine Lösung hat einen Haken: Was passiert, wenn ich 256 Bytes schreibe 
ohne eins zu lesen? Dann stehen beide Zeiger auf 0 und es sind keine 
Daten im Puffer? Dafür ist die used Variable.

Und ja, es ist wichtig, dass ich weiß wie viele Daten im Puffer sind, da 
ich Befehlsblöcke verarbeiten muss und ich kann die nur verarbeiten, 
wenn alle Daten des Befehls da sind.

[Fabian S. (jacky2k)]

Alles klar, ich denke ich habs gelöst. Der Ansatz vom Thomas war doch 
nicht so falsch wie ich dachte. Mit einer kleinen Sonderbedingung beim 
Schreiben habe ich das Problem umgangen:

inline void fifoPush(Fifo* fifo, uint8_t data)
{
  if(fifo->writePos+1==fifo->readPos)
    return;
  fifo->data[fifo->writePos]=data;
  fifo->writePos++;
}

Dann habe ich zwar nur 255 statte 256 Bytes zur Verfügung aber das macht 
den Kohl nun auch nicht fett.

Die aktuelle Größe des Puffers müsste sich dann so bestimmen lassen:

(fifo->writePos-fifo->readPos)

Ich bedanke mich recht herzlich für die Hilfe.
Hat noch jemand etwas auszusetzen? Habe ich etwas übersehen?

[Thomas E. (thomase)]

Fabian S. schrieb:
> Deine Lösung hat einen Haken: Was passiert, wenn ich 256 Bytes schreibe
> ohne eins zu lesen?

Dann habe ich festgestellt, daß der Puffer zu klein ist.

Die nächste Frage wäre dann logischerweise: Was passiert, wenn 257 Bytes 
angekommen sind, ohne eins gelesen zu haben?

Dafür muß man natürlich Sorge tragen, daß beides nicht passiert.

Fabian S. schrieb:
> Hat noch jemand etwas auszusetzen? Habe ich etwas übersehen?

> if(fifo->writePos+1==fifo->readPos)
>return;
Damit gehen dir u.U. Daten verloren.

> Die aktuelle Größe des Puffers müsste sich dann so bestimmen lassen:
>(fifo->writePos-fifo->readPos)

Gilt nicht nach einem Wrap-Around des WritePtr.

mfg.

[Fabian S. (jacky2k)]

Also der Puffer ist 256 Bytes groß, damit ich eben den Überlauf von den 
Zeigern missbrauchen kann.

Und warum geht das deiner Meinung nach nicht, wenn der writePos 
übergelaufen ist? Wenn ich nun 256 geschrieben habe (writePos wieder auf 
0) und 10 gelesen sind folglich noch 246 im puffer. Rechne ich dann 0-10 
kommt da 246 raus. Wo ist das Problem?

[Thomas E. (thomase)]

Fabian S. schrieb:
> Rechne ich dann 0-10
> kommt da 246 raus. Wo ist das Problem?

Hast recht. Nirgends.

mfg.