cariboulite
cariboulite copied to clipboard
potential asynchronous FIFO issues
First of all, many thanks for your contribution! This is a great foundation for tailor made SDR design. So, I could not wait for your delivery so I decided to review and play with your design (using verilog simulation first and lately using a home made board).
Back to the issue, it seems you have built your own asynchronous FIFO from dual_clock_fifo module which looks suitable and you get rid of graycode handling (which is commonly used to address metastablity issues). Is there a reason for that?
Furthermore, it seems that you should take into account address width when your compute full_o and empty_o, otherwise you may have overflow issues
basically, I would expect something like
full_o <= ((wr_addr + 2) & ((2**ADDR_WIDTH)-1)) == rd_addr;
instead of
full_o <= (wr_addr + 2) == rd_addr;
https://github.com/cariboulabs/cariboulite/blob/main/firmware/complex_fifo.v#L30 https://github.com/cariboulabs/cariboulite/blob/main/firmware/complex_fifo.v#L34 https://github.com/cariboulabs/cariboulite/blob/main/firmware/complex_fifo.v#L49
Hi @hea-lab, thanks!
- the gray code - you are right I got rid of it to save some space in the FPGA logic. I will add it when I come back to the firmware after taking care of some silicon shortages :)
- I the pushing and popping are both 32bit samples (16-I, 16-Q) and there are no bit-level writing operations. so it seems to me that the sub-32bit checking is redundant. I will revisit it later, but if you find my answer not sufficient, can you please give me an example where this will cause a problem?
- Ok and good luck with the part shortages :-), it seems the most challenging part in hardware projects nowadays.
- I have attached a quick test bench to describe the issue, it instantiates 2 complex fifo (original and patched as https://github.com/cariboulabs/cariboulite/issues/24#issue-1160928086) and fills them up without reading from them. Without the fix the full_o is not properly computed as the signal stay low during the whole simulation. The second waveform is generated from the patched complex_fifo.
fixed with thanks!