You should never drive a signal from multiple always blocks. This will create multiple drivers (as the warning says). You’re basically connecting two outputs together. What happens when one says it should be 0 and the other says it should be 1?
If you want an asynchronous reset, you need to add the reset signal to the sensitivity list and use it in the same always block. You generally shouldn’t use asynchronous resets in an FPGA though due to timing.
It’s unclear to me what is supposed to be DFFs in your code and what isn’t. Verilog’s proclivity to allow this kind of mistake was the original motivation for creating Lucid.
The way this module is written I don’t think it can ever be reliable since not everything is clocked together. Even if it builds and even simulates fine, timing will be violated in come cases causing issues in hardware.
Generally, you should either have one always block with @(posedge clk)
with everything in it or you should have one @(posedge clk)
with all your DFFs explicitly assigned and another @*
block with only combinational logic.
The first style leads to easily messing up and accidentally creating latches, which often fail to work due again to timing. The second style is very verbose.
Lucid implements things in the second style but all the extra verboseness is taken care of for you.
It’s important to remember that reg
isn’t special in Verilog. It doesn’t actually make a DFF. The reason your code is having issues is because you’re assigning values to things with various triggers. Any signal should either have an explicit value at all times (so it is essentially just a wire) or have its value only change at the edge of a clock (so it’s a DFF).
Here is a working version of your Verilog. Note that your constants were actually 27 bits so I dropped the LSB (rightmost 0).
module leds (input clk, input rst, output reg [7:0] leds);
reg [2:0] pointer;
reg [25:0] delay;
reg onoff = 1'b1;
initial begin
leds <= 8'b00000000;
end
always @(posedge clk) begin
if(onoff == 1'b1) begin
leds[pointer] = 1'b1;
end
if(onoff == 1'b0) begin
leds[pointer] = 1'b0;
end
if(delay < 26'b10111110101111000010000000) begin
delay <= delay + 1'b1;
end
if(delay >= 26'b10111110101111000010000000) begin
delay <= 26'b0;
pointer <= pointer + 1'b1;
end
if(pointer == 3'b111) begin
onoff <= ~onoff;
pointer <= 3'b000;
end
if (rst) begin
leds <= 8'b00000000;
onoff <= 1'b1;
end
end
endmodule
Here is how I would’ve written it in Lucid.
module leds_lucid (
input clk, // clock
input rst, // reset
output leds[8]
) {
.clk(clk), .rst(rst) {
dff led_reg[8]
dff pointer[3]
dff delay[26]
}
always {
delay.d = delay.q + 1
if (delay.q >= 26b10111110101111000010000000) {
delay.d = 0
pointer.d = pointer.q + 1
led_reg.d[pointer.q] = ~led_reg.q[pointer.q]
}
leds = led_reg.q
}
}
And just for completeness, here’s the generated SystemVerilog.
module leds_lucid (
input wire clk,
input wire rst,
output reg [7:0] leds
);
logic [7:0] D_led_reg_d, D_led_reg_q = 0;
logic [2:0] D_pointer_d, D_pointer_q = 0;
logic [25:0] D_delay_d, D_delay_q = 0;
always @* begin
D_delay_d = D_delay_q;
D_pointer_d = D_pointer_q;
D_led_reg_d = D_led_reg_q;
D_delay_d = D_delay_q + 1'h1;
if (D_delay_q >= 26'h2faf080) begin
D_delay_d = 1'h0;
D_pointer_d = D_pointer_q + 1'h1;
D_led_reg_d[D_pointer_q] = ~D_led_reg_q[D_pointer_q];
end
leds = D_led_reg_q;
end
always @(posedge (clk)) begin
if ((rst) == 1'b1) begin
D_led_reg_q <= 0;
D_pointer_q <= 0;
D_delay_q <= 0;
end else begin
D_led_reg_q <= D_led_reg_d;
D_pointer_q <= D_pointer_d;
D_delay_q <= D_delay_d;
end
end
endmodule