SPI Master & Slave on MEGA328PB Write Collisions

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

Having spent about 8 hours over the last few days, I am beyond my wits end with this...

 

I am using a MEGA328PB Xplained Mini and attempting to exchange data between SPI0 (MASTER) and SPI1 (SLAVE) using interrupts.  

 

SPI0 sends the string "MASTER" to SPI1, while SPI1 sends the string "SLAVE " to SPI0.  For reasons I have been unable to determine, SPI1 generates a write collision (WCOL) when its ISR writes the "L" in "SLAVE " to SPDR1.  I inserted code in the SPI1 ISR to set an output pin to the value of the WCOL flag in the SPSR.  This is the bottom trace on the logic analyzer.

 

I am completely lost...  Any suggestions would be appreciated.

 

 

Code:


#include <string.h>
#include <avr/cpufunc.h>
#include <avr/interrupt.h>
#include <avr/io.h>
#include <avr/pgmspace.h>
#include <util/delay.h>
#include "port.h"

// state flags
#define SPI_BUSY_bm		(1 << 0)
#define SPI_MASTER_bm		(1 << 1)
#define SPI_ONLINE_bm		(1 << 7)

// i/o pin bitmaps
#define SCK0_bm			PIN5_bm	/* PORT_B */
#define MISO0_bm		PIN4_bm	/* PORT_B */
#define MOSI0_bm		PIN3_bm	/* PORT_B */
#define SS0_bm			PIN2_bm	/* PORT_B */
#define SCK1_bm			PIN1_bm	/* PORT_C */
#define MISO1_bm		PIN0_bm	/* PORT_C */
#define MOSI1_bm		PIN3_bm	/* PORT_E */
#define SS1_bm			PIN2_bm	/* PORT_E */
#define WCOL_bm			PIN2_bm	/* PORT_C */

// miscel
#define SPIF_bm			(1 << SPIF)
#define SPI_SPR_DIV_128_gc	3
#define SPI_BUFFER_SIZE		16
#define SS_DELAY_us		50

typedef struct spi_status_struct
{
	char* rx_buf;			//!< buffer to transfer to/from
	char* tx_buf;			//!< buffer to transfer to/from
	uint8_t count;		//!< number of bytes left to transfer
	uint8_t flags;		//!< state flags
} spi_status_t;

volatile spi_status_t spi0_status;

volatile spi_status_t spi1_status;

int main(void)
{

	char spi0_rx_buffer[SPI_BUFFER_SIZE];
	char spi0_tx_buffer[SPI_BUFFER_SIZE];
	char spi1_rx_buffer[SPI_BUFFER_SIZE];
	char spi1_tx_buffer[SPI_BUFFER_SIZE];

	// init master SPI
	PORT_B.PORT = MISO0_bm | SS0_bm;
	PORT_B.DDR = SCK0_bm | MOSI0_bm | SS0_bm;
	SPCR0 = (1 << SPE) | (1 << MSTR);

	// init slave SPI
	PORT_E.PORT = SS1_bm;
	PORT_C.DDR = MISO1_bm | WCOL_bm;
	SPCR1 = (1 << SPE);

	_delay_ms(5);

    do
    {
		cli();

		// clear Rx buffers
		memset(spi0_rx_buffer, 0, SPI_BUFFER_SIZE);
		memset(spi1_rx_buffer, 0, SPI_BUFFER_SIZE);

		// load Tx buffers
		strcpy_P(spi0_tx_buffer, PSTR("MASTER"));
		strcpy_P(spi1_tx_buffer, PSTR("SLAVE "));

		// init spi0 status
		spi0_status.rx_buf = spi0_rx_buffer;
		spi0_status.tx_buf = spi0_tx_buffer;
		spi0_status.count = strlen(spi0_tx_buffer);
		spi0_status.flags = SPI_ONLINE_bm | SPI_MASTER_bm;
		SPCR0 = (1 << SPE) | (1 << MSTR) | (1 << SPIE) | SPI_SPR_DIV_128_gc;

		// init spi1 status
		spi1_status.rx_buf = spi1_rx_buffer;
		spi1_status.tx_buf = spi1_tx_buffer;
		spi1_status.count = strlen(spi1_tx_buffer);
		spi1_status.flags = SPI_ONLINE_bm;
		SPCR1 = (1 << SPE) | (1 << SPIE) | SPI_SPR_DIV_128_gc;

		// clear WCOL trigger output
		PORT_C.PORT &= ~WCOL_bm;

		sei();

		// output byte to be sent after slave is selected
		SPDR1 = *spi1_status.tx_buf++;

		// select and pause
		PORT_B.PORT &= ~SS0_bm;
		_delay_us(SS_DELAY_us);

		// output first byte from master
		SPDR0 = *spi0_status.tx_buf++;

		while (spi0_status.flags & SPI_BUSY_bm || \
				spi1_status.flags & SPI_BUSY_bm);

		_delay_ms(25);

    } while (1);

	return 0;
}

ISR(SPI0_STC_vect)
{
	if (spi0_status.count)
	{
		// transfer next byte
		*spi0_status.rx_buf++ = SPDR0;
		SPDR0 = *spi0_status.tx_buf++;
		spi0_status.count--;
	}
	else
	{
		// disable SPIF interrupt and clear BUSY flag
		spi0_status.flags = SPI_ONLINE_bm | SPI_MASTER_bm;
		SPCR0 = (1 << SPE) | (1 << MSTR) | SPI_SPR_DIV_128_gc;

		// pause and deselect
		_delay_us(SS_DELAY_us);
		PORT_B.PORT |= SS0_bm;
	}
}

ISR(SPI1_STC_vect)
{
	if (spi1_status.count)
	{
		// transfer next byte
		SPDR1 = *spi1_status.tx_buf++;
		*spi1_status.rx_buf++ = SPDR1;
		spi1_status.count--;
	}
	else
	{
		// disable SPIF interrupt and clear BUSY flag
		SPCR1 = (1 << SPE) | SPI_SPR_DIV_128_gc;
		spi1_status.flags = SPI_ONLINE_bm;
	}

	if (SPSR1 & (1 << WCOL))
	{
		// set WCOL trigger output
		PORT_C.PORT |= WCOL_bm;
	}
	else
	{
		// clear WCOL trigger output
		PORT_C.PORT &= ~WCOL_bm;
	}
}

 

Logic analyzer capture:

 

 

Here is "port.h":


#ifndef PORT_H_
#define PORT_H_

#include <stdint.h>
#include <avr/io.h>

typedef struct PORT_struct
{
	volatile uint8_t 	PIN;
	volatile uint8_t 	DDR;
	volatile uint8_t 	PORT;
} PORT_t;

#define PORT_B		(*(PORT_t*) 0x23)
#define PORT_C		(*(PORT_t*) 0x26)
#define PORT_D		(*(PORT_t*) 0x29)
#define PORT_E		(*(PORT_t*) 0x2C)

#define PIN0_bm		(1 << PIN0)
#define PIN1_bm		(1 << PIN1)
#define PIN2_bm		(1 << PIN2)
#define PIN3_bm		(1 << PIN3)
#define PIN4_bm		(1 << PIN4)
#define PIN5_bm		(1 << PIN5)
#define PIN6_bm		(1 << PIN6)
#define PIN7_bm		(1 << PIN7)

#endif

 

 

edit: added "Mini" after "Xplained"

 

This topic has a solution.

Greg Muth

Portland, OR, US

Xplained/Pro/Mini Boards mostly

 

Make Xmega Great Again!

 

Last Edited: Fri. Jul 13, 2018 - 05:33 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I'm not claiming to be an SPI expert; and I've always thought that AVR8 SPI slave was ... wimpy.  And no experience with PB.  So what help can I be?

 

I'll need to dig through the datasheet(s).  If the AVR generated an interrupt a) when selected and also b) when SPDR full, wouldn't that expalin your symptoms?

Bit 7 – SPIF0: SPI Interrupt Flag
When a serial transfer is complete, the SPIF Flag is set. An interrupt is generated if SPIE in SPCR is set
and global interrupts are enabled. If SS is an input and is driven low when the SPI is in Master mode, this
will also set the SPIF Flag. SPIF is cleared by hardware when executing the corresponding interrupt
handling vector. Alternatively, the SPIF bit is cleared by first reading the SPI Status Register with SPIF
set, then accessing the SPI Data Register (SPDR)
 

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

Last Edited: Thu. Jul 12, 2018 - 09:29 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 If the AVR generated an interrupt a) when selected and also b) when SPDR full, wouldn't that expalin your symptoms?

The way I understand it (which appears to be not so well...), the only time SS going low generates an interrupt is when the SPI is in master mode with SS an input.  Other than that SPDR Full generates the interrupts on both master and slave.

 

 

 

Greg Muth

Portland, OR, US

Xplained/Pro/Mini Boards mostly

 

Make Xmega Great Again!

 

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The obvious thing from the waveform is that ISR(SPI1_ STC_vect) processing is too late.
The time from ISR(SPI0_STC_vect) placing data in SPDR0 until ISR(SPI1_STC_vect) places data in SPDR1 must be within 64 clocks.

If the master is polled, there is a margin for the slave interrupt processing and there is a possibility of normal operation.

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

The obvious thing from the waveform is that ISR(SPI1_ STC_vect) processing is too late.
The time from ISR(SPI0_STC_vect) placing data in SPDR0 until ISR(SPI1_STC_vect) places data in SPDR1 must be within 64 clocks.

So SPI0 has written SPDR0 and begun shifting out its next byte before SPI1 has written to SPDR1, right?  That explains the WCOL.  How did you arrive at "64 clocks?"

 

Greg Muth

Portland, OR, US

Xplained/Pro/Mini Boards mostly

 

Make Xmega Great Again!

 

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

I guess the next step is to put master and slave on different boards so both ISRs can run concurrently.

Greg Muth

Portland, OR, US

Xplained/Pro/Mini Boards mostly

 

Make Xmega Great Again!

 

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

SCK starts from the point of writing the data.
You are setting DIV128.
64CPU clocks up to the SCK up edge is the output setup period. And the next 64CPU clocks to the SCK down edge is the slave sampling period. At the total of 128CPU clocks finishes 1SCK clock.

 

It is possible to predict the timing of output to SPDR0 from the SCK up edge of the second byte. Subsequent counter increment processing etc. consume CPU cycles.
Even if the slave interrupt can finally start, WCOL is generated because the SCK up edge has already arrived. This is obvious from the timing of WCOL change.

 

As a result, the "MASTER" data received by the slave is output as it is without being updated.

 

Last Edited: Fri. Jul 13, 2018 - 04:43 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

in SPI, the slave never sends, the master fetches from the slave. That's what the interrupt is for. If a slave needs the master's attention, it asserts the interrupt and the master fetches the information from it.

 

277,232,917 -1 The largest known Mersenne Prime

Measure twice, cry, go back to the hardware store

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
		// output byte to be sent after slave is selected
		SPDR1 = *spi1_status.tx_buf++;

outputs a byte does not decrement the count - so the count will be off by one won't it?

 

Similarly:

		// output first byte from master
		SPDR0 = *spi0_status.tx_buf++;

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

Because he is not decrement there, the sending size is increased by 1 byte and NULL is sended.

 

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

What happens if you switch roles, make SPI1 the master and SPI0 the slave (SPI0 has higher priority then SPI1)?

 

Jim

 

Click Link: Get Free Stock: Retire early!

share.robinhood.com/jamesc3274

 

 

 

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

@clawson said:

outputs a byte does not decrement the count - so the count will be off by one won't it?

Yes, fixed that. 

 

@ki0bk said:

What happens if you switch roles, make SPI1 the master and SPI0 the slave (SPI0 has higher priority then SPI1)?

Every other slave byte makes it through. 

 

@kabasan said:

SCK starts from the point of writing the data.
You are setting DIV128.
64CPU clocks up to the SCK up edge is the output setup period. And the next 64CPU clocks to the SCK down edge is the slave sampling period. At the total of 128CPU clocks finishes 1SCK clock.

I understand now.

 

The fix was indeed to put master and slave on two different MCUs.  The proof is in the logic analyzer capture:

 

 

MASTER code:


#include <string.h>
#include <avr/cpufunc.h>
#include <avr/interrupt.h>
#include <avr/io.h>
#include <avr/pgmspace.h>
#include <util/delay.h>
#include "port.h"


// state flags
#define SPI_BUSY_bm	(1 << 0)
#define SPI_MASTER_bm	(1 << 1)
#define SPI_ONLINE_bm	(1 << 7)


// i/o pin bitmaps
#define SCK0_bm			PIN5_bm	/* PORT_B */
#define MISO0_bm		PIN4_bm	/* PORT_B */
#define MOSI0_bm		PIN3_bm	/* PORT_B */
#define SS0_bm			PIN2_bm	/* PORT_B */


// miscel
#define SPIF_bm		        (1 << SPIF)
#define SPI_SPR_DIV_128_gc	3
#define SPI_BUFFER_SIZE		16
#define SS_DELAY_us		5


typedef struct spi_status_struct
{
    volatile char* rx_buf;		//!< buffer to transfer to/from
    volatile char* tx_buf;		//!< buffer to transfer to/from
    volatile uint8_t count;		//!< number of bytes left to transfer
    volatile uint8_t flags;		//!< state flags
} spi_status_t;


spi_status_t spi0_status;


int main(void)
{

    char spi0_rx_buffer[SPI_BUFFER_SIZE];
    char spi0_tx_buffer[SPI_BUFFER_SIZE];

    // init master SPI
    PORT_B.PORT = SS0_bm;
    PORT_B.DDR = MOSI0_bm | SS0_bm | SCK0_bm;
    SPCR0 = (1 << SPE) | (1 << MSTR) | SPI_SPR_DIV_128_gc;

    _delay_ms(1);
	
    do 
    {
	cli();

	// clear Rx buffers
	memset(spi0_rx_buffer, 0, SPI_BUFFER_SIZE);

	// load Tx buffers
	strcpy_P(spi0_tx_buffer, PSTR("MASTER"));

	// init spi0 status
	spi0_status.rx_buf = spi0_rx_buffer;
	spi0_status.tx_buf = spi0_tx_buffer;
	spi0_status.count = strlen(spi0_tx_buffer) - 1;
	spi0_status.flags = SPI_ONLINE_bm;
	SPCR0 = (1 << SPE) | (1 << SPIE) | (1 << MSTR) | SPI_SPR_DIV_128_gc;
	
	sei();

	// select and pause
	PORT_B.PORT &= ~SS0_bm;
	_delay_us(SS_DELAY_us);

	// output first byte from master
	SPDR0 = *spi0_status.tx_buf++;

	while (spi0_status.flags & SPI_BUSY_bm);
				
	_delay_ms(25);

    } while (1);
	
    return 0;
}


ISR(SPI0_STC_vect)
{
    if (spi0_status.count)
    {
	// transfer next byte
	*spi0_status.rx_buf++ = SPDR0;
	SPDR0 = *spi0_status.tx_buf++;
	spi0_status.count--;
    }
    else
    {
	// disable SPIF interrupt and clear BUSY flag
	spi0_status.flags = SPI_ONLINE_bm;
	SPCR0 = (1 << SPE) | (1 << MSTR);

	// pause and deselect
	_delay_us(SS_DELAY_us);
	PORT_B.PORT |= SS0_bm;

    }

}


 

SLAVE code:


#include <string.h>
#include <avr/cpufunc.h>
#include <avr/interrupt.h>
#include <avr/io.h>
#include <avr/pgmspace.h>
#include <util/delay.h>
#include "port.h"


// state flags
#define SPI_BUSY_bm		(1 << 0)
#define SPI_MASTER_bm	(1 << 1)
#define SPI_ONLINE_bm	(1 << 7)


// i/o pin bitmaps
#define SCK0_bm			PIN5_bm	/* PORT_B */
#define MISO0_bm		PIN4_bm	/* PORT_B */
#define MOSI0_bm		PIN3_bm	/* PORT_B */
#define SS0_bm			PIN2_bm	/* PORT_B */


// miscel
#define SPIF_bm			    (1 << SPIF)
#define SPI_SPR_DIV_128_gc	3
#define SPI_BUFFER_SIZE		16
#define SS_DELAY_us		    5


typedef struct spi_status_struct
{
    volatile char* rx_buf;		//!< buffer to transfer to/from
    volatile char* tx_buf;		//!< buffer to transfer to/from
    volatile uint8_t count;		//!< number of bytes left to transfer
    volatile uint8_t flags;		//!< state flags
} spi_status_t;


spi_status_t spi0_status;


int main(void)
{

    char spi0_rx_buffer[SPI_BUFFER_SIZE];
    char spi0_tx_buffer[SPI_BUFFER_SIZE];

    // init slave SPI
    PORT_B.PORT = SS0_bm;
    PORT_B.DDR = MISO0_bm;
    SPCR0 = (1 << SPE);

    _delay_ms(1);
	
    do 
    {
	cli();

	// clear Rx buffers
	memset(spi0_rx_buffer, 0, SPI_BUFFER_SIZE);

	// load Tx buffers
	strcpy_P(spi0_tx_buffer, PSTR("SLAVE "));

	// init spi0 status
	spi0_status.rx_buf = spi0_rx_buffer;
	spi0_status.tx_buf = spi0_tx_buffer;
	spi0_status.count = strlen(spi0_tx_buffer) - 1;
	spi0_status.flags = SPI_ONLINE_bm;
	SPCR0 = (1 << SPE) | (1 << SPIE);
	
	sei();

	// output byte to be sent after slave is selected
	SPDR0 = *spi0_status.tx_buf++;

	while (spi0_status.flags & SPI_BUSY_bm);
				
	_delay_ms(20);

    } while (1);
	
    return 0;
}


ISR(SPI0_STC_vect)
{
    if (spi0_status.count && (!(PORT_B.PIN & SS0_bm)))
    {
	// transfer next byte
	*spi0_status.rx_buf++ = SPDR0;
	SPDR0 = *spi0_status.tx_buf++;
	spi0_status.count--;
    }
    else
    {
	// disable SPIF interrupt and clear BUSY flag
	spi0_status.flags = SPI_ONLINE_bm;
	SPCR0 = (1 << SPE);
    }
}


 

Greg Muth

Portland, OR, US

Xplained/Pro/Mini Boards mostly

 

Make Xmega Great Again!

 

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

This is a soliloquy.

I will never use SPI slaves on devices without send buffer.
Another interrupt will delay the SPI interrupt  and easily generate WCOL.

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

I will never use SPI slaves on devices without send buffer.
Another interrupt will delay the SPI interrupt  and easily generate WCOL.

Sounds like  a good rule-of-thumb.  Thanks for identifying the problem!  smiley 

Greg Muth

Portland, OR, US

Xplained/Pro/Mini Boards mostly

 

Make Xmega Great Again!