Getting latch errors on my module, can anyone please give me some insight?

[johnbizzee]

Hi, I need some help on some warnings on my module. Xilinx is saying I have latches at counter, ph, and product. The output is also incorrect but I am sure I have the logic correct. Can anyone please give me some insight, thank you very much.

module multiply
(
input clk, reset, start,
input [7:0] A, B,
output reg done_tick,
output wire [15:0] out );

localparam [1:0]
	idle = 2'b00,
	op   = 2'b01,
	done = 2'b10;
	
// signal declaration
reg carry;
reg[1:0] state, next_state;
reg [15:0] product;
reg [7:0] ph;
reg [4:0] counter; 

always @(posedge clk, posedge reset)
	if (reset)
		begin
			state <= idle;
		end
	else
		begin
			state <= next_state;
		end
	
	// next state logic
	
always @ *
	begin
		next_state = state;
		done_tick = 1'b0;
	
	case(state)
		idle:
			begin
				if(start)
					begin
						product[15:8] = 8'd0;
						product[7:0] = B;
						ph = A;
						counter = 7;
						carry = 1'd0;
						next_state = op;
					end
				
			end
		
		op:
			begin
				if(product[0] == 1)
					begin
						{carry,product[15:8]} = product[15:8] + ph; 
						// shift to the right
						product[15:0] = {carry,product[15:1]};
						carry = 0;
						counter = counter - 1;
					end
				else
					begin
						product[15:0] = {carry,product[15:1]};
						carry = 0;
						counter = counter - 1;
					end
			  if(counter == 0)
					begin
						next_state = done;
					end
			end
		
		done:
			begin
				done_tick = 1'b1;
				next_state = idle;
			end
			
		default: next_state = idle;
		
	endcase
	
end

assign out = product;

endmodule

 

[kommu4946]

Hi, I need some help on some warnings on my module. Xilinx is saying I have latches at counter, ph, and product. The output is also incorrect but I am sure I have the logic correct

Latches are generating due to incomplete case statements. To avoid this initialize each signal to default value like this.

always @ *
	begin
		next_state = state;
		done_tick = 1'b0;
	    product=0;
		 ph=0;
		 counter=0;
		 carry=0;
		 done_tick=0;
		 
	case(state)
		idle:
                .......
.               op:
               ........

and also check the fallowing two lines
product[15:8] = 8'd0; product[7:0] = B;
replace with this statement
product={8'b0,B};

 

[johnbizzee]

Thanks a lot, that got rid of all the errors. The only problem I have now is the output. When I try 4 x 4 the result is 4. I am pretty sure the logic is correct it has to be some little detail I am messing up in the code.

 

[ads-ee]

Think about what your case represents in hardware especially the count = count -1; lines. How are you supposed to perform a combinational counter without saving the value for the next time you need to decrement the value? Well the only way would be to have a latch, which I'm not even sure this code will do (depends on the defaults you added).

This is one of the problems with using this FSM style, when you include outputs in the FSM state transition combinational logic you might make the mistake of including a counter, which can't be realized in hardware unless a latch is implemented. Personally if I had to write code using this style of FSM (i.e. forced by someone writing my paycheck) I would never include outputs in the FSM state transition logic. I would decode the outputs outside the FSM and use clocked always blocks for any counters.

Either remove the counter from the combinational always block and move it into a separate clocked always block and decode the state (i.e. state == op) or get rid of the combinational always and make it a single clocked always block with all the state transitions and outputs generated in that single clocked always block.

Don't forget to change all the = to <= if you decide to use a clocked always implementation.

Regards