[SOLVED] Verilog: What is the proper way to write a test module?

[Alper özel]

I am trying to write an SPI master module on my own to learn FPGA-Verilog efficiently.

This is my spi_master module:

module spi_master(

		input [15:0] tx_data,
		output [15:0] rx_data,
		output mosi,
		input miso,
		output cs,
		output sck,
		input start,
		input clk
	
    );
	 
	 wire [15:0] tx_data;
	 
	 reg [3:0] tx_counter;
	 reg [3:0] rx_counter;
	 
	 wire start;
	 reg cs;
	 reg mosi;
	 reg [15:0] rx_data;
	 wire miso;
	 reg sck;
	 
	 reg [3:0] clk_divider;

	 initial begin
	 
	 clk_divider [3:0] = 4'b0;
	 tx_counter [3:0] = 4'b0;
	 rx_counter [3:0] = 4'b0;
	 cs = 1;
	 sck = 0;
	 
	 end
	 

	always @(start) begin
		
		if (clk_divider == 4'b1000) begin   // divided by 8
			sck =  ~sck;
		end
		if (clk_divider >= 4'b1000) begin
			clk_divider [3:0] = 4'b0;
		end
		else begin
			#1 clk_divider = clk_divider + 4'b1;
		end
		
	end

	always @(negedge sck) begin
	
		if (cs == 0 && tx_counter != 4'b1111) begin
			#(clk_divider/2) mosi <= tx_data[tx_counter];
			#(clk_divider/2 + 1) tx_counter <= tx_counter + 4'b1;
			
		end
		else if(cs == 0 && tx_counter == 4'b1111) begin
			#(clk_divider/2) cs <= 1'b1;
		end
		else if (cs == 1 && tx_counter == 0) begin
			#(clk_divider/2) cs <= 1'b0;
		end
		
	end
	
	always @(posedge sck) begin
	
		if (cs == 0 && tx_counter <= 4'b1111 && tx_counter >= 4'b0001) begin
			#(clk_divider/2) rx_data[rx_counter] <= miso;
			#(clk_divider/2 + 1) rx_counter <= rx_counter + 4'b1;
			
		end
		
	end
	

endmodule

and this is my test module:

`include "spi_master.v"

module spi_master_tb(
    );

reg clk;
reg start;
wire mosi;
reg miso;

reg cs;
reg sck;

reg [15:0] tx_data;
//reg [15:0] rx_data;

initial begin

clk = 0;
tx_data = 16'hF0AA;
#20 start = 1'b1;
miso = 1;
#1000 $finish;

end

always begin 
	#1 clk = ~clk;
end
	

spi_master SPI_block(

		tx_data,
		rx_data,
		mosi,
		miso,
		cs,
		sck,
		start,
		clk
	
    );

endmodule

The problem is test module seem like not communication with the spi module(as seen in the picture). When you check the ISim diagrams, clk is working and any initialization on test module is working(Start, miso) but Tx_data is not passed to mosi and sck is not dividing clk. Where am I doing wrong? I can understand the other s not working but sck is pretty easy don't you think?

I appreciate any help. Thanks.

 

[ads-ee]

problem 1 you can't create asynchronous counters if you want something that will actually work on hardware. Verilog isn't like C programming it's a hardware description language and the bellow code does not describe a counter.

	always @(start) begin
		
		if (clk_divider == 4'b1000) begin   // divided by 8
			sck =  ~sck;
		end
		if (clk_divider >= 4'b1000) begin
			clk_divider [3:0] = 4'b0;
		end
		else begin
			#1 clk_divider = clk_divider + 4'b1;
		end
		
	end

Your counter also doesn't do a divide by 8 it's a divide by 9.... 0,1,2,3,4,5,6,7,8 (there are 9 values before it repeats)
The #1 has no place in synthesizable code in my opinion. Besides that you don't have the timescale defined for this anywhere in the posted code. From the waveform it looks like it's defaulted to 1ns probably 1ns/1ps. So your clock is 500 MHz which is highly unlikely for SPI.

SCK doesn't toggle because both the initial value is X and stays X as there is no statements initializing it to a value at the start of simulation (not necessarily synthesizable) or a reset on the flip-flop generating it.

The typical way to "do" SPI is to use a much faster clock. You then sample the SPI bus (slave) and generate the SPI signals (master) using that faster clock domain. The only time you don't do this is if you have a design that does not have any clocks available when starting up, e.g. a design that programs the board VCOs that are used by the FPGA/CPLD. In such a case you have to use the SPI clock to run the slave interface.

I'm sure there are other problems, but I only glanced at your code.

- - - Updated - - -

Edit...

Don't use positional port assignments it's a really bad practice and can render code un-maintainable or at minimum make it really easy to add a port and end up with a design that is no longer connected correctly. Instead use the named port association syntax.
e.g.

spi_master SPI_block(
  .tx_data (tx_data),
  .rx_data (rx_data),
  .mosi    (mosi),
  .miso    (miso),
  .cs      (cs),
  .sck     (sck),
  .start   (start),
  .clk     (clk)
);

If you add a new port it doesn't matter where you add the line assigning the new connection to the module.

 

[Alper özel]

Thanks for such detailed answer. But I thought that I initialized sck in the line:

	 initial begin
	 
	 clk_divider [3:0] = 4'b0;
	 tx_counter [3:0] = 4'b0;
	 rx_counter [3:0] = 4'b0;
	 cs = 1;
	 sck = 0;
	 
	 end

does not that cover it?

Also, to make a hardware counter is this the way?

	always @(posedge clk) begin
	
			if (!clk_divider != 3'b111) begin
				clk_divider <= clk_divider + 1;
			end
			else
				clk_divider <= 3'b0;
			end
			
	end

 

[ads-ee]

Didn't see the sck initialization, the X is probably a result of the asynchronous counter, which won't simulate correctly. Your new counter code has an extra ! before the clk_divider in the if expression.

I would probably code this counter with a reset like this..

always @(posedge clk) begin
  if (rst)
    clk_divider <= 0;
  else
    clk_divider <= clk_divider + 1;
end

you don't need to set it back to 0 as the counter will rollover without requiring any compare operations. Or a more generic counter.

parameter N = 8; // maximum number of counts (does not need to be 2^N)
always @(posedge clk) begin
  if (rst)
    clk_divider <= 0;
  else if (clk_divider < N-1)
    clk_divider <= clk_divider + 1;
  else
    clk_divider <= 0;
end