From 45250d837ce566740d1e1c8feb97ed9edde49f54 Mon Sep 17 00:00:00 2001
From: Kevin Mehall <km@kevinmehall.net>
Date: Wed, 24 Aug 2016 22:05:06 -0700
Subject: [PATCH] Avoid bridge state corruption when the SoC polls too fast

Increase the delay in spid and make the bridge code more resilient
against the EVSYS ISR preempting the DMA ISR when the bridge gets a new
transaction before finishing processing the last one.

There could be a better solution with more invasive changes to tell the
SoC when it is allowed to poll. This fix doesn't guarantee correctness,
and costs performance, but substantially improves stability when using
all three bridge pipes simultaneously.
---
 firmware/bridge.c | 35 ++++++++++++++++++++++++++---------
 soc/spid.c        |  3 +--
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/firmware/bridge.c b/firmware/bridge.c
index 0310be7..14653c4 100644
--- a/firmware/bridge.c
+++ b/firmware/bridge.c
@@ -120,6 +120,7 @@ void bridge_handle_sync() {
         for (u8 chan=0; chan<BRIDGE_NUM_CHAN; chan++) {
             u8 size = ctrl_rx.size[chan];
             if (ctrl_tx.status & (1<<chan) && size > 0) {
+                out_chan_ready &= ~ (1<<chan);
                 dma_fill_sercom_tx(&dma_chain_data_tx[desc], SERCOM_BRIDGE, NULL, size);
                 dma_fill_sercom_rx(&dma_chain_data_rx[desc], SERCOM_BRIDGE, out_chan_ptr[chan], size);
                 desc++;
@@ -127,6 +128,7 @@ void bridge_handle_sync() {
 
             size = ctrl_tx.size[chan];
             if (ctrl_rx.status & (1<<chan) && size > 0) {
+                in_chan_size[chan] = 0;
                 dma_fill_sercom_tx(&dma_chain_data_tx[desc], SERCOM_BRIDGE, in_chan_ptr[chan], size);
                 dma_fill_sercom_rx(&dma_chain_data_rx[desc], SERCOM_BRIDGE, NULL, size);
                 desc++;
@@ -161,26 +163,33 @@ void bridge_handle_sync() {
 void bridge_dma_rx_completion() {
     if (bridge_state == BRIDGE_STATE_DATA) {
 
+        // Copy the global state to this stack frame in case SYNC changes and the ISR overwrites these
+        uint8_t rx_status = ctrl_rx.status;
+        uint8_t tx_status = ctrl_tx.status;
+        uint8_t rx_size[BRIDGE_NUM_CHAN];
+        memcpy(rx_size, ctrl_rx.size, sizeof(rx_size));
+        uint8_t tx_size[BRIDGE_NUM_CHAN];
+        memcpy(tx_size, ctrl_tx.size, sizeof(tx_size));
+        __asm__ __volatile__ ("" : : : "memory");
+
         #define CHECK_OPEN(x) \
-            if ((ctrl_rx.status & (0x10<<x)) && !(was_open & (0x10<<x))) { \
-                bridge_open_##x(ctrl_rx.size[x]); \
+            if ((rx_status & (0x10<<x)) && !(was_open & (0x10<<x))) { \
+                bridge_open_##x(rx_size[x]); \
             }
 
         #define CHECK_COMPLETION_OUT(x) \
-            if (ctrl_tx.status & (1<<x) && ctrl_rx.size[x] > 0) { \
-                out_chan_ready &= ~ (1<<x); \
-                bridge_completion_out_##x(ctrl_rx.size[x]); \
+            if (tx_status & (1<<x) && rx_size[x] > 0) { \
+                bridge_completion_out_##x(rx_size[x]); \
             }
 
         #define CHECK_COMPLETION_IN(x) \
-            if (ctrl_rx.status & (1<<x) && ctrl_tx.size[x] > 0) { \
-                in_chan_size[x] = 0; \
+            if (rx_status & (1<<x) && tx_size[x] > 0) { \
                 bridge_completion_in_##x(); \
             }
 
         #define CHECK_CLOSE(x) \
-            if (!(ctrl_rx.status & (0x10<<x)) && (was_open & (0x10<<x))) { \
-                bridge_close_##x(ctrl_rx.size[x]); \
+            if (!(rx_status & (0x10<<x)) && (was_open & (0x10<<x))) { \
+                bridge_close_##x(rx_size[x]); \
             }
 
         CHECK_OPEN(0)
@@ -211,24 +220,32 @@ void bridge_dma_rx_completion() {
 }
 
 void bridge_start_in(u8 channel, u8* data, u8 length) {
+    __disable_irq();
     in_chan_ptr[channel] = data;
     in_chan_size[channel] = length;
+    __enable_irq();
     pin_high(PIN_BRIDGE_IRQ);
 }
 
 void bridge_start_out(u8 channel, u8* data) {
+    __disable_irq();
     out_chan_ptr[channel] = data;
     out_chan_ready |= (1<<channel);
+    __enable_irq();
     pin_high(PIN_BRIDGE_IRQ);
 }
 
 void bridge_enable_chan(u8 channel) {
+    __disable_irq();
     out_chan_ready |= (0x10<<channel);
+    __enable_irq();
     pin_high(PIN_BRIDGE_IRQ);
 }
 
 void bridge_disable_chan(u8 channel) {
+    __disable_irq();
     out_chan_ready &= ~(0x11<<channel); // Also clears the "ready to accept data" bit
     in_chan_size[channel] = 0; // Clears any data that was waiting to be sent
+    __enable_irq();
     pin_high(PIN_BRIDGE_IRQ);
 }
diff --git a/soc/spid.c b/soc/spid.c
index 5a59b39..462924d 100644
--- a/soc/spid.c
+++ b/soc/spid.c
@@ -108,8 +108,7 @@ struct pollfd fds[N_POLLFDS];
 int usbd_sock_fd;
 struct sockaddr_un usbd_sock_addr;
 void delay() {
-    volatile int i = 1000;
-    while(i--);
+    usleep(10);
 }
 
 /*