# Strange bug when Interfacing with Shift Register (CPLD) via SPI

I've implemented a 8-bit Parallel in Serial Out (PISO) Shift register in VHDL on my Max V CPLD. I'm using SPI to interface with the CPLD using my AVR. The circuit works but only partially. Suppose I have an input which contains only one set bit i.e. 00010000 or 10000000 etc. In this case, the AVR reads in the data correctly.

However, if the input to the PISO register is 10100000, the AVR reads in the data as 11100000. This is also reflected in my oscilloscope. I've attached the picture to illustrate this.

The top waveform is the serial output from my shift register and the bottom waveform is the clock. Shouldn't the serial output become 0 on the 2nd clock cycle?

If the data is 10000001, then its read in as 10000011. What could be causing this? Thinking it could be my VHDL code, I've even copy pasted other people's codes from the web to see if it works but unfortunately I get the same results.

The following is the code for my AVR

int main(void)
{
DDRB = (1 << DD_MOSI) | (1 << DD_SCK) | (1 << 2) | (1 << 0);

SPCR = (1 << SPE) | (1 << MSTR) | (1 << SPR0)  | (1 << SPR1);
DDRD = 0xFF;
PORTD = 0x00;

char datain;

while(1)
{
PORTB = 1; // This loads the shift register.
_delay_ms(10);
PORTB = 0x00;
_delay_ms(10);

SPDR = 0b01011101;

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

_delay_ms(10);

PORTD = datain;
}

return 1;
}


And this is the VHDL code I'm using. Note that I've intentionally left out the input. I'm hardcoding the input as then I don't have to mess around with additional wires. Which I was hoping would provide less room for error.

library ieee;
use ieee.std_logic_1164.all;
entity PISO is
port(C, CS_N : in std_logic;
--D   : in std_logic_vector(7 downto 0);
SO  : out std_logic);
end PISO;
architecture archi of PISO is
signal tmp: std_logic_vector(7 downto 0);
begin
process (C, CS_N)
begin
if CS_N = '1' then
tmp <= "10000001";
elsif rising_edge(C) then
tmp <= tmp(tmp'high - 1 downto tmp'low) & '0';
end if;
end process;
SO <= tmp(7);
end archi;


Note that I'm quite new to VHDL/CPLDs and I keep running across something regarding timings and how critical they are. I've tried to read up on them but don't seem to quite get it. I understand gate delays etc. but how am I supposed to set up the CPLD such that this is not an issue? My clock is not very fast. I'm running it at the lowest possible frequency i.e. 62.5kHz. Could this error have something to do with breadboards? I've run out of photoresist copper clads so can't make a PCB just yet. The Max V is on a development board.

I'll appreciate any sort of pointers on how to solve this issue. I've been at it for the past 8 hours or so to no avail.

ADDED INFO: Initially, when I programmed the CPLD I tested it via the buttons on my development board. That showed no problems whatsoever and the register spat out bits serially just fine. Not only that, I've even clocked it via the AVR. That worked great as well. It was only after I confirmed that it was working ok that I started work on the SPI interface which is where things went wrong.

New Waveforms:

OK, here we go. For all screenshots, the serial out was the same (10000001). I've made a slight improvement such that 10000001 no longer comes out as 10000011 but instead is 10000010. When I was debugging before, I inadvertently set CPHA bit to 1. When I set it back to 0, the data was 100000010 instead of 10000011. Still not correct, of course. Here are the waveforms from the scope.

Top waveform: Serial Out

Bottom waveform: Clock. Triggered at rising edge.

Top waveform: Chip Select (Active Low)

Bottom waveform: Clock. Triggered at rising edge.

Top waveform: Chip Select

Bottom waveform: Clock. Triggered at falling edge.

Top waveform: Serial Out.

Bottom waveform: Clock. Triggered at falling edge.

EDIT: Forgot to add, this is my cleaned up AVR code now.

while(1)
{
bit_clear(PORTB,BIT(CS)); // Low Chip Select
SPDR = 0xFF;
while(!(SPSR & (1 << SPIF)));
datain = SPDR;
PORTD = datain;
bit_set(PORTB,BIT(CS)); // High Chip select
_delay_us(100); // Keep high for 100us. This made it easier to see it on o-scope.
}

• Since you said it worked before, I'm wondering what edge the AVR clocks SPI data on? If the AVR is clocking data on the rising edge, you must change it on the falling edge. Changing while it's read will do wonky stuff like that. – darron Sep 17 '11 at 16:00
• All this talk of what the AVR sees is a red herring that distracts and wastes time. If the data is wrong on the scope, then it's clearly the sender and not the receiver having the problem. Therefore forget about the AVR and fix your CPLD code. – Olin Lathrop Sep 17 '11 at 16:12

Well let's look at your VHDL code. Please keep in mind that I'm not trying to be critical or harsh-- I'm just trying to get you up to speed quickly. Your code is currently this:

process (C, ALOAD)
begin
tmp <= "10000001";
elsif (C'event and C='1') then
tmp <= tmp(6 downto 0) & '0';
SO <= tmp(7);
end if;
end process;


But should look like this:

process (C, ALOAD)
begin
tmp <= "10000001";
elsif rising_edge(C) then
tmp <= tmp(tmp'high-1 downto tmp'low) & "0";  -- '0' and "0" are equivalent in this context
end if;
end process;

SO <= tmp(tmp'high);


The changes I made are minor, for sure, but important.

The first thing is my use of "rising_edge()". This is a new-ish thing to VHDL and isn't covered in some of the VHDL books. This is considered to be better than using 'event. Likewise, there is a falling_edge().

The next thing is my use of 'high and 'low instead of hard-coding the values. This really doesn't matter for this code, but when you start doing bigger things then these will help you a lot. For example, you could just change the definition of tmp to be bigger and the rest of the code will just automatically adjust (except for the initialization to "10000001").

I should also point out that an async reset or load in FPGAs is discouraged, but is fine in CPLDs.

Also note that I moved the assignment of SO to outside the process. This may or may not be what you intended. The way you have it there is an extra flip-flop going from tmp(7) to SO. Normally, with SPI, this isn't want you want because the SPI Clock could go away at the end of the transfer and you'll never get that last bit out. On the other hand, with the way that I did it you'll start getting that first bit out when ALOAD='1', not at the rising edge of C.

Unfortunately, this doesn't really answer your question on why you're getting bad data into the AVR. There just isn't enough information in your question. Here's the kinds of things I would be looking at, or be concerned about:

Your o-scope pic doesn't show the other spi signals, like CS. CS is critical to understanding where the bits are supposed to go. Also, knowing what the SPI mode is will help with this.

On the o-scope pic, the first clock cycle where SO='1' is NOT the first bit of the SPI transfer. You loaded the shift register 10-20 mS before that, and your clock period is about 20 uS. So you had at least 1 clock cycle before SO='1', and probably more. So there is some weird stuff going on here-- we don't have enough info to understand the behavior.

You're using ALOAD to load the shift register, but normally you'd use CS_N (active low). While CS_N='1' you do an async load of the shift register, while CS_N='0' you shift it out. Using ALOAD like you have here is OK, but probably not what you wanted and doesn't work with what appears to be some strange SPI clock stuff (from the previous point).

So, here's what you should do...

Clean up the VHDL a bit. Repost the updated version. Since your scope only has 2 channels, hook up one channel to CS_N and the other channel to CLK. Trigger on the falling edge of CLK. Capture a waveform showing 2 clocks before the falling edge of CLK and 5 clocks after. Without changing the settings on the scope, remove CLK and put the probe on SO. Capture another image. Do this again for the rising edge of CLK. So, 4 waveform images total.

Do that and we can re-evaluate what might be wrong.

Edit: Updated to reflect the updated question.

I see two issues: First, if the AVR is sampling on the rising clk edge, you should clock your shift register off of the falling edge. As supercat mentioned, this will give you +/- 0.5 clock periods of setup & hold going into your AVR.

And Second: As you mentioned, you're getting 10000010 instead of 10000001. I do not believe that your VHDL code is in error on this one, but it is obviously coming out of the CPLD wrong. If I had to guess, I would guess that the problem is with some signal integrity issues with your CLK. It's hard to tell with your scope, but it looks like you have over a volt of overshoot and undershoot on that signal (and with that would come a lot of ringing). That ringing, if bad enough, could cause the CPLD to "double clock"-- meaning run the shift register twice for a single clock edge. And if _really_bad_ it could cause the CPLD to latch up and literally explode (I've seen it happen).

Here's some experiments to try:

1. Instead of 10000001, use 10101010 or 01010101. This will help you see which bit is getting double clocked, and if it is always the same bit.

2. Zoom in on the clock edges with the scope. Make sure that your scope probes are at the CPLD, not the AVR, when you do this. Yes, it makes a HUGE difference.

Assuming that it is over/undershoot and ringing on the clock, the solution is to add proper signal termination on the line. I would start with a 50 ohm series resistor at the AVR. Note: this will slow down the clock edges, but since you are clocking the CPLD on the falling edge you have a lot of time available.

How is the clock ran from AVR to CPLD? A long wire between PCB's? That would be my guess.

• Thank you very much, David. The book that I have is old and does not cover the new things you talked about. I can see the advantage of using 'high/'low attributes. Similarly, rising edge also cleans things up and improves readability. However, I'm confused about CS. The CPLD is the only device I've connected to the SPI bus (besides the ISP programmer) and since I don't select any chips, I've set CS as the output but I'm not doing anything with it. Is my next step to connect it such that it loads the register? – Saad Sep 17 '11 at 15:59
• Also, I've left the SPI mode to default. Since the CPLD shifts at the rising edge, I thought that the default mode will do. Am I incorrect regarding this? – Saad Sep 17 '11 at 16:12
• @Saad SPI Chip select does more than just "selecting the chip". It frames the whole SPI transfer (says when it starts and ends). Otherwise all you get is a stream of bits & clocks and you don't know when one byte/word/transfer starts and the other ends. Some applications don't "need" a CS, but in most cases it would tell us what the AVR is expecting. The SPI mode defines two things: Polarity of the clock and timing of the CS. These are important if you're trying to figure out what the AVR is doing or expecting. – user3624 Sep 17 '11 at 17:13
• It is common in SPI for devices to shift out data on one clock edge and latch data on the other. This generally allows +/- half a clock skew between the clock and data lines. Some hardware devices (e.g. 74HC595 and 74HC165) always latch data on the same clock edge as they change their output, and some SPI devices allow transmit and receive clock edges to be configured independently. If a slave puts out the first bit on /CS and succeeding bits with each leading clock edge, a master which generates the clock will be able to tolerate a full clock of round-trip delay instead of 1/2 clock. – supercat Sep 17 '11 at 17:33
• @David Kessner: I wonder why I've not seen many SPI-ish devices use some means of framing other than a /CS wire? With SPI there generally won't be more than two edges of MISO between clock edges. If MISO changes multiple times while the clock stays in one state, such excess transitions could signal a start of packet. There are lots of possible transitions one could use. – supercat Sep 17 '11 at 18:58