[pinmux/tops] Move JTAG mux logic into pinmux strap logic #5486

msfschaffner · 2021-03-06T01:59:11Z

This is a continuation of the pinmux alignments tracked in #5221.

In particular, this moves moves the temporary JTAG mux logic into the pinmux, and removes the jtag_mux module, which was more sort of a helper for Bronze.

This PR is still WIP - I will ping you once this is ready to review.

msfschaffner · 2021-03-10T01:40:25Z

hw/top_earlgrey/rtl/top_earlgrey_verilator.sv

+  // TODO: this is currently not supported.
+  // connect this to the correct pins once pinout is final and once the
+  // verilator testbench supports DFT/Debug strap sampling.
+  // See also #5221.


This is a temporary fix, and I believe it does not affect our CI at this point, since we're not using the JTAG DPI there.

The issue is that the JTAG signals are still mapped onto the SPI signals due to the FTDI chip being used on the nexysvideo board. So now that we move the JTAG mux into the pinmux, the verilator testbench does not have access to the JTAG signals at the top level anymore.

However, in the final ASIC pinout the JTAG will be mapped to a different location (just regular MIOs). Hence, once that new pinout is in, I will uncomment this and remap the JTAG signals appropriately.

Signed-off-by: Michael Schaffner <[email protected]>

tjaychen · 2021-03-10T22:22:51Z

hw/ip/pinmux/rtl/pinmux_strap_sampling.sv

@@ -119,6 +115,11 @@ module pinmux_strap_sampling
        continue_sampling_d = lc_ctrl_pkg::On;
      end
    end
+    // TODO: this can currently be overridden with a parameter for legacy reasons.


can you remind me why this has to be kept for now? is it because of fpga..?

yes this is because FPGA and Verilator do not know anything about the discrete strap sampling event.
eventually this should be removed.

i thought the discrete vs continuously sampling was mostly an internally controlled thing though

it is meant to be internally controlled, that's right - however the continuous sampling mode is currently only available in the TEST life cycle states, and only activates if the straps are nonzero upon the first sample event.

at the moment, the FPGA tests run in DEV state and switch between JTAG and SPI at runtime, which would not be compatible with the internally timed, discrete strap sampling that is enabled in DEV.

tjaychen · 2021-03-10T22:30:00Z

hw/ip/pinmux/rtl/pinmux_strap_sampling.sv

  assign tap_strap = tap_strap_t'(tap_strap_q);
  `ASSERT_KNOWN(TapStrapKnown_A, tap_strap)

  always_comb begin : p_tap_mux
    jtag_rsp     = '0;
+    // Note that this holds the JTAGs in reset


just to confirm, this is the area we discussed where after a change, we recommend the system level issue a jtag reset to make sure the switch didn't cause any issues right?

yeah that's right!

tjaychen · 2021-03-10T22:33:36Z

hw/ip/pinmux/rtl/pinmux_strap_sampling.sv

+        k == TargetCfg.trst_idx ||
+        k == TargetCfg.tdi_idx  ||
+        k == TargetCfg.tdo_idx) begin : gen_jtag_signal
+      assign in_core_o[k] = (jtag_en) ? TargetCfg.tie_offs[k] : in_padring_i[k];


is the idea here we want the pinmux side to see a fixed value when this pin is in use by jtag?
This is not a great example in this case..but you could technically imagine a case...where jtag might be used as a wakeup source (doesn't apply to us..so just humor me).

In that case, we probably would want the signal to feed through and actuate the downstream logic. Is there any harm in relying on downstream pinmux logic to just disable the input or maybe mux away? Maybe things are a bit more complicated if the backing pin is dedicated.

I dont think this is something that needs to be addressed in this PR yet, but might be worth filing an issue to look at waaaaaaaay later.

Indeed, the pinmux side is tied off to a fixed value in this case.
However, what you mention regarding the wakeup detectors is already possible.
I.e., the wakeup taps are taken directly from the pad inputs, hence the JTAG mux does not affect them.

ah excellent, if wakeups are handled, there really isn't much of a problem then.

tjaychen · 2021-03-10T22:34:56Z

hw/top_earlgrey/data/top_earlgrey.hjson

@@ -204,7 +204,17 @@
        },
      ],
    }
-
+    { name: "rv_dm",


ah! glad someone one is using the "host" structure!

we are only....a few PRs away from fully automating the host instantiation! probably another day...

tjaychen · 2021-03-10T22:37:02Z

hw/top_earlgrey/rtl/top_earlgrey_asic.sv

+  localparam logic [pinmux_pkg::NumIOs-1:0] TieOffValues = pinmux_pkg::NumIOs'(1'b1 << (
+      pinmux_reg_pkg::NMioPads + top_earlgrey_pkg::TopEarlgreyDioPinSpiDeviceCsb));
+
+  // DFT and Debug signal positions in the pinout.


do you think in the future it makes sense to generate this from the new pinmux parsing structure you had in mind? Maybe that's exactly where you're headed?

this is exactly where I am headed - I just need a way to pass this in in a manual way for now

tjaychen

LGTM with a few clarifying questions.

Signed-off-by: Michael Schaffner <[email protected]>

tjaychen · 2021-03-11T00:56:00Z

ahhh okay, the FPGA is in DEV state, that makes sense.

…

On Wed, Mar 10, 2021 at 4:51 PM Michael Schaffner ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In hw/ip/pinmux/rtl/pinmux_strap_sampling.sv <#5486 (comment)>: > @@ -119,6 +115,11 @@ module pinmux_strap_sampling continue_sampling_d = lc_ctrl_pkg::On; end end + // TODO: this can currently be overridden with a parameter for legacy reasons. it is meant to be internally controlled, that's right - however the continuous sampling mode is currently only available in the TEST life cycle states, and only activates if the straps are nonzero upon the first sample event. at the moment, the FPGA tests run in DEV state and switch between JTAG and SPI at runtime, which would not be compatible with the internally timed, discrete strap sampling that is enabled in DEV. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#5486 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAH2RSXHVF7R6IUISWV2LFDTDAAXZANCNFSM4YWGLG4Q> .

The pads of the dedicated IOs and the capture trigger got accidentally disconnected with lowRISC#5486. Signed-off-by: Pirmin Vogel <[email protected]>

The pads of the dedicated IOs and the capture trigger got accidentally disconnected with #5486. Signed-off-by: Pirmin Vogel <[email protected]>

msfschaffner added Component:DV DV issue: testbench, test case, etc. Component:FPGA FPGA related issues Component:RTL labels Mar 6, 2021

msfschaffner self-assigned this Mar 6, 2021

msfschaffner force-pushed the jtag-mux branch 2 times, most recently from 7ee4aff to e91f9b0 Compare March 6, 2021 02:03

msfschaffner mentioned this pull request Mar 6, 2021

[pinmux] Tracker for various modifications and updates for Silver #5221

Closed

17 tasks

msfschaffner force-pushed the jtag-mux branch 10 times, most recently from b8ce7cd to 5e05701 Compare March 10, 2021 01:27

msfschaffner requested review from tjaychen and imphil March 10, 2021 01:27

msfschaffner commented Mar 10, 2021

View reviewed changes

msfschaffner marked this pull request as ready for review March 10, 2021 01:41

msfschaffner requested review from eunchan, sriyerg and weicaiyang as code owners March 10, 2021 01:41

msfschaffner force-pushed the jtag-mux branch 2 times, most recently from 3a24baf to 8db9a89 Compare March 10, 2021 02:51

msfschaffner added 2 commits March 10, 2021 09:29

[pinmux] Move JTAG muxing into pinmux strap sample logic

112ac6a

Signed-off-by: Michael Schaffner <[email protected]>

[top] Align tops to use JTAG port from pinmux

27856b1

Signed-off-by: Michael Schaffner <[email protected]>

msfschaffner force-pushed the jtag-mux branch 2 times, most recently from b714afe to 0d319c9 Compare March 10, 2021 20:59

tjaychen reviewed Mar 10, 2021

View reviewed changes

tjaychen approved these changes Mar 10, 2021

View reviewed changes

msfschaffner force-pushed the jtag-mux branch from 0d319c9 to caec31b Compare March 10, 2021 23:06

msfschaffner added 2 commits March 10, 2021 16:44

[top] Align FPGA, Verilator and DV tops

def1fbc

Signed-off-by: Michael Schaffner <[email protected]>

[jtag_mux] Delete JTAG mux module

b543982

Signed-off-by: Michael Schaffner <[email protected]>

msfschaffner force-pushed the jtag-mux branch from caec31b to b543982 Compare March 11, 2021 00:44

msfschaffner merged commit bd440d2 into lowRISC:master Mar 11, 2021

vogelpi added a commit that referenced this pull request Mar 18, 2021

[fpga] Fix dedicated IOs and capture trigger for top_englishbreakfast

4b8a43c

The pads of the dedicated IOs and the capture trigger got accidentally disconnected with #5486. Signed-off-by: Pirmin Vogel <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pinmux/tops] Move JTAG mux logic into pinmux strap logic #5486

[pinmux/tops] Move JTAG mux logic into pinmux strap logic #5486

msfschaffner commented Mar 6, 2021

msfschaffner Mar 10, 2021 •

edited

Loading

tjaychen Mar 10, 2021

msfschaffner Mar 10, 2021

tjaychen Mar 10, 2021

msfschaffner Mar 11, 2021

tjaychen Mar 10, 2021

msfschaffner Mar 10, 2021

tjaychen Mar 10, 2021 •

edited

Loading

msfschaffner Mar 10, 2021

tjaychen Mar 10, 2021

tjaychen Mar 10, 2021

tjaychen Mar 10, 2021

msfschaffner Mar 10, 2021

tjaychen Mar 10, 2021

msfschaffner Mar 10, 2021

tjaychen left a comment

tjaychen commented Mar 11, 2021 via email

[pinmux/tops] Move JTAG mux logic into pinmux strap logic #5486

[pinmux/tops] Move JTAG mux logic into pinmux strap logic #5486

Conversation

msfschaffner commented Mar 6, 2021

msfschaffner Mar 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjaychen Mar 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjaychen left a comment

Choose a reason for hiding this comment

tjaychen commented Mar 11, 2021 via email

msfschaffner Mar 10, 2021 •

edited

Loading

tjaychen Mar 10, 2021 •

edited

Loading