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

[Feature] clock/reset port connection check #708

Closed
taichi-ishitani opened this issue May 9, 2024 · 5 comments · Fixed by #891
Closed

[Feature] clock/reset port connection check #708

taichi-ishitani opened this issue May 9, 2024 · 5 comments · Fixed by #891
Labels
tools Tools feature

Comments

@taichi-ishitani
Copy link
Contributor

taichi-ishitani commented May 9, 2024

refs: #622 (comment)

To prevent clock/reset type mismatch, we need to check type mismatch on clocl/reset port connections.

module ModA (
  i_clk:   input clock_posedge,
  i_rst_n: input reset_async_low
) {}

module ModB (
  i_clk: input clock,
  i_rst: input reset
){
  inst u_moda: ModA (
    i_clk:   i_clk, // these connection include type mismatch
    i_rst_n: i_rst  // so Veryl should report an error.
  );
}
@dalance dalance added the tools Tools feature label May 10, 2024
@nblei
Copy link
Contributor

nblei commented Jun 4, 2024

I'm a bit confused.

What does clock_posedge mean? Does it mean that any flops driven by this clock must be positive edge-triggered? In which case, what is the issue with connecting i_clk : clock to i_clk : clock_posedge?

@taichi-ishitani
Copy link
Contributor Author

taichi-ishitani commented Jun 4, 2024

For clock type, its polarity will be specified by the clock_type build option. So there is a case that its polarity is negedge.
For clock_posedge type, its polarity is fixed to posedge.

@nblei
Copy link
Contributor

nblei commented Jun 4, 2024

I get that, I think I'm more questioning the notion of this being a property of the clock wire rather than a property of the registers.

@taichi-ishitani
Copy link
Contributor Author

I'd like to simplify always_ff declarations so I've moved clock/reset prorperties from always_ff block to clock/reset types.

@dalance
Copy link
Collaborator

dalance commented Jun 5, 2024

I think that using clock_posedge is more explicit than placing posedge in sensitivity list.
Espacially I like it can show that the clock/reset connection should be careful at the module port declaration which is public API.

module ModuleA (
    i_clk: clock_posedge,
    i_rst: reset_async_low,
) {
    always_ff {
        if_reset {
        } else {
        }
    }
}
module ModuleB (
    i_clk: clock,
    i_rst: reset,
) {
    let w_clk: clock_negedge = i_clk;
    let w_rst: reset_async_high = i_rst;
    always_ff (w_clk, w_rst) {
        if_reset {
        } else {
        }
    }
}
module ModuleA (
    i_clk: clock,
    i_rst: reset,
) {
    always_ff (posedge i_clk, async_low i_rst) {
        if_reset {
        } else {
        }
    }
}
module ModuleB (
    i_clk: clock,
    i_rst: reset,
) {
    always_ff (negedge i_clk, async_high, i_rst) {
        if_reset {
        } else {
        }
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Tools feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants