StanfordVLSI / dragonphy2

Open Source PHY v2
Apache License 2.0
21 stars 3 forks source link

Tx block level tests added but not finalized, local prbs test passed #136

Closed CansWang closed 3 years ago

CansWang commented 3 years ago

Tx input order distorted to match normal order [15:0], preliminary prbs test passed. Testbench needs to be finished.

sgherbst commented 3 years ago

@CansWang thanks for this PR. I notice that this branch is out-of-date with the master branch. To fix that, click the "Resolve conflicts" button, and that will walk you through the process of fixing the conflicting files. Afterwards, run git pull locally to pull the changes that you applied on GitHub. This should also fix the problem that is currently causing regression tests to fail on this branch (related to the email I sent out earlier about pysmt).

sgherbst commented 3 years ago

@CansWang looks all tests passed except tx_top_sim. I reviewed the BuildKite log and found this error:

xmelab: *E,CUVPOM (../test.sv,82|7): Port name 'cke' is invalid or has multiple connections.

Looks like you need to delete this line of code: https://github.com/StanfordVLSI/dragonphy2/blob/0f06736c3d3d425c0ef915cc394da0b03367c7db/tests/cpu_block_tests/tx_top_sim/test.sv#L82

I suggest trying to run/fix that test locally, since it takes 1hr to get a pass/fail result for all tests.

sgherbst commented 3 years ago

Couple of small comments:

  1. Could you delete the rst and cke signals in the tx_intf.sv file? Neither is wired up to a JTAG control register, and TX reset is already in the dcore_debug_intf.sv (and wired up to a JTAG register). For cke, since it not used or wired up to anything, it should be removed to avoid confusion. These are the lines I'm referring to: https://github.com/StanfordVLSI/dragonphy2/blob/24b4e6b31d5985930adafd9455b2ea381eee41bc/vlog/chip_src/tx_16t1/tx_debug_intf.sv#L5-L6

  2. On a related note, tx_top has a regular pin called rst that acts as the reset (not tx_intf.rst). I think that you should remove the references to tx_intf.rst in tx_top_sim/test.sv, and wire up the rst pin of tx_top in that testbench to the rst signal, as before.

Sorry to nitpick on this. Since we have already instantiated the TX in DragonPHY top, it is helpful to keep the pinout of tx_top the same, if possible. Thanks!

CansWang commented 3 years ago

Couple of small comments:

  1. Could you delete the rst and cke signals in the tx_intf.sv file? Neither is wired up to a JTAG control register, and TX reset is already in the dcore_debug_intf.sv (and wired up to a JTAG register). For cke, since it not used or wired up to anything, it should be removed to avoid confusion. These are the lines I'm referring to: https://github.com/StanfordVLSI/dragonphy2/blob/24b4e6b31d5985930adafd9455b2ea381eee41bc/vlog/chip_src/tx_16t1/tx_debug_intf.sv#L5-L6
  2. On a related note, tx_top has a regular pin called rst that acts as the reset (not tx_intf.rst). I think that you should remove the references to tx_intf.rst in tx_top_sim/test.sv, and wire up the rst pin of tx_top in that testbench to the rst signal, as before.

Sorry to nitpick on this. Since we have already instantiated the TX in DragonPHY top, it is helpful to keep the pinout of tx_top the same, if possible. Thanks!

Thanks for pointing out this confusion,

rst and cke has been removed from tx_debug_intf.sv