0
\$\begingroup\$

update: I will rewrite my whole VHDL-Statemachine because I had just trouble with the basics of FSM. I will try to find a easier FSM for this.

I have a state machine and as I really forced to use a state machine I maybe didn't check some LATCH issues you discovered.

This results in:

In one state I have a counter which should only increment by 1. But:

My counter increments by 2. Why?

How is that possible?


My Code Ressources:

Here is my checker-module:

https://paste.ofcode.org/DrbFJc78h84dRFz7DvVGXQ

Here is my generator-module:

https://paste.ofcode.org/xhkssfnbBukhQyM7QcpqMj

Here is the dedicated crc calculator:

https://paste.ofcode.org/ALUBzZCUrvTcdVRmKGnv5U

Here is my testbench you asked for:

https://paste.ofcode.org/354Lk9k7NTNk75NXrN9ymSw


I want that it only increases by one if

  • my crc was right ( my axi checker checks all incoming frame data and calculates the crc, then it compares it with the last 4 Bytes of the Frame [which is also a crc].)
  • it is in the state data check

This simulation-foto shows my problem:

enter image description here

signal identic_crc_in_frame_cnt : std_logic_vector (10 downto 0 ):= (others => '0');

enter image description here

I tried another counter, and now it always adds 4.

signal testcnt : unsigned (3 downto 0):= X"0";
testcnt <= testcnt +2 ;
I have a sync method....

SYNC_PROC2 : PROCESS (axi_reset, m_axis_clk, m_axis_tkeep, m_axis_tvalid, m_axis_tlast)
    BEGIN
        IF rising_edge(m_axis_clk) THEN
 
 
 
            IF (axi_reset = '1') THEN
                -- reset..
                current_state <= IDLE;
                --received_frame_cnt <= (others => '0');
 
            ELSE
                -- normal mode
                
                current_state <= next_state;

                IF (current_state = CHECK_ADR1) THEN
                    column_data_correct_cnt <= (OTHERS => '0');
                ELSE
                END IF;
 

And this would be a part of the case


 WHEN CHECK_DATA => 
                         --- Check tdata only if tvalid = 1 , because tvalid means MAC wants to send valid data
                         IF (m_axis_tvalid = '0') THEN


IF (m_axis_tlast = '1') THEN
                                    next_state <= CHECK_ADR1;
                                    -- do this maybe somewhere else:
                                        crc_calc_enabled <= '0';
                                        if ( m_axis_tdata(31 downto 0) = prev_prev_CRC_REG_tb(31 downto 0)) then
                                            crc_ok <= '1';
                                            error_flag_crc <= '0';
                                            identic_crc_in_frame_cnt <= identic_crc_in_frame_cnt + '1';
                                            testcnt <= testcnt +3 ;
else
                                            crc_ok <= '0';
                                            error_flag_crc <= '1';
                                        end if;


After the code below it should go to next state...
IF (m_axis_tlast = '1') THEN
                                    next_state <= CHECK_ADR1;

I changed SYNC_PROC to (like u said)

    SYNC_PROC2 : PROCESS ( m_axis_clk)
    BEGIN
        IF rising_edge(m_axis_clk) THEN

  -- AXI pattern Checker
   my_axi_checker : AXI_CHECKER port map (
     m_axis_tkeep => tkeep,
    m_axis_tvalid => tvalid,
    m_axis_tdata => taxi_data_tb,
    m_axis_tready => open, -- not needed
    m_axis_clk  => m_axis_clk, 
    axi_start => start,
    axi_reset => axi_reset,
    m_axis_tlast => tlast  
  ); 
--- start the axi interface tb
axi_reset <= '0' after 10 ns;
start <= '1' after 12 ns;                    -- Starts an axi_stream transaction
-- 
--  axi_data (63 downto 0)<= axi_data_content(63 downto 0); -- axi data 
--  m_axis_tdata   <= std_logic_vector(axi_data); -- data is converted to std logic vector and layed onto signal!
 -- generates testbench clock: 
axiclk : process 
    begin
    wait for 10 ns;
    m_axis_clk <= '0';
    wait for 10 ns;
    m_axis_clk <= '1';

end process axiclk; 


On this picture you can see tlast and the testebnch clk and the two counters...( increment should be like in the code - anyway, its always double xD)

enter image description here enter image description here enter image description here

After I updated my code it gets stranger :D

enter image description here

\$\endgroup\$
9
  • \$\begingroup\$ Need to provide more context. Maybe the process is triggered twice or the way it is checked is not correct. \$\endgroup\$
    – Joan
    May 19 at 9:19
  • \$\begingroup\$ I added a link to the vhd file above.. I never thought, that it would add two times.. how is that possible. \$\endgroup\$
    – t1fpga1
    May 19 at 9:28
  • \$\begingroup\$ To start with, process SYNC_PROC2 is sensitive to a load of signals (axi_reset, m_axis_clk, m_axis_tkeep, m_axis_tvalid, m_axis_tlast) but is actually sensitive to only one: m_axis_clk. You should take the rest out of the sensitivity list. Next, please edit your question to show (not a link) the source of m_axis_clk. \$\endgroup\$
    – TonyM
    May 19 at 9:29
  • \$\begingroup\$ it still counts up by 2 xD \$\endgroup\$
    – t1fpga1
    May 19 at 9:33
  • \$\begingroup\$ Yes - I didn't say it would fix the problem, it won't. You can delete that section on it you added to your question, it's misleading. But do fix your original source file. Then as asked, please edit your question to show (not a link) the source of m_axis_clk. Thanks. \$\endgroup\$
    – TonyM
    May 19 at 9:35

1 Answer 1

4
\$\begingroup\$

You have latches everywhere due to unassigned signals throughout the if-else statements within your case.

For example, even your next state is a latch.

enter image description here

Make sure signal assignments are handled in every path. Eg: try changing to something like this:

      WHEN CHECK_ADR2 => 
        
            IF (m_axis_tvalid = '1' AND m_axis_tkeep = X"FF") THEN
                IF (FRAME_PART2 = m_axis_tdata) THEN
                    expected_left_cur_framebytes <= expected_left_cur_framebytes - 2;
                    --- this can be processed by the CRC-CHECKER
                    crc_calc_enabled <= '1';
                    
                    next_state <= CHECK_DATA;  -- this is the only line that you had included
                    DETECTED_ADR2 <= '1';
                else
                    next_state <= CHECK_ADR2; -- <--- added
                END IF;
            else
                next_state <= CHECK_ADR2; -- <--- added
            END IF;
            

This is true for all such signals. Above is just an example. You didn't post your benchtest code, so I couldn't run it. But I found that your counter is also generated via latch:

enter image description here

I suspect that, if you clean all of this up so that you have everything properly clocked, your design will run in a much more predictable way.

edit:
Incidentally, Vivado wouldn't let me use this (from your code):

expected_col_data(15 DOWNTO 0)  <= std_logic_vector( - 11 + 0 + to_unsigned(4 * to_integer(received_cur_framecolumn_cnt + 1), 16));
expected_col_data(31 DOWNTO 16) <= std_logic_vector( - 11 + 1 + to_unsigned(4 * to_integer(received_cur_framecolumn_cnt + 1), 16));
expected_col_data(47 DOWNTO 32) <= std_logic_vector( - 11 + 2 + to_unsigned(4 * to_integer(received_cur_framecolumn_cnt + 1), 16));
expected_col_data(63 DOWNTO 48) <= std_logic_vector( - 11 + 3 + to_unsigned(4 * to_integer(received_cur_framecolumn_cnt + 1), 16));

so, I changed it to this:

expected_col_data(15 DOWNTO 0)  <= std_logic_vector(x"fff5" + to_unsigned(4 * to_integer(received_cur_framecolumn_cnt + 1), 16));
expected_col_data(31 DOWNTO 16) <= std_logic_vector(x"fff6" + to_unsigned(4 * to_integer(received_cur_framecolumn_cnt + 1), 16));
expected_col_data(47 DOWNTO 32) <= std_logic_vector(x"fff7" + to_unsigned(4 * to_integer(received_cur_framecolumn_cnt + 1), 16));
expected_col_data(63 DOWNTO 48) <= std_logic_vector(x"fff8" + to_unsigned(4 * to_integer(received_cur_framecolumn_cnt + 1), 16));

I'm guessing it's functionally identical?

\$\endgroup\$
4
  • \$\begingroup\$ I will check your first steps !!! (ah I wont use the expected col data, because I now use crc now :P I was just scarred to delete my masterpiece xD) \$\endgroup\$
    – t1fpga1
    May 20 at 2:24
  • \$\begingroup\$ hmm tlast needs to be high and also the calculated crc must be like the crc in the current frame.. only then it should increment. but it increments three times . lol I should rewrite my whole checker method. it just doesnt make any sense. \$\endgroup\$
    – t1fpga1
    May 20 at 6:20
  • \$\begingroup\$ That means I will try it now completly different. Maybe I just should check every clock cycle which data are available. I will do it now and post my new code ! \$\endgroup\$
    – t1fpga1
    May 20 at 6:38
  • \$\begingroup\$ @t1fpga1 I still count 4 or 5 latches in your new code. Again, I was just showing one example of what should be fixed - I was leaving it to you to fix the others. Think of a unclocked if-statement as a mux generator for any one signal. If you don't give the signal an assignment for every if-else clause, you are requiring the signal to have memory (i.e. to remember what it used to be)... hence a latch. For instance, what is 'crc_calc_enabled' if m_axis_tvalid is not equal to 1? What is identic_crc_in_frame_cnt if tlast is not 1? Or if your tdata is not equal to your prev_prev_CRC_REG_tb? \$\endgroup\$ May 20 at 10:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.