AVR SPI Occasional Junk Transfer from Slave

I've written the two very simple programs below to collect and display a value from a slave device via SPI. Both devices are Arduino Nanos.

When trying to return a value (decimal 100 in the case below), I'm occasionally getting 50 returned instead. Looking at the binary values of 100 = 01100100 and 50 = 00110010, it's clear that the occasional transfer is being shifted to the right 1 place, but I can't work out why. Help would be much appreciated!

Master

#define F_CPU 16000000UL
#define CS 2
#define MOSI 3
#define MISO 4
#define SCK 5

#include <avr/io.h>
#include <util/delay.h>

uint8_t spiByteReceive;

int main(void){
  DDRB |= (1<<CS)|(1<<MOSI)|(1<<SCK);     //Set CS, MOSI and SCK as outputs
  PORTB |= (1<<CS);     //Set CS pin as defualt high

  Serial.begin(9600);     //Being Arduino sperial comms

  SPCR |= (1<<SPE)|(1<<MSTR)|(1<<SPR1)|(1<<SPR0);     //Enable SPI, set as master, set SCK to F_CPU / 128

  while(1){
    PORTB &= ~(1<<CS);      //Pull CS pin low to caputre slave
    SPDR = 0x00;      //Load dummy byte to SPI DR, start tx
    while(!(SPSR |= (1<<SPIF)));      //Wait for SPI tx
    spiByteReceive = SPDR;     //Read return from slave, clear SPIF
    _delay_ms(10);      //Allow tx to complete before releasing slave
    PORTB |= (1<<CS);     //Pull CS pin high to release slave

    Serial.print(spiByteReceive);      //Print spiByte
    Serial.print("\n");

    _delay_ms(200);
  }

  return(0);
}

Slave

#define F_CPU 16000000UL
#define CS 2
#define MOSI 3
#define MISO 4
#define SCK 5

#include <avr/io.h>

uint8_t spiByteSend = 100;

int main(void){
  DDRB |= (1<<MISO);      //Set MISO as output

  SPCR |= (1<<SPE);     //Enable SPI

  while(1){
    SPDR = spiByteSend;     //Load byte to send to master
    while(!(SPSR |= (1<<SPIF)));      //Wait for tx
  }

  return(0);
}

I've tried adding/changing delays and have put a 4.7k pull-up on the CS line all to no avail.

Solution

There is a bit strange way to read the flag bit in your code:

while(!(SPSR |= (1<<SPIF)));

Note, instead of bitwise and & there is bitwise or assignment |= is being used.

Although this construction can be used to check and clear some flags (if you are sure all other bits in the register are zeroes), there are some issues with this approach:

Individual bits in many IO registers can be more effectively accessed using SBI, CBI, SBIC, SBIS instructions. Using |= prevents the compiler from using those instructions and leads to less effective code being generated.
Although most of the interrupt flag bits in AVR can be cleared by writing 1 to them, SPIF in SPSR is not the case. It is cleared either when ISR (if enabled) is executing. Or by reading SPDR after reading SPSR with SPIF is set.

So, in the slave code you have to perform a dummy read of SPDR:

 ...
 SPDR = spiByteSend;     //Load byte to send to master
 while(!(SPSR & (1<<SPIF)));      //Wait for tx  
 SPDR; // dummy read to clear `SPIF`
 ...