[pinmux_aon,rtl] Improve JTAG Muxing

The three jtag request (lc_jtag, dft_jtag, rv_jtag) are now gated with
an AND-gate instead of a combinatorial process. This eliminates the risk
of a glitch on the jtag clock or reset line.

Signed-off-by: Michael Gautschi <mgautschi@lowrisc.org>

Author: gautschimi

hw/ip_templates/pinmux/lint/pinmux.waiver.tpl

@@ -23,6 +23,9 @@ waive -rules RESET_MUX -location {pinmux_strap_sampling.sv} -regexp {Asynchronou
 waive -rules {CLOCK_DRIVER CLOCK_MUX} -location {pinmux_strap_sampling.sv} -regexp {'(lc|rv)_jtag_req.tck' is driven( by a multiplexer)? here,( and)? used as a clock 'tck_i' at dmi_jtag_tap.sv} ${"\\"}
       -comment "These signals are muxed using the JTAG Selection Mux."
 
+waive -rules {CLOCK_DRIVER CLOCK_MUX} -location {pinmux_jtag_buf.sv} -regexp {'tck_gated' is driven by instance 'u_prim_and2_tck' of module 'prim_and2', and used as a clock 'tck_i' at dmi_jtag_tap.sv} ${"\\"}
+      -comment "TCK is gated per TAP by a hand-instantiated prim_and2 cell inside pinmux_jtag_buf, enabled by the TAP Selection Mux, before being re-buffered onto req_o.tck by prim_clock_buf."
+
 waive -rules CLOCK_MUX -location {pinmux_strap_sampling.sv pinmux.sv} -regexp {Clock '(in_padring_i\[38\]|mio_in_i\[38\]|jtag_req.tck)' reaches a multiplexer here, used as a clock 'tck_i' at dmi_jtag_tap.sv} ${"\\"}
       -comment "The 'mio_in_i[TckPadIdx]' input signal is connected to 'jtag_req.tck' which eventually feeds into the JTAG Selection Mux."
 % else:

hw/ip_templates/pinmux/pinmux.core.tpl

@@ -12,6 +12,7 @@ filesets:
       - lowrisc:prim:all
       - lowrisc:prim:clock_buf
       - lowrisc:prim:buf
+      - lowrisc:prim:and2
       - lowrisc:prim:lc_dec
       - lowrisc:prim:lc_sync
       - lowrisc:prim:lc_sender

hw/ip_templates/pinmux/rtl/pinmux_jtag_buf.sv

@@ -3,18 +3,29 @@
 // SPDX-License-Identifier: Apache-2.0
 
 module pinmux_jtag_buf (
+  // Gates TCK/TRSTN below. Holds this TAP's clock off and reset asserted when deselected.
+  input  logic                en_i,
   input  jtag_pkg::jtag_req_t req_i,
   output jtag_pkg::jtag_req_t req_o,
   input  jtag_pkg::jtag_rsp_t rsp_i,
   output jtag_pkg::jtag_rsp_t rsp_o
 );
 
+  logic tck_gated;
+  prim_and2 #(.Width(1)) u_prim_and2_tck (
+    .in0_i(en_i),
+    .in1_i(req_i.tck),
+    .out_o(tck_gated)
+  );
+  // This buffer is used in the constraints to define a generated clock
   prim_clock_buf prim_clock_buf_tck (
-    .clk_i(req_i.tck),
+    .clk_i(tck_gated),
     .clk_o(req_o.tck)
   );
-  prim_buf prim_buf_trst_n (
-    .in_i (req_i.trst_n),
+
+  prim_and2 #(.Width(1)) u_prim_and2_trst_n (
+    .in0_i(en_i),
+    .in1_i(req_i.trst_n),
     .out_o(req_o.trst_n)
   );
   prim_buf prim_buf_tms (

hw/ip_templates/pinmux/rtl/pinmux_strap_sampling.sv.tpl

@@ -331,34 +331,83 @@ module pinmux_strap_sampling
   assign tap_strap = tap_strap_t'(tap_strap_q);
   `ASSERT_KNOWN(TapStrapKnown_A, tap_strap)
 
+  // lc_tap_en_d/rv_tap_en_d/dft_tap_en_d are decoded from tap_strap in p_tap_mux below (qualified
+  // by the live pinmux_hw_debug_en/lc_dft_en signals for the RV_DM and DFT taps) and registered
+  // here into lc_tap_en_q/rv_tap_en_q/dft_tap_en_q. The registered, single-bit enables are what
+  // actually gate the JTAG request lines further down, so those live lines are never a direct
+  // combinational function of the 2-bit tap_strap register. tap_strap_q's two bits are separate
+  // flops that are not guaranteed to switch at exactly the same instant post-synthesis, which
+  // could otherwise glitch the JTAG request lines through an unintended TAP selection for a cycle.
+  logic lc_tap_en_d,  lc_tap_en_q;
+  logic rv_tap_en_d,  rv_tap_en_q;
+  logic dft_tap_en_d, dft_tap_en_q;
+
+  always_ff @(posedge clk_i or negedge rst_ni) begin : p_tap_sel_reg
+    if (!rst_ni) begin
+      lc_tap_en_q  <= 1'b0;
+      rv_tap_en_q  <= 1'b0;
+      dft_tap_en_q <= 1'b0;
+    end else begin
+      lc_tap_en_q  <= lc_tap_en_d;
+      rv_tap_en_q  <= rv_tap_en_d;
+      dft_tap_en_q <= dft_tap_en_d;
+    end
+  end
+
+  // TCK and TRSTN are shared, ungated wires here: the actual gating with the registered,
+  // single-bit tap enables happens inside pinmux_jtag_buf below (en_i), which holds each TAP's
+  // clock off and reset asserted when it is not selected.
+  assign lc_jtag_req.tck     = jtag_req.tck;
+  assign lc_jtag_req.trst_n  = jtag_req.trst_n;
+  assign rv_jtag_req.tck     = jtag_req.tck;
+  assign rv_jtag_req.trst_n  = jtag_req.trst_n;
+  assign dft_jtag_req.tck    = jtag_req.tck;
+  assign dft_jtag_req.trst_n = jtag_req.trst_n;
+
+  // Drives the immediate (unregistered) jtag_rsp/jtag_en response mux used for the TDO override
+  // and pad tie-offs further below, and in the same case statement decodes the per-TAP request
+  // enables (lc_tap_en_d/rv_tap_en_d/dft_tap_en_d) that get registered above. TMS and TDI - the
+  // signals that actually determine what the selected TAP does with its data registers - are
+  // generated here too, directly following tap_strap like jtag_rsp.
   always_comb begin : p_tap_mux
-    jtag_rsp     = '0;
-    // Note that this holds the JTAGs in reset
-    // when they are not selected.
-    lc_jtag_req  = '0;
-    rv_jtag_req  = '0;
-    dft_jtag_req = '0;
+    jtag_rsp = '0;
     // This activates the TDO override further below.
-    jtag_en      = 1'b0;
+    jtag_en  = 1'b0;
+
+    lc_tap_en_d      = 1'b0;
+    rv_tap_en_d      = 1'b0;
+    dft_tap_en_d     = 1'b0;
+    lc_jtag_req.tms  = 1'b0;
+    lc_jtag_req.tdi  = 1'b0;
+    rv_jtag_req.tms  = 1'b0;
+    rv_jtag_req.tdi  = 1'b0;
+    dft_jtag_req.tms = 1'b0;
+    dft_jtag_req.tdi = 1'b0;
 
     unique case (tap_strap)
       LcTapSel: begin
-        lc_jtag_req = jtag_req;
-        jtag_rsp    = lc_jtag_rsp;
-        jtag_en     = 1'b1;
+        jtag_rsp        = lc_jtag_rsp;
+        lc_tap_en_d     = 1'b1;
+        jtag_en         = 1'b1;
+        lc_jtag_req.tms = jtag_req.tms;
+        lc_jtag_req.tdi = jtag_req.tdi;
       end
       RvTapSel: begin
         if (lc_tx_test_true_strict(pinmux_hw_debug_en[HwDebugEnTapSel])) begin
-          rv_jtag_req = jtag_req;
-          jtag_rsp    = rv_jtag_rsp;
-          jtag_en     = 1'b1;
+          jtag_rsp        = rv_jtag_rsp;
+          rv_tap_en_d     = 1'b1;
+          jtag_en         = 1'b1;
+          rv_jtag_req.tms = jtag_req.tms;
+          rv_jtag_req.tdi = jtag_req.tdi;
         end
       end
       DftTapSel: begin
         if (lc_tx_test_true_strict(lc_dft_en[DftEnTapSel])) begin
-          dft_jtag_req = jtag_req;
-          jtag_rsp     = dft_jtag_rsp;
-          jtag_en      = 1'b1;
+          jtag_rsp         = dft_jtag_rsp;
+          jtag_en          = 1'b1;
+          dft_tap_en_d     = 1'b1;
+          dft_jtag_req.tms = jtag_req.tms;
+          dft_jtag_req.tdi = jtag_req.tdi;
         end
       end
       default: ;
@@ -368,18 +417,21 @@ module pinmux_strap_sampling
   // Insert hand instantiated buffers for
   // these signals to prevent further optimization.
   pinmux_jtag_buf u_pinmux_jtag_buf_lc (
+    .en_i(lc_tap_en_q),
     .req_i(lc_jtag_req),
     .req_o(lc_jtag_o),
     .rsp_i(lc_jtag_i),
     .rsp_o(lc_jtag_rsp)
   );
   pinmux_jtag_buf u_pinmux_jtag_buf_rv (
+    .en_i(rv_tap_en_q),
     .req_i(rv_jtag_req),
     .req_o(rv_jtag_o),
     .rsp_i(rv_jtag_i),
     .rsp_o(rv_jtag_rsp)
   );
   pinmux_jtag_buf u_pinmux_jtag_buf_dft (
+    .en_i(dft_tap_en_q),
     .req_i(dft_jtag_req),
     .req_o(dft_jtag_o),
     .rsp_i(dft_jtag_i),

hw/top_darjeeling/ip_autogen/pinmux/pinmux.core

@@ -12,6 +12,7 @@ filesets:
       - lowrisc:prim:all
       - lowrisc:prim:clock_buf
       - lowrisc:prim:buf
+      - lowrisc:prim:and2
       - lowrisc:prim:lc_dec
       - lowrisc:prim:lc_sync
       - lowrisc:prim:lc_sender

hw/top_darjeeling/ip_autogen/pinmux/rtl/pinmux_jtag_buf.sv

@@ -3,18 +3,29 @@
 // SPDX-License-Identifier: Apache-2.0
 
 module pinmux_jtag_buf (
+  // Gates TCK/TRSTN below. Holds this TAP's clock off and reset asserted when deselected.
+  input  logic                en_i,
   input  jtag_pkg::jtag_req_t req_i,
   output jtag_pkg::jtag_req_t req_o,
   input  jtag_pkg::jtag_rsp_t rsp_i,
   output jtag_pkg::jtag_rsp_t rsp_o
 );
 
+  logic tck_gated;
+  prim_and2 #(.Width(1)) u_prim_and2_tck (
+    .in0_i(en_i),
+    .in1_i(req_i.tck),
+    .out_o(tck_gated)
+  );
+  // This buffer is used in the constraints to define a generated clock
   prim_clock_buf prim_clock_buf_tck (
-    .clk_i(req_i.tck),
+    .clk_i(tck_gated),
     .clk_o(req_o.tck)
   );
-  prim_buf prim_buf_trst_n (
-    .in_i (req_i.trst_n),
+
+  prim_and2 #(.Width(1)) u_prim_and2_trst_n (
+    .in0_i(en_i),
+    .in1_i(req_i.trst_n),
     .out_o(req_o.trst_n)
   );
   prim_buf prim_buf_tms (

hw/top_darjeeling/ip_autogen/pinmux/rtl/pinmux_strap_sampling.sv

@@ -325,34 +325,83 @@ module pinmux_strap_sampling
   assign tap_strap = tap_strap_t'(tap_strap_q);
   `ASSERT_KNOWN(TapStrapKnown_A, tap_strap)
 
+  // lc_tap_en_d/rv_tap_en_d/dft_tap_en_d are decoded from tap_strap in p_tap_mux below (qualified
+  // by the live pinmux_hw_debug_en/lc_dft_en signals for the RV_DM and DFT taps) and registered
+  // here into lc_tap_en_q/rv_tap_en_q/dft_tap_en_q. The registered, single-bit enables are what
+  // actually gate the JTAG request lines further down, so those live lines are never a direct
+  // combinational function of the 2-bit tap_strap register. tap_strap_q's two bits are separate
+  // flops that are not guaranteed to switch at exactly the same instant post-synthesis, which
+  // could otherwise glitch the JTAG request lines through an unintended TAP selection for a cycle.
+  logic lc_tap_en_d,  lc_tap_en_q;
+  logic rv_tap_en_d,  rv_tap_en_q;
+  logic dft_tap_en_d, dft_tap_en_q;
+
+  always_ff @(posedge clk_i or negedge rst_ni) begin : p_tap_sel_reg
+    if (!rst_ni) begin
+      lc_tap_en_q  <= 1'b0;
+      rv_tap_en_q  <= 1'b0;
+      dft_tap_en_q <= 1'b0;
+    end else begin
+      lc_tap_en_q  <= lc_tap_en_d;
+      rv_tap_en_q  <= rv_tap_en_d;
+      dft_tap_en_q <= dft_tap_en_d;
+    end
+  end
+
+  // TCK and TRSTN are shared, ungated wires here: the actual gating with the registered,
+  // single-bit tap enables happens inside pinmux_jtag_buf below (en_i), which holds each TAP's
+  // clock off and reset asserted when it is not selected.
+  assign lc_jtag_req.tck     = jtag_req.tck;
+  assign lc_jtag_req.trst_n  = jtag_req.trst_n;
+  assign rv_jtag_req.tck     = jtag_req.tck;
+  assign rv_jtag_req.trst_n  = jtag_req.trst_n;
+  assign dft_jtag_req.tck    = jtag_req.tck;
+  assign dft_jtag_req.trst_n = jtag_req.trst_n;
+
+  // Drives the immediate (unregistered) jtag_rsp/jtag_en response mux used for the TDO override
+  // and pad tie-offs further below, and in the same case statement decodes the per-TAP request
+  // enables (lc_tap_en_d/rv_tap_en_d/dft_tap_en_d) that get registered above. TMS and TDI - the
+  // signals that actually determine what the selected TAP does with its data registers - are
+  // generated here too, directly following tap_strap like jtag_rsp.
   always_comb begin : p_tap_mux
-    jtag_rsp     = '0;
-    // Note that this holds the JTAGs in reset
-    // when they are not selected.
-    lc_jtag_req  = '0;
-    rv_jtag_req  = '0;
-    dft_jtag_req = '0;
+    jtag_rsp = '0;
     // This activates the TDO override further below.
-    jtag_en      = 1'b0;
+    jtag_en  = 1'b0;
+
+    lc_tap_en_d      = 1'b0;
+    rv_tap_en_d      = 1'b0;
+    dft_tap_en_d     = 1'b0;
+    lc_jtag_req.tms  = 1'b0;
+    lc_jtag_req.tdi  = 1'b0;
+    rv_jtag_req.tms  = 1'b0;
+    rv_jtag_req.tdi  = 1'b0;
+    dft_jtag_req.tms = 1'b0;
+    dft_jtag_req.tdi = 1'b0;
 
     unique case (tap_strap)
       LcTapSel: begin
-        lc_jtag_req = jtag_req;
-        jtag_rsp    = lc_jtag_rsp;
-        jtag_en     = 1'b1;
+        jtag_rsp        = lc_jtag_rsp;
+        lc_tap_en_d     = 1'b1;
+        jtag_en         = 1'b1;
+        lc_jtag_req.tms = jtag_req.tms;
+        lc_jtag_req.tdi = jtag_req.tdi;
       end
       RvTapSel: begin
         if (lc_tx_test_true_strict(pinmux_hw_debug_en[HwDebugEnTapSel])) begin
-          rv_jtag_req = jtag_req;
-          jtag_rsp    = rv_jtag_rsp;
-          jtag_en     = 1'b1;
+          jtag_rsp        = rv_jtag_rsp;
+          rv_tap_en_d     = 1'b1;
+          jtag_en         = 1'b1;
+          rv_jtag_req.tms = jtag_req.tms;
+          rv_jtag_req.tdi = jtag_req.tdi;
         end
       end
       DftTapSel: begin
         if (lc_tx_test_true_strict(lc_dft_en[DftEnTapSel])) begin
-          dft_jtag_req = jtag_req;
-          jtag_rsp     = dft_jtag_rsp;
-          jtag_en      = 1'b1;
+          jtag_rsp         = dft_jtag_rsp;
+          jtag_en          = 1'b1;
+          dft_tap_en_d     = 1'b1;
+          dft_jtag_req.tms = jtag_req.tms;
+          dft_jtag_req.tdi = jtag_req.tdi;
         end
       end
       default: ;
@@ -362,18 +411,21 @@ module pinmux_strap_sampling
   // Insert hand instantiated buffers for
   // these signals to prevent further optimization.
   pinmux_jtag_buf u_pinmux_jtag_buf_lc (
+    .en_i(lc_tap_en_q),
     .req_i(lc_jtag_req),
     .req_o(lc_jtag_o),
     .rsp_i(lc_jtag_i),
     .rsp_o(lc_jtag_rsp)
   );
   pinmux_jtag_buf u_pinmux_jtag_buf_rv (
+    .en_i(rv_tap_en_q),
     .req_i(rv_jtag_req),
     .req_o(rv_jtag_o),
     .rsp_i(rv_jtag_i),
     .rsp_o(rv_jtag_rsp)
   );
   pinmux_jtag_buf u_pinmux_jtag_buf_dft (
+    .en_i(dft_tap_en_q),
     .req_i(dft_jtag_req),
     .req_o(dft_jtag_o),
     .rsp_i(dft_jtag_i),

hw/top_earlgrey/ip_autogen/pinmux/lint/pinmux.waiver

@@ -22,6 +22,9 @@ waive -rules RESET_MUX -location {pinmux_strap_sampling.sv} -regexp {Asynchronou
 waive -rules {CLOCK_DRIVER CLOCK_MUX} -location {pinmux_strap_sampling.sv} -regexp {'(lc|rv)_jtag_req.tck' is driven( by a multiplexer)? here,( and)? used as a clock 'tck_i' at dmi_jtag_tap.sv} \
       -comment "These signals are muxed using the JTAG Selection Mux."
 
+waive -rules {CLOCK_DRIVER CLOCK_MUX} -location {pinmux_jtag_buf.sv} -regexp {'tck_gated' is driven by instance 'u_prim_and2_tck' of module 'prim_and2', and used as a clock 'tck_i' at dmi_jtag_tap.sv} \
+      -comment "TCK is gated per TAP by a hand-instantiated prim_and2 cell inside pinmux_jtag_buf, enabled by the TAP Selection Mux, before being re-buffered onto req_o.tck by prim_clock_buf."
+
 waive -rules CLOCK_MUX -location {pinmux_strap_sampling.sv pinmux.sv} -regexp {Clock '(in_padring_i\[38\]|mio_in_i\[38\]|jtag_req.tck)' reaches a multiplexer here, used as a clock 'tck_i' at dmi_jtag_tap.sv} \
       -comment "The 'mio_in_i[TckPadIdx]' input signal is connected to 'jtag_req.tck' which eventually feeds into the JTAG Selection Mux."
 

hw/top_earlgrey/ip_autogen/pinmux/pinmux.core

@@ -12,6 +12,7 @@ filesets:
       - lowrisc:prim:all
       - lowrisc:prim:clock_buf
       - lowrisc:prim:buf
+      - lowrisc:prim:and2
       - lowrisc:prim:lc_dec
       - lowrisc:prim:lc_sync
       - lowrisc:prim:lc_sender

hw/top_earlgrey/ip_autogen/pinmux/rtl/pinmux_jtag_buf.sv

@@ -3,18 +3,29 @@
 // SPDX-License-Identifier: Apache-2.0
 
 module pinmux_jtag_buf (
+  // Gates TCK/TRSTN below. Holds this TAP's clock off and reset asserted when deselected.
+  input  logic                en_i,
   input  jtag_pkg::jtag_req_t req_i,
   output jtag_pkg::jtag_req_t req_o,
   input  jtag_pkg::jtag_rsp_t rsp_i,
   output jtag_pkg::jtag_rsp_t rsp_o
 );
 
+  logic tck_gated;
+  prim_and2 #(.Width(1)) u_prim_and2_tck (
+    .in0_i(en_i),
+    .in1_i(req_i.tck),
+    .out_o(tck_gated)
+  );
+  // This buffer is used in the constraints to define a generated clock
   prim_clock_buf prim_clock_buf_tck (
-    .clk_i(req_i.tck),
+    .clk_i(tck_gated),
     .clk_o(req_o.tck)
   );
-  prim_buf prim_buf_trst_n (
-    .in_i (req_i.trst_n),
+
+  prim_and2 #(.Width(1)) u_prim_and2_trst_n (
+    .in0_i(en_i),
+    .in1_i(req_i.trst_n),
     .out_o(req_o.trst_n)
   );
   prim_buf prim_buf_tms (