state machine help

[niyander]

Hello,

i have written a memory wrapper so that i can use it to interface the
memory controller. i would really appreciate if some one can check it
if its ok?

another issue is that when i try to see the RTL diagram of the wrapper
then the addr_out port is connected to ground, which is not required.
so how do i correct it, so that addr_out always have the present
address from the state machine.

Thanks
----------------------------------------------------------------------------------
-- Company:
-- Engineer:
--
-- Create Date: 02:33:24 01/01/2002
-- Design Name:
-- Module Name: memwrapper - Behavioral
-- Project Name:
-- Target Devices:
-- Tool versions:
-- Description:
--
-- Dependencies:
--
-- Revision:
-- Revision 0.01 - File Created
-- Additional Comments:
--
----------------------------------------------------------------------------------
library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.STD_LOGIC_UNSIGNED.ALL;
use IEEE.NUMERIC_STD.all;

entity memwrapper is
port (
clk : in std_logic;
rst : in std_logic;
din : in std_logic_vector(7 downto 0);
dout : out std_logic_vector(7 downto 0);
data_in : in std_logic_vector(7 downto 0);
data_out : out std_logic_vector(7 downto 0);
wr : out std_logic;
rd : out std_logic;
addr : in std_logic_vector(22 downto 0);
addr_out : out std_logic_vector(22 downto 0);
nxt_addr : in std_logic;
rd_data : in std_logic;
wr_data : in std_logic
);
end memwrapper;

architecture Behavioral of memwrapper is

type statemc is
(
INIT,
READMEM,
WRITEMEM,
ENDMC
);
signal state_r, state_x : statemc;
signal addr_r, addr_x : unsigned(addr'range);

begin

addr_out <= std_logic_vector(addr_r);
dout <= data_in;
data_out <= din;
combinatorialrocess(state_r, addr_r, din, data_in, rd_data,
wr_data, nxt_addr)
begin
rd <= '0';
wr <= '0';
state_x <= state_r;
addr_x <= addr_r;
case state_r is
when INIT =>
addr_x <= unsigned(addr);
if rd_data = '1' then
state_x <= READMEM;
elsif wr_data = '1' then
state_x <= WRITEMEM;
end if;

when READMEM =>
rd <= '1';
wr <= '0';
if nxt_addr = '1' then
addr_x <= unsigned(addr);
else
addr_x <= addr_r;
state_x <= ENDMC;
end if;

when WRITEMEM =>
wr <= '1';
rd <= '0';
if nxt_addr = '1' then
addr_x <= addr_r + 1;
else
addr_x <= unsigned(addr);
state_x <= ENDMC;
end if;

when others =>
addr_x <= unsigned(addr);
state_x <= INIT;

end case;

end process;

update : process(clk)
begin
if clk'event and clk = '1' then
if rst = '1' then
state_r <= INIT;
else
state_r <= state_x;
end if;
end if;
end process;

end Behavioral;

 

[Andy]

Note that JK just hit upon one reason I prefer single process state
machines: there's no separate register assignment to forget. Other
reasons include: latches are impossible, fewer signals need be
declared, process sensitivity lists are simpler, and many more...

Other comments:

Code comments are your friend and ours. You know what you wanted, and
what the ports mean, but we have to figure it out. In another few
weeks, you won't be sure either unless you add comments.

Note that at the end of WRITEMEM, addr_out would revert back to the
original address (probably invalid), and yet you are still driving WR
high. Maybe not what you really wanted?

Not exactly sure what ENDMC is for, unless you need to wait a clock
for the next cycle to begin? Here a comment or two would really help.

It is generally poor form to use "others" to reference named states
that you explicitly enter elsewhere (e.g. ENDMC).

Be consistent in how you handle outputs from state machines. For
instance, in INIT and MC, you let the RD and WR default to '0', yet in
READMEM and WRITEMEM, you explicity set WR and RD, respectively, to
'0'. I prefer to only show what is different from default, but others
prefer to explicitly list every output in every state (probably a
throw-back to a way to avoid latches in combinatorial processes).

Andy