Conversation
Add an active-high synchronous clear `clr_i` to the building-block modules, clearing all flip-flops to their reset values (a synchronous counterpart to the asynchronous `rst_ni`). Hand-written reset blocks are converted to the `FFARNC`/`FFLARNC` clearable register macros. Re-application of the original treewide change onto the v2-dev naming (cc_ prefix, renamed parameters/types). Modules that already had a functional clear which clears all FFs are renamed to `clr_i`; where a functional clear covers only part of the state (cc_max_counter, cc_exp_backoff, cc_serial_deglitch) it is kept alongside a separate general `clr_i`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
to differentiate from `clr_i`.
phsauter
left a comment
phsauter
left a comment
There was a problem hiding this comment.
Some cleanup to explain the difference between clr and other signals and to unify the comments a bit more. Should be easily doable by LLM.
Additionally the Readme needs updating:
- cc_stream_arbiter_flushable is removed
- rst_ni and clr_i need to be mentioned and their purpose explained
- The use of the FF macros should be mentioned in the Readme.
| input logic clk_i, | ||
| input logic rst_ni, | ||
| input logic clear_i, // synchronous clear | ||
| input logic clr_i, // synchronous clear |
There was a problem hiding this comment.
While we are at it, could you make sure the comments here are consistent? Here it is lower case, in cb_filter it is uppercase. I think all modules should have a comment for each clock, the reset and the synchronous clear.
Active high is assumed but not stated, the resets active low is stated, where applicable (eg credit_counter) an additional bit of comment after a semicolon is useful.
| )( | ||
| input logic clk_i, // Clock | ||
| input logic rst_ni, // Asynchronous reset active low | ||
| input logic clr_i, // synchronous clear |
There was a problem hiding this comment.
The difference between clr_i and flush_i (the former clears the elements in the fifo to zero as well as clearing the pointers, which might have a slight energy penalty) should be stated since it is hard to understand without looking at the code.
| ) ( | ||
| input logic clk_i, // Clock | ||
| input logic rst_ni, // Asynchronous reset active low | ||
| input logic clr_i, // Synchronous clear |
There was a problem hiding this comment.
Again state difference between clr and flush in comment.
| ) ( | ||
| input logic clk_i, // Clock | ||
| input logic rst_ni, // Asynchronous reset active low | ||
| input logic clr_i, // Synchronous clear |
There was a problem hiding this comment.
Again state difference between clr and flush in comment.
| ) ( | ||
| input logic clk_i , | ||
| input logic rst_ni , | ||
| input logic clr_i , |
There was a problem hiding this comment.
Again state difference between clr and flush in comment.
2 participants
Add an active-high synchronous clear
clr_ito all modules, clearing all FFS to their reset value (analogous torst_ni). Streamline the codebase by consistently using register macros for sequential logic.Design principles:
clr_ishall clear all FFs, similar torst_ni.clr_i.clr_i. If a an existing functional clear only clears a subset of FFs, rename that port to something distinctively else and addclr_i.The previous
stream_arbiterwrapper is now redundant and replaced by the previousstream_arbiter_flushable.Sequential modules where no synchronous clear was added:
cdc_*clk_*edge_propagator*isochronous_*rstgen*