Help with good verilog practices

[Sink0]

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!