Skip to content

blocking assignment must be used for a clock divider #44

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
27 changes: 27 additions & 0 deletions VerilogCodingStyle.md
Original file line number Diff line number Diff line change
Expand Up @@ -1900,6 +1900,33 @@ separate combinational (`always_comb`) block. Ideally, sequential blocks should
contain only a register instantiation, with perhaps a load enable or an
increment.

Exception: Even in a sequential always block, use blocking assignments (`=`) for
clock dividers. See [Clock Generation](#clock-generation).

### Clock Generation

***All clock signals should be generated using blocking assignment even
Copy link

Choose a reason for hiding this comment

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

This should probably be a new section. It's currently written under Sequential Logic (Registers), but it is not about registers. Perhaps Sequential Logic (Clock Dividers) or some such?

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 agree with you.

On my first PR it was only for clock dividers. During the discussion on this PR @rich-ho suggested me applying this rule to clock generation in general.

... to add a style guide statement that all clocks should be generated using blocking assigns. I think that perhaps the wording needs to be clearer and the reason made explicit.

I agreed with him, too. But if we do that, we also need an example for clock generation other than clock divider, so I added it.

As you wrote this rule is not for the section "Sequential Logic (Registers)" now.

I have separated it into a section called “Generating Clock”.
This time I did not squash the commits so that the changes are visible.

How about this?

for clock dividers.***

See #44 for more details.

👍
```systemverilog {.good}
// only for test bench code
logic clk;
initial begin
clk <= 1'b0;
forever #5 clk = ~clk; // blocking assignment
end
Comment on lines +1915 to +1920
Copy link

@a-will a-will Sep 15, 2024

Choose a reason for hiding this comment

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

This portion of the code isn't really relevant for this section. Since this guidance is about how to construct the divider, we don't need to see how clk is generated. I'd recommend removing it.

Edit: Or am I misunderstanding and you also want to show how to generate clock source stimulus in a testbench?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Edit: Or am I misunderstanding and you also want to show how to generate clock source stimulus in a testbench?

No, I don't.


// for both synthesizable and test bench code
logic clk_div2;
always_ff @(posedge clk or negedge rst_ni) begin
if (!rst_ni) clk_div2 = 1'b0;
else clk_div2 = ~clk_div2; // blocking assignment
end
```

### Don't Cares (`X`'s)

***The use of `X` literals in RTL code is strongly discouraged. RTL must not
Expand Down