airhdl / spi-to-axi-bridge

An SPI to AXI4-lite bridge for easy interfacing of airhdl register banks with any microcontroller.
Apache License 2.0
40 stars 10 forks source link

Multiple driven errors and spi_tx_byte width mismatch truncation and ... #2

Open lmarien94 opened 1 month ago

lmarien94 commented 1 month ago

Hey,

First, complements... Very useful module :)! I've been looking at integrating the SystemVerilog module in a project and came across a couple of problems (didn't simulate the design with this module in particular, so these are elaboration errors of my simulation tool (Cadence Xcelium) and linting tool.

Looking forward to the answers, I have already made some modifications, have to test them with your test environment, but if all seems well I can provide a pull request.

Thanks.

ps. noticed that some of the things where already mentioned by #1.

norteng commented 1 month ago

that is correct, spi_tx_byte is 8b.  We did make this correction.I suspect that blocking vs non-blocking was not done deliberately, but the result of a VHDL-to-Verilog conversion. I did not make any changes to these and we have been using this code successfully for some time.Andy NortonComm Logic Design, IncOn Jul 21, 2024, at 2:11 AM, lmarien94 @.***> wrote: Hey, First, complements... Very useful module :)! I've been looking at integrating the SystemVerilog module in a project and came across a couple of problems (didn't simulate the design with this module in particular, so these are elaboration errors of my simulation tool (Cadence Xcelium) and linting tool.

Multiple driven errors: There are lots of multiple driven errors in the design. I can see that all signals have "initial values", for example: all the internal AXI signals, the states, synchronizers, ... Why do they have these initial values? Signals should be driven from reset or from an external module/testbench I would assume? Width mismatch truncation: the spi_tx_byte signal is declared as 3 bits, considering the name I would assume that it should be 8-bits? The axi_rdata is assigned per 8-bits to the spi_tx_byte, but since this signal is only 3-bits, data gets lost (see code below)

end else begin // CMD_READ if (spi_tx_byte_idx <= 5) begin // null; end else if (spi_tx_byte_idx == 6) begin spi_tx_byte = axi_rdata[31:24]; end else if (spi_tx_byte_idx == 7) begin spi_tx_byte = axi_rdata[23:16]; end else if (spi_tx_byte_idx == 8) begin spi_tx_byte = axi_rdata[15:8]; end else if (spi_tx_byte_idx == 9) begin spi_tx_byte = axi_rdata[7:0]; end else begin

Blocking versus non-blocking statements: I can also see in the code that there is a lot of non-blocking and blocking assignments intermixed within the processes, just to be clear... Just to confirm, this was done deliberately?

Looking forward to the answers, I have already made some modifications, have to test them with your test environment, but if all seems well I can provide a pull request. Thanks.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>