Buggy VHDL UART receiver

fpgauartvhdl

I'm trying to get a simple UART receiver working and am close but I'm receiving slightly off values (some bits are shifted a couple places from where they should be).

Based on this answer I tried running RX through a flip flop (not sure I did that correctly) and oversampled RX. My target baud rate is 2500000, so that's 400ns per baud and 20 samples per baud (FPGA clock is 50 MHz).

Can anyone see what I'm doing wrong? Am I putting RX in the flip flop and oversampling correctly?

entity uart is port(
    clk_50 : in std_logic;
    reset : in std_logic;
    tx : out std_logic;
    rx : in std_logic;
    data_in : out std_logic_vector(7 downto 0); -- data being received
    data_out : in std_logic_vector(7 downto 0); -- data being sent
    leds    : out std_logic_vector(7 downto 0));    
end uart;

architecture behavioral of uart is

signal sample_counter : unsigned(7 downto 0);
signal data_count : unsigned(3 downto 0);
type uart_state_t is (IDLE, START, DATA, STOP, STALL);
signal uart_state: uart_state_t;
signal next_uart_state: uart_state_t;

signal rx_data : std_logic_vector(7 downto 0);

signal rx_d : std_logic;

begin

process(clk_50, reset)
begin
    if (reset = '1') then
        uart_state <= START;
        data_count <= (others => '0');
        sample_counter <= (others => '1');
        leds <= (others => '0');
        rx_data <= (others => '0');

    elsif (rising_edge(clk_50)) then
        uart_state <= next_uart_state;
        rx_d <= rx;

        if (next_uart_state = START) then
            data_count <= (others => '1');

            if (sample_counter = x"ff") then
                if (rx_d = '0') then
                    sample_counter <= sample_counter + 1;
                end if;
            elsif (sample_counter = x"31") then -- 20 samples per baud
                    sample_counter <= (others => '1');
            else
                    sample_counter <= sample_counter + 1;
            end if;
        end if;             
        elsif (next_uart_state = DATA) then         
            if (sample_counter = x"0a") then    -- check rx at sample 10
                rx_data(to_integer(data_count)) <= rx_d;
                leds(to_integer(data_count)) <= rx_d;
                sample_counter <= sample_counter + 1;
            elsif (sample_counter = x"13") then
                sample_counter <= (others => '1');
                data_count <= data_count + 1;
            else
                sample_counter <= sample_counter + 1;
            end if;

        elsif (next_uart_state = STOP) then
            if (sample_counter = x"0a") then    -- check rx at sample 10
                data_in <= rx_data;
                sample_counter <= sample_counter + 1;
            elsif (sample_counter = x"13") then
                sample_counter <= (others => '1');
            else
                sample_counter <= sample_counter + 1;
            end if;
        end if;
    end if;
end process;

process(uart_state, reset, data_count, sample_counter)
begin
    if (reset = '1') then
        next_uart_state <= START;
    end if;
    case uart_state is
    when START =>
        if (sample_counter = x"13") then
            next_uart_state <= DATA;
        else
            next_uart_state <= START;
        end if;
    when DATA =>
        if (data_count = x"8" and sample_counter = x"13") then
            next_uart_state <= STOP;
        else
            next_uart_state <= DATA;
        end if;
    when STOP =>
        if (sample_counter = x"13") then
            next_uart_state <= START;
        else
            next_uart_state <= STOP;
        end if;
    when others =>
        next_uart_state <= START;
    end case;
end process;
end behavioral;

e: I think I got it – I reduced the baud rate to 1000000 and changed the logic for the START state.

Best Answer

Your edit doesn't make any sense — you've changed the timing for the start bit, but not for any of the other bits. Furthermore, you only changed it for the clocked process, but not the unclocked process that actually determines the value of next_uart_state.

The main problem with your original code is your counters. For some reason, you keep resetting them to all-ones instead of all zeros. This adds an extra state to each counting sequence.

For example, your sample_count runs from 0xFF (-1) through 0x13 (+19), which is a total of 21 states, not 20 as your comments imply. This creates a 5% error in the sampling rate, which is enough to create a problem after one or two bytes have been received.

In addition, your code waits for the full period of the stop bit before going back to look for the next start bit, which means that it can never "catch up". Instead, you should verify the value of the stop bit halfway through, and then immediately go back to the START state to look for the leading edge of the next start bit. If you do this, then even if the receive clock is a little slow with respect to the transmit clock, it will be able to keep up.

Finally, I've seen lots of ways to code state machines in HDL, but this one really takes the cake. Why is your clocked process casing on next_uart_state instead of uart_state? This creating a lot of strange boundary cases that will confuse anyone reading this code — and I think you have even confused yourself.

Related Topic