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!
Help with good verilog practices
Started by ●April 27, 2011