Search code examples
serial-portvhdl

VHDL RS232 Receiver not working correctly with Xilinx ISE


So i have this receiver code for the RS232 communication link, I'm supposed to send 8 bits with 1 start bit "0" and one stop bit "1", no parity check bit, I have tried with those code in most kinds of ways but the simulation never worked correctly, even though some people told me my problem is the testbench not the code but it never works on the FPGA implementation, the first signal i sent is always wrong where as any signal after that is correct.

here is the code below

    entity Rs232Rxd is

        port( Reset, Clock16x, Rxd: in std_logic; 

        DataOut1: out std_logic_vector (7 downto 0));

        end Rs232Rxd;

 architecture Rs232Rxd_Arch of Rs232Rxd is 

attribute enum_encoding: string;

-- state definitions

type stateType is (stIdle, stData, stStop, stRxdCompleted);

attribute enum_encoding of statetype: type is "00 01 11 10";

signal iReset : std_logic;

signal iRxd1, iRxd2 : std_logic := '1';

signal presState: stateType; 

signal nextState: stateType;

signal iClock1xEnable, iClock1x, iEnableDataOut: std_logic :='0' ; 

signal iClockDiv: std_logic_vector (3 downto 0) := (others=>'0') ;

signal iDataOut1, iShiftRegister: std_logic_vector (7 downto 0):= (others=>'0');

signal iNoBitsReceived: std_logic_vector (3 downto 0):= (others=>'0') ;

begin

process (Clock16x) begin

        if rising_edge(Clock16x) then 

            if Reset = '1' or iReset = '1' then

                iRxd1 <= '1';

                iRxd2 <= '1';

                iClock1xEnable <= '0'; 

                iClockDiv <= (others=>'0');

            else

                iRxd1 <= Rxd; 

                iRxd2 <= iRxd1;

            end if;

            if iRxd1 = '0' and iRxd2 = '1' then 

                iClock1xEnable <= '1';

            end if;

            if iClock1xEnable = '1' then

                iClockDiv <= iClockDiv + '1';

        end if;

        end if;

end process;


iClock1x <= iClockDiv(3);

process (iClock1xEnable, iClock1x) 

begin

    if iClock1xEnable = '0' then 

            iNoBitsReceived <= (others=>'0');

            presState <= stIdle;

    elsif rising_edge(iClock1x) then

                iNoBitsReceived <= iNoBitsReceived + '1';

                presState <= nextState;

                if iEnableDataOut = '1' then

                iDataOut1 <= iShiftRegister;

                --iShiftRegister <= (others=>'0');

                    else

                        iShiftRegister <= Rxd & iShiftRegister(7 downto 1);

            end if;
        end if;

end process;

DataOut1 <= iDataOut1;

process (presState, iClock1xEnable, iNoBitsReceived) 

begin

-- signal defaults 

iReset <= '0';

iEnableDataOut <= '0';


case presState is

    when stIdle =>

    if iClock1xEnable = '1' then

    nextState <= stData;

    else
        nextState <= stIdle;

    end if; 

    when stData =>

    if iNoBitsReceived = "1000" then

    iEnableDataOut <= '1';

    nextState <= stStop;

    else

    iEnableDataOut <= '0'; 

    nextState <= stData;

    end if; 
    when stStop =>   

    nextState <= stRxdCompleted; 

    when stRxdCompleted =>

    iReset <= '1';

    nextState <= stIdle;

    end case; 

end process;

end Rs232Rxd_Arch;

Solution

  • Your question doesn't present a Minimal Complete and Verifiable Example. The problem cannot be duplicated without writing a testbench and your problem lacks specificity ('signal' and 'wrong' used here are imprecise).

    There are some observations.

    A stop bit followed by a start bit of a successive character leaves no room for state stRxdCompleted. Also iNoBitsReceived is not set to all '0's when iClock1xEnable goes invalid, meaning the sampling point is not determined by the falling edge of the start bit for successive characters:

    rs232rxd_orig_tb.png

    This is a capital 'A' immediately followed by a lower case 'a', the stop bit immediately followed by the start bit of the second character (which is legal).

    In the first character you see that the start bit is counted as one of the character bits.

    You also see that the bit counter isn't reset when the enable goes invalid, which will cause sampling point drift (and may eventually cause sampling errors depending on clock differences or transmission distortion and lack of isosynchronous sampling point reset).

    You also see presState is stStop during the last data bit of the first character and yet the second character is correct. Looking a bit closer we see the start bit of the first character occurs during stData and that doesn't occur for the second character.

    There's a basic issue with number of states and state transitions when iClock1x is stopped.

    You don't need the state machine, you have the counter named iNoBitsReceived that can store all the state should ishiftregister be long enough to accommodate start (and possibly stop) bits should you also detect framing errors.

    Tying operations to specific counts without a separate state machine, along with clearing the bit counter when idle:

    rs232_tb.png

    Gives us something that works with a bit less complexity:

    library ieee;
    use ieee.std_logic_1164.all;
    use ieee.numeric_std.all;
    
    entity Rs232Rxd is
        port ( 
            Reset, 
            Clock16x, 
            Rxd:        in  std_logic; 
            DataOut1:   out std_logic_vector (7 downto 0)
        );
    end entity Rs232Rxd;
    
    architecture foo of Rs232Rxd is
        signal rxd1:                std_logic;
        signal rxd2:                std_logic;
        signal baudctr:             unsigned (3 downto 0);
        signal ctr16x:              unsigned (3 downto 0);
        signal enab1xstart:         std_logic;
        signal enable1x:            std_logic;
        signal ninthbit:            std_logic;
        signal sampleenab:          std_logic;
        signal shiftregister:       std_logic_vector(7 downto 0);
    
    begin
    CLOCK_DOMAIN:
        process (clock16x)
        begin
            if rising_edge(clock16x) then
                rxd1 <= rxd;
                rxd2 <= rxd1;
            end if;
        end process;
    
        enab1xstart <= not rxd1 and rxd2 and not enable1x;
    
    ENABLE_1X:
        process (clock16x, reset)
        begin
            if reset = '1' then
                enable1x <= '0';
            elsif rising_edge(clock16x) then
                if enab1xstart = '1' then
                    enable1x <= '1';
                elsif ninthbit = '1' then
                    enable1x <= '0';
                end if;
            end if;
        end process;
    
    SAMPLE_COUNTER:
        process (clock16x, reset, ninthbit)
        begin
            if reset = '1' or ninthbit = '1' then
                ctr16x <= (others => '0');   -- for simulation
            elsif rising_edge(clock16x) then
                if enab1xstart = '1' or enable1x = '1' then
                    ctr16x <= ctr16x + 1;
                end if;
            end if;
        end process;
    
        sampleenab <= not ctr16x(3) and ctr16x(2) and ctr16x(1) and ctr16x(0);
    
    BAUD_COUNTER:
        process (clock16x, reset)
        begin
            if reset = '1' then
                baudctr <= (others => '0');
            elsif rising_edge(clock16x) and sampleenab = '1' then
                if baudctr = 8 then
                    baudctr <= (others => '0');
                else
                    baudctr <= baudctr + 1;
                end if;
            end if;
        end process;
    
    NINTH_BIT:  -- one clock16x period long, after baudctr changes 
        process (clock16x, reset)
        begin
            if reset = '1' then
                ninthbit <= '0';
            elsif rising_edge(clock16x) then
                ninthbit <= sampleenab and     baudctr(3) and not baudctr(2) and 
                                           not baudctr(1) and not baudctr(0);
            end if;
        end process;
    
    SHIFT_REG:
        process (clock16x, reset)
        begin
            if reset = '1' then
                shiftregister <= (others => '0'); -- for pretty waveforms
            elsif rising_edge(clock16x) and sampleenab = '1' then
                shiftregister <= rxd2 & shiftregister(7 downto 1);
            end if;
        end process;
    
    OUTREG:
        process (clock16x, reset)
        begin
            if reset = '1' then
                dataout1 <= (others => '0');
            elsif rising_edge(clock16x) and ninthbit = '1' then
                dataout1 <= shiftregister;
            end if;
        end process;
    
    end architecture;
    

    VHDL basic identifiers are case insensitive, and the names weren't particularly enlightening. The format of the two above waveforms indicates name changes handily.

    If you extend the shift register length by one or two you can detect framing errors during the stop bit. Changing the shift register length would require slicing the shift register output for writing to your data output.

    Note this architecture is written to use package numeric_std and not Synopsys package std_logic_arith. You also didn't provide the context clause preceding the entity declaration.

    This architecture also produces enables and uses the 16x clock instead of producing a 1x clock.

    It was written after finding the amount of changes to correct issues in the original architecture seemed overwhelming. (When in doubt, start over.)

    This testbench was used:

    library ieee;
    use ieee.std_logic_1164.all;
    
    entity rs232rxd_tb is
    end entity;
    
    architecture foo of rs232rxd_tb is
        signal reset:       std_logic := '0';
        signal clock16x:    std_logic := '0';
        signal rxd:         std_logic := '1'; 
        signal dataout1:    std_logic_vector (7 downto 0);
    begin
    DUT:
        entity work.rs232rxd
            port map (
                reset => reset,
                clock16x => clock16x,
                rxd => rxd,
                dataout1 => dataout1
            );
    CLOCK:
        process
        begin
            wait for 3.255 us;    -- 16X clock divided by 2, 9600 baud 104.16 us
            clock16x <= not clock16x;
            if now > 2.30 ms then
                wait;
            end if;
        end process;
    
       STIMULI:
        process
        begin
            wait for 6.51 us;
            reset <= '1';
            wait for 13.02 us;
            reset <= '0';
            wait for 13.02 us;
            wait for 40 us;
            rxd <= '0';
            wait for 104.16 us;  -- start bit
            rxd <= '1';
            wait for 104.16 us;  -- first data bit, bit 0 =  '1'
            rxd <= '0';
            wait for 104.16 us;  -- second data bit, bit 1 = '0'
            rxd <= '0';
            wait for 104.16 us;  -- third data bit, bit 2 = '0';
            wait for 104.16 us;  -- fourth data bit, bit 3 = '0';
            wait for 104.16 us;  -- fifth data bit, bit 4 = '0';
            wait for 104.16 us;  -- sixth data bit, bit 5 = '0';
            rxd <= '1';
            wait for 104.16 us;  -- seventh data bit, bit 6 = '1';
            rxd <= '0'; 
            wait for 104.16 us;  -- eigth data bit, bit 7 = '0';
            rxd <= '1'; 
            wait for 104.16 us;  -- stop bit ( = '1')
            --wait for 104.16 us;  -- idle 
            rxd <= '0';
            wait for 104.16 us;  -- start bit
            rxd <= '1';
            wait for 104.16 us;  -- first data bit, bit 0 =  '1'
            rxd <= '0';
            wait for 104.16 us;  -- second data bit, bit 1 = '0'
            rxd <= '0';
            wait for 104.16 us;  -- third data bit, bit 2 = '0';
            wait for 104.16 us;  -- fourth data bit, bit 3 = '0';
            wait for 104.16 us;  -- fifth data bit, bit 4 = '0';
            rxd <= '1';
            wait for 104.16 us;  -- sixth data bit, bit 5 = '1';
            wait for 104.16 us;  -- seventh data bit, bit 6 = '1';
            rxd <= '0'; 
            wait for 104.16 us;  -- eigth data bit, bit 7 = '0';
            rxd <= '1'; 
            wait for 104.16 us;  -- stop bit ( = '1')
            wait;
        end process;
    end architecture;
    

    You can see that new architecture has all the same essential elements, although clocked process elements are in separate process statements.

    There is no state machine process.

    The architecture is extensible to a full feature UART receiver by playing around with separating inputs to the shift register (for parity, two stop bits, 7 data bits, etc.). Parity can be performed serially.