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.

Sign up for GitHub

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

[prim,clkmgr,rtl] Simplify prim_clock_meas by removing RefCnt #26029

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

[prim,clkmgr,rtl] Simplify prim_clock_meas by removing RefCnt #26029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
52 changes: 11 additions & 41 deletions hw/ip/prim/rtl/prim_clock_meas.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,12 @@
`include "prim_assert.sv"

module prim_clock_meas #(
// Maximum value of input clock counts over measurement period
// Maximum number of input clock counts over measurement period. Note that this is 1-based
// indexing (so a hardware counter would only count up to Cnt-1)
parameter int Cnt = 16,
// Maximum value of reference clock counts over measurement period
parameter int RefCnt = 1,
parameter bit ClkTimeOutChkEn = 1,
parameter bit RefTimeOutChkEn = 1,
localparam int CntWidth = prim_util_pkg::vbits(Cnt),
localparam int RefCntWidth = prim_util_pkg::vbits(RefCnt)
localparam int CntWidth = prim_util_pkg::vbits(Cnt)
) (
input clk_i,
input rst_ni,
Expand Down Expand Up @@ -155,27 +153,10 @@ module prim_clock_meas #(
.dst_pulse_o(valid_ref)
);


if (RefCnt == 1) begin : gen_degenerate_case
// if reference count is one, cnt_ref is always 0.
// So there is no need to maintain a counter, and
// valid just becomes valid_ref
assign valid = valid_ref;
end else begin : gen_normal_case
logic [RefCntWidth-1:0] cnt_ref;
assign valid = valid_ref & (int'(cnt_ref) == RefCnt - 1);
always_ff @(posedge clk_i or negedge rst_ni) begin
if (!rst_ni) begin
cnt_ref <= '0;
end else if (!cnt_en && |cnt_ref) begin
cnt_ref <= '0;
end else if (cnt_en && valid) begin
cnt_ref <= '0;
end else if (cnt_en && valid_ref) begin
cnt_ref <= cnt_ref + 1'b1;
end
end
end
Comment on lines -164 to -178
Copy link
Contributor

Choose a reason for hiding this comment

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

The following points make me think that it is fine to remove this option but I would like to get your input on this as well Rupert:

  • There is no single instance in any design with RefCnt != 0. So this feature is not used anywhere.
  • We don't have any DV for this feature (from the Git history, I can see that the RefCnt == 1 special case was specifically added to ease coverage closure). So even if we would start using the feature, there would be additional DV work.
  • With a RefCnt value >= 1, the module can count clock cycles across multiple reference clock cycles thereby allowing to average count values across multiple reference clocks. With RefCnt == 1, every reference clock edge clears the counting and triggers a comparison with the threshold registers. I would argue that since we have configurable low and high thresholds in clkmgr, we can just tune these to increase tolerance if needed. But I don't see a strong argument for averaging across multiple reference clocks. Or do I miss something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I completely agree. Annoyingly, my "git grep" promptly found some stray uses of the parameter name! So I'll force-push in a sec to expunge them as well.

And I'll rebase and wait for CI.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks Rupert!

// There isn't really a reference counter (we just wait for the first edge on that clock). The
// valid signal is supposed to be saying that we have seen the synchronised version of the last
// edge we are waiting for, so it is always equal to valid_ref.
assign valid = valid_ref;

logic cnt_ovfl;
logic [CntWidth-1:0] cnt;
Expand Down Expand Up @@ -206,16 +187,13 @@ module prim_clock_meas #(

localparam bit TimeOutChkEn = ClkTimeOutChkEn | RefTimeOutChkEn;

// determine ratio between
localparam int ClkRatio = Cnt / RefCnt;

// maximum cdc latency from the perspective of the measured clock
// 1 cycle to output request
// 2 ref cycles for synchronization
// 1 ref cycle to send ack
// 2 cycles to sync ack
// Double for margin
localparam int MaxClkCdcLatency = (1 + 2*ClkRatio + 1*ClkRatio + 2)*2;
localparam int MaxClkCdcLatency = (1 + 2 + 1 + 2)*2;

// maximum cdc latency from the perspective of the reference clock
// 1 ref cycle to output request
Expand Down Expand Up @@ -261,19 +239,11 @@ module prim_clock_meas #(
// Assertions
//////////////////////////

if (TimeOutChkEn) begin : gen_timeout_assert
// the measured clock must be faster than the reference clock
`ASSERT_INIT(ClkRatios_A, ClkRatio > 2)
end

// reference count has to be at least 1
`ASSERT_INIT(RefCntVal_A, RefCnt >= 1)
// We must be measuring at least two cycles on the input clock.
`ASSERT_INIT(CntMin_A, Cnt > 1)

// if we've reached the max count, enable must be 0 next.
// Otherwise the width of the counter is too small to accommodate the usecase
`ASSERT(MaxWidth_A, (cnt == Cnt-1) |=> !cnt_en )




endmodule // prim_clk_meas
endmodule
3 changes: 1 addition & 2 deletions hw/ip_templates/clkmgr/rtl/clkmgr.sv.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,8 +398,7 @@ rg_srcs = list(sorted({sig['src_name'] for sig
sel_idx = f"Clk{Name.from_snake_case(src).as_camel_case()}Idx"
%>
clkmgr_meas_chk #(
.Cnt(${cnt}),
.RefCnt(1)
.Cnt(${cnt})
) u_${src}_meas (
.clk_i,
.rst_ni,
Expand Down
3 changes: 0 additions & 3 deletions hw/ip_templates/clkmgr/rtl/clkmgr_meas_chk.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ module clkmgr_meas_chk
#(
// Maximum value of input clock counts over measurement period
parameter int Cnt = 16,
// Maximum value of reference clock counts over measurement period
parameter int RefCnt = 1,
localparam int CntWidth = prim_util_pkg::vbits(Cnt)
) (
// the local operating clock
Expand Down Expand Up @@ -42,7 +40,6 @@ module clkmgr_meas_chk

prim_clock_meas #(
.Cnt(Cnt),
.RefCnt(RefCnt),
.ClkTimeOutChkEn(1'b1),
.RefTimeOutChkEn(1'b0)
) u_meas (
Expand Down
9 changes: 3 additions & 6 deletions hw/top_darjeeling/ip_autogen/clkmgr/rtl/clkmgr.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,8 +549,7 @@
end

clkmgr_meas_chk #(
.Cnt(16),
.RefCnt(1)
.Cnt(16)
) u_io_div4_meas (
.clk_i,
.rst_ni,
Expand All @@ -576,8 +575,7 @@


clkmgr_meas_chk #(
.Cnt(64),
.RefCnt(1)
.Cnt(64)
) u_main_meas (
.clk_i,
.rst_ni,
Expand All @@ -603,8 +601,7 @@


clkmgr_meas_chk #(
.Cnt(64),
.RefCnt(1)
.Cnt(64)
) u_usb_meas (
.clk_i,
.rst_ni,
Expand Down
3 changes: 0 additions & 3 deletions hw/top_darjeeling/ip_autogen/clkmgr/rtl/clkmgr_meas_chk.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ module clkmgr_meas_chk
#(
// Maximum value of input clock counts over measurement period
parameter int Cnt = 16,
// Maximum value of reference clock counts over measurement period
parameter int RefCnt = 1,
localparam int CntWidth = prim_util_pkg::vbits(Cnt)
) (
// the local operating clock
Expand Down Expand Up @@ -42,7 +40,6 @@ module clkmgr_meas_chk

prim_clock_meas #(
.Cnt(Cnt),
.RefCnt(RefCnt),
.ClkTimeOutChkEn(1'b1),
.RefTimeOutChkEn(1'b0)
) u_meas (
Expand Down
15 changes: 5 additions & 10 deletions hw/top_earlgrey/ip_autogen/clkmgr/rtl/clkmgr.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,8 +559,7 @@
end

clkmgr_meas_chk #(
.Cnt(1024),
.RefCnt(1)
.Cnt(1024)
) u_io_meas (
.clk_i,
.rst_ni,
Expand All @@ -586,8 +585,7 @@


clkmgr_meas_chk #(
.Cnt(512),
.RefCnt(1)
.Cnt(512)
) u_io_div2_meas (
.clk_i,
.rst_ni,
Expand All @@ -613,8 +611,7 @@


clkmgr_meas_chk #(
.Cnt(256),
.RefCnt(1)
.Cnt(256)
) u_io_div4_meas (
.clk_i,
.rst_ni,
Expand All @@ -640,8 +637,7 @@


clkmgr_meas_chk #(
.Cnt(1024),
.RefCnt(1)
.Cnt(1024)
) u_main_meas (
.clk_i,
.rst_ni,
Expand All @@ -667,8 +663,7 @@


clkmgr_meas_chk #(
.Cnt(512),
.RefCnt(1)
.Cnt(512)
) u_usb_meas (
.clk_i,
.rst_ni,
Expand Down
3 changes: 0 additions & 3 deletions hw/top_earlgrey/ip_autogen/clkmgr/rtl/clkmgr_meas_chk.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ module clkmgr_meas_chk
#(
// Maximum value of input clock counts over measurement period
parameter int Cnt = 16,
// Maximum value of reference clock counts over measurement period
parameter int RefCnt = 1,
localparam int CntWidth = prim_util_pkg::vbits(Cnt)
) (
// the local operating clock
Expand Down Expand Up @@ -42,7 +40,6 @@ module clkmgr_meas_chk

prim_clock_meas #(
.Cnt(Cnt),
.RefCnt(RefCnt),
.ClkTimeOutChkEn(1'b1),
.RefTimeOutChkEn(1'b0)
) u_meas (
Expand Down
12 changes: 4 additions & 8 deletions hw/top_englishbreakfast/ip_autogen/clkmgr/rtl/clkmgr.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,8 +554,7 @@
end

clkmgr_meas_chk #(
.Cnt(1024),
.RefCnt(1)
.Cnt(1024)
) u_io_meas (
.clk_i,
.rst_ni,
Expand All @@ -581,8 +580,7 @@


clkmgr_meas_chk #(
.Cnt(256),
.RefCnt(1)
.Cnt(256)
) u_io_div4_meas (
.clk_i,
.rst_ni,
Expand All @@ -608,8 +606,7 @@


clkmgr_meas_chk #(
.Cnt(1024),
.RefCnt(1)
.Cnt(1024)
) u_main_meas (
.clk_i,
.rst_ni,
Expand All @@ -635,8 +632,7 @@


clkmgr_meas_chk #(
.Cnt(512),
.RefCnt(1)
.Cnt(512)
) u_usb_meas (
.clk_i,
.rst_ni,
Expand Down
3 changes: 0 additions & 3 deletions hw/top_englishbreakfast/ip_autogen/clkmgr/rtl/clkmgr_meas_chk.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ module clkmgr_meas_chk
#(
// Maximum value of input clock counts over measurement period
parameter int Cnt = 16,
// Maximum value of reference clock counts over measurement period
parameter int RefCnt = 1,
localparam int CntWidth = prim_util_pkg::vbits(Cnt)
) (
// the local operating clock
Expand Down Expand Up @@ -42,7 +40,6 @@ module clkmgr_meas_chk

prim_clock_meas #(
.Cnt(Cnt),
.RefCnt(RefCnt),
.ClkTimeOutChkEn(1'b1),
.RefTimeOutChkEn(1'b0)
) u_meas (
Expand Down
Loading