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

[ECO] prim_count.sv CDC #24119

Closed
moidx opened this issue Jul 24, 2024 · 3 comments
Closed

[ECO] prim_count.sv CDC #24119

moidx opened this issue Jul 24, 2024 · 3 comments
Labels
ECO_C A potential candidate for an ECO

Comments

@moidx
Copy link
Contributor

moidx commented Jul 24, 2024

Description

Reported by: @meisnere

sw_alert_handler_ping_ok test fails post synthesis.

The root cause seems to point to glitches on err_o. This is a CDC error as it takes time for the sum to converge here:

// The sum of both counters must always equal the counter maximum.
logic [Width:0] sum;
assign sum = (cnt_q[0] + cnt_q[1]);
assign err_o = (sum != {1'b0, {Width{1'b1}}});

CC @andreaskurth @vogelpi

@moidx moidx added the ECO_C A potential candidate for an ECO label Jul 24, 2024
@moidx moidx added this to the Earlgrey-PROD.M6 milestone Jul 24, 2024
vogelpi added a commit to vogelpi/opentitan that referenced this issue Jul 25, 2024
The escalation output request signal the prim_esc_receiver is a
combinatorial signal. Before feeding this into a synchronizer primitive,
it must be flopped once more in the source clock domain. Otherwise, this
can lead to CDC issues.

This resolves lowRISC#24119.

Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
@vogelpi
Copy link
Contributor

vogelpi commented Jul 25, 2024

Thanks for creating the issue for this here @moidx . I've just created a PR to fix this #24121 and we're discussing with the PD team to assess if this can still be absorbed as ECO.

vogelpi added a commit to vogelpi/opentitan that referenced this issue Jul 25, 2024
This is related to lowRISC#24119.

Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
vogelpi added a commit to vogelpi/opentitan that referenced this issue Jul 25, 2024
This is related to lowRISC#24119.

Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
vogelpi added a commit to vogelpi/opentitan that referenced this issue Jul 25, 2024
This is related to lowRISC#24119.

Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
@vogelpi
Copy link
Contributor

vogelpi commented Jul 25, 2024

Update: it has been decided to take a more holistic approach and fix all prim_count instances + the prim_esc_receiver instances:

vogelpi added a commit that referenced this issue Jul 25, 2024
This is related to #24119.

Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
vogelpi added a commit that referenced this issue Jul 25, 2024
This is related to #24119.

Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
@vogelpi
Copy link
Contributor

vogelpi commented Jul 25, 2024

I am again closing this issue as the corresponding PRs #24125 and #24127 got both merged.

@vogelpi vogelpi closed this as completed Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ECO_C A potential candidate for an ECO
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants