cariboulite icon indicating copy to clipboard operation
cariboulite copied to clipboard

potential asynchronous FIFO issues

Open hea-lab opened this issue 2 years ago • 2 comments

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

hea-lab avatar Mar 07 '22 05:03 hea-lab

Hi @hea-lab, thanks!

  1. 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 :)
  2. 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?

meexmachina avatar Mar 19 '22 10:03 meexmachina

  1. Ok and good luck with the part shortages :-), it seems the most challenging part in hardware projects nowadays.
  2. 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.

test_complex_fifo.zip

image

hea-lab avatar Mar 28 '22 11:03 hea-lab

fixed with thanks!

meexmachina avatar May 05 '23 20:05 meexmachina