extra devices should get elaborated in a single build function

src/main/scala/rocketchip/Configs.scala

@@ -12,6 +12,7 @@ import uncore.converters._
 import coreplex._
 import scala.math.max
 import scala.collection.mutable.{LinkedHashSet, ListBuffer}
+import scala.collection.immutable.HashMap
 import DefaultTestSuites._
 import cde.{Parameters, Config, Dump, Knob, CDEMatchError}
 
@@ -93,13 +94,7 @@ class BasePlatformConfig extends Config (
         res append  "  };\n"
       }
       res append  "};\n"
-      for (device <- site(ExtraDevices)) {
+      res append (site(ExtraDevices).makeConfigString(addrMap))
-        if (device.hasMMIOPort) {
-          val deviceName = device.addrMapEntry.name
-          val deviceRegion = addrMap("io:ext:" + deviceName)
-          res.append(device.makeConfigString(deviceRegion))
-        }
-      }
       res append '\u0000'
       res.toString.getBytes
     }
@@ -123,22 +118,19 @@ class BasePlatformConfig extends Config (
       case NExtInterrupts => 2
       case AsyncDebugBus => false
       case AsyncMMIOChannels => false
-      case ExtraDevices => Nil
+      case ExtraDevices => new EmptyDeviceBlock
       case ExtraTopPorts => (p: Parameters) => new Bundle
       case ExtMMIOPorts => Nil
       case ExtIOAddrMapEntries =>
-        site(ExtraDevices)
+        site(ExtraDevices).addrMapEntries ++ site(ExtMMIOPorts)
-          .filter(_.hasMMIOPort)
-          .map(_.addrMapEntry) ++
-        site(ExtMMIOPorts)
       case NExtMMIOAXIChannels => 0
       case NExtMMIOAHBChannels => 0
       case NExtMMIOTLChannels  => 0
-      case ExportMMIOPort => (site(ExtraDevices).filter(_.hasMMIOPort).size + site(ExtMMIOPorts).size) > 0
+      case ExportMMIOPort => site(ExtraDevices).addrMapEntries.size > 0
       case AsyncBusChannels => false
       case NExtBusAXIChannels => 0
       case NExternalClients => (if (site(NExtBusAXIChannels) > 1) 1 else 0) +
-                                site(ExtraDevices).filter(_.hasClientPort).size
+                                site(ExtraDevices).nClientPorts
       case ConnectExtraPorts =>
         (out: Bundle, in: Bundle, p: Parameters) => out <> in
 
@@ -261,20 +253,19 @@ class TinyConfig extends Config(
 class WithTestRAM extends Config(
   (pname, site, here) => pname match {
     case ExtraDevices => {
-      class TestRAMDevice extends Device {
+      class TestRAMDevice extends DeviceBlock {
         val ramSize = 0x1000
-        def hasClientPort = false
+        def nClientPorts = 0
-        def hasMMIOPort = true
+        def addrMapEntries = Seq(
+          AddrMapEntry("testram", MemSize(ramSize, MemAttr(AddrMapProt.RW))))
         def builder(
-            mmioPort: Option[ClientUncachedTileLinkIO],
+            mmioPorts: HashMap[String, ClientUncachedTileLinkIO],
-            clientPort: Option[ClientUncachedTileLinkIO],
+            clientPorts: Seq[ClientUncachedTileLinkIO],
             extra: Bundle, p: Parameters) {
           val testram = Module(new TileLinkTestRAM(ramSize)(p))
-          testram.io <> mmioPort.get
+          testram.io <> mmioPorts("testram")
         }
-        override def addrMapEntry =
-          AddrMapEntry("testram", MemSize(ramSize, MemAttr(AddrMapProt.RW)))
       }
-      Seq(new TestRAMDevice)
+      new TestRAMDevice
     }
   })

src/main/scala/rocketchip/Devices.scala

@@ -3,45 +3,57 @@ package rocketchip
 import Chisel._
 import junctions._
 import uncore.tilelink._
+import scala.collection.immutable.HashMap
 import cde.{Parameters, Field}
 
 case object ExtraTopPorts extends Field[Parameters => Bundle]
-case object ExtraDevices extends Field[Seq[Device]]
+case object ExtraDevices extends Field[DeviceBlock]
+
+abstract class DeviceBlock {
+  /** How many client ports will the devices use */
+  def nClientPorts: Int
+  /** Address map entries for all of the devices */
+  def addrMapEntries: Seq[AddrMapEntry]
 
-abstract class Device {
-  /** Does this device have an MMIO port? */
-  def hasMMIOPort: Boolean
-  /** Does this device have a client port? */
-  def hasClientPort: Boolean
   /**
-   * This function elaborates the hardware for the device and connects
+   * The function that elaborates all the extra devices and connects them
-   * it to the mmio port, client port, and extra top-level ports.
+   * to the TileLink ports and extra top-level ports.
    *
-   * @param mmioPort The port from the MMIO network that goes to this device.
+   * @param mmioPorts A hashmap for the mmio ports.
-   *  If hasMMIOPort is false, this will be None
+   *    Use the names specified in addrMapEntries to get
-   *
+   *    the mmio port for each device.
-   * @param clientPort The client port provided for this device to make
+   * @param clientPorts All the client ports available for the devices
-   *  requests to the memory system. If hasClientPort is false, this will be None
+   * @param extra The extra top-level IO bundle
+   * @param p The CDE parameters for the devices
    */
   def builder(
-    mmioPort: Option[ClientUncachedTileLinkIO],
+    mmioPorts: HashMap[String, ClientUncachedTileLinkIO],
-    clientPort: Option[ClientUncachedTileLinkIO],
+    clientPorts: Seq[ClientUncachedTileLinkIO],
     extra: Bundle, p: Parameters): Unit
-  /**
-   * The entry that will be placed into the address map for this device.
-   * If hasMMIOPort is false, you do not need to override this
-   */
-  def addrMapEntry: AddrMapEntry =
-    throw new UnsupportedOperationException("no addrMapEntry defined")
 
   /**
    * Create the config string entry for this device that goes into the
    * Boot ROM. You generally won't need to override this
+   *
+   * @param fullAddrMap The full global address map
    */
-  def makeConfigString(region: MemRegion): String = {
+  def makeConfigString(fullAddrMap: AddrMap): String = {
-    s"${addrMapEntry.name} {\n" +
+    addrMapEntries.map { entry =>
+      val region = fullAddrMap("io:ext:" + entry.name)
+      s"${entry.name} {\n" +
       s"  addr 0x${region.start.toString(16)};\n" +
       s"  size 0x${region.size.toString(16)}; \n" +
        "}\n"
+    }.mkString
   }
 }
+
+class EmptyDeviceBlock extends DeviceBlock {
+  def nClientPorts = 0
+  def addrMapEntries = Seq.empty
+
+  def builder(
+      mmioPorts: HashMap[String, ClientUncachedTileLinkIO],
+      clientPorts: Seq[ClientUncachedTileLinkIO],
+      extra: Bundle, p: Parameters) {}
+}

src/main/scala/rocketchip/RocketChip.scala

@@ -12,6 +12,7 @@ import uncore.converters._
 import uncore.coherence.{InnerTLId, OuterTLId}
 import rocket._
 import coreplex._
+import scala.collection.immutable.HashMap
 
 /** Top-level parameters of RocketChip, values set in e.g. PublicConfigs.scala */
 
@@ -193,15 +194,12 @@ class Periphery(implicit val p: Parameters) extends Module
     val extra = p(ExtraTopPorts)(p)
   }
 
-  var client_ind = 0
-
   if (io.bus_axi.size > 0) {
     val conv = Module(new TileLinkIONastiIOConverter)
     val arb = Module(new NastiArbiter(io.bus_axi.size))
     arb.io.master <> io.bus_axi
     conv.io.nasti <> conv.io.tl
     io.clients_out.head <> conv.io.tl
-    client_ind += 1
   }
 
   def connectExternalMMIO(ports: Seq[ClientUncachedTileLinkIO])(implicit p: Parameters) {
@@ -234,22 +232,20 @@ class Periphery(implicit val p: Parameters) extends Module
     val mmioNetwork = Module(new TileLinkRecursiveInterconnect(1, extAddrMap))
     mmioNetwork.io.in.head <> io.mmio_in.get
 
-    for (device <- p(ExtraDevices)) {
+    val extraDevices = p(ExtraDevices)
-      val mmioPort = if (device.hasMMIOPort) 
-        Some(mmioNetwork.port(device.addrMapEntry.name)) else None
 
-      val clientPort = if (device.hasClientPort) {
+    val deviceMMIO = HashMap.newBuilder[String, ClientUncachedTileLinkIO]
-        client_ind += 1
+    for ((entry, i) <- extraDevices.addrMapEntries.zipWithIndex)
-        Some(io.clients_out(client_ind - 1))
+      deviceMMIO += (entry.name -> mmioNetwork.port(entry.name))
-      } else None
+
+    val deviceClients = if (io.bus_axi.size > 0) io.clients_out.tail else io.clients_out
 
     val buildParams = p.alterPartial({
       case InnerTLId => "L2toMMIO" // Device MMIO port
       case OuterTLId => "L1toL2"   // Device client port
     })
 
-      device.builder(mmioPort, clientPort, io.extra, buildParams)
+    extraDevices.builder(deviceMMIO.result(), deviceClients, io.extra, buildParams)
-    }
 
     val ext = p(ExtMMIOPorts).map(
       port => TileLinkWidthAdapter(mmioNetwork.port(port.name), "MMIO_Outermost"))

src/main/scala/rocketchip/TestConfigs.scala

@@ -11,6 +11,7 @@ import uncore.unittests._
 import junctions._
 import junctions.unittests._
 import scala.collection.mutable.LinkedHashSet
+import scala.collection.immutable.HashMap
 import cde.{Parameters, Config, Dump, Knob, CDEMatchError}
 import scala.math.max
 import coreplex._
@@ -183,21 +184,20 @@ class WithBusMasterTest extends Config(
     case BuildGroundTest =>
       (p: Parameters) => Module(new BusMasterTest()(p))
     case ExtraDevices => {
-      class BusMasterDevice extends Device {
+      class BusMasterDevice extends DeviceBlock {
-        def hasClientPort = true
+        def nClientPorts = 1
-        def hasMMIOPort = true
+        def addrMapEntries = Seq(
+          AddrMapEntry("busmaster", MemSize(4096, MemAttr(AddrMapProt.RW))))
         def builder(
-            mmioPort: Option[ClientUncachedTileLinkIO],
+            mmioPorts: HashMap[String, ClientUncachedTileLinkIO],
-            clientPort: Option[ClientUncachedTileLinkIO],
+            clientPorts: Seq[ClientUncachedTileLinkIO],
             extra: Bundle, p: Parameters) {
           val busmaster = Module(new ExampleBusMaster()(p))
-          busmaster.io.mmio <> mmioPort.get
+          busmaster.io.mmio <> mmioPorts("busmaster")
-          clientPort.get <> busmaster.io.mem
+          clientPorts.head <> busmaster.io.mem
         }
-        override def addrMapEntry =
-          AddrMapEntry("busmaster", MemSize(4096, MemAttr(AddrMapProt.RW)))
       }
-      Seq(new BusMasterDevice)
+      new BusMasterDevice
     }
     case _ => throw new CDEMatchError
   })