VHDL weird bit errors seemingly makes no sense

Question

I have a Micro-Nova FPGA dev board with a Xilinx Spartan-3A. I am trying to make it communicate bits over GPIO on a raspberry pi using 3 pins: REQ, ACK, DATA. The code works fine if I uncomment the bit_data assignments and comment out "bit_data := data_out(data_ofs);" and on the Pi i get a continous 10101010... etc. But if I leave the code as pasted below I get the wrong bits at random times eg. 1010110100... etc.

Any idea what could be the problem?:

VHDL on FPGA:

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;

entity xclock is

Port(CLK : in STD_LOGIC;
     REQ : in STD_LOGIC;
     ACK : out STD_LOGIC;
     DATA : out STD_LOGIC);

end xclock;

architecture Behavioral of xclock is

begin

    process(CLK)

        variable data_ofs : integer range 0 to 2 := 0;
        variable data_out : std_logic_vector(1 downto 0) := "01";
        variable bit_data : std_logic := '0';
        variable ack_data : std_logic := '0';
        variable LASTREQ : std_logic := '0';
        variable seconds : integer range 0 to 50000000 := 0;
        variable tick : integer range 0 to 50000000 := 0;

    begin

        if CLK'event and CLK = '1' then

            tick := tick + 1;
            if tick = 49999999 then
                tick := 0;
                seconds := seconds + 1;
                if seconds = 49999999 then
                    seconds := 0;
                end if;
            end if;

            if seconds > 1 then
                if REQ /= LASTREQ and REQ /= ack_data then
                    LASTREQ := REQ;
                    if REQ = '1' then
                        --bit_data := '1';
                        ack_data := '1';
                    else
                        --bit_data := '0';
                        ack_data := '0';
                    end if;
                    bit_data := data_out(data_ofs);
                    data_ofs := data_ofs + 1;
                    if data_ofs = 2 then
                        data_ofs := 0;
                    end if;
                end if;
            end if;

            DATA <= bit_data;
            ACK <= ack_data;

        end if;

    end process;

end Behavioral;

Bash script on Pi:

REQ=27
ACK=17
DATA=22

gpio -g mode $REQ out
gpio -g mode $ACK in
gpio -g mode $DATA in
gpio -g write $REQ 1

e=1

while [ 1 ]; do
  while [ 1 ]; do
    if [ `gpio -g read $ACK` -eq 1 ]; then
        while [ 1 ]; do
            d=`gpio -g read $DATA`
            echo $d
            if [ $d -ne $e ]; then
                echo error DATA should be $e
                sleep 1
            else
                if [ $e -eq 0 ]; then
                    e=1
                else
                    e=0
                fi
                break
            fi
        done
        gpio -g write $REQ 0
        break;
    fi
  done
  while [ 1 ]; do
    if [ `gpio -g read $REQ` -eq 0 ]; then
        while [ 1 ]; do
            d=`gpio -g read $DATA`
            echo $d
            if [ $d -ne $e ]; then
                echo error DATA should be $e
                sleep 1
            else
                if [ $e -eq 0 ]; then
                    e=1
                else
                    e=0
                fi
                break
            fi
        done
        gpio -g write $REQ 1
        break;
    fi
    sleep 1
    done
  done

Any help will be greatly appreciated. Pulling my hair out on this one!

Without getting into detail of what the code should be doing, it sounds like a metastability issue. E.g. you are using some of the top modules input signals in a clocked process without any synchronization. Your VHDL coding style is also very "C-ish". — andrsmllr
if metastability is the case why does it get triggered by using bit_data := data_out(data_ofs);? and how can I get around this issue? also how would this be written "VHDL-ish"? — user1446688
One of the main reason you have issues with your design is that you are using variables in a way that many of the logic in your code are implemented in latches. The rule of thumb when writing code for fpga fpgais to avoid using variables. — FarhadA
Have you simulated your design? Does it work as you would expect in simulation? — Josh

Josh Josh · Accepted Answer · 2016-02-07T22:34:28

I spent some time trying to figure out how your code is supposed to work (what the protocol is), but I gave up. Building off what other have suggested, you might try to make the following modifications and see if it makes any difference.

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;

entity xclock is

port(CLK : in STD_LOGIC;
     REQ : in STD_LOGIC;
     ACK : out STD_LOGIC;
     DATA : out STD_LOGIC);

end xclock;

architecture Behavioral of xclock is

   signal req_reg1, req_reg2, req_int, last_req : std_logic := '0';
   signal bit_data_reg, ack_reg                 : std_logic;

begin

    process(CLK)

        variable data_ofs : integer range 0 to 2 := 0;
        variable data_out : std_logic_vector(1 downto 0) := "01";
        variable bit_data : std_logic := '0';
        variable ack_data : std_logic := '0';
        variable seconds : integer range 0 to 50000000 := 0;
        variable tick : integer range 0 to 50000000 := 0;

    begin

        if CLK'event and CLK = '1' then

            -- Input registers help prevent metastability on REQ line
            -- Note that if there is significant bounce or glitching, then
            -- you will also need a deglitcher.
            req_reg1 <= REQ;
            req_reg2 <= req_reg1;
            req_int  <= req_reg2;

            tick := tick + 1;
            if tick = 49999999 then
                tick := 0;
                seconds := seconds + 1;
                if seconds = 49999999 then
                    seconds := 0;
                end if;
            end if;

            if seconds > 1 then
                -- Using the registered inputs instead of the direct REQ input.
                if req_int /= last_req and req_int /= ack_data then
                    last_req <= req_int;
                    if req_int = '1' then
                        ack_data := '1';
                    else
                        ack_data := '0';
                    end if;
                    bit_data := data_out(data_ofs);
                    data_ofs := data_ofs + 1;
                    if data_ofs = 2 then
                        data_ofs := 0;
                    end if;
                end if;
            end if;

            -- Register outputs to ensure consistent OREG packing, if enabled
            bit_data_reg <= bit_data;
            ack_reg      <= ack_data;

            DATA         <= bit_data_reg;
            ACK          <= ack_reg;

        end if;

    end process;

end Behavioral;

The modifications to the code do two things:

Register the input REQ a couple times to reduce the probability of metastability, and then only use the registered values instead of the pin itself. (Read more)
Register the outputs (twice) to ensure that the output register can be placed in an OREG (in IOB), which ensures consistent clock-to-out timing at the output pad. At the boundaries of your device, when possible, it is nice to place a plain FF register stage which guarantees that packing into OREG is possible. Otherwise there may be a feedback path from last register stage back into your design which prevents packing.

VHDL weird bit errors seemingly makes no sense

1 Answers