search

ucb-bar / testchipip

BSD 3-Clause "New" or "Revised" License
80 stars 59 forks source link

Multi serial tilelink #209

Closed jerryz123 closed 9 months ago

jerryz123 commented 9 months ago

Support multiple serial tilelink links

kevindna commented 9 months ago

General comment is that you added a couple new configs, but no comments about why? Like I could search through the repo, OR I could read a one liner. Rejected

jerryz123 commented 9 months ago

Could you point to the configs for which you'd like a in-line comment explaining the use case?

kevindna commented 9 months ago

I will list line numbers since some are not full configs:

jerryz123 commented 9 months ago

I will list line numbers since some are not full configs:

  • src/main/scala/PeripheryTLSerial.scala: Line 19
  • src/main/scala/PeripheryTLSerial.scala: Line 23
  • src/main/scala/Configs.scala: Line 141 (How should I use this?)
  • src/main/scala/Configs.scala: Line 199 (coherent TLB??)
  • src/main/scala/Configs.scala: Liine 106 (this does __)
  • src/main/scala/TSIHarness.scala: Line 199 ("Iterate over TL connections")

I've addressed most of these, but I couldn't find what you are referring to in TSIHarness.scala (where is line 199?). Use inline comments next time please.

jerryz123 commented 9 months ago

Thanks for the review! I'll merge this together with the Chipyard one once that passes tests