längerer ASM-Inline-Abschnitt - wie?

[Nikolas B. (physikant)]

Hallo,

ich habe das Problem, dass ein Atmega168 in einem engen Zeitfenster per 
Software-SPI Daten auslesen muss. Dabei muss ich eine Rate von 800kbit/s 
- 1Mbit/s erreichen. Der Atmega läuft auf 18,432MHz.
Da ein maximal vereinfachter Code in C nach wie vor zu langsam ist, 
wollte ich einen ASM-Abschnitt inlinen. Leider habe ich das noch nie 
gemacht.
Hier ist, was ich bislang habe:

#define DCLK_PORT       PORTD
#define DCLK_PIN  6
#define DATASIG_PORT  PINB
#define DATASIG_PIN  0
#define RC_PORT   PORTD
#define RC_PIN    5

uint16_t ADC_read(void)
{
  uint16_t data = 0;
  uint8_t lowb = 0;
  uint8_t highb = 0;
  RC_PORT &= ~(1<<RC_PIN);  //ADC-Conversion einleiten
  RC_PORT |= (1<<RC_PIN);
  asm volatile (
  "LDI r27, 8\n\t"
  "SBI %2, DCLK_PIN\n\t"
  "CBI %2, DCLK_PIN\n\t"
  "SBI %2, DCLK_PIN\n\t"
  "CBI %2, DCLK_PIN\n\t"
  "read_byte1_loop:\n\t"
  "SBI %2, DCLK_PIN\n\t"
  "SBIC %3, DATASIG_PIN\n\t"
  "SEC\n\t"
  "SBIS %3, DATASIG_PIN\n\t"
  "CLC\n\t"
  "ROL %0\n\t" 
  "CBI %2, DCLK_PIN\n\t"
  "DEC r27\n\t"
  "BRNE read_byte1_loop\n\t"
  "LDI r27, 8\n\t"
  "read_byte2_loop:\n\t"
  "SBI %2, DCLK_PIN\n\t"
  "SBIC %3, DATASIG_PIN\n\t"
  "SEC\n\t"
  "SBIS %3, DATASIG_PIN\n\t"
  "CLC\n\t"
  "ROL %1 \n\t"
  "CBI %2, DCLK_PIN\n\t"
  "DEC r27\n\t"
  "BRNE read_byte2_loop\n\t"
  :"=r"(highb),"=r"(lowb),"=r"(DCLK_PORT)
  :"r"(DATASIG_PORT)
  : "r27"
  );
  data = lowb + (highb*256);
  return data;            
}

Mir ist klar, dass das lowb + (highb*256) weder guter Stil noch schnell 
ist, aber darauf kommt es gerade nicht an.

Der Compiler meldet einige Male Error: constant value required sowie 
Error: symbol `read_byte1_loop' is already defined (auch für die andere 
Schleife).
Was mache ich falsch?
Ist mein Code so prinzipiell sinnvoll?

Danke schonmal,

Nikolas

[Mw E. (Firma: fritzler-avr.de) (fritzler)]

Also längeres ASM nicht inline machen, sondern auslagern und dann noch 
an die GCC Registerkonventionen halten.
Zudem bei asm immer schön kommentieren, sonst blickt man nach ner Woche 
selbst nicht mehr durch.

[Nikolas B. (physikant)]

Eigentlich hätte ich es lieber so im Code. Es müsste doch so auch gehen, 
oder?
Falls es hilft, das war die Funktion vor der ASM-Ersetzung:

uint16_t ADC_read(void)
{
  uint16_t data = 0;
  uint16_t mask = 0x8000; 
  RC_PORT &= ~(1<<RC_PIN);
  RC_PORT |= (1<<RC_PIN);

  DCLK_PORT |= (1<<DCLK_PIN);
  DCLK_PORT &= ~(1<<DCLK_PIN);
  DCLK_PORT |=  (1<<DCLK_PIN);
  DCLK_PORT &= ~(1<<DCLK_PIN);
   do{
    DCLK_PORT |=  (1<<DCLK_PIN);
    if(DATASIG_PORT&(1<<DATASIG_PIN))data|=mask;
    DCLK_PORT &= ~(1<<DCLK_PIN);    
      }while((mask>>=1));
  return data;
}

Im Prinzip wird also ein CLK-Signal generiert (mit zwei leeren 
CLK-Pulsen vorneweg) und dann Bit für Bit ein Signal eingelesen zwischen 
den Pulsen. In ASM ist das dann in zwei Schleifen aufgeteilt für high- 
und lowbyte.
r27 dient lediglich als Schleifenzähler.

Grüße
  Nikolas

[Stefan E. (sternst)]

Nikolas B. schrieb:
> Was mache ich falsch?

Unter anderem das hier:

#define DCLK_PIN  6
...
  "SBI %2, DCLK_PIN\n\t"

Innerhalb von String-Literalen werden keine Makros ersetzt.

Mache es als separate Assembler-Datei, das erleichtert einiges.

[Stefan (Gast)]

Lohnt sich der AUfwand, an dieser Stelle Assembler einzusetzen? Würde 
mich wundern.

[Nikolas B. (physikant)]

Auch durch Ersetzen mit den richtigen Zahlen meldet er: error:constant 
value required. Seltsamerweise in den Zeilen, in denen zuvor die Makros 
waren.
So sieht der Code in der .s -Datei aus:

LDI r27, 8
SBI r25, 6
CBI r25, 6
SBI r25, 6
CBI r25, 6
read_byte1_loop:
SBI r25, 6  
SBIC r24, 0
SEC
SBIS r24, 0
CLC
ROL r24
CBI r25, 6
DEC r27
BRNE read_byte1_loop
LDI r27, 8
read_byte2_loop:
SBI r25, 6
SBIC r24, 0
SEC
SBIS r24, 0
CLC
ROL r30 
CBI r25, 6
DEC r27
BRNE read_byte2_loop

Seltsamerweise taucht der Code in der .s-Datei mehrfach auf, wodurch er 
read_byte1_loop und read_byte2_loop als already defined bemerkt.

Und ja, der Aufwand lohnt sich (hoffentlich), da die maximale Rate, die 
ich mit dem C-Code erreiche, rund 400kbit/s ist, ich jedoch mindestens 
800 brauche.
Grüße
  Nikolas

[Oliver J. (skriptkiddy)]

Nikolas B. schrieb:
> Und ja, der Aufwand lohnt sich (hoffentlich), da die maximale Rate, die
> ich mit dem C-Code erreiche, rund 400kbit/s ist, ich jedoch mindestens
> 800 brauche.
> Grüße
>   Nikolas

Warum nimmst du nicht HW-SPI?

[Nikolas B. (physikant)]

Weil es nicht funktioniert mit dem ADC.
Diskussion dazu:
Beitrag "SPI mit ADS7825"
Grüße
  Nikolas

[Jörg W. (dl8dtl) (Moderator)]

Nikolas B. schrieb:
> Und ja, der Aufwand lohnt sich (hoffentlich), da die maximale Rate, die
> ich mit dem C-Code erreiche, rund 400kbit/s ist, ich jedoch mindestens
> 800 brauche.

Verstehe ich nicht.  Was GCC (4.7.2) mit -Os draus macht, sieht schon
recht kompakt aus:

.global ADC_read
        .type   ADC_read, @function
ADC_read:
/* prologue: function */
/* frame size = 0 */
/* stack size = 0 */
.L__stack_usage = 0
        cbi 0xb,5
        sbi 0xb,5
        sbi 0xb,6
        cbi 0xb,6
        sbi 0xb,6
        cbi 0xb,6
        ldi r24,lo8(16)
        ldi r25,0
        ldi r20,0
        ldi r21,lo8(-128)
        ldi r18,0
        ldi r19,0
.L3:
        sbi 0xb,6
        sbis 0x3,0
        rjmp .L2
        or r18,r20
        or r19,r21
.L2:
        cbi 0xb,6
        lsr r21
        ror r20
        sbiw r24,1
        brne .L3
        movw r24,r18
        ret

Mit -O3 wird der Code viel größer und "irgendwie entrollt", wobei
ich meine Zweifel habe, dass es wirklich dadurch noch schneller wird.

[Stefan E. (sternst)]

SBI r25, 6
CBI r25, 6
SBI r25, 6
CBI r25, 6

Die sbi/cbi auf ein Register sind völlig sinnlos und das Resultat eines 
falschen Constraints.

Nikolas B. schrieb:
> Seltsamerweise taucht der Code in der .s-Datei mehrfach auf, wodurch er
> read_byte1_loop und read_byte2_loop als already defined bemerkt.

Dann hast du oder der Compiler die Funktion ge-inlined.


Und vom zweifelhaften Vorteil des Ganzen mal abgesehen:
Du hast offenbar größere Probleme mit Inline-Assembler, und die (schon 
mehrfach empfohlene) Alternative hätte keine Nachteile. Warum zum Teufel 
beharrst du also auf Inline-Assembler?

[Nikolas B. (physikant)]

Es hat sich erübrigt. Ich will garnicht wissen wieso, aber ohne 
ersichtlichen Grund läuft das Teil jetzt.
Danke für eure Mühe!
Nikolas

[Karl H. (kbuchegg)]

Nikolas B. schrieb:

> In ASM ist das dann in zwei Schleifen aufgeteilt für high-
> und lowbyte.

... damit man nicht mit 16 Bit rummachen muss. Schon klar.

Aber: Warum kannst du das nicht in C genauso machen?

Ich seh jetzt ehrlich gesagt nicht, wie man in Assembler ein

   uint8_t mask = 0x80;
   uint8_t dataHigh = 0;

   do {
    DCLK_PORT |=  (1<<DCLK_PIN);
    if(DATASIG_PORT&(1<<DATASIG_PIN))
      dataHigh |= mask;
    DCLK_PORT &= ~(1<<DCLK_PIN);    
   }while ( mask >>= 1 );

noch wesentlich beschleunigen könnte. Ich seh aber enormes Potential, 
wie man sich in Assembler ins eigene Knie schiessen kann.

[Oliver J. (skriptkiddy)]

Nikolas B. schrieb:
> Weil es nicht funktioniert mit dem ADC.
> Diskussion dazu:
> Beitrag "SPI mit ADS7825"
> Grüße
>   Nikolas

Kann mir nicht vorstellen, warm das das nicht funktionieren sollte.

Siehe FIGURE 4. Serial Data Timing Using Internal Data Clock (TAG LOW).


Konvertierung Starten. (CS low puls)
Kurz warten. (1,4 µs nach start des CS-Pulses)
Und dann zwei Bytes per SPI abholen (mit HW-SPI).

Gruß Oliver