Verilog - Signal is connected to multiple drivers, error.

[pigtwo]

Hey guys!  I'm still working with FPGAs and I hit kind of an odd problem but I'm sure its very simple.

First off I'm using the Nexys 2 board and I'm trying to make a memory controller for the on board memory. 

I'm getting these two, simple enough, errors:

Code: [Select]

ERROR:Xst:528 - Multi-source in Unit <sram_test> on signal <N0>; this signal is connected to multiple drivers.
ERROR:Xst:528 - Multi-source in Unit <sram_control> on signal <LB_sram>; this signal is connected to multiple drivers.
The strange part about this is for the first error, the signal N0 is never used in my program.  I did a ctrl+f of the file and 'N0' isn't even in it.  The second error at least references an signal I can find.  The problem this time is that the signal is defined as an output and the only time it is used it is assigned the constant zero.

Before I post the code I'd like to say that I am still very terrible at Verilog and when I write Verilog I feel as clumsy as a drunk baby.  But I doubt you would need to look though all of it. 

Here is the program that generated the first error.  It's purpose is just to test the memory controller that I wrote.

Code: [Select]

`timescale 1ns / 1ps
module sram_test(
input [2:0] btn,   // Buttons used for output
input clk, reset,
input [7:0] sw, // Will be used for the data input and address to the memory
output [7:0] led,  // Outputs for the read operation
// Outputs that are generated by the memory controller
output OE_sram, WE_sram, CE_sram, ADV_sram, LB_sram, UB_sram,
output [23:0] addr_sram,
inout [15:0] dio_sram
    );


// Symbolic state defintions
localparam [1:0]
idle = 2'b00,
read = 2'b01,
write = 2'b10;

// Internal definions
reg [7:0] data_reg, data_next, data_out_reg, data_out_next;
reg [1:0] state, state_next;
reg mem, mem_next, rw, rw_next;
wire [2:0] db_btn;
wire [7:0] sram_output;
wire ready;

// Register transistion
always @(posedge clk)
begin
if (reset)
begin
data_reg <= 0;
state <= idle;
data_out_reg <= 0;
mem <= 0;
rw <= 0;
end
else
begin
data_reg <= data_next;
state <= state_next;
data_out_reg <= data_out_next;
mem <= mem_next;
rw <= rw_next;
end
end

// FSMD
always @*
begin
data_next = data_reg;
state_next = state;
data_out_next = data_out_reg;
mem_next = 1'b0;

case(state)
idle:
begin
if (db_btn[0])    // Load to data reg
begin
data_next = sw;
end
else if (db_btn[1]) // Write
begin
state_next = write;
end
else // Read
begin
state_next = read;
end
end
write:
begin
mem_next = 1'b1;
rw = 1'b0;
if (ready)
state_next = idle;
end
read:
begin
mem_next = 1'b1;
rw = 1'b1;
if (ready)
begin
state_next = idle;
data_out_next = sram_output;
end
end
endcase
end

assign led = data_out_reg;

sram_control sram1
( .addr_in(sw), .data_in(data_reg), .mem(mem), .rw(rw), .clk(clk),
  .reset(reset), .ready(ready), .data_out_reg(sram_output), .addr_sram(addr_sram),
  .dio_sram(dio_sram), .OE_sram(OE_sram), .WE_sram(WE_sram), .CE_sram(CE_sram),
  .ADV_sram(ADV_sram), .LB_sram(LB_sram), .UB_sram(UB_sram) );


debounce unit0
( .clk(clk), .reset(reset), .sw(btn[0]), .db_tick(db_btn[0]) );

debounce unit1
( .clk(clk), .reset(reset), .sw(btn[1]), .db_tick(db_btn[1]) );

debounce unit2
( .clk(clk), .reset(reset), .sw(btn[2]), .db_tick(db_btn[2]) );
endmodule

The second one is a little more complicated.  It is attempting to access a Micron M45W8MW16 8MBx16 RAM chip.  I'm being very generous on the timing of this and I'm pretty sure there are some wasted cycles.  This is my first attempt so I wasn't going for the highest speed.

Code: [Select]

`timescale 1ns / 1ps

module sram_control(
// Controller data inputs
input [23:0] addr_in,
input [15:0] data_in,
// Controller control inputs
input mem, rw, clk, reset,
// Controler control outputs
output ready,
// Controler data outputs
output [15:0] data_out, data_out_reg,
// SRAM data outputs  (IE going to the sram)
output [23:0] addr_sram,
inout [15:0] dio_sram,
// SRAM control outputs
output OE_sram, WE_sram, CE_sram, ADV_sram, LB_sram, UB_sram
    );

// Symbolic state definitions
localparam [3:0]
idle = 4'b0000,
read0 = 4'b0001,
read1 = 4'b0010,
read2 = 4'b0011,
read3 = 4'b0100,
write0 = 4'b0101,
write1 = 4'b0110,
write2 = 4'b0111,
write3 = 4'b1000,
write4 = 4'b1001,
write5 = 4'b1010;

// Internal defintions
reg [3:0] state, state_next;
reg [23:0] address_reg, address_next;
reg [15:0] data_to_sram, data_from_sram, data_to_sram_next, data_from_sram_next;
reg we_reg, we_next, oe_reg, oe_next, tri_reg, tri_next, ready_reg, ready_next;

// FSMD register and data transitions
always @(posedge clk)
begin
if (reset)
begin
state <= idle;
address_reg <= 0;
data_to_sram <= 0;
data_from_sram <= 0;
we_reg <= 1'b1;
oe_reg <= 1'b1;
tri_reg <= 1'b1;
ready_reg <= 1'b1;
end
else
begin
state <= state_next;
address_reg <= address_next;
data_to_sram <= data_to_sram_next;
data_from_sram <= data_from_sram_next;
we_reg <= we_next;
oe_reg <= oe_next;
tri_reg <= tri_next;
ready_reg <= ready_next;
end
end

// FSMD next-state logic
always @*
begin
state_next = state;
address_next = address_reg;
data_to_sram_next = data_to_sram;
data_from_sram_next = data_from_sram;
we_next = we_next;
oe_reg = oe_next;
tri_next = tri_reg;
ready_next = ready_reg;

case(state)
idle:
begin
if(mem)
begin
if(rw)    // Read
begin
state_next = read0;
address_next = addr_in;
tri_next = 1'b1;
ready_next = 1'b0;
end
else // Write
begin
state_next = write0;
address_next = addr_in;
data_to_sram_next = data_in;
tri_next = 1'b1;
ready_next = 1'b0;
// More here possibly

end
end
else
begin
ready_next = 1'b1;
oe_next = 1'b1;
we_next = 1'b1;
end
end
read0: // Assert OE
begin
state_next = read1;
oe_next = 1'b0;
end
read1: // Wait 20ns
state_next = read2;
read2:  // Wait 20ns
state_next = read3;
read3: // Read the data
begin
state_next = idle;
data_from_sram = dio_sram;
end
write0: // Assert WE and the tri-state buffer
begin
state_next = write1;
tri_next = 1'b0;
we_next = 1'b0;
end
write1: // Wait 20ns
state_next = write2;
write2: // Wait 20ns
state_next = write3;
write3: // Wait 20ns
state_next = write4;
write4: // Unassert WE and wait 20ns
begin
state_next = write5;
we_next = 1'b1;
end
write5: // Take the data off(unassert the tri-sate buffer)
begin
state_next = idle;
tri_next = 1'b1;
end
default: state_next = idle;
endcase

end



assign AVD_sram = 1'b0;
assign LB_sram = 1'b0;
assign UB_sram = 1'b0;
assign CE_sram = 1'b0;
assign data_out = dio_sram;
assign data_out_reg = data_from_sram;
assign dio_sram = (~tri_reg) ? data_to_sram : 16'bz;

endmodule

I hope that wasn't too irritating to read.  I'm still not very sure how to write readable Verilog(if that exists). 

Thanks!

[SebG]

Hi
The errors sound to me as you are driving a node or signal from two different sources.  This can't be done since, for example, if one driver outputs a 1 on to SIGNAL and another driver outputs a 0 what is the value of SIGNAL. 
In the first code block:  data_next = data_reg;  is assigned unconditionally and then assigned again in the case statement.  If you want that to be assigned as a default value put it in an else case statement.  That goes for the other parameters such as mem_next, state_next, and data_out_next. 

Code: [Select]

// FSMD
always @*
begin
data_next = data_reg; // Unconditionally assigned ***********
state_next = state;
data_out_next = data_out_reg;
mem_next = 1'b0;

case(state)
idle:
begin
if (db_btn[0])    // Load to data reg
begin
data_next = sw; // Conditionally Assigned *************
end
else if (db_btn[1]) // Write
begin
state_next = write;
end
else // Read
begin
state_next = read;
end
end
write:
begin
mem_next = 1'b1;
rw = 1'b0;
if (ready)
state_next = idle;
end
read:
begin
mem_next = 1'b1;
rw = 1'b1;
if (ready)
begin
state_next = idle;
data_out_next = sram_output;
end
end
endcase
end

When you are assigning a value to a signal always specify what the default value should be using else statements rather then assigning them at the top of a block.

[marshallh]

Use nonblocking assignments instead of blocking.

Then you can abuse precedence to your heart's content.

[SebG]

Blocking or non-blocking assignments are not meant to be used to tell the compiler the default values of signals.  It is a good practice to use non-blocking "<=" within always blocks and blocking assignments "=" outside of them.

[pigtwo]

@ SebG
I tried making all the signals explicitly defined like you said but the problem persisted.  But luckily while going through and changing it I noticed a couple of stupid mistakes that seemed to fix the problem. 

Although I'm still a little confused.  You said not to put the default cases at the beginning and instead put them in the cases using else statements(IE be explicit with the signals).  Is the way I was doing it just bad coding practice or will actually cause errors?  I ask because for some reason my book does it the way I was doing it and it seems to work for me(but I wouldn't be surprised if I was just lucky).

Also, you said to use non-blocking inside always blocks but from what I heard you could use blocking as long as the block was all combinational.   Does it matter to combinational circuits whether you use blocking or non-blocking assignment?

@ marshallh
I tried that also but as I said the problem was elsewhere.  Strangely enough it seems to work with blocking assignments.  I'm guessing that nonblocking works all the time while blocking works most of the time (or something like that) and that would be why it is working for me.

Thanks for the help guys!

[romovs]

Take a look at http://www.sunburst-design.com/papers/CummingsSNUG2000SJ_NBA_rev1_2.pdf
It should explain the blocking/non-blocking stuff.

[poorchava]

This is a common problem when you are assigning the same signal on 2 different edges. Doing something like:

Code: [Select]

always @(posedge clk)
foo<=value

always @(posedge other_signal)
foo<=other_value
will quite likely result in such an error, because foo is a single flip-flop and it can be clocked from one source. You could do some additional logic that will take clk and other_signal and generate single write strobe out of those two, but then you will quite possibly run into warnings concerning usage of general routing resources as clock signals. If you add something like this:

Code: [Select]

always @ (a, b, c)
if(a)
foo=b;
else
foo=c;(this should synthesize to a mux)

then you will most definitely get that error, since mux will result in non-registered value, and the clocked part will produce a registered value.

It obviously depends on type of synthesis engine. With Lattice diamond and Xilinx ISE I was having this all the time.