[Darjeeling] Lint fixes to top-level #25982
Conversation
| waive -rules RESET_USE -location {spid_dpram.sv} \ | ||
| -regexp {'(rst_sys_ni|rst_spi_ni)' is connected to 'prim_ram_1r1w_async_adv' port 'rst_a_ni', and used as an asynchronous reset or set} \ | ||
| -comment "Asynchronous reset use in 1r1w ram primitive is legal." |
There was a problem hiding this comment.
I was suspicious because we don't need to waive similar warnings for the 2p variant in Earlgrey and I think the underlying issue here is that inside prim_generic_ram_1r1w which is instantiated inside prim_ram_1r1w_async_adv, the reset inputs are tied off:
logic unused_signals;
assign unused_signals = ^{cfg_i, rst_a_ni, rst_b_ni};So I think this waiver here makes sense and is fine. I am leaving the comment for others.
We may want to make the comment more precise but I don't consider this a must.
There was a problem hiding this comment.
Resets to prim_generic_ram_1r1w where recently added. Previously (like in the 2p version), there was no dedicated reset input to those prims, but they make sense from a generalization point of view and are used downstream.
| waive -rules {RESET_USE} -location {prim_ram_1r1w_async_adv.sv} -regexp {'rst_b_ni' is connected to 'prim_ram_1r1w' port 'rst_b_ni', and used as an asynchronous reset} \ | ||
| -comment "rst_b_ni is a legal asynchronous reset." |
There was a problem hiding this comment.
A this is now the rst_b_ni port of the prim_ram_1r1w_async_adv.sv. The rst_a_ni waiver is in a different commit and I think in a different waiver file. Can you try putting them into the same file and maybe make the comment more precise (see my other review comment please)?
Razer6
force-pushed
the
darjeeling-lint
branch
3 times, most recently
from
4f075fc to
37fd142
Compare
|
@vogelpi Can you take another look? |
Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
…in RTL Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
This was previously unconnected and the default signal is not actually defined. Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
Until we have cleaned up the flow errors in chip_darjeeling_asic, this
lint config allows running lint on top_darjeeling including all the
instantiated modules via dvsim using the following command:
util/dvsim/dvsim.py \
hw/top_darjeeling/lint/top_darjeeling_lint_cfgs.hjson \
--tool ascentlint --local --purge --select-cfgs top_darjeeling
Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
vogelpi
force-pushed
the
darjeeling-lint
branch
from
37fd142 to
2e12602
Compare
There was a problem hiding this comment.
LGTM!
As discussed offline, I've connected the missing soc_dbg_ctrl input signal that prevented running lint. I've also added a lint config for top_darjeeling. You can now run it via
util/dvsim/dvsim.py hw/top_darjeeling/lint/top_darjeeling_lint_cfgs.hjson --tool ascentlint --local --purge --select-cfgs top_darjeeling
You get IMO easier digestable reports with this and we should be able to add this to CI next week as well. I've also waived some SAME_NAME_TYPE errors (most of them we also have in Earlgrey) none of them are critical as the mentions are in different hierarchies.
|
I am merging this the failing CW310 ROM Test (e2e_bootstrap_rma_fpga_cw310_rom_with_fake_keys) is known to be flaky. |
This PR waives some expected lint errors.