I've been learning VHDL coding for the past two weeks to be able to understand the VHDL code of what I'll be working on. In other words, I'm still a beginner and I need help understanding some codes that I have came across.
when WAIT_ST =>
CAN_CS <= '0';
can_init_start <= '0';
can_read_start <= '0';
CPU_CAN_WE <= '0';
mux_sel <= "00";
CPU_CAN_DATA <= X"00";
CPU_CAN_ADDR <= X"01";
ID_START <= '0';
INST <= Frame_Type & Sub_Frame_Type;
WAIT_COUNT_N <= WAIT_COUNT + 1;
if WAIT_COUNT = WAIT_FOR then
state_n <= IDLE_ST;
else
state_n <= WAIT_ST;
end if;
And there are tons of When statements coming after this one, almost similar.
Help needed to understand what is happening in this loop, thank you!
2 Answers 2
It means somebody hasn't thought much about how to write their state machines.
This isn't a loop, it is one state in a state machine. There are presumably many states that do similar things, with minor variations, and it is too easy to lose track of the details.
In every clock cycle, a state machine executes the default actions (things you do in every cycle) and the code for the specific state it is in now. So it has to keep track of its state, and, if necessary, switch to another state.
So a typical single-process state machine looks like
process(reset,clk) is -- and nothing else in the sensitivity list!
begin
if reset = '1' then
state <= Idle;
elsif rising_edge(clk) then
-- default actions
ID_START <= '0';
if Counter /= 0 then
Counter <= Counter - 1;
end if;
-- state machine proper
case state is
when WAIT_ST =>
-- state actions
...
when others =>
state <= Idle;
end case;
end if;
end process;
Hopefully you can find this basic structure and each of its components in yours. (There are also 2-process and 3-process state machines which bring additional problems and no real advantage, I'll ignore them).
Several tools to improve a state machine:
(1) Use default actions.
Here, I'm assuming your ID_START
signal is set to '0' in every state except one, probably labelled START_ST
. If that's the case, you can add a default action, ID_START <= '0';
as I have done above. Then you can override this action in the one (or few) state(s) that need any other value, as
when START_ST =>
ID_START <= '1';
and you can delete all the other ID_START <= '0';
assignments in other states. Slightly less clutter...
However, if ID_START
is set in one state, cleared in one other, and is unassigned (holds its value) in all other states, then you can't do this. So use judgment, as always, when refactoring.
(2) Use named constants rather than magic numbers. What register is address X"01", and what does X"00" do to it?
CPU_CAN_DATA <= NOP;
CPU_CAN_ADDR <= Control_Reg;
(3) Use records to group associated signals.
(You can put them in a package and re-use it anywhere you need it)
type CPU_CAN is record
WE : std_logic;
DATA : std_logic_vector(7 downto 0);
ADDR : unsigned(7 downto 0)
end record;
signal CPU_CAN_BUS : CPU_CAN;
NB addresses are usually natural numbers, hence unsigned. Data can be anything, not necessarily a number, hence std_logic_vector.
And now you can assign related signals in a single statement:
CPU_CAN_BUS <= ( WE=> '0', ADDR => Ctrl_Reg, DATA => NOP);
You can probably do the same for CAN_CS, can_init_start, can_read_start.
(4) Use procedures and functions to streamline repetitive operations.
(5) Use a programmable delay counter. If WAIT_ST is the only state that needs a delay counter, the current approach is fine. However if you need several delays, they can share a counter - which counts as one of the default actions above. Because it counts down, you only need to compare it to 0, (saving a little hardware) and it's programmed when you jump INTO a state like WAIT_ST
if something then
Counter <= WAIT_FOR;
state_n <= WAIT_ST;
end if;
(6) stop before you make things more complicated! i.e. if mux_sel
doesn't logically fit any of the above, leave it alone...
The end result of these changes is something like
when WAIT_ST =>
-- CAN_CS, can_init_start, can_read_start, ID_START are defaults
mux_sel <= "00";
CPU_CAN_BUS <= ( WE=> '0', ADDR => Ctrl_Reg, DATA => NOP);
INST <= Frame_Type & Sub_Frame_Type;
if Counter = 0 then then
state_n <= IDLE_ST;
end if; -- else we stay in WAIT_ST
Better?
-
\$\begingroup\$ It seems that the code from the OP is the combinational part of a 2-process state machine. Thus, you should mention, that assigning outputs in a clocked process instead, the behaviour would be different because the value will be visible not until the next clock period. Also, in your example code, extra logic is synthesized to keep the state of the output registers when reset is asserted. \$\endgroup\$Martin Zabel– Martin Zabel2015年12月24日 11:16:03 +00:00Commented Dec 24, 2015 at 11:16
-
\$\begingroup\$ @Martin : I don't know how you can infer that from the posted code : assuming you're right, this is the unclocked process (because it determines the state) so all these outputs (and the counter!) are unregistered. That would be absurd, so I infer the reverse. I haven't written a 2-process SM this century, so if you think an answer from that perspective is useful, go ahead and write it. \$\endgroup\$user16324– user163242015年12月24日 11:28:58 +00:00Commented Dec 24, 2015 at 11:28
I assume that the posted code is part of a 2-process state machine implementation because WAIT_COUNTER_N
and STATE_N
seem to be the next values of the registers WAIT_COUNT
and STATE
respectively.
Thus, there should be a clocked process describing the registers similar to this:
process(clock)
begin
if rising_edge(clock) then
WAIT_COUNT <= WAIT_COUNT_N;
STATE <= STATE_N;
end if;
end process;
The above process may contain further reset logic.
Then, the second process is combinational logic defining the output values and next register values based on the current state and the inputs of the state machine. The process starts like this:
process (STATE, WAIT_COUNT)
begin
-- default signal assignments if any
-- ..
case STATE is
After this, a WHEN
block follows describing the behaviour for each state, e.g. as in your code:
when WAIT_ST =>
CAN_CS <= '0';
can_init_start <= '0';
can_read_start <= '0';
CPU_CAN_WE <= '0';
mux_sel <= "00";
CPU_CAN_DATA <= X"00";
CPU_CAN_ADDR <= X"01";
ID_START <= '0';
INST <= Frame_Type & Sub_Frame_Type;
WAIT_COUNT_N <= WAIT_COUNT + 1;
if WAIT_COUNT = WAIT_FOR then
state_n <= IDLE_ST;
else
state_n <= WAIT_ST;
end if;
For example, the above block contains assignments to outputs like this:
CAN_CS <= '0';
The value on the right side is assigned immediatly after the STATE
has changed to WAIT_ST
at the rising edge of the clock.
Furthermore, the wait counter is incremented by immediatly assigning the respective value to WAIT_COUNT_N
here:
WAIT_COUNT_N <= WAIT_COUNT + 1;
The new value is stored in the register WAIT_COUNT
at the rising edge of the clock by the clocked process.
Thus, the final if
checks the current (old) counter value and selects the desired follow-up state. Again, the next state is immediatly assigned to the signal state_n
first. The value is stored in the register STATE
at the rising edge of the clock by the clocked process.
-
\$\begingroup\$ On re-reading. you are probably correct, +1. I apparently didn't scroll far enough back to see the OP's counter, must have been looking at my own! If it is a 2-process state machine there are more places for bugs to hide. Not all of my cleanup suggestions are appropriate for the 2-process form. \$\endgroup\$user16324– user163242015年12月26日 00:26:16 +00:00Commented Dec 26, 2015 at 0:26
case
andprocess
statement. Could you please post these single lines too. \$\endgroup\$