[4.4.2] If CS is asserted between transfers then consider bus to be busy for … (#12784)

If CS is asserted between transfers then consider bus to be busy for all but owning device (#12604)

* If CS is asserted between transfers then consider bus to be busy for all but owning device

* Track if MAX7456 is mid DMA transfer, not simply that the SPI bus is busy

* Enable SPI DMA TX/RX together

Co-authored-by: Steve Evans <SteveCEvans@users.noreply.github.com>

src/main/drivers/bus.h

@@ -42,6 +42,7 @@ typedef enum {
     BUS_ABORT
 } busStatus_e;
 
+struct extDevice_s;
 
 // Bus interface, independent of connected device
 typedef struct busDevice_s {
@@ -74,6 +75,7 @@ typedef struct busDevice_s {
 #endif
 #endif // UNIT_TEST
     volatile struct busSegment_s* volatile curSegment;
+    volatile struct extDevice_s *csLockDevice;
     bool initSegment;
 } busDevice_t;
 

src/main/drivers/bus_spi.c

@@ -156,6 +156,10 @@ bool spiInit(SPIDevice device)
 // Return true if DMA engine is busy
 bool spiIsBusy(const extDevice_t *dev)
 {
+    if (dev->bus->csLockDevice && (dev->bus->csLockDevice != dev)) {
+        // If CS is still asserted, but not by the current device, the bus is busy
+        return true;
+    }
     return (dev->bus->curSegment != (busSegment_t *)BUS_SPI_FREE);
 }
 
@@ -163,7 +167,7 @@ bool spiIsBusy(const extDevice_t *dev)
 void spiWait(const extDevice_t *dev)
 {
     // Wait for completion
-    while (dev->bus->curSegment != (busSegment_t *)BUS_SPI_FREE);
+    while (spiIsBusy(dev));
 }
 
 // Wait for bus to become free, then read/write block of data
@@ -419,6 +423,11 @@ FAST_IRQ_HANDLER static void spiIrqHandler(const extDevice_t *dev)
             spiSequenceStart(nextDev);
         } else {
             // The end of the segment list has been reached, so mark transactions as complete
+            if (bus->curSegment->negateCS) {
+                bus->csLockDevice = (extDevice_t *)NULL;
+             } else {
+                bus->csLockDevice = (extDevice_t *)dev;
+            }
             bus->curSegment = (busSegment_t *)BUS_SPI_FREE;
         }
     } else {
@@ -729,6 +738,11 @@ void spiSequence(const extDevice_t *dev, busSegment_t *segments)
             // Safe to discard the volatile qualifier as we're in an atomic block
             busSegment_t *endCmpSegment = (busSegment_t *)bus->curSegment;
 
+            /* It is possible that the endCmpSegment may be NULL as the bus is held busy by csLockDevice.
+             * If this is the case this transfer will be silently dropped. Therefore holding CS low after a transfer,
+             * as is done with the SD card, MUST not be done on a bus where interrupts may trigger a transfer
+             * on an idle bus, such as would be the case with a gyro. This would be result in skipped gyro transfers.
+             */
             if (endCmpSegment) {
                 while (true) {
                     // Find the last segment of the current transfer
@@ -750,11 +764,11 @@ void spiSequence(const extDevice_t *dev, busSegment_t *segments)
                         endCmpSegment = (busSegment_t *)endCmpSegment->u.link.segments;
                     }
                 }
-            }
 
-            // Record the dev and segments parameters in the terminating segment entry
-            endCmpSegment->u.link.dev = dev;
-            endCmpSegment->u.link.segments = segments;
+                // Record the dev and segments parameters in the terminating segment entry
+                endCmpSegment->u.link.dev = dev;
+                endCmpSegment->u.link.segments = segments;
+            }
 
             return;
         } else {

src/main/drivers/bus_spi_ll.c

@@ -398,13 +398,11 @@ void spiInternalStartDMA(const extDevice_t *dev)
         LL_DMA_Init(dmaTx->dma, dmaTx->stream, bus->initTx);
         LL_DMA_Init(dmaRx->dma, dmaRx->stream, bus->initRx);
 
-        LL_SPI_EnableDMAReq_RX(dev->bus->busType_u.spi.instance);
-
         // Enable channels
         LL_DMA_EnableChannel(dmaTx->dma, dmaTx->stream);
         LL_DMA_EnableChannel(dmaRx->dma, dmaRx->stream);
 
-        LL_SPI_EnableDMAReq_TX(dev->bus->busType_u.spi.instance);
+        SET_BIT(dev->bus->busType_u.spi.instance->CR2, SPI_CR2_TXDMAEN | SPI_CR2_RXDMAEN);
 #else
         DMA_Stream_TypeDef *streamRegsTx = (DMA_Stream_TypeDef *)dmaTx->ref;
         DMA_Stream_TypeDef *streamRegsRx = (DMA_Stream_TypeDef *)dmaRx->ref;

src/main/drivers/max7456.c

@@ -194,6 +194,7 @@ extDevice_t *dev = &max7456Device;
 
 static bool max7456DeviceDetected = false;
 static uint16_t max7456SpiClockDiv;
+static volatile bool max7456ActiveDma = false;
 
 uint16_t maxScreenSize = VIDEO_BUFFER_CHARS_PAL;
 
@@ -540,7 +541,7 @@ bool max7456LayerCopy(displayPortLayer_e destLayer, displayPortLayer_e sourceLay
 
 bool max7456DmaInProgress(void)
 {
-    return spiIsBusy(dev);
+    return max7456ActiveDma;
 }
 
 bool max7456BuffersSynced(void)
@@ -611,13 +612,23 @@ bool max7456ReInitIfRequired(bool forceStallCheck)
     return stalled;
 }
 
+// Called in ISR context
+busStatus_e max7456_callbackReady(uint32_t arg)
+{
+    UNUSED(arg);
+
+    max7456ActiveDma = false;
+
+    return BUS_READY;
+}
+
 // Return true if screen still being transferred
 bool max7456DrawScreen(void)
 {
     static uint16_t pos = 0;
     // This routine doesn't block so need to use static data
     static busSegment_t segments[] = {
-            {.u.link = {NULL, NULL}, 0, true, NULL},
+            {.u.link = {NULL, NULL}, 0, true, max7456_callbackReady},
             {.u.link = {NULL, NULL}, 0, true, NULL},
     };
 
@@ -706,6 +717,8 @@ bool max7456DrawScreen(void)
             segments[0].u.buffers.txData = spiBuf;
             segments[0].len = spiBufIndex;
 
+            max7456ActiveDma = true;
+
             spiSequence(dev, &segments[0]);
 
             // Non-blocking, so transfer still in progress if using DMA