Skip to content

Commit

Permalink
[rtl] Fix counter reset value on FPGA
Browse files Browse the repository at this point in the history
If the counter width is >= 49, we do not use a DSP on the FPGA.
Then, we should use an asynchronous reset to initialize the counter.

This bug was detected when enabling the lockstep for the CW340. A
lockstep mismatch happend as the mcycle counters of the main and
shadow core did not match due to this bug.

Signed-off-by: Pascal Nasahl <nasahlpa@lowrisc.org>
  • Loading branch information
nasahlpa authored and vogelpi committed Nov 29, 2024
1 parent d2d55ed commit 54985d2
Showing 1 changed file with 12 additions and 9 deletions.
21 changes: 12 additions & 9 deletions rtl/ibex_counter.sv
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,17 @@ module ibex_counter #(
end

`ifdef FPGA_XILINX
// Set DSP pragma for supported xilinx FPGAs
localparam int DspPragma = CounterWidth < 49 ? "yes" : "no";
(* use_dsp = DspPragma *) logic [CounterWidth-1:0] counter_q;

// DSP output register requires synchronous reset.
`define COUNTER_FLOP_RST posedge clk_i
// On Xilinx FPGAs, 48-bit DSPs are available that can be used for the
// counter.
if (CounterWidth < 49) begin : g_dsp_counter
// Set DSP pragma for supported xilinx FPGAs
(* use_dsp = "yes" *) logic [CounterWidth-1:0] counter_q;
// DSP output register requires synchronous reset.
`define COUNTER_FLOP_RST posedge clk_i
end else begin : g_no_dsp_counter
(* use_dsp = "no" *) logic [CounterWidth-1:0] counter_q;
`define COUNTER_FLOP_RST posedge clk_i or negedge rst_ni
end
`else
logic [CounterWidth-1:0] counter_q;

Expand All @@ -65,6 +70,7 @@ module ibex_counter #(

// Counter flop
always_ff @(`COUNTER_FLOP_RST) begin
`undef COUNTER_FLOP_RST
if (!rst_ni) begin
counter_q <= '0;
end else begin
Expand Down Expand Up @@ -98,6 +104,3 @@ module ibex_counter #(
assign counter_val_o = counter;

endmodule

// Keep helper defines file-local.
`undef COUNTER_FLOP_RST

2 comments on commit 54985d2

@Adam11072000
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im having a compilation issue after this change.
i have a verilog define of XILINX_FPGA set to 1 in my xdc file:
set_property verilog_define {FPGA_XILINX=1 } [get_filesets sources_1]

Image

@vogelpi
Copy link
Contributor

@vogelpi vogelpi commented on 54985d2 Dec 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Adam11072000 ,
it's unfortunate but there went something wrong, @nasahlpa is currently working on fixing that.

Please sign in to comment.