0
\$\begingroup\$

Given the following code

library ieee;
use ieee.numeric_std.all;
use ieee.std_logic_1164.all;

library unisim;
use unisim.vcomponents.all;

entity sw_ctrl_top is

  generic (
    switch_window_g        : positive := 50;
    trig_heralding_delay_g : positive := 40);

  port (
    sysclk_p : in  std_logic;
    sysclk_n : in  std_logic;
    trig     : in  std_logic;
    dl_inc   : in  std_logic;
    dl_dec   : in  std_logic;
    dl_sign  : in  std_logic;
    rst      : in  std_logic;
    d        : in  std_logic_vector(3 downto 0);
    q0       : out std_logic;
    q1       : out std_logic;
    dl_led   : out std_logic_vector(3 downto 0));

end entity sw_ctrl_top;

architecture struct of sw_ctrl_top is

  -------------------------------------------------------------------------------
  -- Internal signals
  -------------------------------------------------------------------------------
  signal rst_i            : std_logic := '0';
  signal dl_inc_i         : std_logic := '0';
  signal dl_dec_i         : std_logic := '0';
  signal dl_sign_i        : std_logic := '0';
  signal clk_s, clk_div_s : std_logic := '0';

  -----------------------------------------------------------------------------
  -- Components
  -----------------------------------------------------------------------------
  component debouncer is
    port (
      CLK         : in  std_logic;
      D           : in  std_logic;
      D_DEBOUNCED : out std_logic);
  end component debouncer;

  component clk_divider is
    generic (
      div_factor_g : positive);
    port (
      I : in  std_logic;
      O : out std_logic);
  end component clk_divider;

  component sw_ctrl is
    generic (
      switch_window_g        : positive;
      trig_heralding_delay_g : positive);
    port (
      clk     : in  std_logic;
      trig    : in  std_logic;
      dl_inc  : in  std_logic;
      dl_dec  : in  std_logic;
      dl_sign : in  std_logic;
      rst     : in  std_logic;
      d       : in  std_logic_vector(3 downto 0);
      q0      : out std_logic;
      q1      : out std_logic;
      dl_led  : out std_logic_vector(3 downto 0));
  end component sw_ctrl;

begin  -- architecture struct

  clk_buf : IBUFGDS_LVDS_25
    port map(
      O  => clk_s,
      I  => sysclk_p,
      IB => sysclk_n
      );
  clk_div : component clk_divider
    generic map (
      div_factor_g => 3)
    port map (
      I => clk_s,
      O => clk_div_s);
  inc_deb : component debouncer
    port map (
      CLK         => clk_div_s,
      D           => dl_inc,
      D_DEBOUNCED => dl_inc_i);
  dec_deb : component debouncer
    port map (
      CLK         => clk_div_s,
      D           => dl_dec,
      D_DEBOUNCED => dl_dec_i);
  sign_deb : component debouncer
    port map (
      CLK         => clk_div_s,
      D           => dl_sign,
      D_DEBOUNCED => dl_sign_i);
  rst_deb : component debouncer
    port map (
      CLK         => clk_div_s,
      D           => rst,
      D_DEBOUNCED => rst_i);
  sw_controller : component sw_ctrl
    generic map (
      switch_window_g        => switch_window_g,
      trig_heralding_delay_g => trig_heralding_delay_g)
    port map (
      clk     => clk_s,
      trig    => trig,
      dl_inc  => dl_inc_i,
      dl_dec  => dl_dec_i,
      dl_sign => dl_sign_i,
      rst     => rst_i,
      d       => d,
      q0      => q0,
      q1      => q1,
      dl_led  => dl_led);



end architecture struct;

and the relevant part of the sw_ctrl component

sw_proc : process (clk) is
    type sw_state_t is (
      idle, wait_delay, save_detectors_state,
      keep_sw_state, reset);            -- state type
    variable state_v                 : sw_state_t                         := idle;  -- current state variable
    variable switch_window_counter_v : natural range 0 to switch_window_g := 0;
    variable delay_counter_v         : natural range 0 to 7               := 0;
  begin
    if rising_edge(clk) then
      if rst = '1' then
        state_v := reset;
      end if;
      case state_v is
        when idle =>
          if trig = '0' then
            state_v := idle;
          end if;
          if trig = '1' and trig_heralding_delay_g = 0 then
            state_v := save_detectors_state;
          end if;
          if trig = '1' and trig_heralding_delay_g /= 0 then
            state_v := wait_delay;
          end if;
        when wait_delay =>
          if delay_counter_v < trig_heralding_delay_g + delay_after_trigger_s then
            delay_counter_v := delay_counter_v + 1;
            state_v         := wait_delay;
          end if;
          if delay_counter_v = trig_heralding_delay_g + delay_after_trigger_s then
            state_v := save_detectors_state;
          end if;
        when save_detectors_state =>
          state_v := keep_sw_state;
          if xor_result_s = '1' then
            switch_signal_s <= '1';
          else
            switch_signal_s <= '0';
          end if;
        when keep_sw_state =>
          if switch_window_counter_v < switch_window_g then
            switch_window_counter_v := switch_window_counter_v + 1;
            state_v                 := keep_sw_state;
          end if;
          if switch_window_counter_v = switch_window_g then
            state_v := reset;
          end if;
        when reset =>
          if trig = '0' then
            state_v := idle;
          end if;
          if trig = '1' then
            state_v := reset;
          end if;
          switch_window_counter_v := 0;
          delay_counter_v         := 0;
          switch_signal_s         <= '0';
        when others => state_v := reset;
      end case;
    end if;
  end process sw_proc;

I fail to see why the variable state_v would be unconnected. XST says that it has a constant value of 0, which I assume it means that it never leaves the idle state and also that is "unconnected". I don't see how idle could be a dead state and ISE does not really help with its messages being very vague. Also, state_v has an assignment in all the branches in the FSM logic and the rst signal is initialized to '0' in the top component (which I thought could be the only reason why XST says that state_v is always 0), so I am apparently missing something very obvious to ISE but not to me.

For clarity and completeness, delay_after_trigger_s is a natural signal defined in another process that does not trigger any errors or warnings.

Where is this "roadblock" hiding?

Edit #1

Expected results

Basically, this design is a overly-complicated XOR. The d input, which is a 4-bit vector, is fed to a XOR4 and its output is routed to both q0 and q1. So if any, and only one, of the d inputs is high, q0 and q1 are also high, and vice versa.

Testbench and simulation results

Simulating this design with the following test bench (disregard the signal delayed_photons, as it is not connected to the uut)

ENTITY tb IS
END tb;

ARCHITECTURE behavior OF tb IS 

    -- Component Declaration for the Unit Under Test (UUT)

    COMPONENT sw_ctrl_top
    PORT(
         SYSCLK_P : IN  std_logic;
         SYSCLK_N : IN  std_logic;
         TRIG : IN  std_logic;
         DL_INC : IN  std_logic;
         DL_DEC : IN  std_logic;
            DL_SIGN : IN std_logic;
         RST : IN  std_logic;
         D : IN  std_logic_vector(3 downto 0);
         Q0 : OUT  std_logic;
         Q1 : OUT  std_logic;
         DL_LED : OUT  std_logic_vector(3 downto 0)
        );
    END COMPONENT;


   --Inputs
   signal SYSCLK_P : std_logic := '0';
   signal SYSCLK_N : std_logic := '0';
   signal TRIG : std_logic := '0';
   signal DL_INC : std_logic := '0';
   signal DL_DEC : std_logic := '0';
   signal DL_SIGN : std_logic := '0';
   signal RST : std_logic := '0';
   signal D : std_logic_vector(3 downto 0) := (others => '0');

    --Outputs
   signal Q0 : std_logic;
   signal Q1 : std_logic;
   signal DL_LED : std_logic_vector(3 downto 0);

    -- Clocking settings
   constant clk_period : time := 5 ns;
    constant trig_period : time := 1 us;

    -- Other simulation signals
    signal delayed_photons : std_logic_vector(3 downto 0) := (others => '0');

BEGIN

    -- Instantiate the Unit Under Test (UUT)
   uut: sw_ctrl_top PORT MAP (
          SYSCLK_P => SYSCLK_P,
          SYSCLK_N => SYSCLK_N,
          TRIG => TRIG,
          DL_INC => DL_INC,
          DL_DEC => DL_DEC,
             DL_SIGN => DL_SIGN,
          RST => RST,
          D => D,
          Q0 => Q0,
          Q1 => Q1,
          DL_LED => DL_LED
        );

   -- Clock process definitions
   clk_process :process
   begin
        SYSCLK_N <= '0';
        SYSCLK_P <= '1';
        wait for clk_period/2;
        SYSCLK_N <= '1';
        SYSCLK_P <= '0';
        wait for clk_period/2;
   end process;

    -- Trigger process
    trig_process : process
    begin
        TRIG <= '0';
        wait for trig_period/2;
        TRIG <= '1';
        wait for trig_period/2;
    end process;


   -- Stimulus process
   stim_proc: process
   begin
        --DL_INC <= '1', '0' after 10 ms;
        wait for 700 ns;
        D <= X"1", X"0" after 60 ns;
        wait for 5 ns;
        delayed_photons <= X"1", X"0" after 60 ns;
        wait for 0.995 us;
        D <= X"3", X"0" after 60 ns;
        wait;
   end process;

END;

Gives the results I expect, i.e. q0 is 1 during the first trigger pulse and 0 during the second, since the d inputs in those cycles are, respectively, "0001" and "0011".

What I still do not understand is: isn't synthesis more important than simulation? What I care about is a design that works also in the "real world" and if XST tells me that some nodes are not connected shouldn't I be concerned more about this than the simulation results?

Edit #2

My system is the following:

  • Software suite: Xilinx ISE 14.7
  • Synthesizer: XST
  • Simulator : ISim
  • FPGA: Spartan-6

Edit #3

Since simulation went good, I tried implementing the design on the chip. There is a much bigger problem during the mapping step, in which it warns me that all of the d(i) inputs have been removed. This is not really a surprise given that the only assignment to those signals is inside the FSM, which according to the synth it's not progressing.

During synth, XST gives out the warning (as example):

Xst:2677 - Node <sw_proc.state_v_0> of sequential type is unconnected in block <sw_ctrl>.

During map, the warning is (again just one as example):

MapLib:701 - Signal d<0> connected to top level port d<0> has been removed.
\$\endgroup\$
  • \$\begingroup\$ Might be your debounce component always outputting a constant. Will look further. Asides are avoid thinking variables are 'local signals' for a process, use signals here, and use else/elsif to prioritise clauses instead of priority by order like you have, delete the redundant 'others' which does nothing or replace with assert for bug trapping in simulation :-) \$\endgroup\$ – TonyM Jan 17 '17 at 13:40
  • \$\begingroup\$ @TonyM I read in some Xilinx User Guide that is "better" to use "mutually exclusive parallel if clauses" because it's a better practice, since there's less lag compared to the "if/elsif" case. Also, in some other course notes found online, I read that if I do not need a signal across multiple processes, a variable is fine, as long as you do not assign it to signals (due to how signals and variables are updated in a different way). What would be the the advantage of using a signal in this case? (except that I can trace it in ISim) \$\endgroup\$ – mmassaro Jan 17 '17 at 13:43
0
\$\begingroup\$

Simulate.

Write a testbench, add that to the question, and simulate.. Use asserts to see what it does when you trigger it. Can you get it out of Idle in simulation? Does it do all you expect?

My guess ... in the process of answering that, you'll find some mistake (probably in the top level connections) that keeps it Idle.

And ... advice to avoid elsif/else is monumentally clueless. But your use of variables is fine PROVIDED you are fully aware of the difference between signal and variable assignment semantics.

\$\endgroup\$
  • \$\begingroup\$ see updated result for the first two parts of your answer. As for the third: I just assumed that the Xilinx folks knew more about VHDL than I do (since I am not an engineer but a scientist) and went for their advice. Can you argument on why they are wrong? \$\endgroup\$ – mmassaro Jan 17 '17 at 15:26
  • \$\begingroup\$ Oh it's just a warning of unconnected nodes? Those aren't errors - synthesis typically generates hundreds of warnings and you can ignore most of them. Getting it working in sim is a MUCH more important step than worrying about synth warnings. The question now is, does the hardware do the same as the sim? If it doesn't, THEN the warnings may provide some clues. More likely, the warning was something like, that one bit of State_V is unused because it's a duplicate of an output variable and synthesis verified you don't need both copies. \$\endgroup\$ – Brian Drummond Jan 17 '17 at 18:20
0
\$\begingroup\$

After scanning line-by-line, I pinpointed the problem.

signal delay_counter_v : natural range 0 to 7 := 0;
...
when wait_delay =>
  if delay_counter_v < trig_heralding_delay_g + delay_after_trigger_s then
    delay_counter_v := delay_counter_v + 1;
    state_v         := wait_delay;

There the FSM was stuck because its range was less than what I needed. Adding the value of trig_heralding_delay_g to the range everything synths and maps smoothly.

This really baffles me, since the simulations gave correct results... I'm happy I solved the problem but I'm actually more confused.

\$\endgroup\$
  • \$\begingroup\$ Ah... are you using Xilinx ISIM? It's broken by default ... turn range checking on (yes you need the "advanced options" tab to get a non-broken simulator) and try again. Yes I really wish they didn't do that ... and considering how much time you spent to find such a trivial problem ... nuff said? \$\endgroup\$ – Brian Drummond Jan 27 '17 at 16:58
  • \$\begingroup\$ @BrianDrummond really? I did not know that... Thanks for the info. I'm unfortunately stuck with ISE and ISim, so I just guess I have to live with them :/ \$\endgroup\$ – mmassaro Jan 30 '17 at 9:43
  • \$\begingroup\$ Then learn how to turn on the option. \$\endgroup\$ – Brian Drummond Jan 30 '17 at 12:21

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.