Need comments on Single port RAM

[Altera_Forum]

I am doing a design of Single port ram in VHDL. I need inputs if there are rooms for improvement. Thanks! 

 --------------------------------------------------------------------------------------------   Library ieee; use ieee.std_logic_1164.all; use ieee.std_logic_arith.all; use ieee.std_logic_unsigned.all; entity ram1 is PORT(address: in std_logic_vector(7 downto 0); clk: in std_logic; we: in std_logic; oe: in std_logic; data_io: inout std_logic_vector(7 downto 0)); end ram1; ARCHITECTURE rtl OF ram1 IS signal data_out : std_logic_vector(7 downto 0); signal mem : std_logic_vector(7 downto 0); TYPE ram_type IS ARRAY(255 downto 0) OF std_logic_vector(7 DOWNTO 0); SIGNAL my_ram256x8 : ram_type:=(others=>(others=>'0')); BEGIN process (clk) begin if rising_edge (clk) then if (oe = '1' and we = '0') then data_io <= data_out; else data_io <= "ZZZZZZZZ"; end if; end if; end process; process(clk) begin if (clk'event and clk='1') then if (we='1') then mem<=data_io; end if; end if; end process; process(clk) begin if (clk'event and clk='1') then if (we='0' and oe='1') then data_out<=mem; end if; end if; end process; END rtl;

[Altera_Forum]

It doesn't work. 

my_ram256x8 is defined but not used in your design, so there's no RAM operation at all. I suggest to use the VHDL template design offered in Quartus editor instead.

[Altera_Forum]

Thanks for pointing out the mistake. It was a mistake there. Sorry! Using automatic generation of code would not do any good for learning process. 

 ____________________________________________________________________________  Library ieee; use ieee.std_logic_1164.all; use ieee.std_logic_arith.all; use ieee.std_logic_unsigned.all; entity ram1 is PORT(address: in std_logic_vector(7 downto 0); clk: in std_logic; we: in std_logic; oe: in std_logic; data_io: inout std_logic_vector(7 downto 0)); end ram1; ARCHITECTURE rtl OF ram1 IS signal data_out : std_logic_vector(7 downto 0); TYPE ram_type IS ARRAY(255 downto 0) OF std_logic_vector(7 DOWNTO 0); SIGNAL my_ram256x8 : ram_type:=(others=>(others=>'0')); BEGIN process (clk) begin if rising_edge (clk) then if (oe = '1' and we = '0') then data_io <= data_out; else data_io <= "ZZZZZZZZ"; end if; end if; end process; process(clk) begin if (clk'event and clk='1') then if (we='1') then my_ram256x8(conv_integer(address))<=data_io; end if; end if; end process; process(clk) begin if (clk'event and clk='1') then if (we='0' and oe='1') then data_out<=my_ram256x8(conv_integer(address)); end if; end if; end process; END rtl;

[Altera_Forum]

Hi, 

 comment 1:  why use bidirectional data. If you are inside the fpga you got plenty of noodles to connect. bidir is meant for pins   comment 2: be careful about bus contention in relation to clocks. My gut feeling you are driving data_io while reading it in...you better use comb for Z assignment

[Altera_Forum]

Hi Kaz, 

 Thanks for comments. One thing I do not understand is "comb for Z assignment". Could you please clarify?   I hope it is not too obvious thing to ask. :)

[Altera_Forum]

Hi, 

 I mean apply Z in a combinatorial assignment, not on the clk edge due to clk latency issues.   Imagine at a clk edge your wr = '1' so your module reads in data_io:      --- Quote Start ---   if (we='1') then my_ram256x8(conv_integer(address))<=data_io; end if;  --- Quote End ---     At the same clk edge when wr = '1' you apply Z on data_io, fair enough but this means data_io will become Z(will be cutoff) just after this edge. so when you take data_io to your ram it is still not cutoff from your own drive.      --- Quote Start ---   if rising_edge (clk) then if (oe = '1' and we = '0') then data_io <= data_out; else data_io <= "ZZZZZZZZ"; end if; end if; end process;  --- Quote End ---       Here is my version(off my head so check for mistakes):   entity ram1 is port( clk : in std_logic; we : in std_logic; oe : in std_logic; address: in std_logic_vector(7 downto 0); data_io : inout std_logic_vector(7 downto 0) ); end ram1;   architecture rtl of ram1 is   signal data_out : std_logic_vector(7 downto 0);   type ram_type is array (255 downto 0) of std_logic_vector(7 downto 0); signal my_ram256x8 : ram_type := (others => (others => '0'));   begin   data_io <= data_out when (oe = '1' and we = '0') else (others => 'Z');   process(clk) begin if rising_edge(clk) then if oe = '1' then if we = '1' then my_ram(conv_integer(address)) <= data_io; -- write else data_out <= my_ram(conv_integer(address)); -- read end if; end if; end if; end process; end rtl;

[Altera_Forum]

Due to assigning data_io in a clock sensitive process, you add another clock cycle delay and require a read access duration of two cycles, which is most likely unwanted. 

 P.S.: I see kaz, already mentioned this.

[Altera_Forum]

Thanks for comments! Have a nice weekend.