What is wrong with my Verilog code ?!

[djnik1362]

hi
i wrote a serial BUS module that have 4
pins for communication.

This is my code :

module serial_bus_state(
	input ref_clk,
	input reset,
	input sclk,
	input sdout,
	input sen,
	output sdin,
	output reg [7:0] ADDR,
	output reg [7:0] DOUT,
	input [7:0] DIN,
	output rdy
    );
	
	localparam  [2:0]
			STATE_IDLE               = 3'd0,
			STATE_WAIT_ADDR_CLK_HIGH = 3'd1,
			STATE_WAIT_ADDR_CLK_LOW  = 3'd2,
			STATE_WAIT_WR_CLK_HIGH   = 3'd3,
			STATE_WAIT_WR_CLK_LOW    = 3'd4,
			STATE_WAIT_RD_CLK_HIGH   = 3'd5,
			STATE_WAIT_RD_CLK_LOW    = 3'd6,
			STATE_WR_READY           = 3'd7;

	reg [2:0] NEXT_STATE;
	reg [7:0] NEXT_BIT_MASK;
	reg [7:0] NEXT_ADDR;
	reg [7:0] NEXT_DOUT;
	
	
	reg [2:0] CUR_STATE;
	reg [7:0] CUR_BIT_MASK;
	
	always @(posedge ref_clk)
	begin
		if(reset | sen)
		begin
			CUR_STATE    <= STATE_IDLE;
			CUR_BIT_MASK <= 8'd1;
			ADDR         <= 8'd0;
			DOUT         <= 8'd0;
		end
		else
		begin
			CUR_STATE    <= NEXT_STATE;
			CUR_BIT_MASK <= NEXT_BIT_MASK;
			ADDR         <= NEXT_ADDR;
			DOUT         <= NEXT_DOUT;
		end
	end
	
	// -------------------------------
	
	always @(*)
	begin
		NEXT_STATE    = CUR_STATE;
		NEXT_ADDR     = ADDR;
		NEXT_DOUT     = DOUT;
		NEXT_BIT_MASK = CUR_BIT_MASK;
		

		case(CUR_STATE)
		
			STATE_IDLE :
			begin
				if(~sclk)
				begin
					NEXT_BIT_MASK = 8'd1;
					NEXT_ADDR     = 8'd0;
					NEXT_STATE    = STATE_WAIT_ADDR_CLK_HIGH;
				end	
			end
			
			// *******************

			STATE_WAIT_ADDR_CLK_HIGH :
			begin
				if(sclk)
				begin
					NEXT_ADDR  = {ADDR[6:0], sdout};
					NEXT_STATE = STATE_WAIT_ADDR_CLK_LOW;
				end
			end	
			
			// *******************
			
			STATE_WAIT_ADDR_CLK_LOW :
			begin
				if(~sclk)
				begin
					if(CUR_BIT_MASK[7])
					begin
						NEXT_BIT_MASK = 8'd1;
						
						if(ADDR[7])
							NEXT_STATE = STATE_WAIT_RD_CLK_HIGH;
						else	
						begin
							NEXT_DOUT  = 8'd0;
							NEXT_STATE = STATE_WAIT_WR_CLK_HIGH;
						end	
					end
					else
					begin
						NEXT_BIT_MASK = {CUR_BIT_MASK[6:0], 1'b0};
						NEXT_STATE = STATE_WAIT_ADDR_CLK_HIGH;
					end
				end
			end
			
			// *******************
			
			STATE_WAIT_WR_CLK_HIGH :
			begin
				if(sclk)
				begin
					NEXT_DOUT  = {DOUT[6:0], sdout};
					NEXT_STATE = STATE_WAIT_WR_CLK_LOW;
				end
			end
			
			// *******************
			
			STATE_WAIT_WR_CLK_LOW :
			begin
				if(~sclk)
				begin
					if(CUR_BIT_MASK[7])
						NEXT_STATE = STATE_WR_READY;
					else
					begin
						NEXT_BIT_MASK = {CUR_BIT_MASK[6:0], 1'b0};
						NEXT_STATE = STATE_WAIT_WR_CLK_HIGH;
					end
				end
			end	
			
			// *******************
			
			STATE_WAIT_RD_CLK_HIGH :
			begin
				if(sclk)
					NEXT_STATE = STATE_WAIT_RD_CLK_LOW;
			end
			
			// *******************
			
			STATE_WAIT_RD_CLK_LOW :
			begin
				if(~sclk)
				begin
					if(CUR_BIT_MASK[7])
						NEXT_STATE = STATE_IDLE;
					else
					begin	
						NEXT_BIT_MASK = {CUR_BIT_MASK[6:0], 1'b0};
						NEXT_STATE        = STATE_WAIT_RD_CLK_HIGH;
					end	
				end
			end
			
			// *******************

			STATE_WR_READY :
			begin
				NEXT_STATE = STATE_IDLE;
			end
				
		endcase
	end

	// -------------------------------
	
	assign sdin = ((CUR_BIT_MASK & DIN) == 0) ? 1'b0 : 1'b1;
	assign rdy = (CUR_STATE == STATE_WR_READY);
	

endmodule

In simulation (Behavioral and Post-Route) it works O.K. without any error.

In real word a micro controller master this bus
and with 10000 tries in reading FPGA there is
random success tries . With changing Timing
Constraints result can be better or worst.
Best result that can be achieved is about 8500 success reads and
Worst result that i have encountered is about only 1700
success reads from 10000 read try.
I am use only a Global Clock Timing Constraint
with my 50 MHz input clock.

I am using Spartan 6 LX45 and ISE 12.1.

Thanks for your support.

 

[FvM]

There's serious problem caused by processing the unrelated external signals sen and sclk without synchronizing them properly to the design clock (ref_clk). The problem occurs if the external signals accidently change their state near to the active edge of the design clock and FFs with an input depending on this signals experience a violation of setup or hold timing requirements. Very rarely this causes true metastable events, more likely multiple FFs see different external signal states due to logic delay skew. As a result, the state machine can be switched to a wrong or even being caught in an illegal state. Likewise, counters can advance to an unexpected value.

I also think that sen shouldn't be used as an asynchronous reset, because it brings up additional timing problems.

Synchronization schemes in FPGA design are serial topic at edaboard, you'll find many references like this recent thread https://www.edaboard.com/threads/308514/

 

[djnik1362]

Thanks FvM

I check sclk at positive edge of ref_clk in FSM .
Isn't this synchronous to ref_clk ?

There is something exciting i forget to tell :
When i optimize synthesis to Area i get better results
than when i am using Speed optimization .

Thanks for your support.

 

[FvM]

But sclk is toggling at arbitrary times related to ref_clk, isn't it? That's causing the problem.

You should take the time to study existing SPI slave or similar serial receiver designs, try to understand how they synchronize external signals and why they do it like that.

 

[djnik1362]

But sclk is toggling at arbitrary times related to ref_clk, isn't it? That's causing the problem.

yes . sclk is not related to ref_clk .
any quick suggestion to solve this problem?

 

[mrflibble]

yes . sclk is not related to ref_clk .
any quick suggestion to solve this problem?

step 1: add a synchronizer
step 2: wtf
step 3: read about synchronizers
step 4: add a working synchronizer

- - - Updated - - -

And I should say add synchronizers since the data signal need to be synced as well. You should just forget that the spi clock is a clock, and treat it as regular data as far as synchronization is concerned.

 

[djnik1362]

thank you guys.
I used synchronizer and my problem has been solved.