SPI: move to a single interrupt per TX/RX transaction

board/drivers/spi.h

@@ -8,11 +8,11 @@ uint8_t spi_buf_tx[SPI_BUF_SIZE];
 
 uint16_t spi_error_count = 0;
 
-static uint8_t spi_state = SPI_STATE_HEADER;
-static uint16_t spi_data_len_mosi;
 static bool spi_can_tx_ready = false;
 static const unsigned char version_text[] = "VERSION";
 
+static uint8_t spi_state = SPI_STATE_HEADER;
+
 static uint16_t spi_version_packet(uint8_t *out) {
   // this protocol version request is a stable portion of
   // the panda's SPI protocol. its contents match that of the
@@ -57,13 +57,17 @@ static uint16_t spi_version_packet(uint8_t *out) {
   return resp_len;
 }
 
+void spi_reset(void) {
+  spi_state = SPI_STATE_HEADER;
+  llspi_dma(spi_buf_tx, 0U, spi_buf_rx, SPI_HEADER_SIZE);
+}
+
 void spi_init(void) {
   // platform init
   llspi_init();
 
-  // Start the first packet!
-  spi_state = SPI_STATE_HEADER;
-  llspi_mosi_dma(spi_buf_rx, SPI_HEADER_SIZE);
+  spi_error_count = 0U;
+  spi_reset();
 }
 
 static bool validate_checksum(const uint8_t *data, uint16_t len) {
@@ -75,40 +79,42 @@ static bool validate_checksum(const uint8_t *data, uint16_t len) {
   return checksum == 0U;
 }
 
-void spi_rx_done(void) {
-  uint16_t response_len = 0U;
-  uint8_t next_rx_state = SPI_STATE_HEADER_NACK;
-  bool checksum_valid = false;
+void spi_done(void) {
   static uint8_t spi_endpoint;
   static uint16_t spi_data_len_miso;
+  static uint16_t spi_data_len_mosi;
+  uint8_t next_rx_state = SPI_STATE_HEADER;
+  bool checksum_valid = false;
 
   // parse header
   spi_endpoint = spi_buf_rx[1];
   spi_data_len_mosi = (spi_buf_rx[3] << 8) | spi_buf_rx[2];
   spi_data_len_miso = (spi_buf_rx[5] << 8) | spi_buf_rx[4];
 
   if (memcmp(spi_buf_rx, version_text, 7) == 0) {
-    response_len = spi_version_packet(spi_buf_tx);
-    next_rx_state = SPI_STATE_HEADER_NACK;;
+    llspi_dma(spi_buf_tx, spi_version_packet(spi_buf_tx), spi_buf_rx, SPI_HEADER_SIZE);
+    next_rx_state = SPI_STATE_HEADER;
   } else if (spi_state == SPI_STATE_HEADER) {
     checksum_valid = validate_checksum(spi_buf_rx, SPI_HEADER_SIZE);
     if ((spi_buf_rx[0] == SPI_SYNC_BYTE) && checksum_valid) {
       // response: ACK and start receiving data portion
       spi_buf_tx[0] = SPI_HACK;
-      next_rx_state = SPI_STATE_HEADER_ACK;
-      response_len = 1U;
+      llspi_dma(spi_buf_tx, 1U, &spi_buf_rx[SPI_HEADER_SIZE], spi_data_len_mosi + 1U);
+      next_rx_state = SPI_STATE_DATA;
+
     } else {
       // response: NACK and reset state machine
       #ifdef DEBUG_SPI
         print("- incorrect header sync or checksum "); hexdump(spi_buf_rx, SPI_HEADER_SIZE);
       #endif
       spi_buf_tx[0] = SPI_NACK;
-      next_rx_state = SPI_STATE_HEADER_NACK;
-      response_len = 1U;
+      llspi_dma(spi_buf_tx, 1U, spi_buf_rx, SPI_HEADER_SIZE);
+      next_rx_state = SPI_STATE_HEADER;
     }
-  } else if (spi_state == SPI_STATE_DATA_RX) {
+  } else if (spi_state == SPI_STATE_DATA) {
     // We got everything! Based on the endpoint specified, call the appropriate handler
     bool response_ack = false;
+    uint16_t response_len = 0U;
     checksum_valid = validate_checksum(&(spi_buf_rx[SPI_HEADER_SIZE]), spi_data_len_mosi + 1U);
     if (checksum_valid) {
       if (spi_endpoint == 0U) {
@@ -168,8 +174,8 @@ void spi_rx_done(void) {
 
     if (!response_ack) {
       spi_buf_tx[0] = SPI_NACK;
-      next_rx_state = SPI_STATE_HEADER_NACK;
-      response_len = 1U;
+      llspi_dma(spi_buf_tx, 1U, spi_buf_rx, SPI_HEADER_SIZE);
+      next_rx_state = SPI_STATE_HEADER;
     } else {
       // Setup response header
       spi_buf_tx[0] = SPI_DACK;
@@ -184,47 +190,19 @@ void spi_rx_done(void) {
       spi_buf_tx[response_len + 3U] = checksum;
       response_len += 4U;
 
-      next_rx_state = SPI_STATE_DATA_TX;
+      llspi_dma(spi_buf_tx, response_len, spi_buf_rx, SPI_HEADER_SIZE);
+      next_rx_state = SPI_STATE_HEADER;
     }
   } else {
     print("SPI: RX unexpected state: "); puth(spi_state); print("\n");
   }
 
-  // send out response
-  if (response_len == 0U) {
-    print("SPI: no response\n");
-    spi_buf_tx[0] = SPI_NACK;
-    spi_state = SPI_STATE_HEADER_NACK;
-    response_len = 1U;
-  }
-  llspi_miso_dma(spi_buf_tx, response_len);
-
   spi_state = next_rx_state;
   if (!checksum_valid) {
     spi_error_count += 1U;
   }
 }
 
-void spi_tx_done(bool reset) {
-  if ((spi_state == SPI_STATE_HEADER_NACK) || reset) {
-    // Reset state
-    spi_state = SPI_STATE_HEADER;
-    llspi_mosi_dma(spi_buf_rx, SPI_HEADER_SIZE);
-  } else if (spi_state == SPI_STATE_HEADER_ACK) {
-    // ACK was sent, queue up the RX buf for the data + checksum
-    spi_state = SPI_STATE_DATA_RX;
-    llspi_mosi_dma(&spi_buf_rx[SPI_HEADER_SIZE], spi_data_len_mosi + 1U);
-  } else if (spi_state == SPI_STATE_DATA_TX) {
-    // Reset state
-    spi_state = SPI_STATE_HEADER;
-    llspi_mosi_dma(spi_buf_rx, SPI_HEADER_SIZE);
-  } else {
-    spi_state = SPI_STATE_HEADER;
-    llspi_mosi_dma(spi_buf_rx, SPI_HEADER_SIZE);
-    print("SPI: TX unexpected state: "); puth(spi_state); print("\n");
-  }
-}
-
 void can_tx_comms_resume_spi(void) {
   spi_can_tx_ready = true;
 }

board/drivers/spi_declarations.h

@@ -21,12 +21,8 @@ __attribute__((section(".sram12"))) extern uint8_t spi_buf_tx[SPI_BUF_SIZE];
 
 // SPI states
 enum {
-  SPI_STATE_HEADER,
-  SPI_STATE_HEADER_ACK,
-  SPI_STATE_HEADER_NACK,
-  SPI_STATE_DATA_RX,
-  SPI_STATE_DATA_RX_ACK,
-  SPI_STATE_DATA_TX
+  SPI_STATE_HEADER = 0U,
+  SPI_STATE_DATA = 1U
 };
 
 extern uint16_t spi_error_count;
@@ -35,10 +31,10 @@ extern uint16_t spi_error_count;
 
 // low level SPI prototypes
 void llspi_init(void);
-void llspi_mosi_dma(uint8_t *addr, int len);
-void llspi_miso_dma(uint8_t *addr, int len);
+void llspi_dump_state(void);
+void llspi_dma(uint8_t *tx_addr, int tx_len, uint8_t *rx_addr, int rx_len);
 
 void can_tx_comms_resume_spi(void);
 void spi_init(void);
-void spi_rx_done(void);
-void spi_tx_done(bool reset);
+void spi_reset(void);
+void spi_done(void);

board/stm32h7/llspi.h

@@ -1,9 +1,26 @@
-// master -> panda DMA start
-void llspi_mosi_dma(uint8_t *addr, int len) {
+static uint8_t *llspi_rx_addr;
+static int llspi_rx_len;
+static int llspi_tx_len;
+
+static void llspi_disable(void) {
   // disable DMA + SPI
-  register_clear_bits(&(SPI4->CFG1), SPI_CFG1_RXDMAEN);
   DMA2_Stream2->CR &= ~DMA_SxCR_EN;
-  register_clear_bits(&(SPI4->CR1), SPI_CR1_SPE);
+  DMA2_Stream3->CR &= ~DMA_SxCR_EN;
+  while((DMA2_Stream2->CR & DMA_SxCR_EN) != 0U);
+  while((DMA2_Stream3->CR & DMA_SxCR_EN) != 0U);
+  SPI4->CR1 &= ~SPI_CR1_SPE;
+  DMA2->LIFCR = DMA_LIFCR_CTCIF2 | DMA_LIFCR_CTCIF3;
+  register_clear_bits(&(SPI4->CFG1), SPI_CFG1_RXDMAEN | SPI_CFG1_TXDMAEN);
+}
+
+void llspi_dma(uint8_t *tx_addr, int tx_len, uint8_t *rx_addr, int rx_len) {
+  // set global for later use
+  llspi_rx_addr = rx_addr;
+  llspi_rx_len = rx_len;
+  llspi_tx_len = tx_len;
+
+  int total_len = tx_len + rx_len;
+  (void) memset(&tx_addr[tx_len], 0xcd, rx_len);
 
   // drain the bus
   while ((SPI4->SR & SPI_SR_RXP) != 0U) {
@@ -13,95 +30,76 @@ void llspi_mosi_dma(uint8_t *addr, int len) {
 
   // clear all pending
   SPI4->IFCR |= (0x1FFU << 3U);
-  register_set(&(SPI4->IER), 0, 0x3FFU);
-
-  // setup destination and length
-  register_set(&(DMA2_Stream2->M0AR), (uint32_t)addr, 0xFFFFFFFFU);
-  DMA2_Stream2->NDTR = len;
+  register_clear_bits(&(SPI4->IER), SPI_IER_EOTIE);
 
-  // enable DMA + SPI
-  DMA2_Stream2->CR |= DMA_SxCR_EN;
+  // setup destinations and length
   register_set_bits(&(SPI4->CFG1), SPI_CFG1_RXDMAEN);
-  register_set_bits(&(SPI4->CR1), SPI_CR1_SPE);
-}
-
-// panda -> master DMA start
-void llspi_miso_dma(uint8_t *addr, int len) {
-  // disable DMA + SPI
-  DMA2_Stream3->CR &= ~DMA_SxCR_EN;
-  register_clear_bits(&(SPI4->CFG1), SPI_CFG1_TXDMAEN);
-  register_clear_bits(&(SPI4->CR1), SPI_CR1_SPE);
-
-  // setup source and length
-  register_set(&(DMA2_Stream3->M0AR), (uint32_t)addr, 0xFFFFFFFFU);
-  DMA2_Stream3->NDTR = len;
-
-  // clear under-run while we were reading
-  SPI4->IFCR |= (0x1FFU << 3U);
-
-  // setup interrupt on TXC
-  register_set(&(SPI4->IER), (1U << SPI_IER_EOTIE_Pos), 0x3FFU);
-
-  // enable DMA + SPI
-  register_set_bits(&(SPI4->CFG1), SPI_CFG1_TXDMAEN);
+  register_set(&(DMA2_Stream3->M0AR), (uint32_t)tx_addr, 0xFFFFFFFFU);
+  DMA2_Stream3->NDTR = total_len;
   DMA2_Stream3->CR |= DMA_SxCR_EN;
-  register_set_bits(&(SPI4->CR1), SPI_CR1_SPE);
-}
-
-static bool spi_tx_dma_done = false;
-// master -> panda DMA finished
-static void DMA2_Stream2_IRQ_Handler(void) {
-  // Clear interrupt flag
-  DMA2->LIFCR = DMA_LIFCR_CTCIF2;
+  register_clear_bits(&(SPI4->CFG2), SPI_CFG2_COMM);
 
-  spi_rx_done();
-}
+  register_set(&(DMA2_Stream2->M0AR), (uint32_t)rx_addr, 0xFFFFFFFFU);  
+  DMA2_Stream2->NDTR = total_len;
+  DMA2_Stream2->CR |= DMA_SxCR_EN;
 
-// panda -> master DMA finished
-static void DMA2_Stream3_IRQ_Handler(void) {
-  ENTER_CRITICAL();
+  SPI4->CR2 = total_len;
 
-  DMA2->LIFCR = DMA_LIFCR_CTCIF3;
-  spi_tx_dma_done = true;
+  register_set_bits(&(SPI4->CFG1), SPI_CFG1_TXDMAEN);
 
-  EXIT_CRITICAL();
+  // setup interrupt on EOT and start transfer
+  register_set_bits(&(SPI4->IER), SPI_IER_EOTIE);
+  SPI4->CR1 |= SPI_CR1_SPE;
+  print("B");
 }
 
 // panda TX finished
 static void SPI4_IRQ_Handler(void) {
-  // clear flag
-  SPI4->IFCR |= (0x1FFU << 3U);
+  uint32_t sr = SPI4->SR;
+  SPI4->IFCR |= (0x1FFU << 3U);  
+
+  if (((sr & SPI_SR_EOT) != 0U)) {
+    llspi_disable();
+
+    if ((((sr & SPI_SR_RXPLVL) >> SPI_SR_RXPLVL_Pos) == 0U) && (((sr & SPI_SR_RXWNE) == 0U))) {
+      // shift any received data down in the rx buffer
+      (void)memcpy(llspi_rx_addr, &((uint8_t *)llspi_rx_addr)[llspi_tx_len], llspi_rx_len);
 
-  if (spi_tx_dma_done && ((SPI4->SR & SPI_SR_TXC) != 0U)) {
-    spi_tx_dma_done = false;
-    spi_tx_done(false);
+      spi_done();
+    } else {
+      spi_reset();
+    }
   }
 }
 
+// static void DMA2_Stream2_IRQ_Handler(void) {
+//   // Clear interrupt flag
+//   DMA2->LIFCR = DMA_LIFCR_CTCIF2;
+// }
 
 void llspi_init(void) {
   REGISTER_INTERRUPT(SPI4_IRQn, SPI4_IRQ_Handler, (SPI_IRQ_RATE * 2U), FAULT_INTERRUPT_RATE_SPI)
-  REGISTER_INTERRUPT(DMA2_Stream2_IRQn, DMA2_Stream2_IRQ_Handler, SPI_IRQ_RATE, FAULT_INTERRUPT_RATE_SPI_DMA)
-  REGISTER_INTERRUPT(DMA2_Stream3_IRQn, DMA2_Stream3_IRQ_Handler, SPI_IRQ_RATE, FAULT_INTERRUPT_RATE_SPI_DMA)
+  // REGISTER_INTERRUPT(DMA2_Stream2_IRQn, DMA2_Stream2_IRQ_Handler, SPI_IRQ_RATE, FAULT_INTERRUPT_RATE_SPI_DMA)
 
   // Setup MOSI DMA
   register_set(&(DMAMUX1_Channel10->CCR), 83U, 0xFFFFFFFFU);
-  register_set(&(DMA2_Stream2->CR), (DMA_SxCR_MINC | DMA_SxCR_TCIE), 0x1E077EFEU);
+  register_set(&(DMA2_Stream2->CR), DMA_SxCR_MINC, 0x1E077EFEU);
+  // register_set(&(DMA2_Stream2->CR), DMA_SxCR_MINC | DMA_SxCR_TCIE, 0x1E077EFEU);
   register_set(&(DMA2_Stream2->PAR), (uint32_t)&(SPI4->RXDR), 0xFFFFFFFFU);
 
   // Setup MISO DMA, memory -> peripheral
   register_set(&(DMAMUX1_Channel11->CCR), 84U, 0xFFFFFFFFU);
-  register_set(&(DMA2_Stream3->CR), (DMA_SxCR_MINC | DMA_SxCR_DIR_0 | DMA_SxCR_TCIE), 0x1E077EFEU);
+  register_set(&(DMA2_Stream3->CR), (DMA_SxCR_MINC | DMA_SxCR_DIR_0), 0x1E077EFEU);
   register_set(&(DMA2_Stream3->PAR), (uint32_t)&(SPI4->TXDR), 0xFFFFFFFFU);
 
   // Enable SPI
-  register_set(&(SPI4->IER), 0, 0x3FFU);
+  register_set(&(SPI4->IER), 0U, 0x7FFU);
   register_set(&(SPI4->CFG1), (7U << SPI_CFG1_DSIZE_Pos), SPI_CFG1_DSIZE_Msk);
+  register_set(&(SPI4->CFG2), 0U, 0xF7FE80FFU);
   register_set(&(SPI4->UDRDR), 0xcd, 0xFFFFU);  // set under-run value for debugging
-  register_set(&(SPI4->CR1), SPI_CR1_SPE, 0xFFFFU);
-  register_set(&(SPI4->CR2), 0, 0xFFFFU);
+  SPI4->CR2 = 0U;
+  llspi_disable();
 
-  NVIC_EnableIRQ(DMA2_Stream2_IRQn);
-  NVIC_EnableIRQ(DMA2_Stream3_IRQn);
   NVIC_EnableIRQ(SPI4_IRQn);
+  // NVIC_EnableIRQ(DMA2_Stream2_IRQn);
 }

python/spi.py

@@ -159,18 +159,15 @@ def _transfer_spidev(self, spi, endpoint: int, data, timeout: int, max_rx_len: i
       return b""
     else:
       logger.debug("- waiting for data ACK")
-      preread_len = USBPACKET_MAX_SIZE + 1  # read enough for a controlRead
-      dat = self._wait_for_ack(spi, DACK, timeout, 0x13, length=3 + preread_len)
+      dat = self._wait_for_ack(spi, DACK, timeout, 0x13, length=3)
 
-      # get response length, then response
+      # get response length
       response_len = struct.unpack("<H", dat[1:3])[0]
       if response_len > max_rx_len:
         raise PandaSpiException(f"response length greater than max ({max_rx_len} {response_len})")
 
       # read rest
-      remaining = (response_len + 1) - preread_len
-      if remaining > 0:
-        dat += bytes(spi.readbytes(remaining))
+      dat += bytes(spi.readbytes(response_len + 1))
 
       dat = dat[:3 + response_len + 1]
       if self._calc_checksum(dat) != 0: