STM32-SPI-Code Feedback

[ffs_jz (Gast)]

Guten Tag,
vorweg, ich bin relativ neu im Bereich Microcontrollertechnik und 
befasse mich das erste Mal mit dem SPI-Bus.
Ich habe mir nun einige Beispiele im Internet angesehen und den groben 
Ablauf einer Kommunikation verstehe ich nun auch.
Mein Problem liegt darin, dass ich immer nur fertige Funktionen z.B. 
void transmitData(...), void receiveData(...) finde.
In diesem Forum (Beitrag "STM32 SPI Slave ohne HAL") habe ich 
dann gelesen, dass die Programmierung einer solchen Funktion nicht 
unmöglich sein soll, und nun habe ich mich selbst daran gewagt.

void transmitByteSPI1(uint8_t byte)
{
  GPIOA->BSRR |= GPIO_BSRR_BR4;        //CS-->low: Übertragung bereit
  while(!(SPI1->SR & SPI_SR_TXE))        //warten TX Register bereit  
  {
    ;
  }
  SPI1->DR = byte;                  
  while(SPI1->SR & SPI_SR_BSY)        //warten bis Kommunikation abgschlossen
  {
    ;
  }
  GPIOA->BSRR |= GPIO_BSRR_BS4;        //CS-->high
}

void receiveByteSPI1(uint8_t byte)
{
  GPIOA->BSRR |= GPIO_BSRR_BR4;        //CS-->low: Übertragung bereit
  while(!(SPI1->SR & SPI_SR_RXNE))      //warten RX Register bereit  
  {
    ;
  }
  byte = SPI1->DR;
  while(SPI1->SR & SPI_SR_BSY)        //warten bis Kommunikation abgschlossen
  {
    ;
  }
  GPIOA->BSRR |= GPIO_BSRR_BS4;        //CS-->high
}

Ich wollte mir einfach einmal Feedback abholen, ob diese Ideen brauchbar 
sind, oder totaler Schwachsinn. Vorallem beim Einsatz von Pointern bin 
ich noch sehr unsicher. Über Verbesserungsvorschläge wäre ich sehr 
dankbar

[Stefan F. (Gast)]

ffs_jz schrieb:
> Ich wollte mir einfach einmal Feedback abholen,
> ob diese Ideen brauchbar sind, oder totaler Schwachsinn.

Sieht brauchbar aus, abgesehen davon dass da noch die ganze 
Initialisierung fehlt, die wesentlich spannender wäre.

Was mir auffällt ist, dass du blockierend kommunizierst. Bei jedem 
einzelnen byte wartest du darauf, dass es gesendet wurde. Kann man 
machen, aber dann braucht man eigentlich keine SPI "teure" Schnittstelle 
zu verwenden sondern kann das gleich per Bit-Banging "zu Fuß" machen. 
Vorteil: Es geht dann mit jedem beliebigen I/O Pin und man hat die volle 
Kontrolle über eventuell gemeine Feinheiten des Übertragungsprotokolls.

> Vor allem beim Einsatz von Pointern bin ich noch sehr unsicher.
> Über Verbesserungsvorschläge wäre ich sehr dankbar

Du hast keine Pointer verwendet, außer bei den Zugriffen auf die 
Register.

Das geht nicht:

void receiveByteSPI1(uint8_t byte)
{
  ...
  byte = SPI1->DR;
}

Weil Parameter immer als Kopie an die Funktion übergeben werden. Es gibt 
aber keinen Weg zurück, das ist eine Einbahnstraße. Die Funktion sollte 
stattdessen einen Rückgabewert haben:

void uint8_t receiveByteSPI1()
{
  ...
  uint8_t byte = SPI1->DR;
  ...
  return byte;
}

Oder einen Zeiger auf ein Byte:

void receiveByteSPI1(uint8_t *byte)
{
  ...
  *byte = SPI1->DR;
}

Hier bekommt die Funktion eine Kopie des Zeigers auf das Byte. Durch den 
indirekten Schreibzugriff wird die originale Variable beschrieben, auf 
die sowohl der originale als auch der kopierte Zeiger zeigt.

[temp (Gast)]

Da ist es aber sinnvoller nur eine Funktion zu benutzen:

uint8_t transmitByteSPI1(uint8_t byte)
{
  GPIOA->BSRR |= GPIO_BSRR_BR4;     //CS-->low: Übertragung bereit
  while(!(SPI1->SR & SPI_SR_TXE))   //warten TX Register bereit  
    ;
  SPI1->DR = byte;                  
  while(SPI1->SR & SPI_SR_BSY)      //warten bis Kommunikation abgschlossen
    ;
  GPIOA->BSRR |= GPIO_BSRR_BS4;     // CS-->high
  return SPI1->DR;
}

Damit zwingt man den benutzer auch ein Sendebyte anzugeben auch wenn nur 
gelesen werden soll. (0x00 oder 0xff z.B.) Das schafft klare 
Verhältnisse.

[Paul (Gast)]

Hmm also wenn ich mich nicht irre wird dein receive zumindest so nicht 
funktionieren. Du scheinst ja code für den SPI Master zu schreiben da du 
die Kommunikation ja mit dem CS startest. Also müsstest du für jedes Bit 
was du vom Slave über MISO rein bekommen möchtest vorher ein Bit über 
MOSI raussenden. Wenn nicht anders vom Slave vorgegeben und du nur 
stumpf Daten von ihm abrufen willst dann können das einfach dummy Bytes 
sein die du  schickst.

[ffs_jz (Gast)]

>Sieht brauchbar aus, abgesehen davon dass da noch die ganze
Initialisierung fehlt, die wesentlich spannender wäre.

Zur Initialisierung:
Ich will mit einem STM32F466 über SPI Bus einen Schrittmotortreibe 
TMC5160  betreiben.
Der Schrittmotor wird intern getaktet --> maximaler SCK = 4 MHz und es 
muss im SPI-Mode 3 kommuniziert werden --> CPOL = 1 CPHA = 1.

/*
 * @brief:  Initialisierung SPI1-Bus:
 *       PA4  -> CS-Pin     =>   SSM   =   1
 *                   SSI   =   1
 *       Baud Rate = 2 MHz  =>  BR    =   101 (fpclk/8 = 16MHz/8 = 2MHz)
 *       STM32F446 = Master  =>   MSTR  =   1
 *       SPI MODE 3      =>  CPOL  =   1
 *                  CPHA  =   1
 *      SPI Enable      =>  SPE    =  1
 * @param[in]  -
 * @param[out]  -
 */

void initSPI1()
{
  RCC->APB2ENR |= RCC_APB2ENR_SPI1EN;

  SPI1->CR1 |= (SPI_CR1_SSM | SPI_CR1_SSI);
  SPI1->CR1 |=  SPI_CR1_BR_1;
  SPI1->CR1 |=  SPI_CR1_MSTR;
  SPI1->CR1 |= (SPI_CR1_CPOL | SPI_CR1_CPHA);

  SPI1->CR1 |=  SPI_CR1_SPE;
}

>Oder einen Zeiger auf ein Byte:
So habe ich es jetzt realisiert (nur bei receiveByteSPI1(...)?

>Also müsstest du für jedes Bit
was du vom Slave über MISO rein bekommen möchtest vorher ein Bit über
MOSI raussenden.

Ich habe jetzt die Funktion transmit_receiveData() gebaut:
Der TMC5160 kommuniziert mit 40bit. 1 Adressbyte und 4 Datenbyte.
Ich habe mir jetzt gedacht ich "zerhacke" das uint64_t tx_data in 5 
uint_8_t tx_byte[0..4] und sende sie nacheinander an den TMC5160. 
Anschließend erhalte ich 5 uint8_t *rx_byte[0...4]. Diese muss ich dann 
wieder zu dem uint64_t *rx_data zusammenbauen. Hierbei komm ich momentan 
nicht weiter.
Bin ich mit dem Lösungsansatz auf dem richtigen Weg, oder ist das eine 
Sackgasse.

void transmit_receiveData(uint64_t tx_data, uint64_t* rx_data)
{
  uint8_t tx_count = 0;
  uint8_t  rx_count = 0;

  uint8_t rx_byte[5];
  uint8_t tx_byte[5];

  tx_byte[0] = tx_data >> 32;
  tx_byte[1] = tx_data >> 24;
  tx_byte[2] = tx_data >> 16;
  tx_byte[3] = tx_data >> 8;
  tx_byte[4] = tx_data;

  GPIOA->BSRR |= GPIO_BSRR_BR4;

  while(tx_count < 5)
  {
    transmitByteSPI1(tx_byte[tx_count]);
    tx_count ++;
  }
  while(rx_count < 5)
  {
    receiveByteSPI1(&rx_byte[rx_count]);
    rx_count ++;
  }
  //Hier sollten die rx_byte[0...4] in rx_data zusammengefasst werden 
  
  GPIOA->BSRR |= GPIO_BSRR_BS4;
}

Zur Info, das CS wurde aus den Funktionen transmitByteSPI1() und 
receiveByteSPI1() entfernt ;-)

Hier wäre noch das Datenblatt des TMC5160: 
https://www.trinamic.com/fileadmin/assets/Products/ICs_Documents/TMC5160A_Datasheet_Rev1.14.pdf

[ffs_jz (Gast)]

>Der Schrittmotor wird intern getaktet
Der Schrittmotortreiber meine ich natürlich

[Stefan F. (Gast)]

ffs_jz schrieb:
>>Oder einen Zeiger auf ein Byte:
> So habe ich es jetzt realisiert (nur bei receiveByteSPI1(...)?

Ja, denn nur da willst du ein Ergebnis zurück liefern.

[Darth Moan (Gast)]

Moin,

   GPIOA->BSRR |= GPIO_BSRR_BR4;
   while(tx_count < 5)
   {
     transmitByteSPI1(tx_byte[tx_count]);
     tx_count ++;
   }
   while(rx_count < 5)
   {
     receiveByteSPI1(&rx_byte[rx_count]);
     rx_count ++;
   }
   //Hier sollten die rx_byte[0...4] in rx_data zusammengefasst werden
 
   GPIOA->BSRR |= GPIO_BSRR_BS4;

du verlässt dich voll darauf, dass die SPI mindestens 5 byte HW FIFO 
hat.
Das mag so sein bei dem STM32, den du benutzt. Aber ich würde das 
trotzdem nicht so machen. Du wartest in der sendByteSPI1() darauf, dass 
alle bits rausgesendet wurden. Und erst dann willst Du empfangen. Du 
hast ein vollduplex Interface. Wenn 1 byte gesendet wurde, wurde auch 1 
byte empfangen. Also würde ich (wenn es schon byte weise sein muss) eine 
Funktion machen, die ein Byte sendet und das empfangene Byte zurück 
gibt. Eigentlich würde ich eine Funktion machen, die 2 Pointer (RX/TX 
buffer) und Anzahl zu sendender Bytes bekommt und ein OK oder NOT_OK 
zurück gibt.

Ich vermute dein Ansatz kommt daher, das dieser TMC5160 so seltsam 
arbeitet.
Um etwas zu lesen brauchst du tatsächlich 2 Transfers zu je 40bit.
Der 1. Transfer enthält das Lesekommando und 4 byte dummy daten.
Dabei empfängst du 8bit SPI Status und entweder 4 byte dummy daten oder 
die zu lesenden daten aus einem vorangegangenen Lesekommando. Der 2. 
Transfer kann irgendwas enthalten zB Schreibkommando + Daten und liefert 
SPI Status und die Daten zum Lesekommando des 1. Transfers.


Noch was, das BSRR Register verodert man eigentlich nicht. Pins die 
manipuliert werden sollen, schreibt man mit 1 und die anderen mit 0.
Also GPIOA->BSRR = GPIO_BSRR_BS4; setzt nur PA4 nach high. Alle anderen 
werden nicht angefasst.

[Adib (Gast)]

Hallo,

wie bereits Darth Moan beschrieb, sollte der Zugriff auf die SPI anders 
ausgeführt werden.
Also wenn schreiben UND lesen gleichzeitig erfolgt, dann sollte das in 
einer Funktion erfolgen.
Die Read Funktion sollte dafür sorgen dass auch Daten geshifted werden. 
Diese muss also Dummy Bytes in das DR Register schreiben.
Du solltest eine Read(), Write() und WriteRead() haben. Oder eine 
universelle, die alles abdeckt.
Diese Funktionen sollten nur beenden, wenn der Transfer wirklich 
abgeschlossen ist. Also bei Write() das BSY beachten.


Noch 2 Ergänzungen:
1) die ARM Archiketur ist stark parallel. Nach dem Aktivieren des 
CS-Signals musst du unbedingt darauf warten, dass es am Ausgang 
aktiviert wurde.
Das erfolgt mit einem Barrier-Befehl:
   GPIOA->BSRR = GPIO_BSRR_BR4;
   DSB();
auch wenn jetzt hier noch viel Code dazwischen steht, kann ein Compiler 
vieles davof weglassen oder umstellen!!!
PS: bei NXP gibt es eine Integrierte CS-Logik, so dass man das CS nicht 
"per Hand" setzen muss.

2) ich würde nicht bei der Kommunikation darauf vertrauen, dass das 
Read-DR Register leer ist.
Also VOR dem Aktivieren der CS-Leitung mal den Read-Buffer leeren.
  while(SPI1->SR & SPI_SR_RXNE) {
    SPI1->DR;
  }


HTH, Adib.
--

[PittyJ (Gast)]

Ich hatte mir mal Cube32 Von ST heruntergeladen.
Für 'meinen' Prozessor STM32H7 sind die HAL Funktionen vorhanden und 
funktionieren prima. Ich benutze nur die HAL Funktionen.
Aber das ist Geschmackssache.

Das Gute ist aber, dass die Sourcen der HAL-Funktionen dabei liegen. Man 
kann also selber schauen, was die machen und dann eigene Modifikationen 
erstellen.

Bevor man rätselt, was alles notwendig ist, einfach mal in den HAL 
Sourcecode schauen. Da sind sogar Kommentare drin.

[Stefan F. (Gast)]

PittyJ schrieb:
> Bevor man rätselt, was alles notwendig ist, einfach mal in den HAL
> Sourcecode schauen. Da sind sogar Kommentare drin.

Ich mag die HAL nicht, aber für diesen Zweck nutze ich sie auch oft 
gerne. Manche Zusammenhänge stehen nämlich im Reference Manual irgendwo 
zwischen den Zeilen vieler Kapitel verteilt, so dass man sie als 
Anfänger leicht übersieht.

[Johannes Z. (ffs_jz)]

Darth Moan schrieb:
> Du wartest in der sendByteSPI1() darauf, dass
> alle bits rausgesendet wurden. Und erst dann willst Du empfangen. Du
> hast ein vollduplex Interface. Wenn 1 byte gesendet wurde, wurde auch 1
> byte empfangen. Also würde ich (wenn es schon byte weise sein muss) eine
> Funktion machen, die ein Byte sendet und das empfangene Byte zurück
> gibt.
Was meinst du mit: (wenn es schon byte weise sein muss)? Das 
DataRegister meines STM ist doch 16bit lang aber ich muss aber 40bit 
versenden. Ich hätte dann einfach 5 mal 8bit versendet. Wenn das zu 
umständlich ist, würde es mir helfen wenn du mir erklärst wie ich dann 
dieses "Problem" lösen kann.

Darth Moan schrieb:
> Noch was, das BSRR Register verodert man eigentlich nicht. Pins die
> manipuliert werden sollen, schreibt man mit 1 und die anderen mit 0.
> Also GPIOA->BSRR = GPIO_BSRR_BS4; setzt nur PA4 nach high. Alle anderen
> werden nicht angefasst.
Danke für den Tipp, ich werde es ab sofort beherzigen.

Adib schrieb:
> Also wenn schreiben UND lesen gleichzeitig erfolgt, dann sollte das in
> einer Funktion erfolgen.
Ok, dann war das ein grundsätzliches Verständnisproblem im Ablauf des 
SPI Bus, nach eurer Erklärung klingt es aber logisch.

PittyJ schrieb:
> Für 'meinen' Prozessor STM32H7 sind die HAL Funktionen vorhanden und
> funktionieren prima.
Danke für den Tipp, ich habe mithilfe der HAL-Funktion auch schon eine 
Kommunikation mit dem TMC5160 zustande gebracht wollte aber versuchen, 
es ohne vorgefertigte Funktion zu lösen, um ein besseres Verständnis für 
die Thematik zu bekommen.

Vielen Dank für die tollen Hilfestellungen :-D.
Leider kann ich an der Funktion kaum weiterbasteln, bis ich verstehe, 
wie ich die 40bit versende und empfange, wenn mir nur maximal 16bit 
DataRegister zur Verfügung stehen.
Ich bin noch sehr frisch im Gebiet Mikrocontrolling, und habe 
wahrscheinlich einfach nicht die "Denke" wie ich solche Probleme löse. 
Ein Denkanstoß würde mir evtl. helfen ;-)

[Darth Moan (Gast)]

Moin,

Johannes Z. schrieb:
> Was meinst du mit: (wenn es schon byte weise sein muss)? Das
> DataRegister meines STM ist doch 16bit lang aber ich muss aber 40bit
> versenden. Ich hätte dann einfach 5 mal 8bit versendet. Wenn das zu
> umständlich ist, würde es mir helfen wenn du mir erklärst wie ich dann
> dieses "Problem" lösen kann.

Mit byte weise meine ich deine Funktionen die genau ein Byte senden oder
empfangen wollen. Wenn du mehr als ein Byte senden willst, rufst du die 
Funktion entsprechend oft auf.
Wenn du das so haben willst, würde ich eben nur eine Funktion empfehlen, 
die ein Byte sendet und gleichzeitig ein Byte empfängt.

uint8_t TransceiveByteSpi1(const uint8 *TxBuf, uint8_t *RxBuf)
{
  ...//check Rx/Tx Fifos empty 
  SPI1->DR = TxBuf[0];
  ...// warte auf byte in RxFifo ggf. mit timeout abbrechen ->NOK
  RxBuf[0] = SPI1->DR;
  ...
  return OK/NOK; //je nach ergebnis der checks/timeouts
}

Aber eigentlich würde ich eine Funktion empfehlen die n Bytes sendet und 
empfängt.
Dann kannst du immer noch 1 Byte senden und empfangen indem du die 
Anzahl der zu sendenden Bytes mit 1 übergibst.

In deinem Fall würde diese Funktion genau einmal aufgerufen werden, mit 
einem Zeiger auf die zu sendenden Daten, Zeiger auf den Empfangsbuffer, 
und Anzahl Bytes, die gesendet und empfangen werden sollen.
Also ungefähr so:

uint8_t TransceiveSPI1(const uint8_t *TxBufer, unit8_t *RxBufer, uint8_t Length);
{
  ...//check Rx/Tx Fifos empty 
  while(Length--)
  {
    SPI1->DR = *TxBuf++;
    ...// warte auf byte in RxFifo ggf. mit timeout abbrechen ->NOK
    *RxBuf++ = SPI1->DR;
  }
  ...
  return OK/NOK; //je nach ergebnis der checks/timeouts
}

Rückgabewert wäre ein Error Code. OK wenn alles glatt ging sonst NOK. 
Ggf. kannst du da auch detailiertere Error Codes basteln. Aber für 
Daheim reicht mir meistens OK/NOK.

Da kann man dann auch so Sachen einbauen, dass wenn zum Bleistift der 
RxBuf pointer NULL ist, die Empfangsdaten in den Orcus gehen. Nützlich 
wenn man grosse Messages hat die man aber nur senden will und keinen 
Dummy RxBuffer zur Verfügung stellen möchte, den man dann eh nicht 
auswertet.

Aber bei deinem Baustein wird ja eigentlich immer gesendet und 
empfangen.

[Florian S. (vatterger)]

Ich habe hier auch noch ein Projekt mit nem TMC5160 herumliegen, ist 
jetzt grade auf der Warteliste wegen STM32-Mangel, habe mit dem 
Prototyp-Board aber schonmal die Funktionen der Hardware getestet.

So habe ich die blockierende SPI Kommunikation zum Testen des Treibers 
implementiert:

// General purpose TMC5160 transaction
uint8_t tmc_spi_transaction(uint8_t address, uint8_t *data) {

  // CSS low
  HAL_GPIO_WritePin(SPI1_NSS_GPIO_Port, SPI1_NSS_Pin, GPIO_PIN_RESET);

  // 5 Bytes per Transaction
  HAL_SPI_TransmitReceive(&hspi1, &address, &address, 1/*len*/, 10/*timeout*/);
  HAL_SPI_TransmitReceive(&hspi1, data, data, 4/*len*/, 10/*timeout*/);

  // CSS high
  HAL_GPIO_WritePin(SPI1_NSS_GPIO_Port, SPI1_NSS_Pin, GPIO_PIN_SET);

  return address;
}

// Read register using 2 transactions
uint8_t tmc_spi_read(uint8_t address, uint8_t *data) {

  address = address & ~0x80;

  // First read makes sure result is in MISO queue
  tmc_spi_transaction(address, data);

  // Second read returns result
  return tmc_spi_transaction(address, data);
}

// Write register using one transaction
uint8_t tmc_spi_write(uint8_t address, uint8_t *data) {
  return tmc_spi_transaction(address | 0x80, data);
}

// TMC5160 integer transaction
uint8_t tmc_spi_transaction_int(uint8_t address, uint32_t *data) {

  uint8_t buf[4];

  buf[0] = ((*data) >> 24) & 0xFF;
  buf[1] = ((*data) >> 16) & 0xFF;
  buf[2] = ((*data) >> 8) & 0xFF;
  buf[3] = ((*data) >> 0) & 0xFF;

  uint8_t flags = tmc_spi_transaction(address, buf);

  *data = (buf[0] << 24) | (buf[1] << 16) | (buf[2] << 8) | (buf[3] << 0);

  return flags;
}

// Read an integer using two integer transactions
uint8_t tmc_spi_read_int(uint8_t address, uint32_t *data) {

  // Clear address write bit => read command
  address = address & ~0x80;

  // First read makes sure result is in MISO queue, data is unused here
  tmc_spi_transaction_int(address, data);

  // Second read returns the content of the register
  return tmc_spi_transaction_int(address, data);
}

// Write an integer using an integer transaction
uint8_t tmc_spi_write_int(uint8_t address, uint32_t data) {
  // Set address write bit => write command
  return tmc_spi_transaction_int(address | 0x80, &data);
}

Register schreiben:

tmc_spi_write_int(0x0B, gScale); // GLOBALSCALER

Register lesen:

tmc_spi_read_int(0x39, (uint32_t*) &X_ENC); // Read Encoder position into variable "X_ENC"

Du musst nur "HAL_SPI_TransmitReceive" und "HAL_GPIO_WritePin" mit 
deinem Register-level Code ersetzen.

[Johannes Z. (ffs_jz)]

Vielen Dank für die tolle Hilfestellung.
Die Kommunikation funktioniert nun.