tillitis / tillitis-key1

Board designs, FPGA verilog, firmware for TKey, the flexible and open USB security key 🔑
https://www.tillitis.se
382 stars 24 forks source link

Verilog linter warnings #182

Closed dehanj closed 2 weeks ago

dehanj commented 3 months ago

While updating our tools to a later version of Verilator and updating the lint flags to Verilog-2005 we came across a few new warnings, see below.

We don't have a quick solution to this for a few reasons:

  1. Some of these comes from dependencies we have not written and don't want to edit at the moment.
  2. The warning for the oscillator ( Circular combinational logic ) is true, but also the intended design of our oscillator. We need to find a way to suppress it. Setting the the linter off with UNOPTFLAT didn't seem to take for some reason.
verilator +1364-2005ext+ --lint-only  -Wall -Wno-DECLFILENAME --timescale 1ns/1ns -DNO_ICE40_DEFAULT_ASSIGNMENTS \
-DBRAM_FW_SIZE=1536 \
-DFIRMWARE_HEX=\"/build/firmware.hex\" \
-DUDS_HEX=\"/build/data/uds.hex\" \
-DUDI_HEX=\"/build/data/udi.hex\" \
--top-module application_fpga \
config.vlt /build/rtl/application_fpga.v /build/rtl/clk_reset_gen.v /build/rtl/ram.v /build/rtl/rom.v /build/rtl/fw_ram.v /build/core/picorv32/rtl/picorv32.v /build/core/timer/rtl/timer_core.v /build/core/timer/rtl/timer.v /build/core/uds/rtl/uds.v /build/core/uds/rtl/uds_rom.v /build/core/touch_sense/rtl/touch_sense.v /build/core/tk1/rtl/tk1.v /build/core/tk1/rtl/udi_rom.v /build/core/uart/rtl/uart_core.v /build/core/uart/rtl/uart_fifo.v /build/core/uart/rtl/uart.v /build/core/trng/rtl/rosc.v /usr/local/share/yosys/ice40/cells_sim.v \
>lint_issues.txt 2>&1 \
&& { rm -f lint_issues.txt; exit 0; } \
|| {   cat lint_issues.txt; exit 1; }
%Warning-WIDTHEXPAND: /usr/local/share/yosys/ice40/cells_sim.v:184:19: Operator VAR 'LUT_INIT' expects 16 bits on the Initial value, but Initial value's CONST '2'h1' generates 2 bits.
                                                                     : ... In instance application_fpga.tk1_inst.rom_i.luts[0].lut_i
  184 |  parameter [15:0] LUT_INIT = 0;
      |                   ^~~~~~~~
                      ... For warning description see https://verilator.org/warn/WIDTHEXPAND?v=5.012
                      ... Use "/* verilator lint_off WIDTHEXPAND */" and lint_on around source to disable this message.
%Warning-UNOPTFLAT: /build/rtl/fw_ram.v:38:16: Signal unoptimizable: Circular combinational logic: 'application_fpga.fw_ram_inst.fw_app_cs'
   38 |   wire         fw_app_cs;
      |                ^~~~~~~~~
                    /build/rtl/fw_ram.v:38:16:      Example path: application_fpga.fw_ram_inst.fw_app_cs
                    /build/rtl/fw_ram.v:128:3:      Example path: ALWAYS
                    /build/rtl/fw_ram.v:34:16:      Example path: application_fpga.fw_ram_inst.tmp_read_data
                    /build/rtl/application_fpga.v:360:3:      Example path: ALWAYS
                    /build/rtl/application_fpga.v:121:17:      Example path: application_fpga.fw_ram_cs
                    /build/rtl/fw_ram.v:48:20:      Example path: ASSIGNW
                    /build/rtl/fw_ram.v:38:16:      Example path: application_fpga.fw_ram_inst.fw_app_cs
%Warning-UNOPTFLAT: /usr/local/share/yosys/ice40/cells_sim.v:178:9: Signal unoptimizable: Circular combinational logic: 'application_fpga.trng_inst.__Vcellout__oscillators[15].osc_inv_f__O'
  178 |  output O,
      |         ^
                    /usr/local/share/yosys/ice40/cells_sim.v:178:9:      Example path: application_fpga.trng_inst.__Vcellout__oscillators[15].osc_inv_f__O
                    /build/core/trng/rtl/rosc.v:118:53:      Example path: ASSIGNW
                    /usr/local/share/yosys/ice40/cells_sim.v:179:8:      Example path: application_fpga.trng_inst.__Vcellinp__oscillators[15].osc_inv_f__I0
                    /usr/local/share/yosys/ice40/cells_sim.v:188:11:      Example path: ASSIGNW
                    /usr/local/share/yosys/ice40/cells_sim.v:178:9:      Example path: application_fpga.trng_inst.__Vcellout__oscillators[15].osc_inv_f__O
%Error: Exiting due to 3 warning(s)
make: *** [Makefile:183: lint] Error 1
secworks commented 4 weeks ago

We should add simulation or linter models that allow us to run lint. See also https://github.com/tillitis/tillitis-key1/pull/233 for more details on lint warnings and how we solve them for the tk1 core.

dehanj commented 2 weeks ago

We still have to keep an eye out for linting, there could be introduced lint warnings that are globally ignored.