Forum Discussion

Altera_Forum's avatar
Altera_Forum
Icon for Honored Contributor rankHonored Contributor
14 years ago

Verilog PIPO Shift-register issue

Hi,

I have this Verilog code that implements a Shift-Register Parallel In - Parallel Out.

It should concatenate 2 parallel bits in vector until it reaches 16 (divide the input frequency by 8), when it does it should set an output flag (setOut). 


module data_packer
(
	input  pin,			// Parallel Input
	input	clk,
	input reset,			// Synchronous and assynchronous state machine controllers
	output reg  pout,	// Parallel Output
	output reg setOut		// Output ready signal
);
	// Declare registers
	reg	 delay_line;
	//reg	 delay_line_out;
	reg	 state;
	
	// Declare states
	parameter	S0 = 0, S1 = 1, S2 = 2, S3 = 3,
			S4 = 4, S5 = 5, S6 = 6, S7 = 7;
	
	// Output depends only on the state
	always @ (state) begin
		case (state)
			S0:
				delay_line <= {delay_line, pin};
			S1:
				delay_line <= {delay_line, pin};
			S2:
				delay_line <= {delay_line, pin};
			S3:
				delay_line <= {delay_line, pin};
			S4:
				delay_line <= {delay_line, pin};
			S5:
				delay_line <= {delay_line, pin};
			S6:
				delay_line <= {delay_line, pin};
			S7:
				delay_line <= {delay_line, pin};
		endcase
	end
	// Determine the next state
	always @ (posedge clk or negedge reset) begin
		if (~reset) begin
			state <= S0;
			setOut <= 0;
		end
		else
			case (state)
				S0:
					state <= S1;
				S1:
					state <= S2;
				S2:
					state <= S3;
				S3:
					begin
					state <= S4;
					setOut <= 1'b0;
					end
				S4:
					state <= S5;
				S5:
					state <= S6;
				S6:
					state <= S7;
				S7:
				begin
					state <= S0;
					pout <= delay_line;
					setOut <= 1'b1;
				end
					
				default:
					begin
					state <= S0;
					setOut <= 1'b0;
					end
			endcase
	end
endmodule

When I compile it it gives me this Warning: 


Warning (10235): Verilog HDL Always Construct warning at data_packer_rev0.v(31): variable "delay_line" is read inside the Always Construct but isn't in the Always Construct's Event Control
Warning (10235): Verilog HDL Always Construct warning at data_packer_rev0.v(31): variable "pin" is read inside the Always Construct but isn't in the Always Construct's Event Control

Repeated for every case of the "always @ (state) begin".

Since, it isn't supposed to enter the case whenever delay_line or pin changes, what it should be done?

Even though the warnings, it compiles successfully, but doesn't give any "setOut".

Kind regards

Pedro Ferreira

3 Replies

  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    14 years ago

    The Warning is that the delay_line result is based on state, delay_line and pin conditions, but the always block is only dependent on state.

    This warning is telling you synthesis may give you different results than simulation in this case.

    If you look at your code, you are in essence making a mux that all the inputs are the same.

    So the mux doesn't need to be there. delay_line <= {delay_line[13:0], pin[1:0]};

    in the always @ (posedge clk or negedge reset) begin block and remove the trouble always block completely.
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    14 years ago

    Hi anakha,

    Yes indeed it was a really messy way to do that...

    Using the delay_line in the always @ (posedge clk or negedge reset) block the warning doesn't appear.

    Could you explain why is that? Since the same registers are read inside the clock block.

    Another question. Is there a way to start the state machine without first giving the reset? Because now after programming the state machine isn't running, I first need to give the reset and then it works normally.

    I read in a state machine if it isn't given a reset, the Altera compiler will assume state S0 as the first state, and a reset isn't needed.

    Kind regards

    Pedro Ferreira
  • Altera_Forum's avatar
    Altera_Forum
    Icon for Honored Contributor rankHonored Contributor
    14 years ago

    On the first question: The warning is based on sensitivity list requirements. Normally all inputs that effect a change on the output are required in the sensitivity list, unless the change is based on a clock signal (IE posedge/negedge clk)

    So in your case, the always @(STATE) doesn't have all the required items. You can fix this in two other ways, then the one I mentioned. One is making it an always @(*) block, or always @(STATE or delay_line or pin). this would effectively make the block a combinational mux. Since delay_line is dependent upon itself, it would oscillate and not be the desired logic.

    The other way would be to have this as also be an always @(posedge clk) block.

    This would give you the desired logic, but would also allow you to have the function of delay_line be different depending on the state it's in. (IE some states could just hold it's value).

    on the second question, reset: It's always good practice to use a reset regardless. Yes FPGA logic can power up in a know state, but if you are depended upon this fact, you will get burned if you ever attempt to convert the logic to an ASIC.

    Every design I've done I put a reset in. In FPGA's sometimes I've left this reset to a pin that is just externally floating or tied high, and I have a weak pull-up on the pin. I've never had an issue with the FPGA starting up in the correct fashion. (Although you have to apply the reset in simulation to get out of the X states) HOWEVER in an ASIC you must tie and assert the reset, because the registers can power up in random states. So for proper functionality you need to get your statemachines into a know state.

    Pete