FPGARelated.com
Forums

Help with good verilog practices

Started by Sink0 April 27, 2011
Hi, i have written the following verilog code. Its basically a WB
master to read and write from OC PCI bridge core.

Can you guys point me what is "ugly" on my code, or what is a bad
practice? I just want to learn what is a good practice programming
with verilog, and why.

`include "network_controller_constants.v"

module NETWORK_CONTROLLER_WB_MASTER
(
	CLK_I,
   RST_I,

	MINT_O,
	MADR_O,
	MDAT_O,
	MDAT_I,
	MSEL_O,
	MCYC_O,
	MSTB_O,
	MWE_O,
	MCAB_O,
	MACK_I,
	MRTY_I,
   MERR_I,

	tx_b1_offset,
	tx_b2_offset,
	rx_b1_offset,
	rx_b2_offset,

	r_cnt_reg,
	r_cmnd_flag,

	tx_b_1_int,
	tx_b_2_int,
	rx_b_1_int,
	rx_b_2_int,

	tx_b_1_over,
	tx_b_2_over,
	rx_b_1_over,
	rx_b_2_over,

	r_counter_empty,
	counter_loaded,

	w_cmnd_flag,
	w_counter_loaded,
	w_cnt_reg,

	leds,

	wb_reading,
	wb_r_data
);


input			   	CLK_I;
input             RST_I;

output				MINT_O;
output   [31:0]  	MADR_O;
input    [31:0]  	MDAT_I;
output   [31:0]  	MDAT_O;
output   [3:0]   	MSEL_O;
output            MCYC_O;
output            MSTB_O;
output            MWE_O;
output            MCAB_O;
input            	MACK_I;
input            	MRTY_I;
input            	MERR_I;

input		[31:0]	tx_b1_offset;
input		[31:0]	tx_b2_offset;
input		[31:0]	rx_b1_offset;
input		[31:0]	rx_b2_offset;

output				tx_b_1_over;
output				tx_b_2_over;
output				rx_b_1_over;
output				rx_b_2_over;

input		[31:0]	r_cnt_reg;
input					r_cmnd_flag;

input					tx_b_1_int;
input					tx_b_2_int;
input					rx_b_1_int;
input					rx_b_2_int;

output				r_counter_empty;

output	[31:0]	wb_r_data;
output				wb_reading;

input					w_cmnd_flag;
output				w_counter_loaded;
input		[31:0]	w_cnt_reg;

output	[3:0]		leds;

output				counter_loaded;

parameter WB_IDLE	= 	  4'b0001;
parameter WB_WRITING	= 4'b0010;
parameter WB_READING	= 4'b0100;
parameter WB_W_WAIT	= 4'b1000;

reg		[31:0]	MADR_O = 32'h00000000;
wire		[31:0]	MDAT_O;
wire		[31:0]	MDAT_I;
wire		[3:0]		MSEL_O;
reg					MCYC_O = 0;
reg					MSTB_O = 0;
wire					MWE_O;
reg					MCAB_O;
wire					MACK_I;
wire					MRTY_I;
wire					MINT_O;

reg		[31:0]	r_counter = 0;
reg		[3:0]		state_machine = WB_IDLE;
reg		[3:0]		next_state = WB_IDLE;
reg					write_done = 0;
wire					r_counter_empty;
wire					wb_int_gen;


reg					tx_b_1_over = 0;
reg					tx_b_2_over = 0;
reg					rx_b_1_over = 0;
reg					rx_b_2_over = 0;

reg		[31:0]	w_counter = 0;
wire					w_counter_empty;

wire		[31:0]	wb_r_data;
wire					wb_reading;

reg		[30:0]	r_w_addr = 0;

//###########################################################
//							DEBUG VARIABLES
reg 		[3:0]		MRTY_C = 0;
reg		[3:0]		MACK_C = 0;
reg		[3:0]		MINT_C = 0;
reg 		[3:0]		WRITE_C = 0;
reg		[3:0]		leds;

//###########################################################

assign MSEL_O = 4'b1111;
assign MINT_O = wb_int_gen;

assign wb_int_gen = 		tx_b_1_int||
								tx_b_2_int||
								rx_b_1_int||
								rx_b_2_int;

/
*##################################################################################
############################ state_machine CONTROL
#################################
##################################################################################*/

always@(*)
begin
	tx_b_1_over = 1'b0;
	tx_b_2_over = 1'b0;
	rx_b_1_over = 1'b0;
	rx_b_2_over = 1'b0;
	next_state = state_machine;
	case (state_machine)
		WB_IDLE :
			begin
				if(r_counter > 32'h0)
				begin
						next_state = WB_READING;
				end
				else
				begin
					if(w_counter > 32'h0)
					begin
						next_state = WB_WRITING;
					end
				end
			end
		WB_READING :
			begin
				if(r_counter == 32'h0)
				begin
					next_state = WB_IDLE;
					rx_b_1_over = 1'b1;
				end
			end
		WB_WRITING :
		begin
				if(w_counter == 32'h0)
					next_state = WB_W_WAIT;
		end
		WB_W_WAIT :
			begin
				if(write_done)
				begin
					next_state = WB_IDLE;
					tx_b_1_over = 1'b1;
				end
			end
		default:begin
				next_state = WB_IDLE ;
            end
	endcase
end


/
*##################################################################################
############################# write_done CONTROL
###################################
##################################################################################*/

always@(posedge CLK_I)
begin
	write_done = 1'b0;
	if(state_machine == WB_W_WAIT)
	begin
		if(MCYC_O && MACK_I)
			write_done = 1'b1;
	end
end


/
*##################################################################################
############################ state_machine TIMING
##################################
##################################################################################*/

always@(posedge CLK_I)
begin
		state_machine <= next_state;
end


always@(posedge CLK_I)
begin
		case (state_machine)
			WB_IDLE : begin
				if(r_cmnd_flag)
				begin
					r_counter <= r_cnt_reg;
					r_w_addr <= 30'h0;
				end
				else
				begin
				if(w_cmnd_flag && (~tx_b_1_int || ~tx_b_2_int))
				begin
					w_counter <= w_cnt_reg;
					r_w_addr <= 30'h0;
				end
				end
			end
			WB_READING : begin
				if(MCYC_O && MACK_I)
				begin
					if(r_counter > 0)
					begin
						r_counter <= r_counter - 32'h1;
						r_w_addr <= r_w_addr + 30'h4;
					end
				end
			end
			WB_WRITING : begin
				if(MCYC_O && MACK_I)
				begin
					if(w_counter > 0)
					begin
						w_counter <= w_counter - 32'h1;
						r_w_addr <= r_w_addr + 30'h4;
					end
				end
			end
		endcase
end

assign MDAT_O = w_counter;
assign MWE_O = (next_state == WB_WRITING)? 1'b1 : 1'b0;
assign wb_r_data = MDAT_I;

always@(*)
begin
	case(next_state)
		WB_IDLE: begin
			MADR_O[30:0] <= r_w_addr + tx_b1_offset[30:0];
			MADR_O[31] <= 1'b0;
			MCAB_O <= 1;
			end
		WB_READING: begin
			MADR_O[30:0] <= r_w_addr + tx_b1_offset[30:0];
			MADR_O[31] <= 1'b0;
			MCAB_O <= 1;
			end
		WB_WRITING: begin
			MADR_O[30:0] <= r_w_addr + rx_b1_offset[30:0];
			MADR_O[31] <= 1'b1;
			MCAB_O <= 1;
			end
		WB_W_WAIT: begin
			MADR_O[30:0] <= tx_b1_offset[30:0] + `DUMMY_READ_ADDR;
			MADR_O[31] <= 1'b0;
			MCAB_O <= 0;
			end
		default: begin
			MADR_O[31:0] <= 0;
			MCAB_O <= 1;
			end
	endcase
end

always@(*)
begin
	leds[0] <= state_machine[0];
	leds[1] <= state_machine[1];
	leds[2] <= MINT_O;

	if(next_state == WB_IDLE || next_state != state_machine)
	begin
		MSTB_O <= 1'b0;
		MCYC_O <= 1'b0;
	end
	else
	begin
		MSTB_O <= 1'b1;
		MCYC_O <= 1'b1;
	end
end

assign wb_reading		  = (state_machine == WB_READING) ? 1'b1 : 1'b0;
assign r_counter_empty = (r_counter > 0)? 1'b0 : 1'b1;
assign w_counter_empty = (w_counter > 0)? 1'b0 : 1'b1;

pulse_gen read_ld_pulse
(
	.Trigger				(r_counter_empty),
	.Pulse_Out			(counter_loaded),
	.Clock				(CLK_I)
);

pulse_gen write_ld_pulse
(
	.Trigger				(w_counter_empty),
	.Pulse_Out			(w_counter_loaded),
	.Clock				(CLK_I)
);

endmodule


Thank you!