From 77a0f76289bf82c8356916e5a64f2dc56ae8d47b Mon Sep 17 00:00:00 2001 From: mwachs5 Date: Mon, 26 Sep 2016 11:10:27 -0700 Subject: [PATCH] Cleanup jtag dtm (#342) * debug: Clean up Debug TransportModule synchronizer With async reset async queues, I feel its safe/cleaner to remove the one-off "AsyncMailbox verilog black-box and use the common primitive. I also added some comments about correct usage of this block. Probably the 'TRST' signal should be renamed to make it less confusing, as it requires some processing of the real JTAG 'TRST' signal. --- regression/Makefile | 4 +- .../scala/rocketchip/DebugTransport.scala | 68 +++----- src/main/scala/uncore/devices/Debug.scala | 2 +- vsim/Makefrag | 1 - vsrc/AsyncMailbox.v | 154 ------------------ 5 files changed, 23 insertions(+), 206 deletions(-) delete mode 100644 vsrc/AsyncMailbox.v diff --git a/regression/Makefile b/regression/Makefile index b960cb7a..859939ec 100644 --- a/regression/Makefile +++ b/regression/Makefile @@ -222,7 +222,7 @@ JTAG_DTM_TEST ?= SimpleRegisterTest.test_s0 stamps/%/jtag-dtm-32-$(JTAG_DTM_TEST).stamp: install_openocd stamps/%/vsim-ndebug.stamp $(abspath $(TOP))/riscv-tools/riscv-tests/debug/gdbserver.py \ - --run $(abspath $(TOP))/vsim/simv-TestHarness-$* \ + --run $(abspath $(TOP))/vsim/simv-$(PROJECT)-$* \ --cmd="$(OPENOCD_DIR)/bin/openocd \ --s $(OPENOCD_DIR)/share/openocd/scripts" \ --freedom-e300-sim \ @@ -231,7 +231,7 @@ stamps/%/jtag-dtm-32-$(JTAG_DTM_TEST).stamp: install_openocd stamps/%/vsim-ndebu stamps/%/jtag-dtm-64-$(JTAG_DTM_TEST).stamp: install_openocd stamps/%/vsim-ndebug.stamp $(abspath $(TOP))/riscv-tools/riscv-tests/debug/gdbserver.py \ - --run $(abspath $(TOP))/vsim/simv-TestHarness-$* \ + --run $(abspath $(TOP))/vsim/simv-$(PROJECT)-$* \ --cmd="$(OPENOCD_INSTALL)_$(OPENOCD_VERSION)/bin/openocd \ --s $(OPENOCD_INSTALL)_$(OPENOCD_VERSION)/share/openocd/scripts" \ --freedom-u500-sim \ diff --git a/src/main/scala/rocketchip/DebugTransport.scala b/src/main/scala/rocketchip/DebugTransport.scala index 1371a16d..e0d557e2 100644 --- a/src/main/scala/rocketchip/DebugTransport.scala +++ b/src/main/scala/rocketchip/DebugTransport.scala @@ -13,29 +13,34 @@ case object IncludeJtagDTM extends Field[Boolean] * This implements JTAG interface described * in the RISC-V Debug Specification * - * This Module is currently a - * wrapper around a number of black-boxed - * modules. The black-boxing is due to the fact that + * This Module is currently a + * wrapper around a JTAG implementation + * of the Debug Transport Module. + * This is black-boxed because * Chisel doesn't currently support: * - Negative Edge Clocking * - Asynchronous Resets * (The tristate requirements of JTAG are exported from the * Chisel domain with the DRV_TDO signal). - * - * The AsyncDebugBus parameter here is overloaded. - * The DebugTransportModule JTAG definately needs a synchronizer, - * the parameter just currently selects whether the Chisel-generated - * crossing is used or the black-boxed crossing is used. - * Although Top is also capable of generating the - * Chisel sychnronizers, it is done here for consistency - * of keeping the synchronizers in one place when - * instantiating the DTM. + * + * The 'TRST' input is used to asynchronously + * reset the Debug Transport Module and the + * DTM side of the synchronizer. + * This design requires that TRST be + * synchronized to TCK (for de-assert) outside + * of this module. Your top level code should ensure + * that TRST is asserted before the rocket-chip core + * comes out of reset. + * Note that TRST is an optional + * part of the JTAG protocol, but it is not + * optional for interfacing with this logic. * */ -class JtagDTMWithSync(implicit val p: Parameters) extends Module { +class JtagDTMWithSync(depth: Int = 1, sync: Int = 3)(implicit val p: Parameters) + extends Module { - // IO <-> [Chisel Sync?] <-> [DebugBusIO<->UInt] <-> [Black Box Sync?] <-> DTM Black Box + // io.DebugBusIO <-> Sync <-> DebugBusIO <-> UInt <-> DTM Black Box val io = new Bundle { @@ -56,14 +61,7 @@ class JtagDTMWithSync(implicit val p: Parameters) extends Module { val io_debug_bus = Wire (new DebugBusIO) - // Optionally instantiate the Chisel synchronizers. - // These go on this side of the DebugBusIO->UInt translation - // because the Chisel synchronizers understand these data structures. - if (p(AsyncDebugBus)){ - io.debug <> AsyncDebugBusFrom(io.jtag.TCK, io.jtag.TRST, io_debug_bus) - } else { - io.debug <> io_debug_bus - } + io.debug <> AsyncDebugBusFrom(io.jtag.TCK, io.jtag.TRST, io_debug_bus, depth, sync) // Translate from straight 'bits' interface of the blackboxes // into the Resp/Req data structures. @@ -74,32 +72,6 @@ class JtagDTMWithSync(implicit val p: Parameters) extends Module { dtm_resp.valid := io_debug_bus.resp.valid dtm_resp.bits := io_debug_bus.resp.bits.asUInt io_debug_bus.resp.ready := dtm_resp.ready - - // Optionally instantiate the black-box synchronizers - // instead of the chisel ones. - // These go on this side of the DebugBusIO->UInt translation - // because they do not understand the DebugBusIO data structures. - - if (p(AsyncDebugBus)) { - dtm_req <> jtag_dtm.io.dtm_req - jtag_dtm.io.dtm_resp <> dtm_resp - } else { - val req_sync = Module (new AsyncMailbox()) - val resp_sync = Module (new AsyncMailbox()) - req_sync.io.enq := jtag_dtm.io.dtm_req - req_sync.io.enq_clock := io.jtag.TCK - req_sync.io.enq_reset := io.jtag.TRST - req_sync.io.deq_clock := clock - req_sync.io.deq_reset := reset - dtm_req := req_sync.io.deq - - jtag_dtm.io.dtm_resp := resp_sync.io.deq - resp_sync.io.deq_clock := io.jtag.TCK - resp_sync.io.deq_reset := io.jtag.TRST - resp_sync.io.enq_clock := clock - resp_sync.io.enq_reset := reset - resp_sync.io.enq := dtm_resp - } } class DebugTransportModuleJtag(reqSize : Int, respSize : Int)(implicit val p: Parameters) extends BlackBox { diff --git a/src/main/scala/uncore/devices/Debug.scala b/src/main/scala/uncore/devices/Debug.scala index 28e7d9bf..c6aaeb09 100644 --- a/src/main/scala/uncore/devices/Debug.scala +++ b/src/main/scala/uncore/devices/Debug.scala @@ -985,7 +985,7 @@ class DebugModule ()(implicit val p:cde.Parameters) object AsyncDebugBusCrossing { // takes from_source from the 'from' clock domain to the 'to' clock domain - def apply(from_clock: Clock, from_reset: Bool, from_source: DebugBusIO, to_clock: Clock, to_reset: Bool, depth: Int = 3, sync: Int = 2) = { + def apply(from_clock: Clock, from_reset: Bool, from_source: DebugBusIO, to_clock: Clock, to_reset: Bool, depth: Int = 1, sync: Int = 3) = { val to_sink = Wire(new DebugBusIO()(from_source.p)) to_sink.req <> AsyncDecoupledCrossing(from_clock, from_reset, from_source.req, to_clock, to_reset, depth, sync) from_source.resp <> AsyncDecoupledCrossing(to_clock, to_reset, to_sink.resp, from_clock, from_reset, depth, sync) diff --git a/vsim/Makefrag b/vsim/Makefrag index a6e04f2f..1463d5f4 100644 --- a/vsim/Makefrag +++ b/vsim/Makefrag @@ -6,7 +6,6 @@ bb_vsrcs = $(base_dir)/vsrc/DebugTransportModuleJtag.v \ $(base_dir)/vsrc/jtag_vpi.v \ - $(base_dir)/vsrc/AsyncMailbox.v \ $(base_dir)/vsrc/AsyncResetReg.v \ $(base_dir)/vsrc/AsyncSetReg.v \ diff --git a/vsrc/AsyncMailbox.v b/vsrc/AsyncMailbox.v deleted file mode 100644 index 4460e9bf..00000000 --- a/vsrc/AsyncMailbox.v +++ /dev/null @@ -1,154 +0,0 @@ - - -module AsyncMailbox ( - // Write Interface - - enq_clock - , enq_reset - , enq_ready - , enq_valid - , enq_bits - - // Read Interface - , deq_clock - , deq_reset - , deq_ready - , deq_valid - , deq_bits - - ); - - - //-------------------------------------------------------- - // Parameter Declarations - - parameter WIDTH = 64; - - //-------------------------------------------------------- - // I/O Declarations - - // Write Interface - input enq_clock; - wire w_clock = enq_clock; - - input enq_reset; - wire w_reset = enq_reset; - - output enq_ready; - wire w_ready; - assign enq_ready = w_ready; - - input enq_valid; - wire w_valid = enq_valid; - - input [WIDTH - 1 : 0 ] enq_bits; - wire [WIDTH - 1 : 0 ] w_bits = enq_bits; - - // Read Interface - input deq_clock; - wire r_clock = deq_clock; - - input deq_reset; - wire r_reset = deq_reset; - - output deq_valid; - wire r_valid; - assign deq_valid = r_valid; - - input deq_ready; - wire r_ready = deq_ready; - - output [WIDTH - 1 : 0] deq_bits; - wire [WIDTH - 1 : 0] r_bits; - assign deq_bits = r_bits; - - //-------------------------------------------------------- - // FIFO Memory Declaration - - reg [WIDTH - 1 :0] mailboxReg; - - //-------------------------------------------------------- - // Reg and Wire Declarations - - wire w_full; - wire w_fire; - - wire r_empty; - wire r_fire; - - - // Read & Write Address Pointers - reg w_wrAddrReg; - wire w_wrAddrNxt; - reg r_rdAddrReg; - wire r_rdAddrNxt; - - - reg wrAddrReg_sync; - reg rdAddrReg_sync; - - reg r_wrAddrReg; - reg w_rdAddrReg; - - //-------------------------------------------------------- - // Reg and Wire Declarations - - assign w_full = ~(w_wrAddrReg == r_rdAddrReg); - - assign w_wrAddrNxt = ~w_wrAddrReg ; - - assign r_rdAddrNxt = ~r_rdAddrReg; - - assign r_empty = (r_wrAddrReg == r_rdAddrReg); - - assign w_ready = ~w_full; - assign w_fire = w_ready & w_valid; - - // Read Logic - assign r_valid = ~r_empty; - assign r_fire = r_ready & r_valid; - - - assign r_bits = mailboxReg; - - always @(posedge w_clock) begin - if (w_fire) begin - mailboxReg <= w_bits; - end - end - - //-------------------------------------------------------- - // Sequential logic - // - - always @(posedge w_clock or posedge w_reset) begin - if (w_reset ) begin - w_wrAddrReg <= 1'b0; - - rdAddrReg_sync <= 1'b0; - w_rdAddrReg <= 1'b0; - end else begin - if (w_fire) begin - w_wrAddrReg <= w_wrAddrNxt; - end - rdAddrReg_sync <= r_rdAddrReg; - w_rdAddrReg <= rdAddrReg_sync; - end - end - - always @(posedge r_clock or posedge r_reset) begin - if (r_reset) begin - r_rdAddrReg <= 1'b0; - - wrAddrReg_sync <= 1'b0; - r_wrAddrReg <= 1'b0; - end else begin - if (r_fire) begin - r_rdAddrReg <= r_rdAddrNxt; - end - wrAddrReg_sync <= w_wrAddrReg; - r_wrAddrReg <= wrAddrReg_sync; - end - end // always @ (posedge r_clock) - -endmodule // AsyncMailbox