On 20/09/2013 22:56, rickman wrote:
[see end of this post for the entire code]
I'm not very happy with the way you have separated the register code
from the output code. For example the procedure autoread is invoked
from the registered section of the clocked process. This means any
signals assigned a value in this procedure will be registered.
I do not have signals, I have only variables and they are synthesized to
registers or wires according to whether they need to retain a value or not.
The last line shown in this procedure assigns a high impedance value to
what I assume is a signal, that's not realizable, or at least it isn't
the way I would do it.
The DATA_v variable is not the main issue here, I'm concerned about
combinational loops on nRD_v and nWR_v.
OT, on the 'Z' assignment, it is clear that while internal resources of
an FPGA do not provide tri-state logic, they can easily be transformed
to a set of multiplexers which logically provide the same functionality.
It is clear that no condition should exist for multiple drivers on the
line. On a side note, shouldn't be better to use std_ulogic_vector to
spot multiple drivers on the bus instead of std_logic_vector? I'm
naively assuming that an unresolved type would help me spotting multiple
drivers at compilation time...
Unless you show us all your code it will be hard to pin point your
combinatorial loop. The problem is most likely in init_regs or
update_ports as these are not registered.
I believe that not posting the whole code was a bad idea so it follows
at the end of this post, but let me add that init_regs does not mean
they are 'not registered'. The clocked process has an asynchronous reset
which triggers the 'initialization' of everything needs to be
initialized, while the update_ports only 'connects' the variables to the
entity ports (no register involved here).
While I cannot be 100% sure that the problem is in the snippet I
previously posted, I'm quite confident that the rest of the code should
not play an important role. In 'pkg_temp' there are some constant
definitions only.
library ieee;
use ieee.std_logic_1164.all;
use ieee.std_logic_unsigned.all;
use ieee.numeric_std.all;
library work;
use work.pkg_temp.all;
-------------------------------------------------------------------------------
entity tfsm is
port (
ADD_i : in std_logic_vector(2 downto 0);
nRD_i : in std_logic;
nWR_i : in std_logic;
ADD_o : out std_logic_vector(2 downto 0);
nRD_o : out std_logic;
nWR_o : out std_logic;
DATA : inout std_logic_vector(7 downto 0);
TEMP : out std_logic_vector(15 downto 0);
SRNO : out std_logic_vector(63 downto 0);
ST : in std_logic;
CLK : in std_logic;
RST : in std_logic);
end tfsm;
-------------------------------------------------------------------------------
architecture tfsm_arch of tfsm is
-- Note: No signal declarations.
begin -- tfsm_arch
main : process (CLK, RST) is
-------------------------------------------------------------------------------
type state_t is (
SET_CLKRATE,
SET_CTRLREG,
CLR_INTREG,
SET_INEREG,
SEND_READ_ROM,
SEND_SKIP_ROM,
SEND_WRITE_SCRATCHPAD,
SEND_CONVERT,
SEND_READ_SCRATCH,
-- reusable states
SEND_RESET,
TEST_PRESENCE,
SEND_CMD,
TEST_CMD,
SEND_DATA,
TEST_DATA,
WRITE_BGN,
WRITE_END,
READ_BGN,
READ_END,
IDLE
);
variable state_v : state_t; -- current state
variable nstate_v : state_t; -- next state after write/read
variable mstate_v : state_t; -- next state after macro command
variable lstate_v : state_t; -- next state after skip rom
variable cmd_v : integer; -- command to send
variable dat_v : integer; -- data to send
variable cnt_v : integer; -- counter
-------------------------------------------------------------------------------
-- Data Registers
subtype bus_t is std_logic_vector(DATA'range); -- match port width
variable DATA_v : bus_t;
-- Address registers
subtype add_t is std_logic_vector(ADD_i'range); -- match port width
variable ADD_v : add_t;
-- control wires
subtype ctr_t is std_logic;
variable nRD_v : ctr_t;
variable nWR_v : ctr_t;
-- sensor id type
type sens_t is array (7 downto 0) of std_logic_vector (7 downto 0);
-- sensor temperature type
type temp_t is array (1 downto 0) of std_logic_vector (7 downto 0);
-- scratchpad type
type scratch_t is array (2 downto 0) of integer;
-- temperature registers
-- temp_count_v: will count each time there's a new temperature available
-- temp_valid_v: will be set to one when ID AND first temperature readout
-- are both valid
variable temp_count_v : std_logic_vector(2 downto 0);
variable temp_valid_v : std_logic;
variable temp_value_v : temp_t;
-- sensor id register
variable sens_id_v : sens_t;
-- scratchpad register
variable scratchpad_v : scratch_t;
-------------------------------------------------------------------------------
procedure bus_init is -- used in init_regs
begin
DATA_v := (others => 'Z');
ADD_v := (others => '0');
nRD_v := '1'; -- inactive
nWR_v := '1'; -- inactive
temp_count_v := "000";
temp_valid_v := '0';
temp_value_v(0) := x"00";
temp_value_v(1) := x"00";
sens_id_v(0) := x"28"; -- for test only
sens_id_v(1) := x"DE"; -- for test only
sens_id_v(2) := x"D5"; -- for test only
sens_id_v(3) := x"45"; -- for test only
sens_id_v(4) := x"03"; -- for test only
sens_id_v(5) := x"00"; -- for test only
sens_id_v(6) := x"00"; -- for test only
sens_id_v(7) := x"C0"; -- crc
end procedure bus_init;
-------------------------------------------------------------------------------
procedure fsm_init is -- used in init_regs
begin
state_v := IDLE;
nstate_v := IDLE;
mstate_v := IDLE;
lstate_v := IDLE;
cmd_v := 0;
cnt_v := 0;
dat_v := 0;
scratchpad_v (0) := 40; -- x"28" -> +40 degrees
scratchpad_v (1) := 226; -- x"E2" -> -30 degrees
scratchpad_v (2) := 127; -- x"7F" -> I have no idea!
end procedure fsm_init;
-------------------------------------------------------------------------------
procedure init_regs is
begin
bus_init;
fsm_init;
end procedure init_regs;
-------------------------------------------------------------------------------
procedure update_ports is
begin -- purpose: synthesize a wire from the register to the port
-- ST is a register bit that allows start/stop of the automatic
-- temperature readout procedure. It is set in the control register of
-- the DS1WM and propagated up to here (see one_wire_interface.vhd)
mux : if ST then
DATA<= DATA_v;
nRD_o<= nRD_v;
nWR_o<= nWR_v;
ADD_o<= ADD_v;
else
DATA<= (others => 'Z');
nRD_o<= nRD_i;
nWR_o<= nWR_i;
ADD_o<= ADD_i;
end if;
TEMP<= temp_count_v&
temp_valid_v&
temp_value_v(1)(3 downto 0)&
temp_value_v(0);
sensor_id : for i in 0 to 7 loop
SRNO(8*(i+1)-1 downto 8*i)<= sens_id_v(i);
end loop sensor_id;
end procedure update_ports;
-------------------------------------------------------------------------------
procedure autoread is
-- local procedures for statements used more than once:
procedure write_ds1wm (
add : in add_t;
val : in integer) is
begin
state_v := WRITE_BGN;
ADD_v := add;
DATA_v := std_logic_vector(to_unsigned(val, DATA_v'length));
end write_ds1wm;
--
procedure read_ds1wm (
add : in add_t) is
begin
state_v := READ_BGN;
ADD_v := add;
DATA_v := (others => 'Z');
end read_ds1wm;
procedure inc_tic_count is
begin
cnt_v := cnt_v + 1;
end procedure inc_tic_count;
procedure inc_tem_count is
begin
temp_count_v := temp_count_v + '1';
end procedure inc_tem_count;
begin -- procedure autoread
if ST then
case state_v is
when IDLE =>
cnt_v := 0;
state_v := SET_CLKRATE;
-------------------------------------------------------------------------------
-- Write and Read states are special ones since they are called several times
-- by every other state, it is therefore necessary to store the value of the
-- next state once the write/read steps have been completed, in order to
-- continue with the fsm.
-------------------------------------------------------------------------------
when READ_BGN =>
state_v := READ_END; -- this is one clock cycle
nRD_v := not nRD_v;
when READ_END =>
state_v := nstate_v;
nRD_v := not nRD_v;
DATA_v := DATA;
when WRITE_BGN =>
state_v := WRITE_END; -- this is one clock cycle
nWR_v := not nWR_v;
when WRITE_END =>
state_v := nstate_v;
nWR_v := not nWR_v;
DATA_v := (others => 'Z'); -- release the bus
-------------------------------------------------------------------------------
-- Init part for the DS1WM. Set clock rate, control register, clean
-- interrupt register and configure interrupt enable register
-------------------------------------------------------------------------------
when SET_CLKRATE =>
nstate_v := SET_CTRLREG;
write_ds1wm(CLKREG, 138); -- clock enabled + 20 MHz
when SET_CTRLREG =>
nstate_v := CLR_INTREG;
write_ds1wm(CTLREG, 2**STB); -- keep STB active!
when CLR_INTREG =>
nstate_v := SET_INEREG;
read_ds1wm(INFREG);
when SET_INEREG =>
nstate_v := SEND_RESET;
mstate_v := SEND_READ_ROM;
write_ds1wm(INEREG, 2**EPD + 2**ETBE + 2**ERBF);
-------------------------------------------------------------------------------
-- Start DS1WM transactions with a 1wire reset. Each 1-wire transaction has
-- always three phases:
-- 1. initialization (RESET)
-- 2. ROM command
-- 3. FUNCTION command
-------------------------------------------------------------------------------
when SEND_RESET =>
nstate_v := TEST_PRESENCE;
write_ds1wm(CMDREG, 2**OneWR);
when TEST_PRESENCE =>
-- we should foresee a timeout...!!!
if DATA_v(PD) and not DATA_v(PDR) then
state_v := mstate_v;
else
nstate_v := state_v;
read_ds1wm(INFREG);
end if;
-------------------------------------------------------------------------------
-- SEND command cmd_v. This sequence of states is referenced as a 'macro' as it
-- will depend on the mstate_v defined by the calling state.
-------------------------------------------------------------------------------
when SEND_CMD =>
nstate_v := TEST_CMD;
write_ds1wm(DATBUF, cmd_v);
when TEST_CMD =>
-- we should foresee a timeout...!!!
if DATA_v(RBF) then
nstate_v := mstate_v;
read_ds1wm(DATBUF); -- empty data buffer!
else
nstate_v := state_v;
read_ds1wm(INFREG);
end if;
-------------------------------------------------------------------------------
-- Read cnt_v data times to register. This sequence of states is references as
-- a 'macro' as it will depend on the mstate_v defined by the calling state.
-------------------------------------------------------------------------------
when SEND_DATA =>
nstate_v := TEST_DATA;
write_ds1wm(DATBUF, dat_v);
when TEST_DATA =>
-- we should foresee a timeout...!!!
if DATA_v(RBF) then
nstate_v := mstate_v;
read_ds1wm(DATBUF);
else
nstate_v := state_v;
read_ds1wm(INFREG);
end if;
-------------------------------------------------------------------------------
when SEND_READ_ROM =>
mstate_v := SEND_READ_ROM;
-- read data after the first SEND_DATA is over (cnt_v = 2)
if cnt_v> 1 then
sens_id_v(cnt_v - 2) := DATA_v;
end if;
if cnt_v = 0 then
cmd_v := READ_ROM;
state_v := SEND_CMD;
inc_tic_count;
elsif cnt_v<= 8 then
dat_v := GET_DATA;
state_v := SEND_DATA;
inc_tic_count;
else
cnt_v := 0;
state_v := SEND_WRITE_SCRATCHPAD;
end if;
-------------------------------------------------------------------------------
when SEND_WRITE_SCRATCHPAD =>
mstate_v := SEND_WRITE_SCRATCHPAD;
if cnt_v = 0 then
cmd_v := WRITE_SCRATCH;
state_v := SEND_CMD;
inc_tic_count;
elsif cnt_v<= 3 then
dat_v := scratchpad_v(cnt_v - 1);
state_v := SEND_DATA;
inc_tic_count;
else
cnt_v := 0;
mstate_v := SEND_SKIP_ROM;
lstate_v := SEND_CONVERT;
state_v := SEND_RESET;
end if;
-------------------------------------------------------------------------------
when SEND_SKIP_ROM =>
cmd_v := SKIP_ROM;
state_v := SEND_CMD;
mstate_v := lstate_v;
-------------------------------------------------------------------------------
when SEND_CONVERT =>
mstate_v := SEND_CONVERT;
if cnt_v = 0 then
cmd_v := CONVERT;
state_v := SEND_CMD;
inc_tic_count;
elsif cnt_v = WAIT_CONVERSION_TIME then
dat_v := GET_DATA;
state_v := SEND_DATA;
inc_tic_count;
elsif cnt_v = WAIT_CONVERSION_TIME+1 then
cnt_v := 1;
if DATA_v = x"FF" then
cnt_v := 0;
mstate_v := SEND_SKIP_ROM;
lstate_v := SEND_READ_SCRATCH;
state_v := SEND_RESET;
end if;
else
inc_tic_count;
end if;
-------------------------------------------------------------------------------
when SEND_READ_SCRATCH =>
mstate_v := SEND_READ_SCRATCH;
-- read data after the first SEND_DATA is over (cnt_v = 2)
if cnt_v> 1 then
temp_value_v(cnt_v - 2) := DATA_v;
end if;
if cnt_v = 0 then
cmd_v := READ_SCRATCH;
state_v := SEND_CMD;
inc_tic_count;
elsif cnt_v<= 2 then
dat_v := GET_DATA;
state_v := SEND_DATA;
inc_tic_count;
else
inc_tem_count;
temp_valid_v := '1';
cnt_v := 0;
mstate_v := SEND_SKIP_ROM;
lstate_v := SEND_CONVERT;
state_v := SEND_RESET;
end if;
when others => null;
end case;
else
state_v := IDLE;
end if;
end procedure autoread;
-------------------------------------------------------------------------------
procedure update_regs is
begin -- purpose: call the procedures above in the desired order
autoread;
end procedure update_regs;
-------------------------------------------------------------------------------
-- Synchronous Templates for Synthesis: The following templates use
-- all of the declarations above to make a UART. With other
-- declarations, the template could make anything else. Relative
-- synthesis performance will be compared below for each template.
-- If I were not making comparisons, the first template would be
-- pasted into the main process.
-------------------------------------------------------------------------------
procedure template_v_rst is
begin -- a_rst is logically equivalent
if RST = '1' then -- Assumes synched trailing edge RST pulse
init_regs; -- reg_v := init_c; Variables only, ports below.
elsif rising_edge(CLK) then
update_regs; -- reg_v := f(reg_v);Variables only, ports below.
end if; -- Synchronous init optional (state_v = idle_c)
update_ports; -- will infer port wires ok for RST and CLK
end procedure template_v_rst; -- out_port<= reg_v; ports only, no signals
</code>