Inline Assembler nach C

[Thomas (Gast)]

Hallo Forum,

ich verzweifele gerade an der Übersetzung einer inline Assembler Routine 
nach C.

Hier der Assembler Code:

 
//"  /*Check SOFs                                                 \n"
  "  BREQ RX_PAYLOAD        ;20 If not equal check SOF            \n"
  "  LDI  R30,lo8(sof_ary)  ;21 Setup pointer                     \n"
  "  LDI  R31,hi8(sof_ary)  ;22 Setup pointer to sof_ary          \n"
  "  ADD  R30,R25           ;23 Adjust pointer offset             \n"
  "  LD   R31,Z             ;25 Load value pointed to             \n"
  "  LDS  R30,UDR0          ;27 Load data received by the USART   \n"
  "  CP   R31,R30           ;28 Compare usart data and sof_ary[n] \n"
  "  BRNE RESET_RX          ;29 If not equal Jump                 \n"
  "  INC  R25               ;30 Increment rf_rx_state             \n"
  "  STS   rf_rx_state,R25  ;32 Increment rf_rx_state             \n"
  "  CPI   R25,0x04         ;33 Compare rf_rx_state to 0x04       \n"
  "  BRNE END_RX_IRQ        ;34 IF not equal Jump                 \n"

und hier der C-Code

  else//check SOF
  {
    ptr = &sof_ary[0];                          
    ptr += rf_rx_state;                                              
    data = UDR0;                                             
    
    if(data == *ptr)
    {                                                 
      rf_rx_state++;                                                
      if(rf_rx_state==0x04)
      {                      
        ts = input(TCNT1);
      }                                
    }                                                                 
      
    else    
    {
      rf_rx_state=0x00;
    }            
}

rf_rx_state ist beim Start der Routine 0, genauso wie der Inhalt des 
sof_ary[0]. Jedoch scheint *ptr nicht 0 zu sein, weshalb die Bedingung 
nicht greift. Kann jemand helfen?

[Mark B. (markbrandis)]

data = UDR0;                                             
    
    if(data == *ptr)

data wird also damit vergleichen, worauf der Zeiger ptr aktuell zeigt. 
Letzteres soll 0 sein, wenn ich Deine Ausführungen richtig verstanden 
habe. Und wenn nun UDR0 ungleich 0 ist?

[Thomas (Gast)]

Dann fängt die Routine wieder von vorne an.

Ziel ist es die die ersten vier Byte eines SOF zu erkennen um dann mit 
der eigentlichen Auswertung zu beginnen. Und sof_ary[0] ist mit 0 
initialisiert.

Die Assembler Routine funktioniert komischerweise, leider kann ich an 
der Stelle nicht debuggen um mir "data" anzuschauen. Aber es sollte 0 
sein, da der sof_ary[0] vor und nach dem inline Assembler Part ebenfalls 
0 ist. Deshalb kann ich mir nur vorstellen irgendetwas an meinem 
geschriebenen C-Code nicht stimmt.

[Karl H. (kbuchegg)]

Das was erkennbar ist, stimmt meiner Meinung nach insofern überein, als 
das der Assembler Ausschnitt sich korrekt im C-Code wiederfindet. Im 
C-Code ist nach mehr enthalten, allerdings fehlen auch ein paar Dinge, 
die man wissen müsste um die Codestelle zu beurteilen.
Zeig doch mal die ganze Stelle im Zusmmenhang. Was logisch passieren 
muss ist klar, die Frage ist: ist es korrekt umgesetzt?
Gibt es vielleicht eine andere Codestelle, die fehlerhaft ist und dir zb 
mittels eines Array-Overflows das fragliche Array niederbügelt?

[Mark B. (markbrandis)]

Hast Du Dir schon mal den Assembler-Code angeschaut, den der Compiler 
generiert? Wenn man den gcc verwendet, kann man sich den mit der Option 
-S anzeigen lassen.

[c-hater (Gast)]

Thomas schrieb:

>   "  LDI  R30,lo8(sof_ary)  ;21 Setup pointer                     \n"
>   "  LDI  R31,hi8(sof_ary)  ;22 Setup pointer to sof_ary          \n"
>   "  ADD  R30,R25           ;23 Adjust pointer offset             \n"

Das ist entweder unsauber oder der Code ist unvollständig. Hier werden 
Annahmen über die Lage von sof_ary gemacht, die unbewiesen sind. Das 
funktioniert nur dann, wenn sof_ary keine 256Byte-Grenze überschreitet. 
Hinweis: schon ein Array mit nur zwei Bytes kann eine 256Byte-Grenze 
überschreiten...

>     ptr = &sof_ary[0];
>     ptr += rf_rx_state;

Hierfür gilt natürlich genau dasselbe.

>     if(data == *ptr)

Das könnte falsch sein. Das hängt davon ab, wie data und vor allem ptr 
eigentlich deklariert sind. Also auch wieder: der Code ist verdammt 
unvollständig.

>     {
>       rf_rx_state++;
>       if(rf_rx_state==0x04)
>       {
>         ts = input(TCNT1);
>       }
>     }
>
>     else
>     {
>       rf_rx_state=0x00;
>     }
> }

Hier fehlt schon wieder reichlich Zeug im Asm-Quelltext. Kein Mensch 
kann dir deshalb sagen, ob deine C-Umsetzung korrekt ist. Aber es 
genügt, um definitiv sagen zu können, daß du nicht genug von deinem 
hochgeheimen (aber wohl sowieso nur geklauten) Code gepostet hast.

>       rf_rx_state=0x00;

Das könnte das sein, was im Asm-Quelltext ab dem Label "RX_RESET" 
steht, du uns aber nicht mitgepostet hast...

>         ts = input(TCNT1);

Und das taucht im Asm-Quelltext überhaupt nicht auf. Nur ein Hellseher 
könnte sagen, ob du das korrekt umgesetzt hast.


Fazit: Wenn man was portieren will, sollte man sowohl Quell- als auch 
Zielsprache zumindest soweit beherrschen, daß man in der Lage ist, die 
relevanten Teile des Quelltextes extrahieren zu können...

Du hast dich da definitiv vollständig übernommen! Und das schon beim 
Raubkopieren. Was soll das erst werden, wenn du tatsächlich mal was 
eigenes schreiben willst?

[Mark B. (markbrandis)]

Also wenn man schon Code klaut, dann sollte man auch richtig mit einer 
Suchmaschine umgehen können:

http://sourceforge.net/mailarchive/forum.php?forum_name=tux-droid-svn&style=nested&viewmonth=200706

+//*****************************************************************************
+//* Project: RF-Firmware for ISM                                              *
+//* Function: UART RECV                                                       *
+//* Parameters: None                                                          *
+//* Returns: None                                                             *
+//* Action: RX_Data                                                           *
+//* Duration: tbd                                                             *
+//* Size: tbd                                                                 *
+//* Date:                                                                     *
+//*---------------------C-Source Code-----------------------------------------*
+//*SIGNAL_NAKED (SIG_USART_RECV)                        *
+//*{                                      *
+//*    uc_8 data,tmp;                                *
+//*    uc_8 *ptr;                                *
+//*     if(rf_rx_state==0x04){//receive                      *
+//*      data = input(UDR0);                          *
+//*      ptr = &rf_rx_buffer[0];                                           *
+//*         ptr += rf_rx_counter;                                             *
+//*         *ptr = data;                            *
+//*         rf_rx_counter = (rf_rx_counter + 1)&0x1F;                         *
+//*         checksum ^= data;                          *
+//*         if(rf_rx_counter==rf_rx_buffer[0]){                  *
+//*            rf_rx_state = 0xFF;                       *
+//*        output(UCSR0A,0x00);                      *
+//*        output(UCSR0B,0x00);                      *
+//*        output(UCSR0C,0x00);                      *
+//*        PU_PWR = 0;                            *
+//*        RXON = 0;                            *
+//*        ENABLE = 0;                            *         
+//*    }                                    *
+//*    else{//check for SOF                          *
+//*      ptr = &sof_ary[0];                          *
+//*         ptr += rf_rx_state;                                               *
+//*         data = input(UDR0);                                               *
+//*         if(data == *ptr){                                                 *
+//*           rf_rx_state++;                                                *
+//*        if(rf_rx_state==0x04){                      *
+//*          ts = input(TCNT1);                             *
+//*        }                                *
+//*         }                                                                 *
+//*         else{                                                             *
+//*           rf_rx_state = 0x00;                                           *
+//*         }                                                                 *         
+//*    }                                    *
+//*                                           *
+//*}                                      *
+//*****************************************************************************
+SIGNAL_NAKED (SIG_USART_RECV)
+{
+  __asm__ __volatile__ (
+  "  .EQU UDR0,0xC6        ;Assembler defines          \n"
+  "  .EQU PORTB,0x05        ;Assembler defines          \n"
+  "  .EQU PORTD,0x0B        ;Assembler defines          \n"
+  "  .EQU PORTC,0x08        ;Assembler defines          \n"
+  "  .EQU TCNT1L,0x84      ;Assembler defines          \n"
+  "  .EQU TCNT1H,0x85      ;Assembler defines          \n"
+  "  .EQU UCSR0C,0xC2      ;Assembler defines          \n"
+  "  .EQU UCSR0A,0xC0      ;Assembler defines          \n"
+  "  .EQU UCSR0B,0xC1      ;Assembler defines          \n"
+  "  .EQU TMP_REG0,0x1E      ;Assembler defines          \n"
+  "  .EQU TMP_REG1,0x2A      ;Assembler defines          \n"
+  "  .EQU TMP_REG2,0x2B      ;Assembler defines          \n"
+  "  .EQU TMP_REG3,0x28      ;Assembler defines          \n"
+  "  /****************/                        \n"
+  "  /*Save Registers*/                        \n"
+  "  /****************/                        \n"
+  "   OUT  TMP_REG0,R25        ;12 Store R25            \n"
+  "  OUT  TMP_REG1,R30      ;13Store R30            \n"
+  "  OUT  TMP_REG2,R31      ;14 Store R31            \n"
+  "  IN   R31,__SREG__      ;15 Load SREG into R31        \n"
+  "  OUT  TMP_REG3,R31      ;16 Store R31            \n"
+  "  LDS  R25,rf_rx_state    ;18 Load rf_rx_state into R25    \n"
+  "  CPI   R25,0x04        ;19 Compare rf_rx_state to 0x04    \n"
+  "  /****************/                        \n"
+  "  /*Check SOFs    */                        \n"
+  "  /****************/                        \n"
+  "  BREQ RX_PAYLOAD        ;20 If not equal check SOF      \n"
+  "  LDI  R30,lo8(sof_ary)    ;21 Setup pointer          \n"
+  "  LDI  R31,hi8(sof_ary)    ;22 Setup pointer to sof_ary    \n"
+  "  ADD  R30,R25        ;23 Adjust pointer offset      \n"
+  "  LD   R31,Z          ;25 Load value pointed to      \n"
+  "  LDS  R30,UDR0        ;27 Load data received by the USART  \n"
+  "  CP   R31,R30        ;28 Compare usart data and sof_ary[n]\n"
+  "  BRNE RESET_RX        ;29 If not equal Jump        \n"
+  "  INC  R25          ;30 Increment rf_rx_state      \n"
+  "  STS   rf_rx_state,R25    ;32 Increment rf_rx_state      \n"
+  "  CPI   R25,0x04        ;33 Compare rf_rx_state to 0x04    \n"
+  "  BRNE END_RX_IRQ        ;34 IF not equal Jump        \n"
+  "  /*****************/                        \n"
+  "  /*     SYNC      */                        \n"
+  "  /*****************/                        \n"
+  "  LDS R30,TCNT1L        ;36 Take Timestamp from TCNT1    \n"
+  "  STS ts,R30          ;38 and store it to the global variable\n"
+  "  LDS R30,TCNT1H        ;40 called ts            \n"
+  "  STS ts+1,R30        ;42                  \n"
+  "  RJMP END_RX_IRQ        ;44                  \n"
+  "  /****************/                        \n"
+  "  /*RX RF - Data  */                        \n"
+  "  /****************/                        \n"
+  "  RX_PAYLOAD:          ;22 If equal receive the RF data  \n"
+  "  LDS  R25,UDR0        ;24 Load data received by the USART \n"
+  "  LDI  R31,hi8(rf_buffer_rx)  ;25 Setup pointer to rf_buffer_rx  \n"
+  "  LDS  R30,rf_rx_counter    ;27 Setup pointer to rf_buffer_rx  \n"
+  "  ST   Z,R25          ;29 Store rxed value to location pointed\n"
+  "  INC  R30          ;30 Increment rf_rx_counter      \n"
+  "  ANDI R30,0x3F        ;31 Adjust rf_rx_counter      \n"
+  "  STS  rf_rx_counter,R30    ;33 Store back rf_rx_counter    \n"
+  "  LDS   R31,checksum      ;35 Load checksum          \n"
+  "  EOR  R31,R25        ;36 Xor received byte to checksum  \n"
+  "  STS   checksum,R31      ;38 Store back checksum        \n"
+  "  LDS   R25,rf_buffer_rx    ;40 Load packet length byte      \n"
+  "  CP   R25,R30        ;41 Compare length byte to rf_rx_counter\n"
+  "  BRNE END_RX_IRQ        ;42 If not equal Jump        \n"
+  "  /******************/                      \n"
+  "  /*Finish reception*/                      \n"
+  "  /******************/                      \n"
+  "  LDI   R25,0xFF        ;44 Set rf_rx_state to 0xFF      \n"
+  "  STS  rf_rx_state,R25    ;46 indicating a complete RX cycle  \n"
+  "  LDI  R25,0x00        ;47 Disable USART interface      \n"
+  "  STS UCSR0B,R25        ;49 Disable USART interface      \n"
+  "  STS UCSR0C,R25        ;51 Disable USART interface      \n"
+  "  CBI PORTD,2           ;53 Power down the RF - Chip    \n"
+  "  CBI PORTD,5          ;55 Power down the RF - Chip    \n"
+  "  CBI PORTC,1          ;57 Power down the RF - Chip    \n"
+  "  CBI PORTB,1          ;59 Power down the RF - Chip    \n"
+  "  RJMP END_RX_IRQ        ;61 Jump to end of ISR        \n"
+  "  RESET_RX:          ;                  \n"
+  "  STS  rf_rx_state,R1      ;33Set rf_rx_state to zero      \n"
+  "  /****************/                        \n"
+  "  /*Load Registers*/                        \n"
+  "  /****************/                        \n"
+  "  END_RX_IRQ:                            \n"
+  "  IN    R31,TMP_REG3      ;62 Restore R31            \n"
+  "  OUT   __SREG__,R31      ;63 Restore SREG          \n"
+  "  IN    R31,TMP_REG2      ;64 Restore R31            \n"
+  "  IN    R30,TMP_REG1      ;65 Restore R30            \n"
+  "  IN    R25,TMP_REG0      ;66 Restore R25            \n"
+  "   RETI                   ;70 Return from ISR          \n"
+  : : );
+}

[Thomas (Gast)]

Der Code ist weder geklaut noch sonstiges. Er war bei einem 
Entwicklungsboard dabei und ich war mir nicht sicher in wie weit ich 
diesen veröffentlichen darf.

Und was ich machen will ist lediglich die Assembler-Routine gegen die 
auskommentierte C-Routine tauschen um sie entsprechend anzupassen, da 
ich kein Assembler kann. Das oben aufgeführte Beispiel liegt mir genauso 
vor, leider funktioniert die C-Routine nicht so wie es die 
Assembler-Routine macht. Und da meine Assembler Kenntnisse gleich null 
sind war ich der Meinung ich bekomme hier Hilfe.

[Peter D. (peda)]

Mark Brandis schrieb:
> SIGNAL_NAKED (SIG_USART_RECV)

Für ein C-Programm dürfte "NAKED" generell nicht funktionieren. Ein 
C-Compiler möchte gerne Prolog und Epilog erzeugen.

Am besten wäre es, Du würdest in Worten beschreiben, was die Routine tun 
soll. Dann kann man es am schnellsten implementieren.


Meiner Meinung nach wird viel zu oft unnötig Assembler hinzu gemixt und 
dann geht die Lesbarkeit, Wartbarkeit und Fehlersicherheit zum Teufel.

[Thomas (Gast)]

Hallo Peter,

wie bereits geschrieben versuche ich den Assembler Code den Mark 3 Posts 
weiter oben eingefügt hat durch den auskommentierten C-Code, ebenfalls 
in diesem Code, zu ersetzen. Leider funktioniert der C-Code nicht.

Funktionsweise des Assembler-Code's ist folgender:
Ein Transceiver schickt mehrere Bytes über die UART Schnittstelle. Die 
ersten 3 Bytes (sof_ary[1-3]) sind der Start of Frame, der den folgenden 
Payload ankündigt. Als erstes wird der Start of Frame überprüft. Ist 
dieser korrekt wird eine Zeit festgelegt (Timer Interrupt) der im 
folgenden die Auswertung der Daten übernimmt und diese in einen 
Empfangsbuffer schreibt. Nach jedem Datenempfang werden UART und 
Transceiver ausgeschaltet.

Jedoch erkennt der C-Code diesen Start of Frame schon nicht, der 
Assembler_Code hingegen funktioniert einwandfrei.

[c-hater (Gast)]

Peter Dannegger schrieb:

> Meiner Meinung nach wird viel zu oft unnötig Assembler hinzu gemixt und
> dann geht die Lesbarkeit, Wartbarkeit und Fehlersicherheit zum Teufel.

Tja, ich sehe das exakt andersrum: Viel zu oft wird unnötigerweise C 
hinzugemixt, wodurch nicht nur die Lesbarkeit und Wartbarkeit des Codes 
enorm leidet, sondern obendrein ein wallender Nebel sinnloser 
Syntaxblähungen die Erkennung von Fehlern extrem erschwert.

Naja, zum Glück kann ein brauchbarer Assemblerprogrammierer notfalls 
immer noch die C-Seuche kompilieren, das Ergebnis disassemblieren und 
kommt so doch noch zu den gewünschten Erkenntnissen.

[Martin V. (oldmax)]

c-hater schrieb:
> Thomas schrieb:
>
>>   "  LDI  R30,lo8(sof_ary)  ;21 Setup pointer                     \n"
>>   "  LDI  R31,hi8(sof_ary)  ;22 Setup pointer to sof_ary          \n"
>>   "  ADD  R30,R25           ;23 Adjust pointer offset             \n"
>
> Das ist entweder unsauber oder der Code ist unvollständig. Hier werden
> Annahmen über die Lage von sof_ary gemacht, die unbewiesen sind. Das
> funktioniert nur dann, wenn sof_ary keine 256Byte-Grenze überschreitet.
> Hinweis: schon ein Array mit nur zwei Bytes kann eine 256Byte-Grenze
> überschreiten...
>
Deswegen fehlt ja auch die Anweisung für den Übertrag

   adc  R31, Zero   ; Zero ist ein mit 0 besetztes Register

Vielleicht hilft's....
gruß oldmax

[Thomas (Gast)]

Danke für den Tip.

Aber der Assembler-Code funktioniert einwandfrei, beim C-Code scheint 
irgendetwas nicht zu stimmen.

[Martin V. (oldmax)]

Hi
Das funktioniert auch, solange dein Array nicht über die 255 adressiert 
wird. Erst wenn die Zugriffsadresse größer ist, schreibt er ja wieder in 
den unten adressierten Bereich (Adresse 0) da das Register 31 ja 
überhaupt nicht verändert wird. Du hast in Assembler nicht automatisch 
eine 16 Bit Addition. Beispiel :
Basisadresse 248     (R30 = 248, R31 = 0)
Addition Offset 6    (R30 = 251  R31 = 0) ist noch ok
Addition Offset 22   (R30 = 15   R31 = 0)
248 + 22 --> bis 255 = 7 Rest 15
Spätestens hier hast du lustige schwer auffindbare Fehler. Solange du 
unter 256 bleibst, ist alles ok.
Gruß oldmax

[Peter D. (peda)]

Dieses Verwursteln verschiedenster Aufgaben zusammen in einen Interrupt 
ist nicht mein Ding. Die Seiteneffekte sind kaum übersehbar.

Thomas schrieb:
> Ein Transceiver schickt mehrere Bytes über die UART Schnittstelle.

Ich mache dazu im Interrupt immer nur eine FIFO und das Main kann dann 
in aller Ruhe den FIFO auslesen und die Nachricht parsen.

Thomas schrieb:
> Nach jedem Datenempfang werden UART und
> Transceiver ausgeschaltet.

Wozu?
Die UART läßt man besser immer durchlaufen, damit sie kein Startbit 
verpaßt.
Stromsparoptionen schreibe ich in eine separate Funktion.

Also Stromsparen und Protokoll Parsen einfach raus aus dem Interrupt, 
dann wird das wesentlich übersichtlicher und unkritischer.
Und man kann dann auch alles für sich testen.
Es gilt: Teile und Herrsche.

Stromsparoptionen implementiere ich immer erst zum Schluß, nachdem alle 
Funktionalität komplett läuft.

[Peter D. (peda)]

Thomas schrieb:
> Aber der Assembler-Code funktioniert einwandfrei,

Er mag zwar funtionieren, aber es gibt mehrere Einwände, z.B.:
- es ist Assembler
- er verwuselt mehrere Aufgaben, die man besser getrennt ausführen (und 
testen) sollte.

[Mark B. (markbrandis)]

Ist denn das hier:

data = UDR0;

exakt das Gleiche wie das hier?

data = input(UDR0);

Ich kenne die Implementieruing der Funktion input() nicht. Aber 
vielleicht macht sie ja noch mehr als das, was man vielleicht erwartet. 
Oder etwas anderes, als das was man erwartet.

[Thomas (Gast)]

>> Ist denn das hier:

>> data = UDR0;


>> exakt das Gleiche wie das hier?

>> data = input(UDR0);

Es ist das gleiche. input ist ist nem Makro definiert.

Es scheint jetzt aber zu laufen, zumindest bekomme ich den SOF 
ausgewertet. Hab "SIGNAL_NAKED (SIG_USART_RECV)" gegen 
"ISR(USARTC0_RXC_vect)" und nun läuft's soweit.

Besten Dank an alle die geholfen bzw. versucht haben zu helfen. Und 
Sorry für den chaotischen Thread.