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

nonce_offset_i inferred in async reset always block, but is not included in reset #339

Closed
nstewart-amd opened this issue Dec 8, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@nstewart-amd
Copy link
Contributor

Lint violations results from the following:

"./pcrvault/rtl/pv_gen_hash.sv"
198 always_ff @(posedge clk or negedge rst_b) begin : api_regs
199 if (~rst_b) begin
200 gen_hash_fsm_ps <= GEN_HASH_IDLE;
201 block_offset_i <= '0;
202 read_entry <= '0;
203 read_offset <= '0;
204 end
205 else begin
206 gen_hash_fsm_ps <= gen_hash_fsm_ns;
207 block_offset_i <= block_offset_nxt;
208 nonce_offset_i <= nonce_offset_nxt;
209 read_entry <= read_entry_nxt;
210 read_offset <= read_offset_nxt;
211 end
212 end

nonce_offset_i is inferred in an async reset always block, but is not included in the reset condition.

Correction:

  1. Assuming nonce_offset_i is don't care until after it's initialized by the combination assignment of nonce_offset_nxt, nonce_offset_i should be moved to a new (simple) sequential always block with no reset inference.
    198 always_ff @(posedge clk or negedge rst_b) begin : api_regs
    199 if (~rst_b) begin
    200 gen_hash_fsm_ps <= GEN_HASH_IDLE;
    201 block_offset_i <= '0;
    202 read_entry <= '0;
    203 read_offset <= '0;
    204 end
    205 else begin
    206 gen_hash_fsm_ps <= gen_hash_fsm_ns;
    207 block_offset_i <= block_offset_nxt;
    209 read_entry <= read_entry_nxt;
    210 read_offset <= read_offset_nxt;
    211 end
    212 end

198 always_ff @(posedge clk) begin : api_regs
208 nonce_offset_i <= nonce_offset_nxt;
212 end

  1. If nonce_offset_i is NOT don't care, it should be added to the reset begin/end group.
    198 always_ff @(posedge clk or negedge rst_b) begin : api_regs
    199 if (~rst_b) begin
    200 gen_hash_fsm_ps <= GEN_HASH_IDLE;
    201 block_offset_i <= '0;
    **208 nonce_offset_i <= '0;
    202 read_entry <= '0;
    203 read_offset <= '0;
    204 end
    205 else begin
    206 gen_hash_fsm_ps <= gen_hash_fsm_ns;
    207 block_offset_i <= block_offset_nxt;
    208 nonce_offset_i <= nonce_offset_nxt;
    209 read_entry <= read_entry_nxt;
    210 read_offset <= read_offset_nxt;
    211 end
    212 end
@Nitsirks
Copy link
Contributor

Nitsirks commented Dec 8, 2023

Looking into why we aren't seeing this on our end. I thought we had this rule.
@nstewart-amd can you send me the rule that caught this?

Looks like nonce offset was added later and just missed in the reset block. I don't think it was explicitly meant to be a non-reset flop. We should add this to the reset block.

@nstewart-amd
Copy link
Contributor Author

Michael - This is caught by UnInitializedReset-ML, which was not included in 1p0RC LINT rule set recommendations.

@bharatpillilli
Copy link
Collaborator

@Nitsirks - close this?

@calebofearth
Copy link
Collaborator

This is addressed by commit 020cdc6 from #483

@calebofearth calebofearth added the bug Something isn't working label Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants