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

Channel without callback never completes #17

Open
markcha opened this issue Jun 2, 2020 · 1 comment
Open

Channel without callback never completes #17

markcha opened this issue Jun 2, 2020 · 1 comment

Comments

@markcha
Copy link

markcha commented Jun 2, 2020

If a DMA channel is created without calling setCallback, startJob will end up configuring the channel in such a way that it can not be used properly. The problem is that jobStatus is only ever reset to DMA_STATUS_OK in Adafruit_ZeroDMA::_IRQHandler. This means that _IRQHandler must be called for the DMA channel for the channel to ever have Adafruit_ZeroDMA recognize that the transfer is done. The _IRQHandler is only called for DMAC_CHINTENCLR_TCMPL if the proper bit is set in the CHINTENSET field in startJob. And that is only done if the user has registered a callback.

Now, why would anyone ever want to use DMA without knowing when it is done? Answer: SPI. The standard way to use DMA with SPI is to use 2 DMA channels: one to stuff data (or dummy bytes) into the SERCOM, and a second to copy data out of the SERCOM to a memory buffer. Some implementations will only register a callback for the first channel.

This is done in the Adafruit SAMD Core implementation of SPI. Using 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


After putting in a lot of thought, I think this is mainly a bug in the SPI implementation. startJob returns DMA_STATUS_BUSY for the second invocation on the second channel (copy data out), though everyone tends to ignore it (our problem).

However:

  1. It would take a lot of thinking and debugging to figure out that DMA_STATUS_BUSY was caused by a lack of a callback. I still think this should be fixed or documented.
  2. [Related] Getting the error DMA_STATUS_ERROR_IO will only happen if an error callback is registered.
  3. [Related] It appears that even if jobStatus was set to DMA_STATUS_ERROR_IO, the only way to tell is calling printStatus.

This will also affect people trying to use Adafruit_ZeroDMA::isActive() to poll instead of using a callback.

@AndrewSmiles
Copy link

Thanks for the nice description of events @markcha I was having this issue too. I agree that this should be fixed/documented.
For anyone who stumbles across this in the (near?) future, a very dirty but easy fix is just setting the value of jobStatus to OK before it is tested in startJob() seen here:

ZeroDMAstatus Adafruit_ZeroDMA::startJob(void) {'
  ZeroDMAstatus status = DMA_STATUS_OK;

  cpu_irq_enter_critical(); // Job status is volatile
  jobStatus = DMA_STATUS_OK;   //this is the line to insert
  if (jobStatus == DMA_STATUS_BUSY) {

Obviously this isn't a real fix as it eliminates any useful feedback of the jobStatus, but odds are if you're setting up as fast as possible SPI, you might not be using that functionality anyway. Also, goes without saying that the while (!transfer_is_done) ; method of controlling CS/SS pins as seen in the examples will not work here, I have been hardcoding in delays, however more elegant solutions exist.

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

2 participants