Electronic – How to eliminate latches in FSM Verilog implementation

latchstate-machinesverilog

I'm trying to create an FSM that left shifts a register until the MSB is 1 while counting the number of shifts completed. However, I have an issue with latches because I don't re-assign each register on a state change; namely, the shift and ctr registers (see the "if (shift[3])" statements). To get rid of the latches, I know that I need to assign these registers, but I don't want their values to change at the end–I want them to remain constant.

How do I achieve this?

`define RST 2'b00
`define LSH1 2'b01
`define LSH2 2'b10
`define DONE 2'b11

module leftshift(clk, rst, x, shift);
    input [3:0] x;
    input clk, rst;
    output [3:0] shift;

    reg [1:0] state, nextstate;

    reg [3:0] shift; // reg holding shifted bits
    reg [2:0] ctr; // ctr keeps track of how many shifts until completion

    always @ (posedge clk)
        if (rst) state <= `RST;
        else state <= nextstate;

    always @ (state, x, shift[3])
        case (state)
            `RST: begin
                shift <= x;
                ctr <= 0;
                nextstate <= `LSH1;
            end
            `LSH1: begin
                if (shift[3]) begin
                    //shift <= //don't change;
                    //ctr <= //don't change;
                    nextstate <= `DONE;
                end
                else begin
                    shift <= {shift[2:0], 1'b0};
                    ctr <= ctr+1;
                    nextstate <= `LSH2;
                end
            end
            `LSH2: begin
                if (shift[3]) begin
                    //ctr <= //don't change
                    //shift <= //don't change
                    nextstate <= `DONE;
                end
                else begin
                    shift <= {shift[2:0], 1'b0};
                    ctr <= ctr+1;
                    nextstate <= `LSH1;
                end
            end
            `DONE: begin
                //shift <= //don't change;
                //ctr <= //don't change;
                nextstate <= `DONE;
            end
        endcase
endmodule

Best Answer

What I see is the coding style where you have a registered and a combinatorial section. It is a good coding style but it also only works if you are 100% consistent in your code:
Everything you clock (state, counter, shift) must be in the clock section. and you must only use non-blocking "<=" assignments.
All combinatorial code must be fully! encoded and you must only use blocking "=" assignments.
Start by adding a next_shift and next_cnt and in the clocking section use

always @ (posedge clk)
   if (rst) 
   begin
      state <= `RST_STATE;
      shift <= `RST_SHIFT;
      cnt   <= `RST_CNT;
   end
   else
   begin
      state <= next_state;
      shift <= next_shift;
      cnt   <= next_cnt;
   end
 end

I am not going to re-code your whole combinatorial section (sorry) but there you use:

always @ ( * ) // easiest!
begin
    case (state)
        `RST_STATE: begin
            next_shift = x;
            next_ctr   = 0;
            next_state = `DONE;
        end
        `LSH1: begin
             if (shift[3]) begin
               next_shift = shift; // no change
               next_ctr   = cntr;  // no change
               next_state = `DONE; // 
             end
             else begin 
               next_shift = {shift[2:0], 1'b0};
               next_ctr   = cntr+1;  
               next_state = `LSH2;
             end
  etc...

end

As I said it is a good coding style because it can handle some nasty cases better but the other coding style (I just saw another answer appear) is much easier.

Disclaimer: code not compiled there may be typos in there