lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.59k stars 782 forks source link

[CDC|AST] Stopping propagation of AST_CLK_EXT #16897

Closed jeoongp closed 1 year ago

jeoongp commented 1 year ago

MAIN_CLK, IO_CLK, AON_CLK, USB_CLK are propagated clocks from AST_CLK_EXT in AST. The issue is that AST_CLK_EXT is found on paths of these clocks even though AST_CLK_EXT is different from them. So, the tool is complaining about them relative to ast_ext_clk. This is so annoying and causing many unnecessary errors/warnings.. We need to stop propagation of AST_CLK_EXT.

As we discussed with RI folks, two options can be considered.

  1. It really sounds like a modeling / setup issue. Maybe the IP vendor (Nuvoton) can change the liberty, add description of synchronizers for example.
  2. If option 1 is not available, we need to check with Nuvoton if they are OK with the following suggestion to filter out AST_EXT_CLK: set_clock_sense -stop_propagation -clocks { AST_EXT_CLK } { u_ast.clk_ast_ext_i }

@arnonsha can you review?

jeoongp commented 1 year ago

As I run with option 2, it seems to block propagation of ast_ext_clk. However, we will need to see what might break because of this approach.

arnonsha commented 1 year ago

Glad to hear the stop_propagation has removed the excessive noise. However you still need to check both int clocks and ext clocks. Since these are 2 independent scenarios, 2 runs with different set_case analysis will give us full coverage.

jeoongp commented 1 year ago

@arnonsha can you provide detailed guideline or commands for different set_case for two independent scenarios? Currently, we run one general command that includes all clocks.

tjaychen commented 1 year ago

is there any reason we need to check for external clock on the open source side though? We don't have the the real AST for open side, and as far as we are concerned the clock tree built from the output of ast to the open source side is the same no?

tjaychen commented 1 year ago

since the stop clock propagation is done in the cdc side constraint, it no longer impacts synthesis, i think we can just close this now.

jeoongp commented 1 year ago

Yeah, I agree. Let me close this ticket. @arnonsha if you have any comment or reply to my question, feel free to add it later.