AVR-USART-Interrupt Quizfrage

[Cube S. (cube_s)]

Beim Versuch einen Raspberry PI mit einem Atmega8 per USART zu verbinden 
bin ich auf folgendes Problem gestoßen: Um die Programmierung des 
Atmega8 zu testen habe ich den Atmega8 auf RXD mit seinem eigenen TXD 
gefüttert. Das funktioniert auch im Prinzip, nur habe ich mit einer 
seltsamen Zeichenverdopplung zu kämpfen. Dazu folgender Code (mit dem 
Versuch nur das Wesentliche darzustellen):

Interruptroutine für USART RX

.def data = r20
.def flag = r21

usartrx: in data, UDR ; empfangenes Byte nach data sichern
         ser flag     ; flag für Hauptprogramm setzen
         reti

Hauptprogramm Variante 1 (mit sporadischer Zeichenverdopplung):

         cli          ; Interrupts generell im folgenden zugelassen
display: tst flag     ; Warten bis Interruptroutine das flag setzt
         breq display
         clr flag
         mov temp, data
         ; ... Die übliche LCD Ansteuerung zur Anzeige von temp
         rjmp display

Hauptprogramm Variante 2 (funktioniert):

loop:     cli         ; Auskommentieren führt auch hier zur Zeichenverdopplung
          tst   flag
          brne  display
          sei         ; Dem Interrupt auch eine Chance geben
          rjmp  loop 

display:  clr   flag
          mov   temp, data
          sei
          ; ... Die übliche LCD Ansteuerung zur Anzeige von temp
          rjmp loop

Warum kommt es in Variante 1 zu diesen seltsamen Zeichenverdopplungen? 
Klar, dass das Interrupt-Flag in Variante 2 ein Mutex für 'flag' und 
'data' ist, nur kann ich mir nicht erklären, dass ich Zeichen doppelt 
sehe (nein es ist nicht das Bier) sondern eher würde ich vermuten dass 
welche verschluckt werden. Das ganze geschieht bei absolut moderaten 
Bitraten (9600 Baud) und alle paar Sekunden ein Zeichen.

[Peter D. (peda)]

Klassischer Fehler, der Interrupt sichert das SREG nicht, ändert es 
aber.

[Cube S. (cube_s)]

Peter Dannegger schrieb:
> Klassischer Fehler, der Interrupt sichert das SREG nicht, ändert es
> aber.

Nein, das ist es nicht. Weder 'in' noch 'ser' ändern das SREG. Die 
Variante.

usartrx:  push  temp
          in  temp, SREG
          in  data, UDR
          ser  flag
          out  SREG, temp
          pop  temp
          reti

verhält sich ganz genauso.

[Malte S. (maltest)]

Cube S. schrieb:
> eher würde ich vermuten dass
> welche verschluckt werden

Ja denn da ist eine Race Condition zwischen Abfrage und Löschen des 
Flags bei Variante 1. Wenn ich mal unterstelle, dass das cli eigentlich 
ein sei ist? Oder wo ist das sei?

EDIT: so wie oben dürfte es ja eher zu gar keinem Zeichen kommen...

[Cube S. (cube_s)]

> EDIT: so wie oben dürfte es ja eher zu gar keinem Zeichen kommen...

Kommt es aber. Die 'sei' sind durchaus vorhanden (in Variante 1 habe ich 
es allerdings fälschlicherweise als 'cli' getarnt (der Kommentar 
widerspricht dem). Variante 2 funktioniert ohne Probleme.

Sprich: In Variante 1 sind Interrupts generell zugelassen, in Variante 2 
ist es so, dass Zugriffe auf 'data' und 'flag' quasi atomar sind.

[Malte S. (maltest)]

Genau so hatte ich das ja vermutet, umso mehr stimme ich Dir zu, dass da 
eigentlich höchstens was verloren gehen kann. TOCTTOU in Bezug auf das 
Flag selbst als auch die Daten:

tst flag
; ...
clr flag
mov temp, data

bei aktiviertem Interrupt sieht flasch aus.

Wie es stattdessen aber zu einer Verdopplung kommen kann, würde mich 
auch interessieren. Blöde Neugier :)

Wird der ISR zu oft angesprungen oder sonstwie der Inhalt von r21 
vermüllt?

[Peter D. (peda)]

Cube S. schrieb:
> Dazu folgender Code (mit dem
> Versuch nur das Wesentliche darzustellen):

Solange Du den Fehler nicht weißt, weißt Du auch nicht, was wesentlch 
ist!

Zeig ein komplettes Programm (als Anhang) und nichts aus dem Gedächtnis, 
sondern das exakte, getestete Programm mit stimmenden Kommentaren.
Sonst raten wir noch in 100 Jahren rum.

[Cube S. (cube_s)]

Gerne hier auch das komplette Programm so wie es fehlerlos funktioniert. 
Allein das auskommentieren des 'cli' nach dem 'loop:'-Label zeigt den 
Effekt der sporadischen Zeichenverdopplung.

.include  "m8def.inc"


       rjmp main  ; Reset Handler
      rjmp noint ; IRQ0 Handler
      rjmp noint ; IRQ1 Handler
      rjmp noint ; Timer2 Compare Handler
      rjmp noint ; Timer2 Overflow Handler
      rjmp noint ; Timer1 Capture Handler
      rjmp noint ; Timer1 CompareA Handler
      rjmp noint ; Timer1 CompareB Handler
      rjmp timer1ov ; Timer1 Overflow Handler
      rjmp noint ; Timer0 Overflow Handler
      rjmp noint ; SPI Transfer Complete Handler
      rjmp usartrx ; USART RX Complete Handler
      rjmp noint ; UDR Empty Handler
      rjmp usarttx ; USART TX Complete Handler
      rjmp noint ; ADC Conversion Complete Handler
      rjmp noint ; EEPROM Ready Handler
      rjmp noint ; Analog Comparator Handler
      rjmp noint ; Two-wire Serial Interface Handler
      rjmp noint ; Store Program Memory Ready Handler


.def temp = r19
.def data = r20
.def flag = r21
.def count = r22
.def ccount = r23

main:    ldi    temp, low(ramend)
      out    spl, temp
      ldi    temp, high(ramend)
      out    sph, temp

      rcall  lcd_init


; Timer1
; ------
      clr    temp
      out    TCNT1H, temp
      out    TCNT1L, temp
      ldi    temp, (2<<CS10)
      out    TCCR1B, temp
      ldi    temp, (1<<TOIE1)
      out    TIMSK, temp
      ldi    temp, (1<<TOV1)
      out    TIFR, temp

      ldi    ccount, 'A'

; USART initialization
; --------------------

      clr    temp
      out    UBRRH, temp
      ldi    temp, 12
      out    UBRRL, temp

      ldi    temp, (1<<U2X)
      out    UCSRA, temp
      ldi    temp, (1<<RXEN)|(1<<RXCIE)|(1<<TXEN)|(1<<TXCIE)
      out    UCSRB, temp
      ldi     temp, (1<<URSEL)|(3<<UCSZ0)|(0<<UPM0)
      out     UCSRC, temp
      ldi    temp, (1<<RXC)|(1<<TXC)
      out    UCSRA, temp

      clr    flag
      clr    count

loop:    cli
      tst    flag
      brne  display
      sei
      rjmp  loop

display:  mov    lcd_temp, data
      clr    flag
      sei

      rcall  lcd_data
      inc    count
      cpi    count, 8
      brlt  loop
      clr    count
      ldi    lcd_temp, $80
      rcall  lcd_cmd
      ;rcall  lcd_5ms

      rjmp  loop

noint:    reti

; --------------------------------------


usartrx:  push  temp
      in    temp, SREG
      in    data, UDR
      ser    flag
      out    SREG, temp
      pop    temp
      reti

usarttx:  
      reti

; --------------------------------------

timer1ov:  out UDR, ccount
      inc ccount
      cpi ccount, 'z'+1
      brlt timer1l1
      ldi ccount, 'A'
timer1l1:  reti

; --------------------------------------

lcd_hex:  push  lcd_temp
      push  zl
      push  zh
      push  r0

      push  lcd_temp
      swap  lcd_temp
      andi  lcd_temp,15
      rcall  lcd_nibble
      pop    lcd_temp
      andi  lcd_temp,15
      rcall  lcd_nibble

      pop    r0
      pop    zh
      pop    zl
      pop    lcd_temp
      ret

lcd_nibble: ldi    zl, low(2*hex)
      ldi    zh, high(2*hex)
      add    zl, lcd_temp
      clr    lcd_temp
      adc    zh, lcd_temp
      lpm
      mov    lcd_temp, r0
      rcall  lcd_data
      ret

hex:    .db    "0123456789ABCDEF"


; --------------------------------------

.equ    lcd_port = PORTC
.equ    lcd_ddr = DDRC
.equ    lcd_en = 5
.equ    lcd_rs = 4

.def    lcd_temp = r16
.def    lcd_c1 = r17
.def    lcd_c2 = r18

lcd_init:  push  lcd_temp
      ldi    lcd_temp, 0x7f
      out    lcd_ddr, lcd_temp
      ldi    lcd_temp, 50
lcd_init1:  rcall  lcd_5ms
      dec    lcd_temp
      brne  lcd_init1
      ldi    lcd_temp, 3
      out    lcd_port, lcd_temp
      rcall  lcd_enable
      rcall  lcd_5ms
      rcall  lcd_5ms
      rcall  lcd_5ms
      rcall  lcd_5ms
      rcall  lcd_enable
      rcall  lcd_5ms
      rcall  lcd_enable
      rcall  lcd_5ms
      ldi    lcd_temp, 2
      out    lcd_port, lcd_temp
      rcall  lcd_enable
      rcall  lcd_5ms

      ldi    lcd_temp, 0x28  ; Function Set
      rcall  lcd_cmd
      ldi    lcd_temp, 0x08  ; Display Off
      rcall  lcd_cmd
      ldi    lcd_temp, 0x01  ; Clear Display
      rcall  lcd_cmd
      rcall  lcd_5ms
      ldi    lcd_temp, 0x06  ; Entry Mode Set
      rcall  lcd_cmd
      rcall  lcd_5ms
      ldi    lcd_temp, 0x0C  ; Display On
      rcall  lcd_cmd

      pop    lcd_temp
      ret

lcd_cmd:  push  lcd_temp
      push  lcd_temp
      swap  lcd_temp
      andi  lcd_temp, 0b1111
      out    lcd_port, lcd_temp
      rcall  lcd_enable
      pop    lcd_temp
      andi  lcd_temp, 0b1111
      out    lcd_port, lcd_temp
      rcall  lcd_enable
      rcall  lcd_50us
      pop    lcd_temp
      ret

lcd_data:  push  lcd_temp
      push  lcd_temp
      swap  lcd_temp
      andi  lcd_temp, 0b1111
      sbr    lcd_temp, 1<<lcd_rs
      out    lcd_port, lcd_temp
      rcall  lcd_enable
      pop    lcd_temp
      andi  lcd_temp, 0b1111
      sbr    lcd_temp, 1<<lcd_rs
      out    lcd_port, lcd_temp
      rcall  lcd_enable
      rcall  lcd_50us
      pop    lcd_temp
      ret

lcd_enable:  sbi    lcd_port, lcd_en
      nop
      nop
      nop
      cbi    lcd_port, lcd_en
      ret

lcd_50us:   push  lcd_temp
      ldi    lcd_temp, 66
lcd_50us1:  dec    lcd_temp
      brne  lcd_50us1
      pop    lcd_temp
      ret

lcd_5ms:  push  lcd_c1
      push  lcd_c2
      ldi    lcd_c1, $21
lcd_5ms1:   ldi    lcd_c2, $C9
lcd_5ms2:   dec    lcd_c2
             brne  lcd_5ms2
             dec    lcd_c1
             brne  lcd_5ms1
      pop    lcd_c2
      pop    lcd_c1
             ret

[Malte S. (maltest)]

>      tst    flag

> timer1ov:  out UDR, ccount
>       inc ccount
>       cpi ccount, 'z'+1
>       brlt timer1l1
>       ldi ccount, 'A'

>      brne  display

[Cube S. (cube_s)]

Malte S. schrieb:
> bei aktiviertem Interrupt sieht flasch aus.

Da stimme ich absolut zu, deswegen habe ich ja auch die Variante mit den 
gesperrten Interrupts gegenübergestellt. Nur bei der Baudrate und der 
Häufigkeit von selbst gesendeten Zeichen hätte ich das "durchgehen 
lassen". Und ich finde einfach die Erklärung nicht warum es so ist wie 
es ist.

[Malte S. (maltest)]

wenn's das war, wovon ich erstmal ausgehe, dann war

Peter Dannegger schrieb:
> Klassischer Fehler, der Interrupt sichert das SREG nicht, ändert es
> aber.

ins schwarze. Nur ohne den schuldigen ISR auf dem Silbertablett :)

[Cube S. (cube_s)]

Malte S. schrieb:
> ins schwarze

Treffer, versenkt. Das war's. Besten Dank auch. Gute Güte, das sollte ja 
nur zum Testen sein. Naja gescheiter wird man durch scheitern.