Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HasTilesModuleImp should not extends HasPeripheryDebugModuleImp #3276

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/main/scala/devices/debug/Periphery.scala
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,16 @@ class ResetCtrlIO(val nComponents: Int)(implicit val p: Parameters) extends Bund
*/

trait HasPeripheryDebug { this: BaseSubsystem =>
private val tlbus = locateTLBusWrapper(p(ExportDebug).slaveWhere)
def tlbus = locateTLBusWrapper(p(ExportDebug).slaveWhere)

val debugCustomXbarOpt = p(DebugModuleKey).map(params => LazyModule( new DebugCustomXbar(outputRequiresInput = false)))
val apbDebugNodeOpt = p(ExportDebug).apb.option(APBMasterNode(Seq(APBMasterPortParameters(Seq(APBMasterParameters("debugAPB"))))))
lazy val debugCustomXbarOpt = p(DebugModuleKey).map(params => LazyModule( new DebugCustomXbar(outputRequiresInput = false)))
lazy val apbDebugNodeOpt = p(ExportDebug).apb.option(APBMasterNode(Seq(APBMasterPortParameters(Seq(APBMasterParameters("debugAPB"))))))
val debugTLDomainOpt = p(DebugModuleKey).map { _ =>
val domain = ClockSinkNode(Seq(ClockSinkParameters()))
domain := tlbus.fixedClockNode
domain
}
val debugOpt = p(DebugModuleKey).map { params =>
lazy val debugOpt = p(DebugModuleKey).map { params =>
Comment on lines +76 to +83
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lazy is necessary for avoid null pointer issue.

val tlDM = LazyModule(new TLDebugModule(tlbus.beatBytes))

tlDM.node := tlbus.coupleTo("debug"){ TLFragmenter(tlbus) := _ }
Expand Down
11 changes: 7 additions & 4 deletions src/main/scala/groundtest/GroundTestSubsystem.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,18 @@
package freechips.rocketchip.groundtest

import chisel3._
import org.chipsalliance.cde.config.{Parameters}
import freechips.rocketchip.devices.debug.{HasPeripheryDebug, HasPeripheryDebugModuleImp}
import org.chipsalliance.cde.config.Parameters
import freechips.rocketchip.diplomacy.{AddressSet, LazyModule}
import freechips.rocketchip.interrupts.{IntSinkNode, IntSinkPortSimple}
import freechips.rocketchip.subsystem.{BaseSubsystem, BaseSubsystemModuleImp, HasTiles, CanHaveMasterAXI4MemPort}
import freechips.rocketchip.tilelink.{TLRAM, TLFragmenter}
import freechips.rocketchip.subsystem.{BaseSubsystem, BaseSubsystemModuleImp, CanHaveMasterAXI4MemPort, HasTiles}
import freechips.rocketchip.tilelink.{TLFragmenter, TLRAM}

class GroundTestSubsystem(implicit p: Parameters)
extends BaseSubsystem
with HasTiles
with CanHaveMasterAXI4MemPort
with HasPeripheryDebug
{
val testram = LazyModule(new TLRAM(AddressSet(0x52000000, 0xfff), beatBytes=pbus.beatBytes))
pbus.coupleTo("TestRAM") { testram.node := TLFragmenter(pbus) := _ }
Expand All @@ -28,7 +30,8 @@ class GroundTestSubsystem(implicit p: Parameters)
override lazy val module = new GroundTestSubsystemModuleImp(this)
}

class GroundTestSubsystemModuleImp[+L <: GroundTestSubsystem](_outer: L) extends BaseSubsystemModuleImp(_outer) {
class GroundTestSubsystemModuleImp[+L <: GroundTestSubsystem](_outer: L) extends BaseSubsystemModuleImp(_outer)
with HasPeripheryDebugModuleImp {
val success = IO(Output(Bool()))
val status = dontTouch(DebugCombiner(outer.tileStatusNodes.map(_.bundle)))
success := outer.tileCeaseSinkNode.in.head._1.asUInt.andR
Expand Down
19 changes: 11 additions & 8 deletions src/main/scala/subsystem/HasTiles.scala
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ case class TileSlavePortParams(
trait HasTileInterruptSources
extends CanHavePeripheryPLIC
with CanHavePeripheryCLINT
with HasPeripheryDebug
with InstantiatesTiles
{ this: BaseSubsystem => // TODO ideally this bound would be softened to LazyModule
/** meipNode is used to create a single bit subsystem input in Configs without a PLIC */
Expand Down Expand Up @@ -134,7 +133,7 @@ trait HasTileInterruptSources
}

/** These are sources of "constants" that are driven into the tile.
*
*
* While they are not expected to change dyanmically while the tile is executing code,
* they may be either tied to a contant value or programmed during boot or reset.
* They need to be instantiated before tiles are attached within the subsystem containing them.
Expand Down Expand Up @@ -291,10 +290,14 @@ trait CanAttachTile {
// we stub out missing interrupts with constant sources here.

// 1. Debug interrupt is definitely asynchronous in all cases.
domain.tile.intInwardNode :=
context.debugOpt
.map { domain { IntSyncAsyncCrossingSink(3) } := _.intnode }
.getOrElse { NullIntSource() }
context match {
case c: HasPeripheryDebug =>
domain.tile.intInwardNode :=
c.debugOpt
.map { domain { IntSyncAsyncCrossingSink(3) } := _.intnode }
.getOrElse { NullIntSource() }
case _ =>
}

// 2. The CLINT and PLIC output interrupts are synchronous to the TileLink bus clock,
// so might need to be synchronized depending on the Tile's crossing type.
Expand Down Expand Up @@ -395,7 +398,7 @@ case class CloneTileAttachParams(
},
instantiatedTiles(sourceHart).asInstanceOf[TilePRCIDomain[TileType]]
)
tile_prci_domain
tile_prci_domain
}
}

Expand Down Expand Up @@ -440,7 +443,7 @@ trait HasTiles extends InstantiatesTiles with HasCoreMonitorBundles with Default
}

/** Provides some Chisel connectivity to certain tile IOs */
trait HasTilesModuleImp extends LazyModuleImp with HasPeripheryDebugModuleImp {
trait HasTilesModuleImp extends LazyModuleImp {
val outer: HasTiles with HasTileInterruptSources with HasTileInputConstants

val reset_vector = outer.tileResetVectorIONodes.zipWithIndex.map { case (n, i) => n.makeIO(s"reset_vector_$i") }
Expand Down
6 changes: 4 additions & 2 deletions src/main/scala/subsystem/RocketSubsystem.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@

package freechips.rocketchip.subsystem

import freechips.rocketchip.devices.debug.{HasPeripheryDebug, HasPeripheryDebugModuleImp}
import org.chipsalliance.cde.config.Parameters
import freechips.rocketchip.diplomacy._
import freechips.rocketchip.prci.{ResetCrossingType, NoResetCrossing}
import freechips.rocketchip.prci.{NoResetCrossing, ResetCrossingType}
import freechips.rocketchip.tile._

case class RocketCrossingParams(
Expand All @@ -29,9 +30,10 @@ trait HasRocketTiles extends HasTiles { this: BaseSubsystem =>
}).toList
}

class RocketSubsystem(implicit p: Parameters) extends BaseSubsystem with HasRocketTiles {
class RocketSubsystem(implicit p: Parameters) extends BaseSubsystem with HasRocketTiles with HasPeripheryDebug {
override lazy val module = new RocketSubsystemModuleImp(this)
}

class RocketSubsystemModuleImp[+L <: RocketSubsystem](_outer: L) extends BaseSubsystemModuleImp(_outer)
with HasTilesModuleImp
with HasPeripheryDebugModuleImp
3 changes: 3 additions & 0 deletions src/main/scala/system/ExampleRocketSystem.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

package freechips.rocketchip.system

import freechips.rocketchip.devices.debug.{HasPeripheryDebug, HasPeripheryDebugModuleImp}
import org.chipsalliance.cde.config.Parameters
import freechips.rocketchip.subsystem._
import freechips.rocketchip.devices.tilelink._
Expand All @@ -13,6 +14,7 @@ class ExampleRocketSystem(implicit p: Parameters) extends RocketSubsystem
with CanHaveMasterAXI4MemPort
with CanHaveMasterAXI4MMIOPort
with CanHaveSlaveAXI4Port
with HasPeripheryDebug
{
// optionally add ROM devices
// Note that setting BootROMLocated will override the reset_vector for all tiles
Expand All @@ -26,3 +28,4 @@ class ExampleRocketSystemModuleImp[+L <: ExampleRocketSystem](_outer: L) extends
with HasRTCModuleImp
with HasExtInterruptsModuleImp
with DontTouch
with HasPeripheryDebugModuleImp