StanfordVLSI / dragonphy2

Open Source PHY v2
Apache License 2.0
22 stars 2 forks source link

The transmitter directory created #127

Closed CansWang closed 3 years ago

CansWang commented 3 years ago

16 to 1 quarter-rate transmitter created

sgherbst commented 3 years ago

Hi @CansWang -- thanks for submitting this PR! Couple of comments:

  1. I noticed that the tests aren't passing because it duplicates the definition of prbs_generator_syn.sv. In our system, each file should define exactly one module, whose name is given by the file name (e.g., prbs_generator_syn.sv defines exactly one module, called prbs_generator_syn). I think your code is already following that convention. In addition, there should only be one definition of a module in each folder in vlog -- so chip_src can only define prbs_generator_syn once. In this particular case, I think you can just remove that module from the tx_16t1 folder. Our dependency management system should pick up the location of that module automatically.
  2. For the same reason, please remove the files ff_c and mux, since they are already defined in analog_core/gates.
  3. The chip_src folder should only have code that is intended to go on the chip, so the tx_top_tb.sv file should be moved to one of the block- or system-level tests, or maybe to the vlog/tb folder.
CansWang commented 3 years ago

Hi @CansWang -- thanks for submitting this PR! Couple of comments:

  1. I noticed that the tests aren't passing because it duplicates the definition of prbs_generator_syn.sv. In our system, each file should define exactly one module, whose name is given by the file name (e.g., prbs_generator_syn.sv defines exactly one module, called prbs_generator_syn). I think your code is already following that convention. In addition, there should only be one definition of a module in each folder in vlog -- so chip_src can only define prbs_generator_syn once. In this particular case, I think you can just remove that module from the tx_16t1 folder. Our dependency management system should pick up the location of that module automatically.

Yes, I will remove them accordingly. Thanks for the reminder

  1. For the same reason, please remove the files ff_c and mux, since they are already defined in analog_core/gates.
  2. The chip_src folder should only have code that is intended to go on the chip, so the tx_top_tb.sv file should be moved to one of the block- or system-level tests, or maybe to the vlog/tb folder.

I will reorganize the files and push it again