Buffered, interrupt-driven SPI comms failing

Go To Last Post
15 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello! I've been using the examples in application notes about FIFO buffers in USART for a time and they work fine.

I tried to use the same idea to communicate with avr slaves via SPI, and used the SIGNAL(SIG_SPI) to load the next avilable byte in buffer into SPDR.

SIGNAL(SIG_SPI)
{
	unsigned char tmptail;

	/* Check if all data is transmitted */
	if ( SPI_TxBuffer_Head != SPI_TxBuffer_Tail )
	{
		/* Calculate buffer index */
		tmptail = ( SPI_TxBuffer_Tail + 1 ) & SPI_TxBuffer_BUFFER_MASK;
		SPI_TxBuffer_Tail = tmptail;      /* Store new index */
		SPDR = SPI_TxBuffer[tmptail];  /* Start transmition */
	}
	else
	{
		SPI_preReleasing = 1;// All data was transmitted
	}
}

The code works fine in the simulator. Data loaded in the buffer is sent to the slave. The problem is that in the real AVR-AVR circuit, comms doesn't happen.

The master never finish sending data (8 byte-packages) to the slave. I know this because I use the global volatile variable "SPI_preReleasing" to track the comms inside the master's main loop.

I saw the code in the application note "SPI-USART Gateway" again, and it surprised me that polling of the SPIF is used to send data, instead of interrupts.

Does anyone has ever used with success the interrupt driven approach?
What am I doing wrong?
Does anyone know what is physically needed to fire the SPI interrupt (in the master)? I mean, is it needed that the slave really receives data or not?

Thanks.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
I saw the code in the application note "SPI-USART Gateway" again, and it surprised me that polling of the SPIF is used to send data, instead of interrupts.

Generally speaking, the RECEIVE process is critical, not SEND.
So, it is wise to use interrupt-driven Rx (avoids losing data...).
OTAH, sending is not mandatory to be interrupt-driven; efficient control may be obtained also by using the polling technique.

Real men don't use backups, they post their stuff on a public ftp server and let the rest of the world make copies.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You can use something like this (adapted from Pascal Stang's AVRlib):

// SPI Transmit Interrupt Handler
SIGNAL(SIG_SPI)
{
  // check if there's data left in the buffer
  if(spiTxBuffer.datalength)
  {
     // send byte from top of buffer
     SPDR = bufferGetFromFront(&spiTxBuffer));
  }
  else // no data left
     spiReadyTx = TRUE;
}

BTW: what is exactly SPI_TxBuffer_BUFFER_MASK ?

Real men don't use backups, they post their stuff on a public ftp server and let the rest of the world make copies.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

groenhen wrote:
BTW: what is exactly SPI_TxBuffer_BUFFER_MASK ?

It'll likely be a power of two mask ((2^n)-1). This is quite common for circular buffers. This makes the 'wrap-around' feature very elegant and efficient.

So for a 16 byte buffer (2^4) we would have a mask value of 0x0F. So when the pointer increments from 15->16, and then and it with the mask of 0x0F, we will end up back at zero. You can avoid the mask, if the boundary falls on a 8, 16, or 32 bit boundary as the roll-over will be natural, though for AVR's these will usually result in buffers that are much to large, with the possible exception of the 8 bit boundary.

Writing code is like having sex.... make one little mistake, and you're supporting it for life.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks for your answers.

Stanley:

Quote:

// SPI Transmit Interrupt Handler 
SIGNAL(SIG_SPI) 
{ 
  // check if there's data left in the buffer 
  if(spiTxBuffer.datalength) 
  { 
     // send byte from top of buffer 
     SPDR = bufferGetFromFront(&spiTxBuffer)); 
  } 
  else // no data left 
     spiReadyTx = TRUE; 
}


This code is almost identical to my code. The difference is that Pascal's library is generic and my code (my adaptation, to be honest) is a little more specific to the particular application, but just a tiny bit. Indeed, thanks for posting it. I forgot that library!!!! Can you point me to the page where I can download it please? I can't find my old copy.

As glitch says, SPI_TxBuffer_BUFFER_MASK is a power of two mask. It was a very cool trick I found on the app notes!

Quote:

efficient control may be obtained also by using the polling technique.

I hadn't realize this. You're right!

However, the problem still appears!!! :(

I check the logic many times until seems right to me. Then I run the simulator and test it with as much detail as I can. When all the errors and bugs are corrected, the simulator indicates the program runs correctly, but, when I load the program in an actual ATMega16, the transmission doesn't happen!!!

This is a sample of the display on my terminal (ignore the message that says "entering SPI ISR", is just a message to indicate the beginning of the transmission):

Quote:

Processing bytes: 1B:04:01:52:78:42:C4
CRC check: 6a91

Processing an SAVR instruction.

Received package:
Destination address = 0x4
Command sent = Trajectory
Data sent = -777.8800
Sending package to slave...
Entering SPI ISR...

If I send more packages to the slave, just the "processing bytes:xxxxx and CRC check: xxxx" messages appear. According to the simulator, the following should appear:

Quote:

01
02
03
04
05
06
07
08
Package transmitted, SPI_preReleasing = 01
Transmission complete!

The numbers are just an static counter for debugging purposes.

My guess is that the SPIF flag is never set, so the code that loads data into SPDR never executes. My code to process the package is this:

void processSlaveInstruction(void)
{
	unsigned char s[FLOAT_STR_WIDTH+1];
	instruction instr;
	static unsigned char counter=0;
	
	
	if(SPI_transmitting==0) {	// No data is being transmitted to the slaves
		SPI_transmitting = 1;
		send_str(SAVRprocessing);
		// Get instruction from buffer
		SAVRiBufferRetrieve(&instr);
		availableSlaveInstructions++;	// <-- Workaround!!!! Fix it!!!!!!
		send_str(packHeader);
		itoa(instr.dAddr, s, 16);	send_str(dAddr);	sendRAMstr(s);	send_str(crlf);
		send_str(command);
		switch(instr.command) {
			case COM_TRAJECTORY:
				send_str(comTrajectory);
				break;
			case COM_GO:
				send_str(comGo);
				break;
			default:
				itoa(instr.command, s, 16);
				send_str(comUnknown);
				sendRAMstr(s);
		}
		send_str(crlf);
		send_str(data);
		if(isinf(instr.data)) {
			send_str(msgInfinity);
		}
		else if(isnan(instr.data)) {
			send_str(msgNaN);
		}
		else {
			dtostrf(instr.data, FLOAT_STR_WIDTH, FLOAT_STR_PREC, s);
			sendRAMstr(s);
		}
		send_str(crlf);	
		
		send_str(spiSending);
		switch(instr.dAddr) {
			case SAVR_MIN_ADDR:
				PORTC &= ~_BV(SLAVE0);
				send_str(strSlave0);
				break;
			case SAVR_MIN_ADDR+1:
				PORTC &= ~_BV(SLAVE1);
				send_str(strSlave1);
				break;
			case SAVR_MIN_ADDR+2:
				PORTC &= ~_BV(SLAVE2);
				send_str(strSlave2);
				break;
			case SAVR_MIN_ADDR+3:
				PORTC &= ~_BV(SLAVE3);
				send_str(strSlave3);
				break;
			case SAVR_MIN_ADDR+4:
				PORTC &= ~_BV(SLAVE4);
				send_str(strSlave4);
				break;
			case SAVR_MIN_ADDR+5:
				PORTC &= ~_BV(SLAVE5);
				send_str(strSlave5);
				break;
		}
		SPI_preReleasing = 0;
		send_str(strSpiIsr);
		transmitPackage(&instr);
	}
	else if(SPI_transmitting == 1 && SPI_preReleasing==0) {	// transmission in process
		if(SPSR & _BV(SPIF)) {	// Check if previous transmission has finalized.
			counter++;
			xmitByte(counter);
			send_str(crlf);
			/* Check if all data is transmitted */
			if (dataInSPITransmissionBuffer()) {
				SPDR = SPI_TxBufferRetrieve();
			}
			else {
				SPI_preReleasing = 1;// All data was transmitted
				send_str(strSpiExit);
				xmitByte(SPI_preReleasing);
				send_str(crlf);
			}
		}
	}
	else if(SPI_preReleasing) {		// Transmision is complete
		PORTC = 0xFF;	// Deselect slave
		SPI_transmitting = 0;
		availableSlaveInstructions--;
		send_str(spiTerminate);
	}
	// else return;
}


void transmitPackage(instruction *instr)
{
	uint8_t *data, i;
	
	data = (uint8_t*)instr;

	SPI_TxBufferStore((uint8_t)SOT);	// Start of transmision
	for(i=1; i

Although in the simulator the line

SPSR |= _BV(SPIF);

works as intented (I set the flag so the code in processSlaveInstruction that loads the next byte into SPDR works). Do you know if it really works in the hardware?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Real men don't use backups, they post their stuff on a public ftp server and let the rest of the world make copies.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
SPSR |= _BV(SPIF);

Quote:
Do you know if it really works in the hardware?

No, SPIF is read-only; it can't even be cleared (at least not directly) by the CPU.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
   else if(SPI_transmitting == 1 && SPI_preReleasing==0) {   // transmission in process
      if(SPSR & _BV(SPIF)) {   // Check if previous transmission has finalized.

I'm having a bit of trouble figuring out where this function fits in, but if you have a SIG_SPI
running on this side it will consume the SPIF, so this function will never (OK, with very low
probability) see SPIF set.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I suppose _BV(...) it's a sort of bit value...?
[not very common / standard]

Real men don't use backups, they post their stuff on a public ftp server and let the rest of the world make copies.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Afaik _BV() is defined in the AVR Toolset headers

I think its #define _BV(x) (1<<x)

/Bingo

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
I think its #define _BV(x) (1<<x)

What's the big deal replacing something very clear / common / standard (i.e. 1<<x),
with something rather strange... underscore BV, that also implies 6 keystrokes, but it is so ugly...?

Real men don't use backups, they post their stuff on a public ftp server and let the rest of the world make copies.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Notice the differences between the SPIF-interrupt flag and the UDRE-bit in the UART Status Register:

1. The UDRE-bit is initialized to '1' @ reset - the SPIF-interrupt flag to '0'.

2. The SPIF-interrupt flag is cleared when the corresponding interrupt is executed - the UDRE-bit is not.

(at least this was the case on the original 8515 which I've played with)

These differences makes interrupt driven SPI transmit routines hard to write - at least for me (it ended up in an rather nasty, partly polled, state machine).

/
Magnus

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
2. The SPIF-interrupt flag is cleared when the corresponding interrupt is executed - the UDRE-bit is not.

I think UDRE is cleared by writing UDR.

Real men don't use backups, they post their stuff on a public ftp server and let the rest of the world make copies.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

At last!!! Solved!!!!

Quote:

I'm having a bit of trouble figuring out where this function fits in, but if you have a SIG_SPI
running on this side it will consume the SPIF, so this function will never (OK, with very low
probability) see SPIF set.

OK. What happens here is that, after Stanley's suggestion, I tried a polled mode and elliminated SIG_SPI. The function fits in the big picture when you see the main loop:

for(;;) {
		packStream();	
		if(availablePackages) {
			pBufferRetrieve(&instr);
			parsePackage(&instr);	
		}
		if(availableLocalInstructions) {
			processLocalInstruction();	// Parse and execute 1 local instruction, stored in MAVRiBuffer
		}
		if(availableSlaveInstructions) {
			processSlaveInstruction();	/* Parse and send 1 instruction, stored in SAVRiBuffer, to the corresponding slave. Stores the package stream
			in SPI_TxBuffer */
		}
	}


That's why I only use an 'if' sentence, instead of the common "while(flag_some_state);' construction.

Quote:

I suppose _BV(...) it's a sort of bit value...?

As Bingo said, is defined in the avr-libc. I wrote a lot of asm code in my beginings with AVR and got accustomed with the (1<<BitXX) construction, so when I saw this '_BV' stuff for the first time, I though it was unnecesary. However, in the FAQ of the avr-libc documentation the following recomendation appears:
Quote:

"[...] So in order to access a particular bit number as a byte value, use the _BV() macro. Of course, the implementation of this macro is just the usual bit shift[...] However, using the macro often makes the program better readable[...]"

I've seen the use of this macro in lots of code, so I considered it was better to stick to the "standard" used in the avr-gcc toolset and started to use the _BV macro.

Quote:

What's the big deal replacing something very clear / common / standard (i.e. 1<<x),
with something rather strange... underscore BV, that also implies 6 keystrokes, but it is so ugly...?

Once you get used to it, it doesn't seem that ugly ;)

Quote:

These differences makes interrupt driven SPI transmit routines hard to write - at least for me (it ended up in an rather nasty, partly polled, state machine).

I think that the "problem" with SPI is that communication is not asynchronous. In fact, is not that difficult to write the routines...(yeah sure, now that I know how...;)) but only when you write the code for the master. In the slave, it ends being a real mess!

Now to the solution:

Quote:

No, SPIF is read-only; it can't even be cleared (at least not directly) by the CPU.

That was the real problem. Since I wasn't "firing" the SPI comms, the master never transmitted data to the slaves. I realized that there's no need at all to modify directly the SPIF and simply changed the line:

// Buffer is filled with data. Activate transmission by setting transmision complete flag 
   SPSR |= _BV(SPIF);


to:

SPDR = SPI_RxBufferRetrieve();


and everything worked fine!!. The SPI system does all the needed flag modifications. I suppose that in case I need to be sure that the SPIF is cleared (though all the reasons I can think of for doing this are solved by a correct program flow design) the sequence to be used is the one mentioned in the datasheet:

Quote:

Alternatively, the SPIF bit is cleared by first reading the SPI Status register with SPIF set, then accessing the SPI Data Register (SPDR)

Thanks all of you for your posts!!

OT: How do you modify the "[quote ][/quote ]" pair when posting in the forums so that you can read "xxxx wrote:" or anything like that, instead of the ugly "Quote:"?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

Notice the differences between the SPIF-interrupt flag and the UDRE-bit in the UART Status Register:

1. The UDRE-bit is initialized to '1' @ reset - the SPIF-interrupt flag to '0'.

2. The SPIF-interrupt flag is cleared when the corresponding interrupt is executed - the UDRE-bit is not.

That's because of their different meanings:
1) UDRE -> USART Data Register Empty
2) SPIF -> serial transfer complete (I know, a more proper name would be SPTC or something like that, but blame Atmel for this...)

Now one can understand it's value under different situations.
1) UDRE is set when the data register is empty. When you write something to it, this flag is cleared. Otherwise, is set. At reset, there's nothing in the UDR, so it's reasonable that its initial value is 'set' (1).
2) SPIF is set when SPI hardware ends transmitting data between master and slave, so if nothing was transmitted this flag is 'cleared'. Note that SPIF is also set when /SS pin is an input and is driven low when SPI is in Master mode. This is a somehow odd behavior, but nothing can be done about it.