New Users Forum

[farazk86]

Hi..

I get this annoying error only when I instantiate a module into another one.. when I write the code in a single module it simmulates without any problem..

here is the instatiation line:

bounce_counter count_display(.clock(clock), .reset(reset), .out(first), .sw(btn));

 and here is the instatiated module:

module bounce_counter(
    input reset,
    input clock,
    input sw,
	 output reg led,
    output [3:0] out
    );

localparam [1:0]
				low = 2'b00,
				high = 2'b01;
				
reg [3:0]count;
reg [1:0]next;
reg [1:0]now;

always @ (posedge clock or posedge reset)
	begin
		if (reset)
			begin
				now <= low;
				count <= 0;
			end
		else
			now <= next;
	end

always @ (*)
	begin
		next <= now;		//defualt state is the same
		
		case(now)
			
			low:
				begin
					led <= 1'b0;
					if(sw)
						next <= high;
				end
			high:
				begin
					led = 1'b1;
					count <= count + 1;
					if(count == 4'd9) count <= 0;
					if(~sw)
						next <= low;
				end
			default:
						next <= low;
		
		endcase
	end
	
assign out = count;

endmodule

it is pointing to the last line "assign out = count" while giving this error..

Ive been at this all day and its driving me up the wall.

Can you PLEASE tell me what Im doing wrong :(

Thank you

[gszakacs]

The problem is that you're incrementing "count" in an asynchronous process.  This creates a loop

because count is scheduled to increment when the process completes (non-blocking assign) and

due to the @* including count in the sensitivity list, this causes the process to retrigger.

1) Non-blocking assignments are bad in an asynchronous process.

2) You need a clocked process if you want to build any sort of sequential logic including counters.

3) Never pay too much attention to the line marked in an error message.  You usually have to trace back

to the real error source - in this case "count"

-- Gabor

[eteam00]

Gabor,

I think there is a missing begin - end enclosing lines 26-27 of your posted code (shown below).

25    else

        begin
26        now <= next;
27        count <= next_count;

        end

Faraz,

An error in your original code (which is not present in Gabor's code) is that you assign a value to count in two different processes.  This is not allowed!  This will result in a multiple-source error.

-- Bob Elkind

[farazk86]

@Gabor,

Thanks a lot for that.. I thought every statement within an always block must be a non blocking assignment.. so I guess it has to do with always blocks with a clock signal.

also can you please tell me why you used count in else block in line 27 and also later incremented the same in case block.


@Bob

different processes? are you talking about this:

count <= count + 1;
if(count == 4'd9) count <= 0;

?

Thanks

[eteam00]

@Bob
different processes? are you talking about this:

count <= count + 1;
if(count == 4'd9) count <= 0;

This would be legal if/when contained in a single process.  In another (preceding) process you also have:

if (reset)
  begin
    now <= low;
    count <= 0;

And this is the problem:  count is defined (assigned a value) in more than one process.

• Each always procedural block is a separate process, and defines a separate set of logic hardware.

• When you assign a value to a register or signal in a process, you are infering a hardware instance of logic which generates that register or signal.

• When a single register or signal is infered in multiple processes, you are creating multiple and conflicting sets of hardware which 'drive' a single signal.

-- Bob Elkind

[eteam00]

I thought every statement within an always block must be a non blocking assignment.. so I guess it has to do with always blocks with a clock signal.

You misunderstand.  Verilog is (originally) a simulation/modeling language, not a hardware design language.  Verilog has evolved into a HDL, partially by the adoption of certain conventions between the language masters/custodians and the synthesis tool designers.

One of these conventions is that the phrase "always @(posedge <clock>)" is interpreted by synthesis tools as clocked register logic.  Without the "posedge" qualifier, the procedural block (or process) is synthesised as a combination of combinatorial and transparent latch logic.  The entries in the process sensitivity list (absent "posedge") do not affect the hardware synthesis, they only affect the simulator (when the process is re-evaluated).

also can you please tell me why you used count in else block in line 27 and also later incremented the same in case block.

An interesting misunderstanding here.  In Gabor's code, the count register is modified in lines 23 and 27.  The register count is not modified in the case statement (which appears in the next process).  The signal next_count is modified in the case statement code, but the count register is not modified.

-- Bob Elkind

[gszakacs · Accepted solution]

If I may add another 2 cents, I would never have used two processes for this design.

You can do everything you need inside the clocked process, and that would prevent the

sort of error you ran into.

Search the forums for "two process state machine" or "two process FSM" and you will

see that, although many text books teach this approach, most experienced designers

don't use it.

In the attached code, I assign led during the state transitions to mimick the current behaviour

of the two-process logic.

Some more points:

Your state machine really only needs two states, but the now variable is defined with 2 bits.

The default clause in the case statement will not be reached, and therefore synthesis will

ignore it - it does not guarantee synthesis of a "safe" implementation.

This logic has the potential to break in real hardware if the sw input is not already synchronized

to the clock.  This is especially true if XST decides to implement the logic as one-hot.

-- Gabor

[farazk86]

Thanks a lot for your helpful response guys, Ive learned a lot from this thread alone.

@Gabor,

you are right regarding I did not have to code as I did, the only reason I did that was because the book I am following emphasized on coding it like that.

Thanks again for your help Bob and Gabor :)

[gszakacs]

---

farazk86 wrote:

Thanks a lot for your helpful response guys, Ive learned a lot from this thread alone.

@Gabor,

you are right regarding I did not have to code as I did, the only reason I did that was because the book I am following emphasized on coding it like that.

Thanks again for your help Bob and Gabor :)

---

As Bassman is fond of saying - burn that book.

Seriously, it is a good idea to get used to single-process state logic.

-- Gabor

[farazk86]

haha.. will do :)

I personally like the single process style and will continue to code it like this.

Also before closing this topic I would like to ask another question related to the same code.. The instantiation of the input button..

The input button is used in the module I listed above.. it had nothing to do with the module it was instantiated from so initially I did not instantiate it, but when I created the testbench using the top module I found out it did not have access to the input button, so I introduced an input in the top level module and connected them together in this part...

module mux_sseg(
    input clock,
    input reset,
	 input btn,
    output reg a,
    output reg b,
    output reg c,
    output reg d,
    output reg e,
    output reg f,
    output reg g,
    output dp,
    output [3:0] an,
    input [3:0] first
    );

bounce_counter count_display(.clock(clock), .reset(reset), .out(first), .sw(btn));

 the input, "btn" is not used here, it was only introduced so that the input can used in the test bench...

so what Im basically asking.. is this the right method? can I do that?

Thanks