What is the disadvantage of this approach to VHDL state machine design?

Question

A frequently used style for state machines is to change state in a very simple clocked process and either in the same process or another one, a combinatorial section which defines the transition to the next state: (here I use a synchronous reset, because I have a master reset synchronizer)

process( aclk, state, next_state, bob, alice )
begin
  if rising_edge( aclk ) then
    if resetn = '0' then
      state <= IDLE;
    else
      state <= next_state;
    end if;
  end if;

  next_state <= state;
  case state is
    when IDLE =>
      if bob = alice then
        next_state <= ANOTHER_STATE;

...and so on. Some prefer to put the combinatorial section in another process, styles vary. I split the machine and various things such as counters which control state transitions into separate small processes, but I know some dislike this style.

Is there any serious problem with keeping everything in the synchronous block, as in:

process( aclk )
begin
  if rising_edge( aclk ) then
    if resetn = '0' then
      state <= IDLE;
    else
      case state is
        when IDLE =>
          if bob = alice then
            state <= ANOTHER_STATE;
          end if;
        when others =>
          null;
      end case;
    end if;
  end if;
end process;

I ask because when I started programming (very badly, as a software person!) I used to use the second method, and am wondering if I should rewrite the code.

I have a friend who wrote some of the ARM logic, (not Sophie! I think she used a pencil and paper...) who tells me his rule was never to create processes with anything other than the clock in the sensitivity list.

Best regards Geoff

Answer 1

I don't think you can prescribe what to do. As with many coding styles some people prefer the one others the other. There is definitely not a good or a bad style.

For small simple state-machines I use the first method.

For big, complex state-machines I prefer the second method.

Tricks I like about the second method:

1. e.g. in state X you need a register R to have the value A. In the first method you have to track down every state transition which goes to X and add: R <= A;. Also if you make changes your state-machines you should not forget that.
   The second method normally has a state and next_state variable. Then you can write:
   if (next_state==X) R <= A;. You can now play around with your code and know that when you get to state X, R will be have been assigned the value A.

2. Break out: Suppose you need to have an 'emergency stop' or the user negates the 'enable' bit and the whole FSM needs to stop. In method 1 you have to start every state condition with: if (stop_everything) state<=IDLE else...
   In method two you register part becomes:

   if (stop_everything) state <= IDLE; else state <= next_state;

I have worked with IP from a big CPU core vendor (You can guess their name). In their code you see only the second method even for the simplest of code. I assume it is a mandatory coding style if you work for them.