Ring counter in verilog

Question

I have the task of designing a ring counter in verilog using shift operator. Here is the code so far along with test bench :

module ring (

input wire [3:0] data,
input wire clk,
input wire rst,
input wire load,
output reg [3:0] q 
) ;

/*When the load pin is High, the data is loaded in the counter on clock transition.
* Otherwise the data is shifted  upon each clock. 
* Last output is fed back to the first input.
*/

initial     q = 4'b1010 ;

reg tmp = 1'b0 ;

always @ (posedge clk, posedge rst)
begin
if(rst)
    begin
    q <= 4'b0000 ;
    end
else if(load)
    begin
    q[3] <= data[3] ;
    q[2] <= data[2] ;
    q[1] <= data[1] ;
    q[0] <= data[0] ;
    end

else
    begin
    tmp = q[0] ;
    q = q >> 1 ;
    q[3] = tmp ;
    end
end
endmodule 

Test bench

`timescale 1 ns / 1 ns

module ringTest ;

//Signals
reg [3:0] data = 4'b1010 ;
wire [3:0] q = 4'b0000 ;
reg clk = 1'b0 ;
reg rst = 1'b0 ;
reg load = 1'b0 ;

//Instantiate uut
ring uut (  .data(data),
    .q(q),
    .clk(clk),
    .rst(rst),
    .load(load)
);

always  #10 clk = ~clk ;

/*Load data every 100 s
always  
begin
#95 load = 1'b1 ;
#5 load = 1'b0 ;
end
*/
//Data to be loaded
always 
begin
if ( data == 4'b1111) data <= 4'b0000 ;
else    begin
    # 50 data <= data + 4'b0001 ;
    end
end
endmodule

Note : I have temporarily disabled load functionality bcoz the Counter wasnt working. So instead I chose to shift the data with an initial value, which also did not work. The output is always X.

Where am I going wrong ?

Thank you.

Answer 1

First of all, initial commands are not used in synthesizable Verilog code. And I am not sure if I follow why you are using it, given you have already a reset state for it.

Secondly, in you else statement, you are using a blocking assignment in "tmp = q[0]" which is ok, because it stores q[0] value in a variable internal to the process.

What is not OK, is continuing using blocking assignments in "q = q << 1" and in "q[3] = tmp". It does not make any sense, you are modeling a sequential output, you should model it using a non-blocking assignment like: "q <= q << 1" and "q[3] <= temp". I don't know what the simulator will do, but probably he will change q signal as soon as it executes this line, and not at the next rising edge clock as you should expect. As the synthesizer won't be capable of doing that, you are going to have hardware behavior which is not modeled by you simulation. (very serious)

I have also noticed you do not perform a reset in your module. This is surely a malpractice, the first thing you should do, before pumping any data into your module is to reset it.

Lastly, have you checked in you waveform that you have a rising edge at the same time you send a load signal level high? As far as I have read your test bench I do not believe this occurs.

So you don't reset you module, you don't send a load at a rising edge, and you are complaining your output is at X? Well, you should re-read all the basics of Verilog, and learn how to properly use a simulator.