Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
May 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/main/drivers/at32/bus_spi_at32bsp.c
Original file line number Diff line number Diff line change
Expand Up @@ -402,8 +402,10 @@ void spiSequenceStart(const extDevice_t *dev)
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;
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/main/drivers/bus.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ typedef enum {
BUS_ABORT
} busStatus_e;

struct extDevice_s;

// Bus interface, independent of connected device
typedef struct busDevice_s {
Expand Down Expand Up @@ -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;

Expand Down
30 changes: 19 additions & 11 deletions src/main/drivers/bus_spi.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,18 @@ 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);
}

// Wait for DMA completion
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
Expand Down Expand Up @@ -425,12 +429,6 @@ FAST_IRQ_HANDLER static void spiIrqHandler(const extDevice_t *dev)
nextSegment = (busSegment_t *)bus->curSegment + 1;

if (nextSegment->len == 0) {
#if defined(USE_ATBSP_DRIVER)
if (!bus->curSegment->negateCS) {
// Negate Chip Select if not done so already
IOHi(dev->busType_u.spi.csnPin);
}
#endif
// If a following transaction has been linked, start it
if (nextSegment->u.link.dev) {
const extDevice_t *nextDev = nextSegment->u.link.dev;
Expand All @@ -442,6 +440,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 {
Expand Down Expand Up @@ -760,6 +763,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
Expand All @@ -781,11 +789,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 {
Expand Down
17 changes: 15 additions & 2 deletions src/main/drivers/max7456.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -540,7 +541,7 @@ bool max7456LayerCopy(displayPortLayer_e destLayer, displayPortLayer_e sourceLay

bool max7456DmaInProgress(void)
{
return spiIsBusy(dev);
return max7456ActiveDma;
}

bool max7456BuffersSynced(void)
Expand Down Expand Up @@ -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},
};

Expand Down Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions src/main/drivers/stm32/bus_spi_ll.c
Original file line number Diff line number Diff line change
Expand Up @@ -377,13 +377,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;
Expand Down