ATmega168 SPI - why do I have to wait before sending?

[obiwanjacobi]

I am re-programming a PCB that has an ATmega168 on it.
There are a whole bunch of LEDs that are controlled by SPI that shifts bits into 4094 serial in / parallel out ICs.
There are 12 bytes worth of LED bits to be shifted out. So I made a 12 byte buffer that I fill and then call a function to iterate over the buffer and push each byte out over SPI. After each write I wait for the SPI to finish. Then I read what came in (there are switches going the other way) but ignore that data for now.

What I am seeing is that in the loop that writes out the 12 bytes I need to wait for +/- 10us after I've read the incoming byte before writing out the next SPI byte.
If I don't wait a lot of LEDs are turning on that were supposed to be off and the ones I want on are off (most of the time). Looks a bit random.

I don't see any limitation or timing requirement mentioned in the datasheet (or I am looking in the wrong place?).

Can this be explained?

PS: This is the schematic: https://service-tcgroup.tcelectronic.com/files/tech_service/nova/novasystem/schematics/p16517-bbe_schematic_printer.pdf

[pqass]

I'm not sure but the use of the QS2 (vs the QS1) output from the HC4094s to daisy-chain to the next shift register may be causing an off-by-one bit lag.  How fast is your SCK?

"Two serial outputs are available for cascading a number of these devices. Data is available at the QS1 serial output terminal on positive clock edges to allow for high-speed operation in cascaded system in which the clock rise time is fast. The same serial information, available at the QS2 terminal on the next negative clock edge, provides a means for cascading these devices when the clock rise time is slow."  From here page 10. 

Nevertheless, just waiting a couple of clock periods before sending the next byte would be sufficient to overcome this design choice.  How may clocks in 10us?

You've also got some weirdness with IC23 (74AHC541) being used as a SPI clock buffer but SP_CLK_4 leads the other SPI_CLK_0..3 by a propagation delay (12ns). But I don't think this would matter.


ON SECOND THOUGHT...   use of QS2 just means that the 8th bit lags 1/2 a clock but this is before the rising edge of the next (9th) clock.

This is what I use for my SPI routines.  There's a clock or two gap between bytes on the MOSI line due to calls to spi_transfer() and the while loop waiting.

Code: [Select]

#define SPIDDR     (DDRB)
#define SPIPORT    (PORTB)
#define SPISS      (PB2)    // 2x74HC595 LATCH pin 12
#define SPIMOSI    (PB3)    // 2x74HC595 DATA IN pin 14
#define SPIMISO    (PB4)    // unconnected but with pull up enabled; may later add 74HC165 to scan key switches
#define SPISCK     (PB5)    // 2x74HC595 CLOCK pin 11

void spi_setup() {
  SPIDDR  |=  _BV(SPISS);   // output
  SPIPORT &= ~_BV(SPISS);   // default to LOW; device set to normal position; latch occurs on rising edge
  SPIDDR  |=  _BV(SPIMOSI); // output
  SPIPORT &= ~_BV(SPIMOSI); // default to LOW
  SPIDDR  &= ~_BV(SPIMISO); // input
  SPIPORT |=  _BV(SPIMISO); // enable pull-up; no flapping if nothing connected
  SPIDDR  |=  _BV(SPISCK);  // output
  SPIPORT &= ~_BV(SPISCK);  // default to LOW

  uint8_t clr;
  SPCR = (1<<SPE)|(1<<MSTR)|(1<<SPR1);      // F_CPU=16MHz: 250KHz SCK
  clr=SPSR;
  clr=SPDR;

  _delay_ms(10);
}

uint8_t spi_transfer(volatile uint8_t data) {
  SPDR = data;                    // Start the transmission
  while (!(SPSR & (1<<SPIF)));    // Spinwait until the end of the transmission
  return SPDR;                    // return the received byte
}

void spi_load() {
  SPIPORT |=  _BV(SPISS);    // Latch on rising edge.
  SPIPORT &= ~_BV(SPISS);    // Return device to normal position.
}



int main(void) {
  spi_setup();

  ...

  spi_transfer(x);
  spi_transfer(y);
  spi_load();

  ...

}


[obiwanjacobi]

I'm not sure but the use of the QS2 (vs the QS1) output from the HC4094s to daisy-chain to the next shift register may be causing an off-by-one bit lag.  How fast is your SCK?

Good point. Fairly quick 12Mhz/16. I looked at the max frequency of the ICs (74HC4094) and that was higher than the MCU's clock frequency. But the serial output QS2 used of those ICs indeed suggest to take a slower approach. Will try it out. Thanks!

EDIT: Wait, the 74HC4094 datasheet talks about fast and slow clock rise times... ?

EDIT2: Even with the slowest SCK (12Mhz/128) it did not work without a wait. So I guess I keep it at prescaler 16 and wait 10us...

EDIT3: It even works with 12MHz/4 and a wait of 10us. Fast-fast.

[xvr]

After each write I wait for the SPI to finish.

Are you sure? Additional wait hack may be suspend execution until end of SPI transaction.

[DavidAlfa]

Edit:
I don't know atmega mcus, checked the datasheet and SPI doesn't have double buffering/shadowing like most MCUs do, so this is pointless.

Are you waiting for the correct SPI flag?

Often the SPI peripheral has two flags:
- Buffer empty: You can load new data in, but it might still be transmitting the last byte.
- Idle: SPI peripheral is doing nothing.

I've seen this mistake a lot of times, reading the buffer empty flag and assuming it's done, breaking the active transfer.

[NorthGuy]

What I am seeing is that in the loop that writes out the 12 bytes I need to wait for +/- 10us after I've read the incoming byte before writing out the next SPI byte.

You should wait before you read. Like you send a byte, wait 8 SPI clock times (and then some depending on the overhead within AVR), then you read. The SPI module should have a flag indicating you can read. After the read, you don't need to wait and can write again.

[mikerj]

What I am seeing is that in the loop that writes out the 12 bytes I need to wait for +/- 10us after I've read the incoming byte before writing out the next SPI byte.

You should wait before you read. Like you send a byte, wait 8 SPI clock times (and then some depending on the overhead within AVR), then you read. The SPI module should have a flag indicating you can read. After the read, you don't need to wait and can write again.

The OP did state "After each write I wait for the SPI to finish. Then I read what came in", so it sounds like he's using it correctly.  However posting his code would be very helpful.

[obiwanjacobi]

Yep - spin loop testing that flag... 

loop thru 12 bytes
  spi send byte at loop index
  spi wait for transfer to complete
  spi read and store in buffer

  => wait 10 us <=
next

[pqass]

Does the inside of your loop look like this?

Code: [Select]

  SPDR = sdata[i];                 // Start the transmission
  while (!(SPSR & (1<<SPIF)));     // Wait until the end of the transmission
  rdata[i] = SPDR;                 // return the received byte


[obiwanjacobi]

Does the inside of your loop look like this?

Code: [Select]

  SPDR = sdata[i];                 // Start the transmission
  while (!(SPSR & (1<<SPIF)));     // Wait until the end of the transmission
  rdata[i] = SPDR;                 // return the received byte


Exactly!

All the 12 bytes are being delivered correctly to their respective 4094 ICs without any corruption or loss. So I know for sure that sending is correct. I also know that the reading is correct, I just implemented reading the switches this weekend.

[NorthGuy]

All the 12 bytes are being delivered correctly to their respective 4094 ICs without any corruption or loss. So I know for sure that sending is correct. I also know that the reading is correct, I just implemented reading the switches this weekend.

QS2 vs QS1 shouldn't matter. From the point of view of a shift register, nothing special happens between bytes - same as between any bits. So, if it works between bits, it should work the same between bytes. Moreover, you probably already have a delay between bytes anyway.

You should look at the clock/data signal at various points with LA or a scope to see how it is propagating.

How do you operate the strobe?

[obiwanjacobi]

How do you operate the strobe?

A (hi-lo) pulse after the spi transfer of 12 bytes is done. No delay in between.
I did have a delay in there initially but thru trail & error concluded that it also worked without it.

[NorthGuy]

How do you operate the strobe?

A (hi-lo) pulse after the spi transfer of 12 bytes is done. No delay in between.
I did have a delay in there initially but thru trail & error concluded that it also worked without it.

But there should be a clock when strobe is high. If it isn't, the output shouldn't change at all:

[NorthGuy]

But there should be a clock when strobe is high. If it isn't, the output shouldn't change at all:

Looks like I'm wrong. Contrary to truth table, the schematics shows that STR operates a latch, not flip flop.

[obiwanjacobi]

Yep - I did see that too. But the schematics are fixed (not designed by me) and it used to work. So...
A short strobe pulse at the end of the transfer works fine.

[NorthGuy]

I think it is most likely a bug in your code. It should be easy to find what exactly is happening by looking at the signal - your schematics shows lots of TPs.

There also could be SI problems, but this is rather unlikely.

10 us is a huge delay. I cannot think of any mechanism which would require such a delay to work.

[DavidAlfa]

But there should be a clock when strobe is high. If it isn't, the output shouldn't change at all:

Are you sure? TI datasheet doesn't show this.
To me, all it needs is to send data through SPI and then pulse STR, regardless of the clock.

[pqass]

The original complaint:
"If I don't wait a lot of LEDs are turning on that were supposed to be off and the ones I want on are off (most of the time). Looks a bit random."

And clues:
a. "EDIT2: Even with the slowest SCK (12Mhz/128) it did not work without a wait. So I guess I keep it at prescaler 16 and wait 10us..."
b. "A (hi-lo) pulse after the spi transfer of 12 bytes is done. No delay in between."

From HC4094 datasheet here:
"The data of each stage of the shift register is provided with a latch, which latches data on the negative going transition of the STROBE input signal. When the STROBE input is held high, data propagates through the latch to a 3-state output buffer. This buffer is enabled when OUTPUT ENABLE input is taken high."

Therefore:
a. If LED_OE=LOW, then the output goes Hi-Z. So, the code must keep LED_OE=HIGH most of the time.
b. While LED_STROBE=HIGH, any input changes flows through the latches.  At the moment LED_STROBE=LOW the outputs are frozen.

So, while you're clocking out serial data LED_STROBE must be kept LOW, otherwise you'll see blink'nlights during the transmission.

Now, 12MHz/128=93.75KHz. Time to transmit 12 bytes=(1/93750 * 12 * 8 )=1.024ms
Confirm that the post-transmission delay is actually 10us.  Maybe you mean 10 milliseconds?

I'm thinking you're continuously sending a stream (visible to the outputs) and without a 10 millisecond delay (a delay >> than the transmit time), it's going to look like a blur.

Pseudo code:

Code: [Select]

main() {
  LED_OE = HIGH;
  while(1) {                 // loop forever
    // do some stuff here.

    // send/receive LED/switch state
    LED_STROBE = LOW;
    for (int i = 0; i < 12; i++) {
      SPDR = sdata[i];                 // Start the transmission
      while (!(SPSR & (1<<SPIF)));     // Wait until the end of the transmission
      rdata[i] = SPDR;                 // Save the received byte
    }
    LED_STROBE = HIGH;
    LED_STROBE = LOW;

    // do other stuff here.
  }
}


[obiwanjacobi]

My apologies!

I just removed the 10 us wait and it is still working.

The code has been refactored from when I started this topic. I think I had the STR high while clocking in the bits over SPI when I observed this issue - due to the 4094's datasheet 'explanation'.

So now - no wait in the loop and a simple STR pulse at the end. It's all good.
And the SCK is still running at 12MHz/4!

Thank you all for your contributions. 

(I am trying to get the rotary encoders to work now and for some reason the switches work but the rotary encoders don't.)

[obiwanjacobi]

Here's a view of the SPI writing 4094's and reading 165's (first 3 MISO bytes)