1
\$\begingroup\$

I have designed a serial adder, with a small control unit which is supposed to synchronize all the ff states. I'm specifically interested in the state machine that does such stuff (you can see a block diagram here. The design is the following:

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

entity serial_adder is
generic(num_bits : integer := 4);
port(rst : in std_logic;
      start : in std_logic;
      clk : in std_logic;
      x : in std_logic_vector(num_bits - 1 downto 0);
      y : in std_logic_vector(num_bits - 1 downto 0);
      z : out std_logic_vector(num_bits - 1 downto 0);
      ovf : out std_logic;
      done : out std_logic);
end entity serial_adder;

architecture rtl of serial_adder is
    type states is (s0,s1,s2);
    signal load, ce : std_logic;
    signal x_reg, y_reg, z_reg : unsigned(num_bits - 1 downto 0);
    signal s, next_c, c : std_logic;
    subtype count is natural range 0 to num_bits;
    signal counter : count;
    signal current_state, next_state : states;
begin
    s <= x_reg(0) xor y_reg(0) xor c;
    next_c <= (x_reg(0) and y_reg(0)) xor (x_reg(0) and c) xor (y_reg(0) and c);
    z <= std_logic_vector(z_reg);

    regs_and_ff : process(clk)
    begin
        if load <= '1' then
            x_reg <= unsigned(x);
            y_reg <= unsigned(x);
            z_reg <= to_unsigned(0,num_bits);
            c <= '0';
            counter <= 0;
        elsif ce <= '1' then
            x_reg <= '0' & x_reg(num_bits - 1 downto 1);
            y_reg <= '0' & y_reg(num_bits - 1 downto 1);
            z_reg <= s & z_reg(num_bits - 1 downto 1);
            c <= next_c;
        end if;
    end process regs_and_ff;

    control_unit_out : process(current_state)
    begin
        case current_state is
            when s0 =>
                if start <= '0' then
                    load <= '0'; ce <= '0'; done <= '1';
                    next_state <= s1;
                end if;
            when s1 =>
                ce <= '0';
                if start <= '1' then
                    load <= '1'; done <= '0';
                else
                    load <= '0'; done <= '1';
                end if;
            when s2 =>
                load <= '0';
                done <= '0';
                if counter < num_bits then
                    ce <= '1';
                else 
                    ce <= '0';
                end if;
        end case;
    end process control_unit_out;

    control_unit_sm : process(clk,rst)
    begin
        if rst = '1' then current_state <= s0;
        elsif clk'event and clk = '1' then current_state <= next_state;
        end if;
    end process control_unit_sm;

end architecture rtl;

I've also designed a test bench

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

entity tb_serial_adder is
end entity tb_serial_adder;

architecture test of tb_serial_adder is
    signal rst, clk, start, done, ovf : std_logic;
    signal x, y, z : std_logic_vector(3 downto 0);

    component serial_adder is
    generic(num_bits : natural := 4);
    port(rst : in std_logic;
      start : in std_logic;
      clk : in std_logic;
      x : in std_logic_vector(num_bits - 1 downto 0);
      y : in std_logic_vector(num_bits - 1 downto 0);
      z : out std_logic_vector(num_bits - 1 downto 0);
      ovf : out std_logic;
      done : out std_logic);
    end component serial_adder;

    constant clk_period : time := 100 ns;

begin

    DUT : serial_adder
        generic map(num_bits => 4)
        port map(rst => rst,
                    start => start,
                    clk => clk,
                    x => x,
                    y => y,
                    z => z,
                    ovf => ovf,
                    done => done);

    clk_proc : process is
    begin
        clk <= '1';
        wait for clk_period/2;
        clk <= '0';
        wait for clk_period/2;
    end process clk_proc;

    stim_proc : process is
    begin
        x <= "1001";
        y <= "0101";
        rst <= '1';
        start <= '0';
        wait for 200 ns;
        rst <= '0';
        start <= '1';
        wait for 200 ns;
        wait for 1000 ns;
        wait;
    end process stim_proc;


end architecture test;

However I don't understand why it doesn't work. In the following a plot of the simulation results: enter image description here

I'm quite sure the problem is how I've designed the state machine, I'm new to control unit design. Any clue on why it's not working?

thx

Update 1

Following the suggestions:

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

entity serial_adder is
generic(num_bits : integer := 4);
port(rst : in std_logic;
    start : in std_logic;
    clk : in std_logic;
    x : in std_logic_vector(num_bits - 1 downto 0);
    y : in std_logic_vector(num_bits - 1 downto 0);
    z : out std_logic_vector(num_bits - 1 downto 0);
    ovf : out std_logic;
    done : out std_logic);
end entity serial_adder;

architecture rtl of serial_adder is
    type states is (s0,s1,s2);
    signal load, ce : std_logic;
    signal x_reg, y_reg, z_reg : unsigned(num_bits - 1 downto 0);
    signal s, next_c, c : std_logic;
    subtype count is natural range 0 to num_bits;
    signal counter : count;
    signal current_state, next_state : states;
begin
    s <= x_reg(0) xor y_reg(0) xor c;
    next_c <= (x_reg(0) and y_reg(0)) xor (x_reg(0) and c) xor (y_reg(0) and c);
    z <= std_logic_vector(z_reg);

    regs_and_ff : process(clk)
    begin
        if clk'event and clk = '1' then
            if load <= '1' then
                x_reg <= unsigned(x);
                y_reg <= unsigned(x);
                z_reg <= to_unsigned(0,num_bits);
                c <= '0';
                counter <= 0;
            elsif ce <= '1' then
                x_reg <= '0' & x_reg(num_bits - 1 downto 1);
                y_reg <= '0' & y_reg(num_bits - 1 downto 1);
                z_reg <= s & z_reg(num_bits - 1 downto 1);
                c <= next_c;
                counter <= counter + 1;
            end if;
        end if;
    end process regs_and_ff;

    control_unit_out : process(current_state)
    begin
    case current_state is
        when s0 =>
            if start = '0' then
                load <= '0'; ce <= '0'; done <= '1';
            else
                next_state <= s1;
            end if;
        when s1 =>
            ce <= '0';
            if start = '1' then
                load <= '1'; done <= '0';
                next_state <= s2;
            else
                load <= '0'; done <= '1';
            end if;
        when s2 =>
            load <= '0';
            done <= '0';
            if counter < num_bits then
                ce <= '1';
            else 
                ce <= '0';
                next_state <= s0;
            end if;
    end case;
    end process control_unit_out;

    control_unit_sm : process(clk,rst)
    begin
        if rst = '1' then current_state <= s0;
        elsif clk'event and clk = '1' then current_state <= next_state;
        end if;
    end process control_unit_sm;

end architecture rtl;

Unfortunately still doesn't work...

\$\endgroup\$
8
  • \$\begingroup\$ Why aren't you looking at the signals for your serial adder in the simulation instead of just the testbench signals? Seems like that would be a lot more helpful \$\endgroup\$
    – ks0ze
    Commented Mar 23, 2017 at 17:34
  • \$\begingroup\$ You mean the internal signals? I can post a picture, but what it's saying is like "not logged" which I don't know what it means. \$\endgroup\$
    – user8469759
    Commented Mar 23, 2017 at 17:38
  • \$\begingroup\$ When you run the simulation you should be able to add all of the signals in "serial_adder" to your waveform viewer, which would significantly help with debugging. But, I think your problem is you are missing a "if rising_edge(clk) then" in your "regs_and_ff" process \$\endgroup\$
    – ks0ze
    Commented Mar 23, 2017 at 17:40
  • \$\begingroup\$ I can add the "internal signals", but for some reason I can't visualize them. \$\endgroup\$
    – user8469759
    Commented Mar 23, 2017 at 17:43
  • \$\begingroup\$ (I've just tried the rising edge, still doesn't work) but I should have added it, thanks for that. \$\endgroup\$
    – user8469759
    Commented Mar 23, 2017 at 17:45

1 Answer 1

Reset to default
1
\$\begingroup\$

In following process you should add condition like if clk'event and clk = '1' then

regs_and_ff : process(clk)
begin
    if clk'event and clk = '1' then
        if load = '1' then
            x_reg <= unsigned(x);
            y_reg <= unsigned(x);
            z_reg <= to_unsigned(0,num_bits);
            c <= '0';
            counter <= 0;
        elsif ce = '1' then
            x_reg <= '0' & x_reg(num_bits - 1 downto 1);
            y_reg <= '0' & y_reg(num_bits - 1 downto 1);
            z_reg <= s & z_reg(num_bits - 1 downto 1);
            c <= next_c;
        end if;
    end if;
end process regs_and_ff;

I found few folllowing mistakes in your FSM process:

control_unit_out : process(current_state, start, counter) --here in sensitivity list you should add all signals which will use in any comparison (if/elsif/else, case and etc. structures)
begin
    case current_state is
        when s0 =>
            if start = '0' then --here you used incorrect sign for comparison
                load <= '0'; ce <= '0'; done <= '1';
                next_state <= s1;
            end if;
        when s1 => -- in this state you didn't assign next_state so your FSM stop in this state
            ce <= '0';
            if start = '1' then --here you used incorrect sign for comparison
                load <= '1'; done <= '0';
            else
                load <= '0'; done <= '1';
            end if;
        when s2 => --you also should add condition to achieve a next_state in this state too (like in prev)
            load <= '0';
            done <= '0';
            if counter < num_bits then
                ce <= '1';
            else 
                ce <= '0';
            end if;
    end case;
end process control_unit_out;

Also you have counter reset counter <= 0; but you never count and increment it, I think you should add it in your logic if you want that comparison in S2 state will work correctly if counter < num_bits then

Try to fix the mistakes and I think your module will work. All mistakes I noted in comments to specific line in code.
Some mistakes were fixed, but I don't know logic of your FSM, so, please do it yourself.

Update 1: this code is working but I don't think that logic is correct, so you need to solve it but by yourself

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

entity serial_adder is
generic(num_bits : integer := 4);
port(
    rst   :  in std_logic;
    start :  in std_logic;
    clk   :  in std_logic;
    x     :  in std_logic_vector(num_bits - 1 downto 0);
    y     :  in std_logic_vector(num_bits - 1 downto 0);
    z     : out std_logic_vector(num_bits - 1 downto 0);
    ovf   : out std_logic;
    done  : out std_logic);
end entity serial_adder;

architecture rtl of serial_adder is
    type states is (s0,s1,s2);
    signal load, ce : std_logic;
    signal x_reg, y_reg, z_reg : unsigned(num_bits - 1 downto 0);
    signal s, next_c, c : std_logic;
    subtype count is natural range 0 to num_bits;
    signal counter : count;
    signal current_state, next_state : states;
begin
    s <= x_reg(0) xor y_reg(0) xor c;
    next_c <= (x_reg(0) and y_reg(0)) xor (x_reg(0) and c) xor (y_reg(0) and c);
    z <= std_logic_vector(z_reg);

    regs_and_ff : process(clk)
    begin
        if clk'event and clk = '1' then
            if load = '1' then -- here was mistake ('<=' instead '=')
                x_reg <= unsigned(x);
                y_reg <= unsigned(x);
                z_reg <= to_unsigned(0,num_bits);
                c <= '0';
                counter <= 0;
            elsif ce = '1' then -- here was mistake ('<=' instead '=')
                x_reg <= '0' & x_reg(num_bits - 1 downto 1);
                y_reg <= '0' & y_reg(num_bits - 1 downto 1);
                z_reg <= s & z_reg(num_bits - 1 downto 1);
                c <= next_c;
                counter <= counter + 1;
            end if;
        end if;
    end process regs_and_ff;

    control_unit_out : process(current_state, start, counter)
    begin
        case current_state is
            when s0 =>
                if start = '0' then
                    load <= '0'; ce <= '0'; done <= '1';
                    next_state <= s0; -- I added this line to avoid latch implementation. 
                    --You can change the next_state value in for this condition as you need
                else
                    next_state <= s1;
                end if;
            when s1 =>
                ce <= '0';
                if start = '1' then
                    load <= '1'; done <= '0';
                    next_state <= s2;
                else
                    load <= '0'; done <= '1';
                    next_state <= s1; -- I added this line to avoid latch implementation. 
                    --You can change the next_state value in for this condition as you need
                end if;
            when s2 =>
                load <= '0';
                done <= '0';
                if counter < num_bits then
                    ce <= '1';
                    next_state <= s2; -- I added this line to avoid latch implementation. 
                    --You can change the next_state value in for this condition as you need
                else 
                    ce <= '0';
                    next_state <= s0;
                end if;
        end case;
    end process control_unit_out;

    control_unit_sm : process(clk,rst)
    begin
        if rst = '1' then current_state <= s0;
        elsif clk'event and clk = '1' then current_state <= next_state;
        end if;
    end process control_unit_sm;

end architecture rtl;

Below you can find waveform from my simulation the code taken above.waveform

\$\endgroup\$
7
  • \$\begingroup\$ I've updated my code using your suggestions, it still doesn't work. I'll check if the logic of the state machine is fine. \$\endgroup\$
    – user8469759
    Commented Mar 24, 2017 at 9:52
  • \$\begingroup\$ Maybe I've fixed, part of the problem was what you said. I wonder however if could simplify the state machines for this specific design. \$\endgroup\$
    – user8469759
    Commented Mar 24, 2017 at 14:08
  • \$\begingroup\$ I updated my answer. If you will have questions feel free to ask. I'll think about your FSM, but it not looks like so complicate \$\endgroup\$
    – Roman
    Commented Mar 24, 2017 at 14:16
  • \$\begingroup\$ This is my first time I design a unit synchronized using a state machine, which is really confusing to me. \$\endgroup\$
    – user8469759
    Commented Mar 24, 2017 at 14:53
  • \$\begingroup\$ You should use rising_edge(clk) instead of clk'event and clk = '1' \$\endgroup\$
    – scary_jeff
    Commented Mar 24, 2017 at 16:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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