adafruit / ArduinoCore-samd

116 stars 119 forks source link

DMA Based SPI will stop working after first call (in some cases) #230

Open markcha opened 4 years ago

markcha commented 4 years ago

There is a feature in the Adafruit_ZeroDMA code that any channel registered without a callback will never have its status set back to DMA_STATUS_OK (Adafruit_ZeroDMA issue #17). This will cause startJob to fail with an error. In the following code

char szBufOut[10];
setBufferData(szBufIn);
SPI.transfer(NULL, szBufOut, 10);   // block TRUE or FALSE, makes no difference.

// szBufOut contains proper data SPI.transfer(NULL, szBufOut, 10); // block TRUE or FALSE, makes no difference. // szBufOut contains previous data, not new data

The output buffer is not filled in the second time because the DMA channel was never started.

Solution: Register a callback for the readChannel (it copies from SERCOM to output buffer). This has to be done carefully to prevent another possible bug: Currently the SPI transfer is considered done when the writeChannel (the one sending data to the SERCOM). However, this may only write it to a buffer in the SERCOM and the transfer will not be done (all data in the output buffer) until after the readChannel DMA is done. Assuming that we are using a form of transfer that has a readChannel.

BriscoeTech commented 4 years ago

Hello @markcha

Its funny I ran into the exact same problems and had to clean up the library. I had my oscilloscope and sure enough there is a glitch if you dont wait for both the read and write dmas to finish before calling the transfer complete

I will post a pr soon, and it also addresses #225, as well as another undocumented bug that initializes the source and destination buffer pointer only once, and wont update them again on subsequent dma calls.

BriscoeTech commented 4 years ago

Ok pull request posted :-)

BriscoeTech commented 3 years ago

@PaintYourDragon mentioned that he thinks this was fixed in 1.6.2. Can anyone confirm that this was fixed, and you can now do dma read only transactions now without the waveform and data getting cut off?

I will have to get my oscilloscope hooked up again to test it one of these days

DrItanium commented 2 years ago

@BriscoeTech 1.7.5 does not fix the issue. If I set a receive buffer at all on a grand central m4 then I get a hang. Transmit only works just fine. I have my scope hooked up to the icsp connector and I see nothing... it is just locked up. Here is a reduced test case that demonstrates the problem:

#include <SPI.h>
byte transmit[1024] = { 0 };
byte receive[1024] = { 0 };
void setup() {
  Serial.begin(9600);
  while (!Serial) {
    delay(10);
  }
  Serial.println("Console up!");
  // put your setup code here, to run once:
  SPI.begin();
  for (int i = 0; i < 1024; ++i ) {
    transmit[i] = static_cast<byte>(random());
    receive[i] = 0;
  }
}
void doDMATransfer(const char* message, byte* tx, byte* rx) {
  Serial.println(message);
  SPI.transfer(tx, rx, 1024, true);
  delay(1000);
}
void loop() {
  SPI.beginTransaction({12000000, MSBFIRST, SPI_MODE0});
  doDMATransfer("Write only operation (blocking)!", transmit, nullptr);
  doDMATransfer("Read only operation (blocking)!", nullptr, receive);
  doDMATransfer("Read/Write operation (blocking)!", transmit, receive);
  SPI.endTransaction();
}

In the sketch you will never get to the Read/Write operation (blocking) phase. Disabling the Read only operation will lock up when you get to read/write.