teensy 3.0 SPI with DMA -- nice try

Status
Not open for further replies.
I already tried lots of things:

Code: 
  while (SPI0_TCR!=tcrEnd); // wait for last increment of TCR

  SPI0_SR = SPI_SR_TCF; // reset TCF
	  
  // clear DMA int
  DMA_CINT = DMA_CINT_CINT(DMASPI0_RXCHAN);
  DMA_CERQ = DMA_CERQ_CERQ(DMASPI0_TXCHAN);
	  
  // reset SPI
  SPI0_SR = 0xFF0F0000;
  SPI0_MCR |= SPI_MCR_CLR_RXF | SPI_MCR_CLR_TXF;
  SPI0_TCR = 0;

Nothing really helped... Maybe it's a matter of sequencing those blocks correctly, but I admit that I'm lost...
 
A couple of thoughts:

The pause() method just tells the DMA SPI to stop after it has finished the current transfer, i.e. not start a new one after that. Strictly speaking, your code
Code: 
DMASPI0.registerTransfer(trx); // transfer 3 bytes through DmaSpi
while (trx.busy()); // wait until complete

DMASPI0.pause(); // switch to polled mode
is not using it correctly. You should not poll the busy flag of your own transfer, but the paused() method instead:
Code: 
DMASPI0.registerTransfer(trx);

... optional: wait until the transfer is busy to make sure it is really started before we call pause()

DMASPI0.pause();
// only when the DMA SPI driver finished a running transfer it can release the SPI:
while(!DMASPI0.paused());
DMASPI0.releaseSpi();

... do non-DMA SPI stuff

DMASPI0.resume();
It is important to call releaseSpi() because that method disables the SPI's DMA channel requests. This is a very important step in switching between "normal" and DMA mode. If you don't call releaseSpi(), the next SPI transfer you do manually will cause a DMA request to be generated, and my guess is that the DMA channel/controller will write to whatever adress is past the receive buffer of the last transfer object as soon as DMA operation is resumed. That means corrupted memory.

So - did you try it with paused() and releaseSPI()? If not, try adding them. If tried to find a way of dropping releaseSpi(), but that just doesn't fit into the "disable, then poll until disabled" scheme of pause() and paused().

And finally, a suggestion: The registerTransfer() method could accept transfers that are already chained together. That way, you can prepare a chain of transfers before registering them, and make sure that they are handled sequentially (an ISR of another part of your application could insert a foreign transfer between your command and data block transfers! Hard to debug for sure.). Not deselecting the device could then be the task of your chip select class. That's a better separation of responsibilities imho.

Regards

Christoph
 
Last edited:
Christoph,

I agree. You're right, I really forgot the releaseSPI().

However - unfortunately it doesn't make any difference. Even after a SPI0_RSER=0, the DMA interrupt is triggered one byte before the end of the transfer. I had tried this (inserting SPI0_RSER=0 into the code) before as well.

So, here's my new code:

Code: 
  while (1) {
    uint8_t data[]={0xf0, 0xa5, 0x03, 0xcc};
 
    DmaSpi0::Transfer trx(data,3,nullptr,0,&ethCs);
    DMASPI0.registerTransfer(trx);
    while (trx.busy());
    
    DMASPI0.pause();
    while(!DMASPI0.paused());
    DMASPI0.releaseSpi();    

    SPDR=0x0;
    while(!(SPSR&(1<<SPIF)));    
    SPDR=0x0;
    while(!(SPSR&(1<<SPIF)));    

    DMASPI0.resume();

    DMASPI0.registerTransfer(trx);
    while (trx.busy());
}

The result is the same: First transfer OK, after switching to poll mode CS raises early.

Schöne Grüße nach Hamburg aus Brasilien.

Jörg

DmaSpi.png
 
Hm....that will take some time to figure out. When I tried the current DMA SPI implementation with SDFatlib, it worked well. If it had screwed up my DMA transfers, I would see garbage on my display.

Can you elaborate a bit more on your scope picture? I'm having difficulties figuring out what's (not) happening when.
 
Christoph,

I have decorated the image a bit. Please pay attention to the timing relationship between the end of the yellow clock cycles and the rising edge on the CS line.

As CS is handled entirely in software, I omitted changing the line during the direct SPI register access.

The first DmaSpi transfer (3 bytes) works fine and CS is raised about 1 us after the last clock cycle. Then the software switches to direct SPI access and outputs two bytes directly to the emulated SPDR (which comes down to SPI0_PUSHR) and waits for each byte to be shifted out completely. There is no CS handling for that.

Then we have the second DmaSpi transfer, which again outputs three bytes. However, CS raises roughly 1us BEFORE the end of the transfer. Also note that the gap between the DmaSpi transfer and the direct transfer decreases, as the ISR is called earlier and marks the SPI object as idle.

The while loop ends and the three transfers repeat, however, CS always raises early.

Just out of curiosity, I have tested the code on a Teensy 3.1 and the result is the same.

I have attached my complete test sketch for your reference here: View attachment DmaSpiTest.ino.

Please let me know if you discover possible reasons and/or solutions.

Jorg

DmaSpi2.png
 
Unfortunately, I don't have a scope, so I'll have to use yours for this:

What happens if you don't use the emulated AVR SPI between DMA transfers, but still pause(), release() and resume()? As in:
Code: 
while (1)
{
    uint8_t data[]={0xf0, 0xa5, 0x03, 0xcc};
 
    DmaSpi0::Transfer trx(data,3,nullptr,0,&ethCs);
    DMASPI0.registerTransfer(trx);
    while (trx.busy());
    
    DMASPI0.pause();
    while(!DMASPI0.paused());
    DMASPI0.releaseSpi();    

//    SPDR=0x0;
//    while(!(SPSR&(1<<SPIF)));    
//    SPDR=0x0;
//    while(!(SPSR&(1<<SPIF)));    

    DMASPI0.resume();

    DMASPI0.registerTransfer(trx);
    while (trx.busy());
}
Do you still get an early deassertion of the CS line?

Regards

Christoph
 
Christoph,

without writing to SPDR, everything works fine:

DmaSpi3.png

You should be able to reproduce the behavior with any SPI device that relies on a valid CS until the end of the transfer. I ran into this problem with an ENC28J60 (Ethernet chip) that stopped responding after I switched to direct SPI mode.

So, do I really need direct SPI mode? Yes, I do. The Ethernet stack is calculating a checksum of the ENC28J60's internal buffer memory and retrieves one byte at a time through SPI from the buffer to calculate the checksum. The alternative would be to allocate memory within the MK20DX and copy the whole ENC28J60 buffer to the MK20DX's memory, only to sum up the bytes. Waste of valuable memory space, I think.

I have tried some other things, like exchanging

Code: 
    SPDR=0x0;
    while(!(SPSR&(1<<SPIF)));

for

Code: 
    SPI0_MCR |= SPI_MCR_CLR_RXF | SPI_MCR_CLR_TXF;
    SPI0_SR=SPI_SR_EOQF;  // use EOQF instead of TCF
    SPI0_PUSHR=SPI_PUSHR_EOQ;
    while (!(SPI0_SR&SPI_SR_EOQF));

I thought I had found something when I noticed that the processor reference manual specifies "Do not write to the RSER while the DSPI is in the Running state.". So I framed releaseSpi() and resume() like this (maybe, you should include this in your code anyway):

Code: 
    uint32_t mcr = SPI0_MCR;    
    SPI0_MCR = mcr | SPI_MCR_MDIS | SPI_MCR_HALT;
    DMASPI0.releaseSpi();    
    SPI0_MCR = mcr;

...

    mcr = SPI0_MCR;    
    SPI0_MCR = mcr | SPI_MCR_MDIS | SPI_MCR_HALT;
    DMASPI0.resume();
    SPI0_MCR = mcr;

However, no change. I also checked DMA_HRS for pending transfers before and after acessing SPDR, it always showed 0. Same thing with DMA_ERR. I tried to assert DMA_CERQ on both DMASPI0_RXCHAN and DMASPI0_TXCHAN. Same result.

Disclaimer: I'm reporting all of these checks are on a trial-and-error basis and it might be a good idea to repeat them.

So, I'm still stuck.
 
Found it!

Things started working after I added another DMASPI0.begin() into the loop. I then stripped it down line by line and found that the magic command that made everything working was

Code: 
SPI0_MCR = SPI_MCR_MSTR | SPI_MCR_DCONF(0) | SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF;

Taking a closer look, it was resetting the SPI_MCR_CLR_RXF bit. that made the difference. And it explains why you couldn't reproduce the problem: As I wrote to SPI0_PUSHR, but didn't read the result from SPI0_POPR, the byte read during the cycle remained in the FIFO and generated an additional SPI DMA request as soon as a DmaSpi transfer started. Reading SPI0_POPR after the transfer also eliminates the problem:

Code: 
    DmaSpi0::Transfer trx(data,3,nullptr,0,&cs);
    DMASPI0.registerTransfer(trx);
    while (trx.busy());
    
    DMASPI0.pause();
    while(!DMASPI0.paused());
 
    uint32_t mcr = SPI0_MCR;    
    SPI0_MCR = mcr | SPI_MCR_MDIS | SPI_MCR_HALT;
    DMASPI0.releaseSpi();    
    SPI0_MCR = mcr;


    SPDR=0xF;
    while(!(SPSR&(1<<SPIF)));    
    Serial.print(SPDR,HEX);

    SPDR=0xFA;
    while(!(SPSR&(1<<SPIF)));    
    Serial.print(SPDR,HEX);

    mcr = SPI0_MCR;    
    SPI0_MCR = mcr | SPI_MCR_MDIS | SPI_MCR_HALT;
    DMASPI0.resume();
    SPI0_MCR = mcr;

    DMASPI0.registerTransfer(trx);
    while (trx.busy());

DmaSpi4.png

Without reading SPI_POPR, we must make sure to clear the receive FIFO before starting a new DMA transfer:

Code: 
    uint8_t data[]={0xf0, 0xa5, 0x03};

    DmaSpi0::Transfer trx(data,3,nullptr,0,&cs);
    DMASPI0.registerTransfer(trx);
    while (trx.busy());
    
    DMASPI0.pause();
    while(!DMASPI0.paused());
 
    uint32_t mcr = SPI0_MCR;    
    SPI0_MCR = mcr | SPI_MCR_MDIS | SPI_MCR_HALT;
    DMASPI0.releaseSpi();    
    SPI0_MCR = mcr;


    SPDR=0xF;
    while(!(SPSR&(1<<SPIF)));    

    SPDR=0xFA;
    while(!(SPSR&(1<<SPIF)));    

    mcr = SPI0_MCR;    
    SPI0_MCR = mcr | SPI_MCR_MDIS | SPI_MCR_HALT | SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF;
    DMASPI0.resume();
    SPI0_MCR = mcr;

    DMASPI0.registerTransfer(trx);
    while (trx.busy());

DmaSpi5.png

With these changes, DmaSpi works like a charm. Can you update your code on GitHub, please?

Regards, Jorg
 
I'm glad you solved it, that's great! And thanks for sharing the fix. A few questions, though.

First: pausing / releasing
Your code is:
Code: 
uint32_t mcr = SPI0_MCR;    
SPI0_MCR = mcr | SPI_MCR_MDIS | SPI_MCR_HALT;
DMASPI0.releaseSpi();    
SPI0_MCR = mcr;
This will create a backup of the current SPI MCR value, release the SPI (which currently on affects SPI_RSER by setting it to zero), and then disables the SPI by setting MDIS and HALT. I'd add this into releaseSpi():
Code: 
static void releaseSpi()
{
  uint32_t mcr = SPI0_MCR;
  SPI0_MCR = mcr | SPI_MCR_MDIS | SPI_MCR_HALT;
  SPI0_RSER = 0;
  SPI0_MCR = mcr;
}
MDIS disables the module - do we actually want that? Wouldn't it be enough to just set HALT? Would the HALT flag have an effect at all? It's cleared almost immediately afterwards anyway.

Second: resuming
Your code is:
Code: 
mcr = SPI0_MCR;    
SPI0_MCR = mcr | SPI_MCR_MDIS | SPI_MCR_HALT | SPI_MCR_CLR_TXF | SPI_MCR_CLR_RXF;
DMASPI0.resume();
SPI0_MCR = mcr;
resume() calls beginNextTransfer() internally, while the SPI is halted (or not, actually we don't know, because the HALT flag might not have an effect yet). Then the MDIS and HALT flags are cleared and the SPI starts working again. However, and that is a possible problem, we don't know the state of SPI0_MCR when the backup is created in the first line, and the "foreign" driver that took control of the SPI before might have disabled the SPI as well, so the last line
Code: 
SPI0_MCR = mcr;
might leave the SPI disabled. There should be a backup of SPI0_MCR in the DMA SPI driver, and that backup should be created in releaseSpi(). I have stubs for those methods in the driver already, but I wasn't sure what they should do. Now I have more of an idea...

So the bottom line is: Shouldn't it be enough to just clear the TX and RX counters before resuming()?

Regards

Christoph
 
Would the HALT flag have an effect at all?
Click to expand...

Don't know. That's just the code I found everywhere to stop the SPI, so I just copied it.

Shouldn't it be enough to just clear the TX and RX counters before resuming()?
Click to expand...

Yes, I agree that restoring a backup that was made in releaseSpi should be sufficient. Maybe you can also make backup of the other registers, like CTAR and the DMA stuff and restore it in resume.

I tried to implement the linked transfer mode you suggested, but things became a bit complicated when I had to recode beginNextTransfer and handle two distinct types of links. Maybe you can work on that and include it as well. For now, I'm using the simple m_bDontDeselect flag I suggested to link two transfers without deasserting CS, which works fine.

Thanks again for your library, which really makes difference. I know that Paul is working on redesigning the SPI library and maybe he can consider your work.
 
Last edited:
Rereading the set of our last posts, just remember to set SPI_MCR_HALT and/or SPI_MCR_MDIS in resume before writing to SPI0_RSER and clearing them afterwards. I believe that that really MADE a difference. Of course you can combine that with a backup created in releaseSpi.
 
Sounds good. I'll update my code on github as soon as I can. I think it should really be sufficient to just set the HALT flag, i.e. not set the MDIS bit.

Can you try that as a last test?
 
Non-blocking writing via SDFat would certainly be a huge feature. Too bad though that the Teensy 3 only has one SPI hardware bus - that limits the usefulness of the non-blocking writes if there is more than one SPI device on the bus... or did you guys figure out a way to have the SDFat writes happen during lulls in the SPI bus?
 
Constantin said:
... or did you guys figure out a way to have the SDFat writes happen during lulls in the SPI bus?
Click to expand...

I will have to work exactly on this issue for my data logger project. For the first version, I thought about implementing non-blocking writes only for files created with SdBaseFile::createContiguous, as this is less subject to jitter and only involves simple SPI data block transfers.

I'll let you know about the results.
 
As it is, the SdFat library interface is designed for blocking reads and writes, and anything asynchronous/non-blocking would need a redesign from the ground up. Starting with createContiguous is probably a good idea. Personally, I'd need read access to files - but that's low priority.
 
@jbliesener any progress? You're not complaining so I assume your code still works - I just don't know if that's with or without the MDIS flag...
 
Christoph,

sorry, the world cup here in Brazil is keeping me busy - apart from organizing the house for rent, I, as a bi-national still "have to" watch the games of Brazil as well as Germany. :eek:

Didn't have a chance to check it. Won't be able to do it until next weekend. I'll let you know. For now the code works setting both flags (MDIS and HALT).
 
Hi Jorg,

any updates? I'm writing an SPI backup class that stores/restores SPI settings for different drivers (adafruit CC3000, SDFat, DMA SPI in my case) and I'd appreciate your feedback. To me it seems that the MDIS bit is not necessary.

Regards

Christoph
 
christoph said:
any updates? ... To me it seems that the MDIS bit is not necessary.
Click to expand...

Christoph, sorry for the late reply. Soccer worldcup is over, so I'm back at work ;)

You're right. No need for the MDIS bit, HALT alone gets it done.

Will you update the DmaSpi class in github?

There is another discussion about oscilloscopes on this forum, you might want to take a look: http://forum.pjrc.com/threads/25730-Oscilloscope-advice. Actually, I was able to debug this problem with a simple DSO-2090. You can find it for US$130. Might be worth a look...

Regards

Jorg
 
Last edited:
Unfortunately, I'm on vacation until next thursday. I'll add this to the dma spi and also look at dynamic channel allocation when I'm back.

And you're right, getting a simple oscilloscope might be a good idea...

Regards

Christoph
 
jbliesener said:
Rereading the set of our last posts, just remember to set SPI_MCR_HALT and/or SPI_MCR_MDIS in resume before writing to SPI0_RSER and clearing them afterwards. I believe that that really MADE a difference. Of course you can combine that with a backup created in releaseSpi.
Click to expand...

christoph said:
Sounds good. I'll update my code on github as soon as I can. I think it should really be sufficient to just set the HALT flag, i.e. not set the MDIS bit.

Can you try that as a last test?
Click to expand...

Hi christoph and jbliesener,

I see you have some fixes and improvement for the DMA SPI library, do you mind committing them to github?

Thanks for the work! Appreciate it!
 
Status
Not open for further replies.
Back
Top