REFACTOR: SPI segment handling (#14345)

* Move common code (spiIrqHandler) from src/platform/common/stm32/ to src/main/drivers/bus_spi.c
* Move repeated code from inside spiSequenceStart to bus_spi.c as spiProcessSegmentsDMA, spiProcessSegmentsPolled
* spiInternalReadWriteBufPolled becomes non-static

Co-authored-by: Matthew Selby <matthewjselby@aol.com>

src/main/drivers/bus_spi.c

@@ -485,4 +485,157 @@ void spiSequence(const extDevice_t *dev, busSegment_t *segments)
 
     spiSequenceStart(dev);
 }
+
+// Process segments using DMA - expects DMA irq handler to have been set up to feed into spiIrqHandler.
+FAST_CODE void spiProcessSegmentsDMA(const extDevice_t *dev)
+{
+    // Intialise the init structures for the first transfer
+    spiInternalInitStream(dev, false);
+
+    // Assert Chip Select
+    IOLo(dev->busType_u.spi.csnPin);
+
+    // Start the transfers
+    spiInternalStartDMA(dev);
+}
+
+// Interrupt handler common code for SPI receive DMA completion.
+// Proceed to next segment as required.
+FAST_IRQ_HANDLER void spiIrqHandler(const extDevice_t *dev)
+{
+    busDevice_t *bus = dev->bus;
+    busSegment_t *nextSegment;
+
+    if (bus->curSegment->callback) {
+        switch(bus->curSegment->callback(dev->callbackArg)) {
+        case BUS_BUSY:
+            // Repeat the last DMA segment
+            bus->curSegment--;
+            // Reinitialise the cached init values as segment is not progressing
+            spiInternalInitStream(dev, true);
+            break;
+
+        case BUS_ABORT:
+            // Skip to the end of the segment list
+            nextSegment = (busSegment_t *)bus->curSegment + 1;
+            while (nextSegment->len != 0) {
+                bus->curSegment = nextSegment;
+                nextSegment = (busSegment_t *)bus->curSegment + 1;
+            }
+            break;
+
+        case BUS_READY:
+        default:
+            // Advance to the next DMA segment
+            break;
+        }
+    }
+
+    // Advance through the segment list
+    // OK to discard the volatile qualifier here
+    nextSegment = (busSegment_t *)bus->curSegment + 1;
+
+    if (nextSegment->len == 0) {
+        // If a following transaction has been linked, start it
+        if (nextSegment->u.link.dev) {
+            const extDevice_t *nextDev = nextSegment->u.link.dev;
+            busSegment_t *nextSegments = (busSegment_t *)nextSegment->u.link.segments;
+            // The end of the segment list has been reached
+            bus->curSegment = nextSegments;
+            nextSegment->u.link.dev = NULL;
+            nextSegment->u.link.segments = NULL;
+            spiSequenceStart(nextDev);
+        } else {
+            // The end of the segment list has been reached, so mark transactions as complete
+            bus->curSegment = (busSegment_t *)BUS_SPI_FREE;
+        }
+    } else {
+        // Do as much processing as possible before asserting CS to avoid violating minimum high time
+        bool negateCS = bus->curSegment->negateCS;
+
+        bus->curSegment = nextSegment;
+
+        // After the completion of the first segment setup the init structure for the subsequent segment
+        if (bus->initSegment) {
+            spiInternalInitStream(dev, false);
+            bus->initSegment = false;
+        }
+
+        if (negateCS) {
+            // Assert Chip Select - it's costly so only do so if necessary
+            IOLo(dev->busType_u.spi.csnPin);
+        }
+
+        // Launch the next transfer
+        spiInternalStartDMA(dev);
+
+        // Prepare the init structures ready for the next segment to reduce inter-segment time
+        spiInternalInitStream(dev, true);
+    }
+}
+
+FAST_CODE void spiProcessSegmentsPolled(const extDevice_t *dev)
+{
+    busDevice_t *bus = dev->bus;
+    busSegment_t *lastSegment = NULL;
+    bool segmentComplete;
+
+    // Manually work through the segment list performing a transfer for each
+    while (bus->curSegment->len) {
+        if (!lastSegment || lastSegment->negateCS) {
+            // Assert Chip Select if necessary - it's costly so only do so if necessary
+            IOLo(dev->busType_u.spi.csnPin);
+        }
+
+        spiInternalReadWriteBufPolled(
+            bus->busType_u.spi.instance,
+            bus->curSegment->u.buffers.txData,
+            bus->curSegment->u.buffers.rxData,
+            bus->curSegment->len);
+
+        if (bus->curSegment->negateCS) {
+            // Negate Chip Select
+            IOHi(dev->busType_u.spi.csnPin);
+        }
+
+        segmentComplete = true;
+        if (bus->curSegment->callback) {
+            switch(bus->curSegment->callback(dev->callbackArg)) {
+            case BUS_BUSY:
+                // Repeat the last DMA segment
+                segmentComplete = false;
+                break;
+
+            case BUS_ABORT:
+                bus->curSegment = (busSegment_t *)BUS_SPI_FREE;
+                segmentComplete = false;
+                return;
+
+            case BUS_READY:
+            default:
+                // Advance to the next DMA segment
+                break;
+            }
+        }
+        if (segmentComplete) {
+            lastSegment = (busSegment_t *)bus->curSegment;
+            bus->curSegment++;
+        }
+    }
+
+    // If a following transaction has been linked, start it
+    if (bus->curSegment->u.link.dev) {
+        busSegment_t *endSegment = (busSegment_t *)bus->curSegment;
+        const extDevice_t *nextDev = endSegment->u.link.dev;
+        busSegment_t *nextSegments = (busSegment_t *)endSegment->u.link.segments;
+        bus->curSegment = nextSegments;
+        endSegment->u.link.dev = NULL;
+        endSegment->u.link.segments = NULL;
+        spiSequenceStart(nextDev);
+    } else {
+        // The end of the segment list has been reached, so mark transactions as complete
+        bus->curSegment = (busSegment_t *)BUS_SPI_FREE;
+    }
+}
+
 #endif

src/main/drivers/bus_spi.h

@@ -140,3 +140,10 @@ bool spiUseSDO_DMA(const extDevice_t *dev);
 void spiBusDeviceRegister(const extDevice_t *dev);
 uint8_t spiGetRegisteredDeviceCount(void);
 uint8_t spiGetExtDeviceCount(const extDevice_t *dev);
+
+// Common code to process linked segments, to be called from spiSequenceStart.
+// DMA path makes use of spiInternalInitStream, spiInternalStartDMA.
+void spiProcessSegmentsDMA(const extDevice_t *dev);
+void spiIrqHandler(const extDevice_t *dev);
+// Polling code calls spiInternalReadWriteBufPolled.
+void spiProcessSegmentsPolled(const extDevice_t *dev);

src/main/drivers/bus_spi_impl.h

@@ -79,4 +79,5 @@ void spiInternalStartDMA(const extDevice_t *dev);
 void spiInternalStopDMA (const extDevice_t *dev);
 void spiInternalResetStream(dmaChannelDescriptor_t *descriptor);
 void spiInternalResetDescriptors(busDevice_t *bus);
+bool spiInternalReadWriteBufPolled(SPI_TypeDef *instance, const uint8_t *txData, uint8_t *rxData, int len);
 void spiSequenceStart(const extDevice_t *dev);

src/platform/APM32/bus_spi_apm32.c

@@ -138,7 +138,7 @@ void spiInternalResetStream(dmaChannelDescriptor_t *descriptor)
     DMA_CLEAR_FLAG(descriptor, DMA_IT_HTIF | DMA_IT_TEIF | DMA_IT_TCIF);
 }
 
-FAST_CODE static bool spiInternalReadWriteBufPolled(SPI_TypeDef *instance, const uint8_t *txData, uint8_t *rxData, int len)
+FAST_CODE bool spiInternalReadWriteBufPolled(SPI_TypeDef *instance, const uint8_t *txData, uint8_t *rxData, int len)
 {
     while (len) {
         while (!DDL_SPI_IsActiveFlag_TXE(instance));
@@ -377,74 +377,9 @@ FAST_CODE void spiSequenceStart(const extDevice_t *dev)
     if (bus->useDMA && dmaSafe && ((segmentCount > 1) ||
                                    (xferLen >= SPI_DMA_THRESHOLD) ||
                                    !bus->curSegment[segmentCount].negateCS)) {
-        // Intialise the init structures for the first transfer
-        spiInternalInitStream(dev, false);
-
-        // Assert Chip Select
-        IOLo(dev->busType_u.spi.csnPin);
-
-        // Start the transfers
-        spiInternalStartDMA(dev);
+        spiProcessSegmentsDMA(dev);
     } else {
-        busSegment_t *lastSegment = NULL;
-        bool segmentComplete;
-
-        // Manually work through the segment list performing a transfer for each
-        while (bus->curSegment->len) {
-            if (!lastSegment || lastSegment->negateCS) {
-                // Assert Chip Select if necessary - it's costly so only do so if necessary
-                IOLo(dev->busType_u.spi.csnPin);
-            }
-
-            spiInternalReadWriteBufPolled(
-                    bus->busType_u.spi.instance,
-                    bus->curSegment->u.buffers.txData,
-                    bus->curSegment->u.buffers.rxData,
-                    bus->curSegment->len);
-
-            if (bus->curSegment->negateCS) {
-                // Negate Chip Select
-                IOHi(dev->busType_u.spi.csnPin);
-            }
-
-            segmentComplete = true;
-            if (bus->curSegment->callback) {
-                switch(bus->curSegment->callback(dev->callbackArg)) {
-                case BUS_BUSY:
-                    // Repeat the last DMA segment
-                    segmentComplete = false;
-                    break;
-
-                case BUS_ABORT:
-                    bus->curSegment = (busSegment_t *)BUS_SPI_FREE;
-                    segmentComplete = false;
-                    return;
-
-                case BUS_READY:
-                default:
-                    // Advance to the next DMA segment
-                    break;
-                }
-            }
-            if (segmentComplete) {
-                lastSegment = (busSegment_t *)bus->curSegment;
-                bus->curSegment++;
-            }
-        }
-
-        // If a following transaction has been linked, start it
-        if (bus->curSegment->u.link.dev) {
-            busSegment_t *endSegment = (busSegment_t *)bus->curSegment;
-            const extDevice_t *nextDev = endSegment->u.link.dev;
-            busSegment_t *nextSegments = (busSegment_t *)endSegment->u.link.segments;
-            bus->curSegment = nextSegments;
-            endSegment->u.link.dev = NULL;
-            endSegment->u.link.segments = NULL;
-            spiSequenceStart(nextDev);
-        } else {
-            // The end of the segment list has been reached, so mark transactions as complete
-            bus->curSegment = (busSegment_t *)BUS_SPI_FREE;
-        }
+        spiProcessSegmentsPolled(dev);
     }
 }
 #endif

src/platform/AT32/bus_spi_at32bsp.c

@@ -135,7 +135,7 @@ void spiInternalResetStream(dmaChannelDescriptor_t *descriptor)
     DMA_CLEAR_FLAG(descriptor, DMA_IT_HTIF | DMA_IT_TEIF | DMA_IT_TCIF);
 }
 
-static bool spiInternalReadWriteBufPolled(spi_type *instance, const uint8_t *txData, uint8_t *rxData, int len)
+bool spiInternalReadWriteBufPolled(spi_type *instance, const uint8_t *txData, uint8_t *rxData, int len)
 {
     uint8_t b;
 
@@ -345,73 +345,9 @@ void spiSequenceStart(const extDevice_t *dev)
     if (bus->useDMA && dmaSafe && ((segmentCount > 1) ||
                                    (xferLen >= SPI_DMA_THRESHOLD) ||
                                    !bus->curSegment[segmentCount].negateCS)) {
-        // Intialise the init structures for the first transfer
-        spiInternalInitStream(dev, false);
-
-        // Assert Chip Select
-        IOLo(dev->busType_u.spi.csnPin);
-
-        // Start the transfers
-        spiInternalStartDMA(dev);
+        spiProcessSegmentsDMA(dev);
     } else {
-        busSegment_t *lastSegment = NULL;
-        bool segmentComplete;
-
-        // Manually work through the segment list performing a transfer for each
-        while (bus->curSegment->len) {
-            if (!lastSegment || lastSegment->negateCS) {
-                // Assert Chip Select if necessary - it's costly so only do so if necessary
-                IOLo(dev->busType_u.spi.csnPin);
-            }
-
-            spiInternalReadWriteBufPolled(bus->busType_u.spi.instance,
-                                          bus->curSegment->u.buffers.txData,
-                                          bus->curSegment->u.buffers.rxData,
-                                          bus->curSegment->len);
-
-            if (bus->curSegment->negateCS) {
-                // Negate Chip Select
-                IOHi(dev->busType_u.spi.csnPin);
-            }
-
-            segmentComplete = true;
-            if (bus->curSegment->callback) {
-                switch(bus->curSegment->callback(dev->callbackArg)) {
-                case BUS_BUSY:
-                    // Repeat the last DMA segment
-                    segmentComplete = false;
-                    break;
-
-                case BUS_ABORT:
-                    bus->curSegment = (busSegment_t *)BUS_SPI_FREE;
-                    segmentComplete = false;
-                    return;
-
-                case BUS_READY:
-                default:
-                    // Advance to the next DMA segment
-                    break;
-                }
-            }
-            if (segmentComplete) {
-                lastSegment = (busSegment_t *)bus->curSegment;
-                bus->curSegment++;
-            }
-        }
-
-        // If a following transaction has been linked, start it
-        if (bus->curSegment->u.link.dev) {
-            const extDevice_t *nextDev = bus->curSegment->u.link.dev;
-            busSegment_t *nextSegments = (busSegment_t *)bus->curSegment->u.link.segments;
-            busSegment_t *endSegment = (busSegment_t *)bus->curSegment;
-            bus->curSegment = nextSegments;
-            endSegment->u.link.dev = NULL;
-            endSegment->u.link.segments = NULL;
-            spiSequenceStart(nextDev);
-        } else {
-            // The end of the segment list has been reached, so mark transactions as complete
-            bus->curSegment = (busSegment_t *)BUS_SPI_FREE;
-        }
+        spiProcessSegmentsPolled(dev);
     }
 }
 #endif

src/platform/STM32/bus_spi_ll.c

@@ -206,7 +206,7 @@ void spiInternalResetStream(dmaChannelDescriptor_t *descriptor)
     DMA_CLEAR_FLAG(descriptor, DMA_IT_HTIF | DMA_IT_TEIF | DMA_IT_TCIF);
 }
 
-FAST_CODE static bool spiInternalReadWriteBufPolled(SPI_TypeDef *instance, const uint8_t *txData, uint8_t *rxData, int len)
+FAST_CODE bool spiInternalReadWriteBufPolled(SPI_TypeDef *instance, const uint8_t *txData, uint8_t *rxData, int len)
 {
 #if defined(STM32H7)
     LL_SPI_SetTransferSize(instance, len);
@@ -618,74 +618,9 @@ FAST_CODE void spiSequenceStart(const extDevice_t *dev)
     if (bus->useDMA && dmaSafe && ((segmentCount > 1) ||
                                    (xferLen >= SPI_DMA_THRESHOLD) ||
                                    !bus->curSegment[segmentCount].negateCS)) {
-        // Intialise the init structures for the first transfer
-        spiInternalInitStream(dev, false);
-
-        // Assert Chip Select
-        IOLo(dev->busType_u.spi.csnPin);
-
-        // Start the transfers
-        spiInternalStartDMA(dev);
+        spiProcessSegmentsDMA(dev);
     } else {
-        busSegment_t *lastSegment = NULL;
-        bool segmentComplete;
-
-        // Manually work through the segment list performing a transfer for each
-        while (bus->curSegment->len) {
-            if (!lastSegment || lastSegment->negateCS) {
-                // Assert Chip Select if necessary - it's costly so only do so if necessary
-                IOLo(dev->busType_u.spi.csnPin);
-            }
-
-            spiInternalReadWriteBufPolled(
-                    bus->busType_u.spi.instance,
-                    bus->curSegment->u.buffers.txData,
-                    bus->curSegment->u.buffers.rxData,
-                    bus->curSegment->len);
-
-            if (bus->curSegment->negateCS) {
-                // Negate Chip Select
-                IOHi(dev->busType_u.spi.csnPin);
-            }
-
-            segmentComplete = true;
-            if (bus->curSegment->callback) {
-                switch(bus->curSegment->callback(dev->callbackArg)) {
-                case BUS_BUSY:
-                    // Repeat the last DMA segment
-                    segmentComplete = false;
-                    break;
-
-                case BUS_ABORT:
-                    bus->curSegment = (busSegment_t *)BUS_SPI_FREE;
-                    segmentComplete = false;
-                    return;
-
-                case BUS_READY:
-                default:
-                    // Advance to the next DMA segment
-                    break;
-                }
-            }
-            if (segmentComplete) {
-                lastSegment = (busSegment_t *)bus->curSegment;
-                bus->curSegment++;
-            }
-        }
-
-        // If a following transaction has been linked, start it
-        if (bus->curSegment->u.link.dev) {
-            busSegment_t *endSegment = (busSegment_t *)bus->curSegment;
-            const extDevice_t *nextDev = endSegment->u.link.dev;
-            busSegment_t *nextSegments = (busSegment_t *)endSegment->u.link.segments;
-            bus->curSegment = nextSegments;
-            endSegment->u.link.dev = NULL;
-            endSegment->u.link.segments = NULL;
-            spiSequenceStart(nextDev);
-        } else {
-            // The end of the segment list has been reached, so mark transactions as complete
-            bus->curSegment = (busSegment_t *)BUS_SPI_FREE;
-        }
+        spiProcessSegmentsPolled(dev);
     }
 }
 #endif

src/platform/STM32/bus_spi_stdperiph.c

@@ -143,7 +143,7 @@ void spiInternalResetStream(dmaChannelDescriptor_t *descriptor)
     DMA_CLEAR_FLAG(descriptor, DMA_IT_HTIF | DMA_IT_TEIF | DMA_IT_TCIF);
 }
 
-static bool spiInternalReadWriteBufPolled(SPI_TypeDef *instance, const uint8_t *txData, uint8_t *rxData, int len)
+bool spiInternalReadWriteBufPolled(SPI_TypeDef *instance, const uint8_t *txData, uint8_t *rxData, int len)
 {
     uint8_t b;
 
@@ -369,74 +369,9 @@ void spiSequenceStart(const extDevice_t *dev)
     if (bus->useDMA && dmaSafe && ((segmentCount > 1) ||
                                    (xferLen >= SPI_DMA_THRESHOLD) ||
                                    !bus->curSegment[segmentCount].negateCS)) {
-        // Intialise the init structures for the first transfer
-        spiInternalInitStream(dev, false);
-
-        // Assert Chip Select
-        IOLo(dev->busType_u.spi.csnPin);
-
-        // Start the transfers
-        spiInternalStartDMA(dev);
+        spiProcessSegmentsDMA(dev);
     } else {
-        busSegment_t *lastSegment = NULL;
-        bool segmentComplete;
-
-        // Manually work through the segment list performing a transfer for each
-        while (bus->curSegment->len) {
-            if (!lastSegment || lastSegment->negateCS) {
-                // Assert Chip Select if necessary - it's costly so only do so if necessary
-                IOLo(dev->busType_u.spi.csnPin);
-            }
-
-            spiInternalReadWriteBufPolled(
-                    bus->busType_u.spi.instance,
-                    bus->curSegment->u.buffers.txData,
-                    bus->curSegment->u.buffers.rxData,
-                    bus->curSegment->len);
-
-            if (bus->curSegment->negateCS) {
-                // Negate Chip Select
-                IOHi(dev->busType_u.spi.csnPin);
-            }
-
-            segmentComplete = true;
-            if (bus->curSegment->callback) {
-                switch(bus->curSegment->callback(dev->callbackArg)) {
-                case BUS_BUSY:
-                    // Repeat the last DMA segment
-                    segmentComplete = false;
-                    break;
-
-                case BUS_ABORT:
-                    bus->curSegment = (busSegment_t *)BUS_SPI_FREE;
-                    segmentComplete = false;
-                    return;
-
-                case BUS_READY:
-                default:
-                    // Advance to the next DMA segment
-                    break;
-                }
-            }
-            if (segmentComplete) {
-                lastSegment = (busSegment_t *)bus->curSegment;
-                bus->curSegment++;
-            }
-        }
-
-        // If a following transaction has been linked, start it
-        if (bus->curSegment->u.link.dev) {
-            busSegment_t *endSegment = (busSegment_t *)bus->curSegment;
-            const extDevice_t *nextDev = endSegment->u.link.dev;
-            busSegment_t *nextSegments = (busSegment_t *)endSegment->u.link.segments;
-            bus->curSegment = nextSegments;
-            endSegment->u.link.dev = NULL;
-            endSegment->u.link.segments = NULL;
-            spiSequenceStart(nextDev);
-        } else {
-            // The end of the segment list has been reached, so mark transactions as complete
-            bus->curSegment = (busSegment_t *)BUS_SPI_FREE;
-        }
+        spiProcessSegmentsPolled(dev);
     }
 }
 #endif

src/platform/common/stm32/bus_spi_hw.c

@@ -39,80 +39,6 @@
 extern spiDevice_t spiDevice[SPIDEV_COUNT];
 extern busDevice_t spiBusDevice[SPIDEV_COUNT];
 
-// Interrupt handler for SPI receive DMA completion
-FAST_IRQ_HANDLER static void spiIrqHandler(const extDevice_t *dev)
-{
-    busDevice_t *bus = dev->bus;
-    busSegment_t *nextSegment;
-
-    if (bus->curSegment->callback) {
-        switch(bus->curSegment->callback(dev->callbackArg)) {
-        case BUS_BUSY:
-            // Repeat the last DMA segment
-            bus->curSegment--;
-            // Reinitialise the cached init values as segment is not progressing
-            spiInternalInitStream(dev, true);
-            break;
-
-        case BUS_ABORT:
-            // Skip to the end of the segment list
-            nextSegment = (busSegment_t *)bus->curSegment + 1;
-            while (nextSegment->len != 0) {
-                bus->curSegment = nextSegment;
-                nextSegment = (busSegment_t *)bus->curSegment + 1;
-            }
-            break;
-
-        case BUS_READY:
-        default:
-            // Advance to the next DMA segment
-            break;
-        }
-    }
-
-    // Advance through the segment list
-    // OK to discard the volatile qualifier here
-    nextSegment = (busSegment_t *)bus->curSegment + 1;
-
-    if (nextSegment->len == 0) {
-        // If a following transaction has been linked, start it
-        if (nextSegment->u.link.dev) {
-            const extDevice_t *nextDev = nextSegment->u.link.dev;
-            busSegment_t *nextSegments = (busSegment_t *)nextSegment->u.link.segments;
-            // The end of the segment list has been reached
-            bus->curSegment = nextSegments;
-            nextSegment->u.link.dev = NULL;
-            nextSegment->u.link.segments = NULL;
-            spiSequenceStart(nextDev);
-        } else {
-            // The end of the segment list has been reached, so mark transactions as complete
-            bus->curSegment = (busSegment_t *)BUS_SPI_FREE;
-        }
-    } else {
-        // Do as much processing as possible before asserting CS to avoid violating minimum high time
-        bool negateCS = bus->curSegment->negateCS;
-
-        bus->curSegment = nextSegment;
-
-        // After the completion of the first segment setup the init structure for the subsequent segment
-        if (bus->initSegment) {
-            spiInternalInitStream(dev, false);
-            bus->initSegment = false;
-        }
-
-        if (negateCS) {
-            // Assert Chip Select - it's costly so only do so if necessary
-            IOLo(dev->busType_u.spi.csnPin);
-        }
-
-        // Launch the next transfer
-        spiInternalStartDMA(dev);
-
-        // Prepare the init structures ready for the next segment to reduce inter-segment time
-        spiInternalInitStream(dev, true);
-    }
-}
-
 // Interrupt handler for SPI receive DMA completion
 FAST_IRQ_HANDLER static void spiRxIrqHandler(dmaChannelDescriptor_t* descriptor)
 {