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

SPI DMA write-only transfer(txBuf, nullptr, count) corrupts following receives. #335

Open
greiman opened this issue Aug 15, 2022 · 0 comments

Comments

@greiman
Copy link

greiman commented Aug 15, 2022

Write-only calls to void transfer(const void *txbuf, void *rxbuf, size_t count, bool block) in this file:

https://github.com/adafruit/ArduinoCore-samd/blob/master/libraries/SPI/SPI.cpp

with rxbuf equal to nullptr cause following reads to fail.

The problem is that SPI receive is enabled in SERCOM but no DMA receive job is enabled so the receive FIFO is not emptied and may even overflow.

A following receive using any transfer function retrieves leftover bytes in the FIFO.

I tested a possible fix by enabling a DMA receive job with a static dummy byte.

It solves the problem in my SdFat library for Feather M0 SAMD21 and Feather M4 Express. The fix would need further tests.

I have attached a copy of the modified SPI.cpp.

Here is a program that demonstrates the problem. Connect MISO to MOSI and run the program to see the problem.
When nullptr is used, rx does not match tx data for rx = transfer(tx).

#include "SPI.h"
// Connect MISI to MOSI.
void spiTest(bool useNull) {
  uint8_t txBuf[5] = {0, 1, 2, 3, 4};
  uint8_t rxBuf[5];
  Serial.println();
  Serial.println(useNull ? "use nullptr" : "use rxBuf");
  SPI.begin();
  SPI.beginTransaction(SPISettings(1000000, MSBFIRST, SPI_MODE0));

  if (useNull) {
    SPI.transfer(txBuf, nullptr, sizeof(txBuf));
  } else {
    SPI.transfer(txBuf, rxBuf, sizeof(txBuf));
  }
  for (uint8_t tx = 5; tx < 20; tx++) {
    // rx should equal tx here.
    uint8_t rx = SPI.transfer(tx);
    Serial.print("tx: ");
    Serial.print(tx);
    Serial.print(", rx: ");
    Serial.println(rx);
  }
  SPI.endTransaction();
  SPI.end();
}

void setup() {
  Serial.begin(115200);
  while (!Serial) {}
  Serial.println("Connect MISO to MOSI then type any character.");
  while (!Serial.available()) {}
  spiTest(false);
  spiTest(true);
}
void loop() {}

Here is a diff of the original and modified SPI.cpp file:

381a382,385
> #define SPI_DMA_WRITE_ONLY_FIX
> #ifdef SPI_DMA_WRITE_ONLY_FIX
>     static uint8_t rxDum;  // Dummy byte for write-only xfers
> #endif  // SPI_DMA_WRITE_ONLY_FIX
398a403
> #ifndef SPI_DMA_WRITE_ONLY_FIX
399a405,411
> #else    // SPI_DMA_WRITE_ONLY_FIX
>         rDesc->BTCTRL.bit.DSTINC = 1;
>       } else {
>         rDesc->DSTADDR.reg = (uint32_t)&rxDum;
>         rDesc->BTCTRL.bit.DSTINC = 0; // Don't increment  pointer
>       }
> #endif  // SPI_DMA_WRITE_ONLY_FIX
425a438
> #ifndef SPI_DMA_WRITE_ONLY_FIX
432a446,449
> #else   // SPI_DMA_WRITE_ONLY_FIX
>     writeChannel.setCallback(dmaDoNothingCallback);
>     readChannel.startJob();
> #endif  // SPI_DMA_WRITE_ONLY_FIX

Modified_SPI_cpp.zip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant