Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix non-synchronized path in cdc_2phase_clearable #243

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Bender.yml
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ sources:
- test/addr_decode_tb.sv
- test/cb_filter_tb.sv
- test/cdc_2phase_tb.sv
- test/cdc_4phase_tb.sv
- test/cdc_2phase_clearable_tb.sv
- test/cdc_fifo_tb.sv
- test/cdc_fifo_clearable_tb.sv
Expand Down
34 changes: 13 additions & 21 deletions src/cdc_4phase.sv
Original file line number Diff line number Diff line change
Expand Up @@ -298,27 +298,19 @@ module cdc_4phase_dst #(
end
end

if (DECOUPLED) begin : gen_decoupled
// Decouple the output from the asynchronous data bus without introducing
// additional latency by inserting a spill register
spill_register #(
.T(T),
.Bypass(1'b0)
) i_spill_register (
.clk_i,
.rst_ni,
.valid_i(data_valid),
.ready_o(output_ready),
.data_i(async_data_i),
.valid_o,
.ready_i,
.data_o
);
end else begin : gen_not_decoupled
assign valid_o = data_valid;
assign output_ready = ready_i;
assign data_o = async_data_i;
end
spill_register #(
.T(T),
.Bypass(1'b0)
) i_spill_register (
.clk_i,
.rst_ni,
.valid_i(data_valid),
.ready_o(output_ready),
.data_i(async_data_i),
.valid_o,
.ready_i,
.data_o
);

// Output assignments.
assign async_ack_o = ack_dst_q;
Expand Down
104 changes: 70 additions & 34 deletions test/cdc_2phase_clearable_tb.sv
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

module cdc_2phase_clearable_tb;

parameter int UNTIL = 100000;
parameter int UNTIL = 10_000_000;
parameter bit INJECT_DELAYS = 1;
parameter int CLEAR_PPM = 2000;
parameter int SYNC_STAGES = 3;
Expand Down Expand Up @@ -308,11 +308,16 @@ module cdc_2phase_clearable_tb_delay_injector #(
logic async_ack_o, async_ack_i;
logic [31:0] async_data_o, async_data_i;


logic s_src_clear;
logic s_src_clear_req;
logic s_src_clear_ack_q;
logic s_src_ready;
logic s_dst_clear;
logic s_src_isolate_req;
logic s_src_isolate_ack_q;
logic s_dst_clear_req;
logic s_dst_clear_ack_q;
logic s_dst_valid;
logic s_dst_isolate_req;
logic s_dst_isolate_ack_q;

always @(async_req_o) begin
automatic time d = $urandom_range(1ps, MAX_DELAY);
Expand All @@ -332,49 +337,80 @@ module cdc_2phase_clearable_tb_delay_injector #(
end

cdc_2phase_src_clearable #(logic [31:0], SYNC_STAGES) i_src (
.rst_ni ( src_rst_ni ),
.clk_i ( src_clk_i ),
.clear_i ( s_src_clear ),
.data_i ( src_data_i ),
.valid_i ( src_valid_i & !s_src_clear ),
.ready_o ( s_src_ready ),
.async_req_o ( async_req_o ),
.async_ack_i ( async_ack_i ),
.async_data_o ( async_data_o )
.rst_ni ( src_rst_ni ),
.clk_i ( src_clk_i ),
.clear_i ( s_src_clear_req ),
.data_i ( src_data_i ),
.valid_i ( src_valid_i & !s_src_isolate_req ),
.ready_o ( s_src_ready ),
.async_req_o ( async_req_o ),
.async_ack_i ( async_ack_i ),
.async_data_o ( async_data_o )
);

assign src_ready_o = s_src_ready & !s_src_clear;
assign src_ready_o = s_src_ready & !s_src_isolate_req;

cdc_2phase_dst_clearable #(logic [31:0], SYNC_STAGES) i_dst (
.rst_ni ( dst_rst_ni ),
.clk_i ( dst_clk_i ),
.clear_i ( s_dst_clear ),
.data_o ( dst_data_o ),
.valid_o ( s_dst_valid ),
.ready_i ( dst_ready_i & !s_dst_clear ),
.async_req_i ( async_req_i ),
.async_ack_o ( async_ack_o ),
.async_data_i ( async_data_i )
.rst_ni ( dst_rst_ni ),
.clk_i ( dst_clk_i ),
.clear_i ( s_dst_clear_req ),
.data_o ( dst_data_o ),
.valid_o ( s_dst_valid ),
.ready_i ( dst_ready_i & !s_dst_isolate_req ),
.async_req_i ( async_req_i ),
.async_ack_o ( async_ack_o ),
.async_data_i ( async_data_i )
);

assign dst_valid_o = s_dst_valid & !s_dst_clear;
assign dst_valid_o = s_dst_valid & !s_dst_isolate_req;

// Synchronize the clear and reset signaling in both directions (see header of
// the cdc_reset_ctrlr module for more details.)
cdc_reset_ctrlr #(
.SYNC_STAGES(SYNC_STAGES-1)
) i_cdc_reset_ctrlr (
.a_clk_i ( src_clk_i ),
.a_rst_ni ( src_rst_ni ),
.a_clear_i ( src_clear_i ),
.a_clear_o ( s_src_clear ),
.b_clk_i ( dst_clk_i ),
.b_rst_ni ( dst_rst_ni ),
.b_clear_i ( dst_clear_i ),
.b_clear_o ( s_dst_clear )
.a_clk_i ( src_clk_i ),
.a_rst_ni ( src_rst_ni ),
.a_clear_i ( src_clear_i ),
.a_clear_o ( s_src_clear_req ),
.a_clear_ack_i ( s_src_clear_ack_q ),
.a_isolate_o ( s_src_isolate_req ),
.a_isolate_ack_i ( s_src_isolate_ack_q ),
.b_clk_i ( dst_clk_i ),
.b_rst_ni ( dst_rst_ni ),
.b_clear_i ( dst_clear_i ),
.b_clear_o ( s_dst_clear_req ),
.b_clear_ack_i ( s_dst_clear_ack_q ),
.b_isolate_o ( s_dst_isolate_req ),
.b_isolate_ack_i ( s_dst_isolate_ack_q )
);

assign src_clear_pending_o = s_src_clear;
assign dst_clear_pending_o = s_dst_clear;
// Just delay the isolate request by one cycle. We can ensure isolation within
// one cycle by just deasserting valid and ready signals on both sides of the CDC.
always_ff @(posedge src_clk_i, negedge src_rst_ni) begin
if (!src_rst_ni) begin
s_src_isolate_ack_q <= 1'b0;
s_src_clear_ack_q <= 1'b0;
end else begin
s_src_isolate_ack_q <= s_src_isolate_req;
s_src_clear_ack_q <= s_src_clear_req;
end
end

always_ff @(posedge dst_clk_i, negedge dst_rst_ni) begin
if (!dst_rst_ni) begin
s_dst_isolate_ack_q <= 1'b0;
s_dst_clear_ack_q <= 1'b0;
end else begin
s_dst_isolate_ack_q <= s_dst_isolate_req;
s_dst_clear_ack_q <= s_dst_clear_req;
end
end


assign src_clear_pending_o = s_src_isolate_req; // The isolate signal stays
// asserted during the whole
// clear sequence.
assign dst_clear_pending_o = s_dst_isolate_req;

endmodule
Loading
Loading