Address code review comments

generators/chipyard/src/main/scala/ChipTop.scala

@@ -46,8 +46,7 @@ class ChipTop(implicit p: Parameters) extends LazyModule with HasTestHarnessFunc
     val implicit_reset = implicitClockSinkNode.in.head._1.reset
 
 
-    // The implicit clock and reset for the system is also, by convention, used for all the IOBinders
-    // TODO: This may not be the right thing to do in all cases
+    // Note: IOBinders cannot rely on the implicit clock/reset, as this is a LazyRawModuleImp
     val (_ports, _iocells, _portMap) = ApplyIOBinders(lazySystem, p(IOBinders))
     // We ignore _ports for now...
     iocells ++= _iocells

generators/chipyard/src/main/scala/Clocks.scala

@@ -120,7 +120,6 @@ object ClockingSchemeGenerators {
         }
       }}
 
-
       chiptop.harnessFunctions += ((th: HasHarnessSignalReferences) => {
         clock_io := th.harnessClock
         Nil

generators/chipyard/src/main/scala/HarnessBinders.scala

@@ -69,7 +69,7 @@ class WithGPIOTiedOff extends OverrideHarnessBinder({
 // DOC include start: WithUARTAdapter
 class WithUARTAdapter extends OverrideHarnessBinder({
   (system: HasPeripheryUARTModuleImp, th: HasHarnessSignalReferences, ports: Seq[Data]) => {
-    UARTAdapter.connect(ports.map(_.asInstanceOf[UARTPortIO]))(system.p)
+    UARTAdapter.connect(ports.map({case p: UARTPortIO => p}))(system.p)
     Nil
   }
 })
@@ -77,7 +77,7 @@ class WithUARTAdapter extends OverrideHarnessBinder({
 
 class WithSimSPIFlashModel(rdOnly: Boolean = true) extends OverrideHarnessBinder({
   (system: HasPeripherySPIFlashModuleImp, th: HasHarnessSignalReferences, ports: Seq[Data]) => {
-    SimSPIFlashModel.connect(ports.map(_.asInstanceOf[SPIChipIO]), th.harnessReset, rdOnly)(system.p)
+    SimSPIFlashModel.connect(ports.map({case p: SPIChipIO => p}), th.harnessReset, rdOnly)(system.p)
     Nil
   }
 })
@@ -132,8 +132,11 @@ class WithSimAXIMem extends OverrideHarnessBinder({
   (system: CanHaveMasterAXI4MemPort, th: HasHarnessSignalReferences, ports: Seq[Data]) => {
     val p: Parameters = chipyard.iobinders.GetSystemParameters(system)
     val clock = WireInit(false.B.asClock)
-    ports.filter(_.isInstanceOf[Clock]).map { case p: Clock => clock := p }
-    val axi4_ports = ports.filter(_.isInstanceOf[AXI4Bundle])
+    ports.map {
+      case p: Clock => clock := p
+      case _ =>
+    }
+    val axi4_ports = ports.collect { case p: AXI4Bundle => p }
     (axi4_ports zip system.memAXI4Node.edges.in).map { case (port: AXI4Bundle, edge) =>
       val mem = LazyModule(new SimAXIMem(edge, size=p(ExtMem).get.master.size)(p))
       withClockAndReset(clock, th.harnessReset) {
@@ -149,7 +152,10 @@ class WithBlackBoxSimMem extends OverrideHarnessBinder({
   (system: CanHaveMasterAXI4MemPort, th: HasHarnessSignalReferences, ports: Seq[Data]) => {
     val p: Parameters = chipyard.iobinders.GetSystemParameters(system)
     val clock = WireInit(false.B.asClock)
-    ports.filter(_.isInstanceOf[Clock]).map { case p: Clock => clock := p }
+    ports.map {
+      case p: Clock => clock := p
+      case _ =>
+    }
     val axi4_ports = ports.collect { case p: AXI4Bundle => p }
     (axi4_ports zip system.memAXI4Node.edges.in).map { case (port: AXI4Bundle, edge) =>
       val memSize = p(ExtMem).get.master.size
@@ -167,11 +173,15 @@ class WithSimAXIMMIO extends OverrideHarnessBinder({
   (system: CanHaveMasterAXI4MMIOPort, th: HasHarnessSignalReferences, ports: Seq[Data]) => {
     val p: Parameters = chipyard.iobinders.GetSystemParameters(system)
     val clock = WireInit(false.B.asClock)
-    ports.filter(_.isInstanceOf[Clock]).map { case p: Clock => clock := p }
-    (ports zip system.mmioAXI4Node.edges.in).zipWithIndex.map { case ((port: AXI4Bundle, edge), i) =>
-      val mmio_mem = LazyModule(new SimAXIMem(edge, size = 4096)(p))
+    ports.map {
+       case p: Clock => clock := p
+       case _ =>
+    }
+    val axi4_ports = ports.collect { case p: AXI4Bundle => p }
+    (axi4_ports zip system.mmioAXI4Node.edges.in).map { case (port: AXI4Bundle, edge) =>
+      val mmio_mem = LazyModule(new SimAXIMem(edge, size = p(ExtBus).get.size)(p))
       withClockAndReset(clock, th.harnessReset) {
-        Module(mmio_mem.module).suggestName(s"mmio_mem_${i}")
+        Module(mmio_mem.module).suggestName("mmio_mem")
       }
       mmio_mem.io_axi4.head <> port
     }
@@ -188,9 +198,11 @@ class WithTieOffInterrupts extends OverrideHarnessBinder({
 
 class WithTieOffL2FBusAXI extends OverrideHarnessBinder({
   (system: CanHaveSlaveAXI4Port, th: HasHarnessSignalReferences, ports: Seq[Data]) => {
-    ports.map { case p: AXI4Bundle =>
-      p := DontCare
-      p.tieoff()
+    ports.map {
+      case p: AXI4Bundle =>
+        p := DontCare
+        p.tieoff()
+      case c: Clock =>
     }
     Nil
   }

generators/chipyard/src/main/scala/IOBinders.scala

@@ -47,8 +47,13 @@ case object IOBinders extends Field[Map[String, (Any) => (Seq[Data], Seq[IOCell]
 object ApplyIOBinders {
   def apply(sys: LazyModule, map: Map[String, (Any) => (Seq[Data], Seq[IOCell])]):
       (Iterable[Data], Iterable[IOCell], Map[String, Seq[Data]]) = {
-    val r = map.map({ case (s,f) => (f(sys), s) }) ++ map.map({ case (s,f) => (f(sys.module), s) })
-    (r.flatMap(_._1._1), r.flatMap(_._1._2), r.map { t => t._2 -> t._1._1 })
+    val lzy = map.map({ case (s,f) => s -> f(sys) })
+    val imp = map.map({ case (s,f) => s -> f(sys.module) })
+
+    val ports: Iterable[Data] = lzy.values.map(_._1).flatten ++ imp.values.map(_._1).flatten
+    val cells: Iterable[IOCell] = lzy.values.map(_._2).flatten ++ imp.values.map(_._2).flatten
+    val portMap: Map[String, Seq[Data]] = map.keys.map(k => k -> (lzy(k)._1 ++ imp(k)._1)).toMap
+    (ports, cells, portMap)
   }
 }
 
@@ -287,13 +292,18 @@ class WithAXI4MMIOPunchthrough extends OverrideIOBinder({
 })
 
 class WithL2FBusAXI4Punchthrough extends OverrideIOBinder({
-  (system: CanHaveSlaveAXI4Port) => {
-    val port = system.l2_frontend_bus_axi4.zipWithIndex.map({ case (m, i) =>
+   (system: CanHaveSlaveAXI4Port) => {
+    val clock = if (!system.l2_frontend_bus_axi4.isEmpty) {
+      Some(BoreHelper("axi4_fbus_clock", system.asInstanceOf[BaseSubsystem].fbus.module.clock))
+    } else {
+      None
+    }
+    val ports = system.l2_frontend_bus_axi4.zipWithIndex.map({ case (m, i) =>
       val p = IO(Flipped(DataMirror.internal.chiselTypeClone[AXI4Bundle](m))).suggestName(s"axi4_fbus_${i}")
       m <> p
       p
     })
-    (port, Nil)
+    (ports ++ clock, Nil)
   }
 })
 

generators/chipyard/src/main/scala/config/ArianeConfigs.scala

@@ -13,7 +13,7 @@ class ArianeConfig extends Config(
   new chipyard.config.AbstractConfig)
 
 class dmiArianeConfig extends Config(
-  new chipyard.harness.WithTiedOffSerial ++          // Tie off the serial port, override default instantiation of SimSerial
+  new chipyard.harness.WithTiedOffSerial ++            // Tie off the serial port, override default instantiation of SimSerial
   new chipyard.config.WithDMIDTM ++                    // have debug module expose a clocked DMI port
   new ariane.WithNArianeCores(1) ++                    // single Ariane core
   new chipyard.config.AbstractConfig)

generators/chipyard/src/main/scala/config/RocketConfigs.scala

@@ -12,21 +12,21 @@ class RocketConfig extends Config(
 
 class HwachaRocketConfig extends Config(
   new chipyard.config.WithHwachaTest ++
-  new hwacha.DefaultHwachaConfig ++                        // use Hwacha vector accelerator
+  new hwacha.DefaultHwachaConfig ++                              // use Hwacha vector accelerator
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 
 // DOC include start: GemminiRocketConfig
 class GemminiRocketConfig extends Config(
-  new gemmini.DefaultGemminiConfig ++                        // use Gemmini systolic array GEMM accelerator
+  new gemmini.DefaultGemminiConfig ++                            // use Gemmini systolic array GEMM accelerator
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 // DOC include end: GemminiRocketConfig
 
 // DOC include start: DmiRocket
 class dmiRocketConfig extends Config(
-  new chipyard.harness.WithTiedOffSerial ++          // don't use serial to drive the chip, since we use DMI instead
-  new chipyard.config.WithDMIDTM ++                    // have debug module expose a clocked DMI port
+  new chipyard.harness.WithTiedOffSerial ++                      // don't use serial to drive the chip, since we use DMI instead
+  new chipyard.config.WithDMIDTM ++                              // have debug module expose a clocked DMI port
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 // DOC include end: DmiRocket
@@ -40,53 +40,53 @@ class GCDTLRocketConfig extends Config(
 
 // DOC include start: GCDAXI4BlackBoxRocketConfig
 class GCDAXI4BlackBoxRocketConfig extends Config(
-  new chipyard.example.WithGCD(useAXI4=true, useBlackBox=true) ++          // Use GCD blackboxed verilog, connect by AXI4->Tilelink
+  new chipyard.example.WithGCD(useAXI4=true, useBlackBox=true) ++            // Use GCD blackboxed verilog, connect by AXI4->Tilelink
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 // DOC include end: GCDAXI4BlackBoxRocketConfig
 
 class LargeSPIFlashROMRocketConfig extends Config(
-  new chipyard.harness.WithSimSPIFlashModel(true) ++      // add the SPI flash model in the harness (read-only)
+  new chipyard.harness.WithSimSPIFlashModel(true) ++        // add the SPI flash model in the harness (read-only)
   new chipyard.config.WithSPIFlash ++                       // add the SPI flash controller
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 
 class SmallSPIFlashRocketConfig extends Config(
-  new chipyard.harness.WithSimSPIFlashModel(false) ++     // add the SPI flash model in the harness (writeable)
+  new chipyard.harness.WithSimSPIFlashModel(false) ++       // add the SPI flash model in the harness (writeable)
   new chipyard.config.WithSPIFlash(0x100000) ++             // add the SPI flash controller (1 MiB)
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 
 class SimAXIRocketConfig extends Config(
-  new chipyard.harness.WithSimAXIMem ++                   // drive the master AXI4 memory with a SimAXIMem, a 1-cycle magic memory, instead of default SimDRAM
+  new chipyard.harness.WithSimAXIMem ++                     // drive the master AXI4 memory with a SimAXIMem, a 1-cycle magic memory, instead of default SimDRAM
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 
 class SimBlockDeviceRocketConfig extends Config(
-  new chipyard.harness.WithSimBlockDevice ++             // drive block-device IOs with SimBlockDevice
-  new testchipip.WithBlockDevice ++                        // add block-device module to peripherybus
+  new chipyard.harness.WithSimBlockDevice ++                // drive block-device IOs with SimBlockDevice
+  new testchipip.WithBlockDevice ++                         // add block-device module to peripherybus
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 
 class BlockDeviceModelRocketConfig extends Config(
-  new chipyard.harness.WithBlockDeviceModel ++           // drive block-device IOs with a BlockDeviceModel
-  new testchipip.WithBlockDevice ++                        // add block-device module to periphery bus
+  new chipyard.harness.WithBlockDeviceModel ++              // drive block-device IOs with a BlockDeviceModel
+  new testchipip.WithBlockDevice ++                         // add block-device module to periphery bus
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 
 // DOC include start: GPIORocketConfig
 class GPIORocketConfig extends Config(
-  new chipyard.config.WithGPIO ++                          // add GPIOs to the peripherybus
+  new chipyard.config.WithGPIO ++                           // add GPIOs to the peripherybus
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 // DOC include end: GPIORocketConfig
 
 class QuadRocketConfig extends Config(
-  new freechips.rocketchip.subsystem.WithNBigCores(4) ++   // quad-core (4 RocketTiles)
+  new freechips.rocketchip.subsystem.WithNBigCores(4) ++    // quad-core (4 RocketTiles)
   new chipyard.config.AbstractConfig)
 
 class RV32RocketConfig extends Config(
-  new freechips.rocketchip.subsystem.WithRV32 ++           // set RocketTiles to be 32-bit
+  new freechips.rocketchip.subsystem.WithRV32 ++            // set RocketTiles to be 32-bit
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 
@@ -104,14 +104,14 @@ class Sha3RocketConfig extends Config(
 
 // DOC include start: InitZeroRocketConfig
 class InitZeroRocketConfig extends Config(
-  new chipyard.example.WithInitZero(0x88000000L, 0x1000L) ++                // add InitZero
+  new chipyard.example.WithInitZero(0x88000000L, 0x1000L) ++   // add InitZero
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 // DOC include end: InitZeroRocketConfig
 
 class LoopbackNICRocketConfig extends Config(
-  new chipyard.harness.WithLoopbackNIC ++                        // drive NIC IOs with loopback
-  new icenet.WithIceNIC ++                                         // add an IceNIC
+  new chipyard.harness.WithLoopbackNIC ++                      // drive NIC IOs with loopback
+  new icenet.WithIceNIC ++                                     // add an IceNIC
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 
@@ -126,8 +126,8 @@ class ScratchpadOnlyRocketConfig extends Config(
 // DOC include end: l1scratchpadrocket
 
 class L1ScratchpadRocketConfig extends Config(
-  new chipyard.config.WithRocketICacheScratchpad ++   // use rocket ICache scratchpad
-  new chipyard.config.WithRocketDCacheScratchpad ++   // use rocket DCache scratchpad
+  new chipyard.config.WithRocketICacheScratchpad ++         // use rocket ICache scratchpad
+  new chipyard.config.WithRocketDCacheScratchpad ++         // use rocket DCache scratchpad
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 
@@ -141,35 +141,35 @@ class MbusScratchpadRocketConfig extends Config(
 
 // DOC include start: RingSystemBusRocket
 class RingSystemBusRocketConfig extends Config(
-  new testchipip.WithRingSystemBus ++ // Ring-topology system bus
+  new testchipip.WithRingSystemBus ++                       // Ring-topology system bus
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 // DOC include end: RingSystemBusRocket
 
 class StreamingPassthroughRocketConfig extends Config(
-  new chipyard.example.WithStreamingPassthrough ++ // use top with tilelink-controlled streaming passthrough
+  new chipyard.example.WithStreamingPassthrough ++          // use top with tilelink-controlled streaming passthrough
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 
 // DOC include start: StreamingFIRRocketConfig
 class StreamingFIRRocketConfig extends Config (
-  new chipyard.example.WithStreamingFIR ++ // use top with tilelink-controlled streaming FIR
+  new chipyard.example.WithStreamingFIR ++                  // use top with tilelink-controlled streaming FIR
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 // DOC include end: StreamingFIRRocketConfig
 
 class SmallNVDLARocketConfig extends Config(
-  new nvidia.blocks.dla.WithNVDLA("small") ++ // add a small NVDLA
+  new nvidia.blocks.dla.WithNVDLA("small") ++               // add a small NVDLA
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 
 class LargeNVDLARocketConfig extends Config(
-  new nvidia.blocks.dla.WithNVDLA("large", true) ++ // add a large NVDLA with synth. rams
+  new nvidia.blocks.dla.WithNVDLA("large", true) ++         // add a large NVDLA with synth. rams
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
 
 class MMIORocketConfig extends Config(
-  new freechips.rocketchip.subsystem.WithDefaultMMIOPort ++ // add default external master port
+  new freechips.rocketchip.subsystem.WithDefaultMMIOPort ++  // add default external master port
   new freechips.rocketchip.subsystem.WithDefaultSlavePort ++ // add default external slave port
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)
@@ -177,7 +177,7 @@ class MMIORocketConfig extends Config(
 // NOTE: This config doesn't work yet because SimWidgets in the TestHarness
 // always get the TestHarness clock. The Tiles and Uncore receive the correct clocks
 class DividedClockRocketConfig extends Config(
-  new chipyard.config.WithTileDividedClock ++                       // Put the Tile on its own clock domain
+  new chipyard.config.WithTileDividedClock ++                     // Put the Tile on its own clock domain
   new freechips.rocketchip.subsystem.WithRationalRocketTiles ++   // Add rational crossings between RocketTile and uncore
   new freechips.rocketchip.subsystem.WithNBigCores(1) ++
   new chipyard.config.AbstractConfig)

generators/chipyard/src/main/scala/config/TracegenConfigs.scala

@@ -3,61 +3,37 @@ package chipyard
 import freechips.rocketchip.config.{Config}
 import freechips.rocketchip.rocket.{DCacheParams}
 
-class TraceGenConfig extends Config(
+class AbstractTraceGenConfig extends Config(
   new chipyard.harness.WithBlackBoxSimMem ++
   new chipyard.harness.WithTraceGenSuccess ++
   new chipyard.iobinders.WithAXI4MemPunchthrough ++
   new chipyard.iobinders.WithTraceGenSuccessPunchthrough ++
   new chipyard.config.WithTracegenSystem ++
   new chipyard.config.WithNoSubsystemDrivenClocks ++
-  new tracegen.WithTraceGen()(List.fill(2) { DCacheParams(nMSHRs = 0, nSets = 16, nWays = 2) }) ++
   new freechips.rocketchip.subsystem.WithCoherentBusTopology ++
   new freechips.rocketchip.groundtest.GroundTestBaseConfig)
 
+
+class TraceGenConfig extends Config(
+  new tracegen.WithTraceGen()(List.fill(2) { DCacheParams(nMSHRs = 0, nSets = 16, nWays = 2) }) ++
+  new AbstractTraceGenConfig)
+
 class NonBlockingTraceGenConfig extends Config(
-  new chipyard.harness.WithBlackBoxSimMem ++
-  new chipyard.harness.WithTraceGenSuccess ++
-  new chipyard.iobinders.WithAXI4MemPunchthrough ++
-  new chipyard.iobinders.WithTraceGenSuccessPunchthrough ++
-  new chipyard.config.WithTracegenSystem ++
-  new chipyard.config.WithNoSubsystemDrivenClocks ++
   new tracegen.WithTraceGen()(List.fill(2) { DCacheParams(nMSHRs = 2, nSets = 16, nWays = 2) }) ++
-  new freechips.rocketchip.subsystem.WithCoherentBusTopology ++
-  new freechips.rocketchip.groundtest.GroundTestBaseConfig)
+  new AbstractTraceGenConfig)
 
 class BoomTraceGenConfig extends Config(
-  new chipyard.harness.WithBlackBoxSimMem ++
-  new chipyard.harness.WithTraceGenSuccess ++
-  new chipyard.iobinders.WithAXI4MemPunchthrough ++
-  new chipyard.iobinders.WithTraceGenSuccessPunchthrough ++
-  new chipyard.config.WithTracegenSystem ++
-  new chipyard.config.WithNoSubsystemDrivenClocks ++
   new tracegen.WithBoomTraceGen()(List.fill(2) { DCacheParams(nMSHRs = 8, nSets = 16, nWays = 2) }) ++
   new freechips.rocketchip.subsystem.WithInclusiveCache ++
-  new freechips.rocketchip.subsystem.WithCoherentBusTopology ++
-  new freechips.rocketchip.groundtest.GroundTestBaseConfig)
+  new AbstractTraceGenConfig)
 
 class NonBlockingTraceGenL2Config extends Config(
-  new chipyard.harness.WithBlackBoxSimMem ++
-  new chipyard.harness.WithTraceGenSuccess ++
-  new chipyard.iobinders.WithAXI4MemPunchthrough ++
-  new chipyard.iobinders.WithTraceGenSuccessPunchthrough ++
-  new chipyard.config.WithTracegenSystem ++
-  new chipyard.config.WithNoSubsystemDrivenClocks ++
   new tracegen.WithL2TraceGen()(List.fill(2)(DCacheParams(nMSHRs = 2, nSets = 16, nWays = 4))) ++
   new freechips.rocketchip.subsystem.WithInclusiveCache ++
-  new freechips.rocketchip.subsystem.WithCoherentBusTopology ++
-  new freechips.rocketchip.groundtest.GroundTestBaseConfig)
+  new AbstractTraceGenConfig)
 
 class NonBlockingTraceGenL2RingConfig extends Config(
-  new chipyard.harness.WithBlackBoxSimMem ++
-  new chipyard.harness.WithTraceGenSuccess ++
-  new chipyard.iobinders.WithAXI4MemPunchthrough ++
-  new chipyard.iobinders.WithTraceGenSuccessPunchthrough ++
-  new chipyard.config.WithTracegenSystem ++
-  new chipyard.config.WithNoSubsystemDrivenClocks ++
   new tracegen.WithL2TraceGen()(List.fill(2)(DCacheParams(nMSHRs = 2, nSets = 16, nWays = 4))) ++
   new testchipip.WithRingSystemBus ++
   new freechips.rocketchip.subsystem.WithInclusiveCache ++
-  new freechips.rocketchip.subsystem.WithCoherentBusTopology ++
-  new freechips.rocketchip.groundtest.GroundTestBaseConfig)
+  new AbstractTraceGenConfig)