tillitis / tillitis-key1

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

Fix tk1 lint #233

Closed secworks closed 3 months ago

secworks commented 3 months ago

This PR fixes linting of the tk1 core. It adds the udi_rom and sb_rbga_led simulation models used to replace the Lattice core instantiations done in the RTL. We also add ignore-pragmas to disable lint warnings on things not supported in the sim model of the rgba core.

dehanj commented 3 months ago

When testing I noticed that this issue did not arise on main, when using make lint in hw/application_fpga.

Is it due to these?

ICE40_SIM_CELLS = $(shell yosys-config --datdir/ice40/cells_sim.v)

Maybe we should reuse them instead of making our own sim models?

secworks commented 3 months ago

using the ICE40_SIM_CELLS models instead of our own sim models cause the linter - Verilator in Verilog 2005 mode to become very disappointed:

verilator +1364-2005ext+ --lint-only  -Wall -Wno-fatal -Wno-DECLFILENAME ../rtl/tk1.v /opt/oss-cad-suite/share/yosys/ice40/cells_sim.v
%Error: /opt/oss-cad-suite/share/yosys/ice40/cells_sim.v:1497:21: syntax error, unexpected '=', expecting ','
 1497 |  input  [15:0] MASK = 16'h 0000,
      |                     ^
%Error: /opt/oss-cad-suite/share/yosys/ice40/cells_sim.v:1533:2: syntax error, unexpected generate
 1533 |  generate
      |  ^~~~~~~~
%Error: /opt/oss-cad-suite/share/yosys/ice40/cells_sim.v:1607:2: syntax error, unexpected initial
 1607 |  initial begin
      |  ^~~~~~~
%Error: /opt/oss-cad-suite/share/yosys/ice40/cells_sim.v:1736:21: syntax error, unexpected '=', expecting ','
 1736 |  input  [15:0] MASK = 16'h 0000,
      |                     ^
%Error: /opt/oss-cad-suite/share/yosys/ice40/cells_sim.v:1761:2: syntax error, unexpected IDENTIFIER
 1761 |  SB_RAM40_4K #(
      |  ^~~~~~~~~~~
%Error: /opt/oss-cad-suite/share/yosys/ice40/cells_sim.v:1872:21: syntax error, unexpected '=', expecting ','
 1872 |  input  [15:0] MASK = 16'h 0000,
      |                     ^
%Error: /opt/oss-cad-suite/share/yosys/ice40/cells_sim.v:1897:2: syntax error, unexpected IDENTIFIER
 1897 |  SB_RAM40_4K #(
      |  ^~~~~~~~~~~
%Error: /opt/oss-cad-suite/share/yosys/ice40/cells_sim.v:2008:21: syntax error, unexpected '=', expecting ','
 2008 |  input  [15:0] MASK = 16'h 0000,
      |                     ^
%Error: /opt/oss-cad-suite/share/yosys/ice40/cells_sim.v:2033:2: syntax error, unexpected IDENTIFIER
 2033 |  SB_RAM40_4K #(
      |  ^~~~~~~~~~~
%Error: Exiting due to 9 error(s)

We also disable linting of the whole FPGA (at least in CI) due to a lot of problems in the ICE40 cell libraries. They may not be updated for Verilog 2005. I propose that we don't use them for the tk1. At least for now. When the libraries have been fixed upstream we could revisit this issue.

secworks commented 3 months ago

The issues the linter throws errors relates to language features not supported in Verilog 2005. For example, Verilog 2005 does not support default values in module port declarations in the way it's used in cells_sim.v.

I suggest we use our own sim models for linting. And do similar things for the rest of the design. It is better that we can lint 90% of the design that not being able to lint at all. I'll create an issue for fixing linting of the whole design (unless it exists or not.)