Why isn't this VHDL falling edge detector reliable?

Question

I'm trying to write an RS232 decoder for my Mojo v3 (Spartan 6 XC6SLX9). I know I can find existing libraries to do this - I'd just like to do it myself. As part of the decoding, I need to detect the high-to-low transition for the start bit.

Many sources, such as fpgacenter's edge detector and Pong Chu's FPGA Prototyping by VHDL Examples book, recommend something like this:

architecture arch of edge_detector is
    signal last: std_logic;
begin
    process(clk)
    begin
        if rising_edge(clk) then
            last <= signal_in;
        end if;
    end process;

    output <= (not signal_in) and last; 
end arch;

I haven't found that to be a reliable detector of falling edges. In particular, it appears to miss falling edges when the FPGA sees the edge near the rising edge of the clock:

SLOW is an edge detector that uses two registers to do the detection. Whereas FAST will fire in the same clock cycle as the falling edge, SLOW fires in the next cycle. BAD goes high when it sees SLOW fire without a corresponding tick from FAST. RAW_RX is direct from the FTDI USB-to-Serial transceiver, bypassing the FPGA; RX is that same signal as seen from inside the FPGA (and routed back out, like FAST, SLOW, etc). CLK is a 50MHz clock.

The same falling edge detector properly fires when it sees a high-to-low transition during the clock cycle:

That successful detection also illustrates why I've called SLOW slow.

What am I doing wrong? Is this detector expected to work when transitions coincide with clock rising edges?

Answer 1

"signal_in" is not timed by the development tools until it is brought into a clocked element (i.e. register). Because of this there is nothing that says "not signal_in" has to be valid before/after "last" is updated.

Therefore, when "last" and "not signal_in" are ANDed together there is essentially a race condition, where the final result can vary (between builds due to routing and between FPGAs due to variations in the specific fabric). This is why signals should always be brought onto a clock before being used in calculations.

Answer 2

You are grabbing last in the same cycle as your test, Signal_In can change between instructions.

It should be more like this.

begin
    if rising_edge(clk) then
        output <= (not signal_in) and last; 
        last <= signal_in;
    end if;
end process;