VHDL code works in a simulation but not on FPGA

Question

I am trying to implement a SPI master module with a buffer. I use this FSM module to test it and to transmit received data via UART to my serial console.

library IEEE;
USE ieee.std_logic_1164.all;
USE ieee.std_logic_arith.all;
USE ieee.std_logic_unsigned.all;

entity FSM_SPI_buf is
Port ( clk: in STD_LOGIC;
          increase: in STD_LOGIC;
          reset: in STD_LOGIC;
          busy : in  STD_LOGIC;
          tx : out  STD_LOGIC_VECTOR (7 downto 0);
          rx : in  STD_LOGIC_VECTOR (7 downto 0);
          transmit : out  STD_LOGIC;
          loadFromRXBuf : out  STD_LOGIC;
          loadToTxBuf : out  STD_LOGIC;
          rxBufEmpty : in  STD_LOGIC;
          rxBufFull : in  STD_LOGIC;
          txBufEmpty : in  STD_LOGIC;
          txBufFull : in  STD_LOGIC;
          led: out STD_LOGIC_VECTOR (7 downto 0);
          uartTXData: out STD_LOGIC_VECTOR(7 downto 0);
          uartRXData: in STD_LOGIC_VECTOR(7 downto 0);
          uartTXSig: out STD_LOGIC;
          uartTXRdy: in STD_LOGIC   ;
          uartRXCont: out STD_LOGIC;
          uartRXSig: in STD_LOGIC;
          uartRXFrameError: in STD_LOGIC
          );
end FSM_SPI_buf;

architecture Behavioral of FSM_SPI_buf is
type statex is (start,updateTX, closeTX, send,openRX, receive, closeRX,
sendUART,closeUART, stop);
signal state: statex:=start;
signal counter: integer range 0 to 5 := 0;
signal bytes:STD_LOGIC_VECTOR(31 downto 0)  := x"030001FF";
signal bytes_rec:STD_LOGIC_VECTOR(31 downto 0):=x"03040506";
begin

process(clk, reset) begin

if(clk'event and clk = '1') then
        case state is
            when start =>
                if(increase = '0') then
                    state <= updateTX;
                    counter <= 0;
                    uartrxCont <= '1';
                else
                    state <= start;
                end if;
            when updateTX =>
                if(counter < 4) then
                    loadToTxBuf <= '1';
                    tx<=bytes(31 - counter * 8 downto 32 - (counter+1) * 8);
                    counter <= counter + 1;
                    state <= closeTX;
                else
                    state <= send;
                end if;
            when closeTX =>                     
                    loadToTxBuf <= '0';
                    state <= updateTX;
            when send =>
                transmit <= '1';
                counter <= 0;
                if (rxbuffull = '1') then
                    state <=openRX;
                end if;
            when openRX =>
                transmit <= '0';
                if(counter < 4) then
                    loadFromRxBuf <= '1';
                    state <=closeRX;
                else
                    counter <= 0;
                    state <= sendUART;
                end if;
            when closeRX =>

                loadFromRXBuf <= '0';
                state <= receive;
            when receive =>
                bytes_rec(31 - (counter) * 8 downto 32 - (counter+1) * 8)<=rx;
                counter <= counter + 1;
                state <= openRX;
            when sendUART =>
                if(counter < 4) then
                    if uarttxRdy = '1' then
                        uarttxData <=bytes_rec(31 - (counter) * 8 downto 32 - (counter+1) * 8);
                        uarttxSig <= '1';
                        counter <= counter + 1;
                        state <= closeUART;
                    end if;

                else
                    state <= stop;
                end if;
            when closeUART =>
                if (uarttxRdy= '0') then
                    uarttxSig <= '0';
                    state <= sendUART;
                else
                    state <= closeUART;
                end if;
            when stop =>
                if (uarttxRdy= '0') then
                    uarttxSig <= '0';
                end if;
                if(increase = '1') then
                    state <= start;
                else
                    state <= stop;
                end if;
        end case;
elsif(reset = '1') then
     counter <= 0;
     state <= updateTX;
end if;

end process;

end Behavioral;

Here is an excerpt from SPI module where I shift out received bytes

if(rx_upd <='0' and loadFromRxBuf ='1') then
        rx_upd <='1';
        rx <= rx_buffer(d_width*buffer_size-1 downto d_width*(buffer_size-1));
        rx_buffer<= rx_buffer(d_width*(buffer_size-1)-1 downto 0) & x"00";
elsif(rx_upd ='1' and loadFromRxBuf ='0') then
        rx_upd <='0';
end if;

Judging by the simulation bytes_rec transitions from its starting value to x"FFFFFFFF" (miso is always high) before UART transmission occurs.

Simulation Screenshot

But when I upload generated bit file to my FPGA (XC6SLX9 on Mojo Board v3) I receive only zeroes via UART even when I tie miso to 3.3v source. I've checked UART implementation that I am using by sending signal "bytes" through it and it's working just fine so I don't think it is to blame.

It's my first time programming FPGA if you don't count a few tutorials I've replicated so I expect error to be attributed to that. But please point me to the possible source of it. I can provide other parts of my code should the need arise. Thank you in advance!

Answer 1

This is pretty good code, overall, you've dodged a lot of potential bullets by writing this as a single process state machine with the correct sensitivity list.

I'd make one observation, that the Transmitter and Receiver halves of a UART would normally be separate state machines in separate processes, to allow full duplex communication. However I'm not clear if this is actually a UART as the state names suggest, or an SPI (synchronous) interface so that observation may not be relevant.

However, there is one crucial error, which applies to increase, rxBufFull, uarttxRdy at least, possibly others.

Let's look at the closeUart state.

        if uarttxRdy = '0' then
            uarttxSig <= '0';
            state <= sendUART;
        -- else
        --    state <= closeUART;
        end if;

(not the main point, but I've commented out lines that are redundant since the current value of state is otherwise preserved - and unnecessary code is usually a bad idea, if only from adding verbosity and clutter. I've also cleaned up the C-style line noise around the boolean expression)

The mistake here is that uarttxRdy is apparently an external input to the FPGA, not synchronised to clk.

As such, the usual guarantees on signal timing and synchronous design, as described here, don't apply to it.

So you expect two outcomes from this statement : either nothing happens, or uarttxRdy is cleared and you start sending.

But imagine that the synthesis process makes the quite legitimate transformation (to combine logic and save gates, or shorten a critical signal path) to:

        if uarttxRdy= '0' then
            uarttxSig <= '0';
        end if;

        if uarttxRdy= '0' then
            state <= sendUART;
        end if;

and the signal delay from your input pin to each of these comparisons is different.

I think you can see there are now four possible outcomes, including starting to Send without clearing uarttxSig, or clearing it and remaining in this state for at least one more clock cycle. Either of these can royally screw up your state machine.

So it's clear that asynchronous inputs need special treatment, to bring them into the synchronous domain.

Simply copy them on a clock edge, and use the synchronous copies. You can either do this in a separate synch process, but it's also perfectly safe and normal practice to add "default actions" before the Case statement in a state machine. For example:

if rising_edge(clk) then
   -- default actions : synch external inputs
   uarttxRdy_s <= uarttxRdy;
   -- state machine
   case state is

   when closeUART =>
      if uarttxRdy_s = '0' then

This problem is often mis-labelled "metastability" but it isn't : it's merely a consequence of different signal path lengths to different uses of an asynchronous input.

Metastability occurs when the asynchronous input arrives at (almost) precisely the same moment as the clock edge - within femtoseconds - so the register doesn't see a valid '0' or '1' logic level, and latches that non-level. It's like balancing a ball bearing on the point of a pin, and the ball will fall off sooner or later.

With current FPGAs it has a vanishingly small probability, but if you're still worried, add a second stage of synchronisation on each external signal.