1
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ I don't see where your testbench ever asserts rst. Also, I'd advise resetting to something other than 4'b0000 or 4'b1111 if you actually want to see that the counter is doing something. \$\endgroup\$ – The Photon Feb 16 '15 at 17:51
  • \$\begingroup\$ I've temporarily disabled load and rst functionality. Neverthless, shouldn't my counter shift the initial value ? \$\endgroup\$ – Plutonium smuggler Feb 16 '15 at 17:53
  • \$\begingroup\$ Maybe the simulator is confused because you assign to q in both modules? q is an output from your counter. Don't assign it in your testbench. \$\endgroup\$ – The Photon Feb 16 '15 at 17:57
  • \$\begingroup\$ @ThePhoton . Thanks a lot. It works after removing it from TB. Was stuck on it for 2 days. \$\endgroup\$ – Plutonium smuggler Feb 16 '15 at 18:03
  • \$\begingroup\$ Feel free to answer your own question. I didn't post my comment as an answer because I was really just shotgunning in case it would help you find the problem. \$\endgroup\$ – The Photon Feb 16 '15 at 19:31
1
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Actually, I did all of this. But my design wouldnt work because as mentioned in the comments above, I was initialising an output port in test bench. So inorder to find out the error, I temporarily removed all non critical functionality like load and reset with an assumption that my counter should be able to shift any initial value as well. Thats why you see an initial block in main module. As far as non blocking assignment is concerned, i'll definitely change it now that you have pointed. \$\endgroup\$ – Plutonium smuggler Feb 17 '15 at 6:34
  • 1
    \$\begingroup\$ @Plutoniumsmuggler Yes, I understand. You should never define the signal connected to an output from your DUT as regs, nor assign values to them, because it may lead to two entities driving the same signal. A good simulator would detect this condition, give an error and stop the simulation. \$\endgroup\$ – gstorto Feb 17 '15 at 8:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.