[alert,rtl] The `skew_cnt_q` signal is bigger than needed in prim_diff_decode.sv
- Dominant language
- SystemVerilog
- Stars
- 3.6k
- Forks
- 1.1k
- Avg merge
- 2d 22h
- Merged PRs (30d)
- 141
Description
### Description
The `prim_diff_decode` module has a counter called `skew_cnt_q` that is used to allow the signal to be non-differential for a short time (`SkewCycles`) before the mismatch is upgraded from a "skew" to a "sigint failure". The signal is quite new: it was only added in July 2025 (in commit cb26912606e) to support a use-case that wanted to tolerate skews of more than a single cycle.
I think the counter is bigger than it needs to be, though. It gets set to `1` (to represent the cycle where things have gone wrong) as we leave the `IsStd` FSM state and I don't think it can ever be zero in FSM state `IsSkewing`. It gets zeroed again as we enter `IsStd` or `SigInt`.
So I suggest "subtracting one" from the value in the only state we care about it:
- Don't set `skew_cnt_d` as we leave `isStd`
- Change the comparison in `IsSkewing` to something like `skew_cnt_q < SkewCycles - 1`
Doing so might let the signal get smaller. At the moment, it's declared as:
```
// Counter for skew cycles tolerated before flagging an issue
// The width needs to accommodate SkewCycles + 1 to count up to SkewCycles.
logic [prim_util_pkg::vbits(SkewCycles + 1)-1:0] skew_cnt_d, skew_cnt_q;
```
but the change would allow
```
logic [prim_util_pkg::vbits(SkewCycles)-1:0] skew_cnt_d, skew_cnt_q;
```
instead. If `SkewCycles` is a power of two, this will matter. For the parameterisation in `EarlGrey`, `SkewCycles = 1`, so the two counters will change from 2 bits to 1 bit each.
It won't make any difference to Darjeeling, where `SkewCycles = 3`.
I should note that I found this by looking at formal coverage numbers for the async alert testbench, where Jasper correctly pointed out that it could never make the comparison on `prim_diff_decode.sv:162` false. Congratulations to the tool vendors!
Contributor guide
Research direction
Start in prim_diff_decode.sv at the skew_cnt_d logic and the comparison near line 162; review the IsStd and IsSkewing transitions first. Check the async alert testbench's formal coverage, and confirm the reduced counter width behaves for EarlGrey (SkewCycles=1) and Darjeeling (SkewCycles=3).
Written by the indexing model from the issue text.
Assessment
- Domain
- embedded-iot, security
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 64/100