search

lnis-uofu / OpenFPGA

An Open-source FPGA IP Generator
https://openfpga.readthedocs.io/en/master/
MIT License
813 stars 161 forks source link

SOFA: incorrect IO_ISOL_N polarity and un-needed SOC_OUT assignment in test_bench #178

Closed liaokevin-ql closed 3 years ago

liaokevin-ql commented 3 years ago

Describe the bug Hi Ganesh/XiFan:

Just want to give you an update on the verification that Rakesh is running and we notice of the following to see if this can be taken care of in OpenFPGA:

a) It looks like the IO_ISOL_N polarity is inverted. Looks like the polarity of “IO_ISOL_N” is incorrectly set in the test bench. The assignment shows that IO_ISOL_N is the inversion of config_done. That essential will put “IO_ISOL_N” to “0”, which will tristate the IO in the normal mode. pic_1 The following simulation shows that if line 141 is used, the output is tristated.The following simulation shows that if line 141 is used, the output is tristated. pic_2

b) All SOC_OUT have an assignment to 1’b0; I understand where this is coming from where Caravel probably requested for tristating the IO during configuration. At the eFPGA level, by assigning the 1’b0 to any unused IO could hide some issue that we may not be observable. For example, if the SOC_OUT is not tristated as intended and it is driving a “0”, we may not know until later that there was an issue.

Do you guys agree?

tangxifan commented 3 years ago

@liaokevin-ql For the config_done signal, you just need to use default_val="0" when defining the ISOL_N signal. When default_val="1", the signal will be connected to an inverted config_done signal.

For the unused SOC_OUT, I think it should not be wired to logic 0. I will patch OpenFPGA

liaokevin-ql commented 3 years ago

The actual issue regarding IO_ISO_N and config_done is, these two individual ports shall be decoupled. for example, (1) when config_done=0 (during configuration cycles), and IO_ISO_N=0, SOC_OUTs are tri-stated. (2) when config_done=0 (during configuration cycles), and IO_ISO_N=1, SOC_OUTs are sitting on a predetermined value. This is based on the "ql_iso_io_logic.v" My original issue statement did not point out this difference.

tangxifan commented 3 years ago

@liaokevin-ql Not 100% clear to me. Can we schedule a talk? I would like to learn more details and we can work out a solution