[test,uart] Rework uart_tx_rx_test to use UART console

This merges the versions of this test for each UART into one C binary and configures the OTTF console to use UART1 when UART0 is under test. Signed-off-by: James Wainwright <[email protected]>
lowRISC · Jan 18, 2024 · 9381e31 · 9381e31
1 parent 73d53e1
commit 9381e31
Show file tree

Hide file tree

Showing 8 changed files with 153 additions and 97 deletions.
diff --git a/hw/top_earlgrey/dv/chip_sim_cfg.hjson b/hw/top_earlgrey/dv/chip_sim_cfg.hjson
@@ -327,9 +327,9 @@
                  // Format SW image names (which are Bazel labels concatenated with an index
                  // and/or flags, see below) into output file names separated by commas to feed into
                  // +sw_images plusarg. For example, if the input list of SW images is
-                 // ["//sw/device/tests:uart0_tx_rx_test:1",
+                 // ["//sw/device/tests:uart_tx_rx_test:1",
                  //  "//sw/device/lib/testing/test_rom:test_rom:0"], then the output of this eval_cmd
-                 // will be: "uart0_tx_rx_test:1,test_rom:0".
+                 // will be: "uart_tx_rx_test:1,test_rom:0".
                  '''+sw_images={eval_cmd} \
                  reformatted_sw_images=; \
                  for image in {sw_images}; do \
@@ -525,39 +525,39 @@
     {
       name: chip_sw_uart_tx_rx
       uvm_test_seq: chip_sw_uart_tx_rx_vseq
-      sw_images: ["//sw/device/tests:uart0_tx_rx_test:1:new_rules"]
+      sw_images: ["//sw/device/tests:uart_tx_rx_test:1:new_rules"]
       en_run_modes: ["sw_test_mode_test_rom"]
       run_opts: ["+uart_idx=0", "+calibrate_usb_clk=1"]
       reseed: 5
     }
     {
       name: chip_sw_uart_tx_rx_idx1
       uvm_test_seq: chip_sw_uart_tx_rx_vseq
-      sw_images: ["//sw/device/tests:uart0_tx_rx_test:1:new_rules"]
+      sw_images: ["//sw/device/tests:uart_tx_rx_test:1:new_rules"]
       en_run_modes: ["sw_test_mode_test_rom"]
       run_opts: ["+uart_idx=1", "+calibrate_usb_clk=1"]
       reseed: 5
     }
     {
       name: chip_sw_uart_tx_rx_idx2
       uvm_test_seq: chip_sw_uart_tx_rx_vseq
-      sw_images: ["//sw/device/tests:uart0_tx_rx_test:1:new_rules"]
+      sw_images: ["//sw/device/tests:uart_tx_rx_test:1:new_rules"]
       en_run_modes: ["sw_test_mode_test_rom"]
       run_opts: ["+uart_idx=2", "+calibrate_usb_clk=1"]
       reseed: 5
     }
     {
       name: chip_sw_uart_tx_rx_idx3
       uvm_test_seq: chip_sw_uart_tx_rx_vseq
-      sw_images: ["//sw/device/tests:uart0_tx_rx_test:1:new_rules"]
+      sw_images: ["//sw/device/tests:uart_tx_rx_test:1:new_rules"]
       en_run_modes: ["sw_test_mode_test_rom"]
       run_opts: ["+uart_idx=3", "+calibrate_usb_clk=1"]
       reseed: 5
     }
     {
       name: chip_sw_uart_tx_rx_bootstrap
       uvm_test_seq: chip_sw_uart_tx_rx_vseq
-      sw_images: ["//sw/device/tests:uart0_tx_rx_test:1:new_rules"]
+      sw_images: ["//sw/device/tests:uart_tx_rx_test:1:new_rules"]
       en_run_modes: ["sw_test_mode_test_rom"]
       run_opts: ["+use_spi_load_bootstrap=1", "+calibrate_usb_clk=1",
                  "+test_timeout_ns=80_000_000"]
@@ -633,7 +633,7 @@
     {
       name: chip_sw_uart_rand_baudrate
       uvm_test_seq: chip_sw_uart_rand_baudrate_vseq
-      sw_images: ["//sw/device/tests:uart0_tx_rx_test:1:new_rules"]
+      sw_images: ["//sw/device/tests:uart_tx_rx_test:1:new_rules"]
       en_run_modes: ["sw_test_mode_test_rom"]
       run_opts: ["+sw_test_timeout_ns=80_000_000", "+calibrate_usb_clk=1"]
       run_timeout_mins: 120
@@ -642,7 +642,7 @@
     {
       name: chip_sw_uart_tx_rx_alt_clk_freq
       uvm_test_seq: chip_sw_uart_rand_baudrate_vseq
-      sw_images: ["//sw/device/tests:uart0_tx_rx_test:1:new_rules"]
+      sw_images: ["//sw/device/tests:uart_tx_rx_test:1:new_rules"]
       en_run_modes: ["sw_test_mode_test_rom"]
       run_opts: ["+sw_test_timeout_ns=80_000_000",
                  "+chip_clock_source=ChipClockSourceExternal96Mhz", "+calibrate_usb_clk=1"]
@@ -652,7 +652,7 @@
     {
       name: chip_sw_uart_tx_rx_alt_clk_freq_low_speed
       uvm_test_seq: chip_sw_uart_rand_baudrate_vseq
-      sw_images: ["//sw/device/tests:uart0_tx_rx_test:1:new_rules"]
+      sw_images: ["//sw/device/tests:uart_tx_rx_test:1:new_rules"]
       en_run_modes: ["sw_test_mode_test_rom"]
       run_opts: ["+sw_test_timeout_ns=80_000_000", "+calibrate_usb_clk=1",
                  "+chip_clock_source=ChipClockSourceExternal48Mhz"]

diff --git a/hw/top_earlgrey/dv/env/seq_lib/chip_sw_uart_tx_rx_vseq.sv b/hw/top_earlgrey/dv/env/seq_lib/chip_sw_uart_tx_rx_vseq.sv
@@ -40,7 +40,7 @@ class chip_sw_uart_tx_rx_vseq extends chip_sw_uart_smoke_vseq;
     super.cpu_init();
     sw_symbol_backdoor_overwrite("kUartTxData", exp_uart_tx_data);
     sw_symbol_backdoor_overwrite("kExpUartRxData", uart_rx_data);
-    sw_symbol_backdoor_overwrite("kUartIdx", uart_idx_data);
+    sw_symbol_backdoor_overwrite("kUartIdxDv", uart_idx_data);
   endtask
 
   virtual task body();

diff --git a/sw/device/tests/BUILD b/sw/device/tests/BUILD
@@ -4447,60 +4447,56 @@ opentitan_binary(
     ],
 )
 
-[
-    opentitan_test(
-        name = "uart{}_tx_rx_test".format(uart_idx),
-        srcs = ["uart_tx_rx_test.c"],
-        copts = ["-DUART_IDX={}".format(uart_idx)],
-        cw310 = new_cw310_params(
-            otp = "//hw/ip/otp_ctrl/data/earlgrey_a0_skus/sival:otp_img_prod_manuf_personalized",
-            test_cmd = " ".join([
-                "--bitstream=\"{bitstream}\"",
-                "--bootstrap=\"{firmware}\"",
-                "--firmware-elf=\"{firmware:elf}\"",
-            ]),
-            test_harness = "//sw/host/tests/chip/uart:uart_tx_rx",
-        ),
-        exec_env = {
-            # TODO: this test currently fails on FPGA.
-            # "//hw/top_earlgrey:fpga_cw310_sival": None,
-            # "//hw/top_earlgrey:fpga_cw310_sival_rom_ext": None,
-            # "//hw/top_earlgrey:fpga_cw310_test_rom": None,
-            "//hw/top_earlgrey:silicon_creator": None,
-            "//hw/top_earlgrey:silicon_owner_sival_rom_ext": "silicon_owner",
-            "//hw/top_earlgrey:silicon_owner_prodc_rom_ext": "silicon_owner",
-            "//hw/top_earlgrey:sim_dv": None,
-        },
-        silicon = silicon_params(
-            test_cmd = " ".join([
-                "--bootstrap=\"{firmware}\"",
-                "--firmware-elf=\"{firmware:elf}\"",
-            ]),
-            test_harness = "//sw/host/tests/chip/uart:uart_tx_rx",
-        ),
-        silicon_owner = silicon_params(
-            tags = ["broken"],
-        ),
-        deps = [
-            "//hw/ip/lc_ctrl/data:lc_ctrl_regs",
-            "//hw/top_earlgrey/ip/clkmgr/data/autogen:clkmgr_regs",
-            "//hw/top_earlgrey/ip/pinmux/data/autogen:pinmux_regs",
-            "//hw/top_earlgrey/sw/autogen:top_earlgrey",
-            "//sw/device/lib/base:mmio",
-            "//sw/device/lib/dif:clkmgr",
-            "//sw/device/lib/dif:lc_ctrl",
-            "//sw/device/lib/dif:rv_plic",
-            "//sw/device/lib/dif:uart",
-            "//sw/device/lib/runtime:hart",
-            "//sw/device/lib/runtime:irq",
-            "//sw/device/lib/runtime:log",
-            "//sw/device/lib/testing:clkmgr_testutils",
-            "//sw/device/lib/testing:uart_testutils",
-            "//sw/device/lib/testing/test_framework:ottf_main",
-        ],
-    )
-    for uart_idx in range(0, 4, 1)
-]
+opentitan_test(
+    name = "uart_tx_rx_test",
+    srcs = ["uart_tx_rx_test.c"],
+    cw310 = new_cw310_params(
+        otp = "//hw/ip/otp_ctrl/data/earlgrey_a0_skus/sival:otp_img_prod_manuf_personalized",
+        test_cmd = " ".join([
+            "--bitstream=\"{bitstream}\"",
+            "--bootstrap=\"{firmware}\"",
+            "--firmware-elf=\"{firmware:elf}\"",
+        ]),
+        test_harness = "//sw/host/tests/chip/uart:uart_tx_rx",
+    ),
+    exec_env = {
+        "//hw/top_earlgrey:fpga_cw310_sival": None,
+        "//hw/top_earlgrey:fpga_cw310_sival_rom_ext": None,
+        "//hw/top_earlgrey:silicon_creator": None,
+        "//hw/top_earlgrey:silicon_owner_sival_rom_ext": "silicon_owner",
+        "//hw/top_earlgrey:silicon_owner_prodc_rom_ext": "silicon_owner",
+        "//hw/top_earlgrey:sim_dv": None,
+    },
+    silicon = silicon_params(
+        test_cmd = " ".join([
+            "--bootstrap=\"{firmware}\"",
+            "--firmware-elf=\"{firmware:elf}\"",
+        ]),
+        test_harness = "//sw/host/tests/chip/uart:uart_tx_rx",
+    ),
+    silicon_owner = silicon_params(
+        tags = ["broken"],
+    ),
+    deps = [
+        "//hw/ip/lc_ctrl/data:lc_ctrl_regs",
+        "//hw/top_earlgrey/ip/clkmgr/data/autogen:clkmgr_regs",
+        "//hw/top_earlgrey/ip/pinmux/data/autogen:pinmux_regs",
+        "//hw/top_earlgrey/sw/autogen:top_earlgrey",
+        "//sw/device/lib/base:mmio",
+        "//sw/device/lib/dif:clkmgr",
+        "//sw/device/lib/dif:lc_ctrl",
+        "//sw/device/lib/dif:rv_plic",
+        "//sw/device/lib/dif:uart",
+        "//sw/device/lib/runtime:hart",
+        "//sw/device/lib/runtime:irq",
+        "//sw/device/lib/runtime:log",
+        "//sw/device/lib/testing:clkmgr_testutils",
+        "//sw/device/lib/testing:uart_testutils",
+        "//sw/device/lib/testing/test_framework:ottf_console",
+        "//sw/device/lib/testing/test_framework:ottf_main",
+        "//sw/device/lib/testing/test_framework:ottf_utils",
+    ],
+)
 
 opentitan_test(
     name = "rv_core_ibex_mem_test_functest",

diff --git a/sw/device/tests/sival/BUILD b/sw/device/tests/sival/BUILD
@@ -57,10 +57,8 @@ test_suite(
         "//sw/device/tests:sram_ctrl_sleep_sram_ret_contents_no_scramble_test",
         "//sw/device/tests:sram_ctrl_sleep_sram_ret_contents_scramble_test",
         "//sw/device/tests:sram_ctrl_smoketest",
-        "//sw/device/tests:uart0_tx_rx_test",
-        "//sw/device/tests:uart1_tx_rx_test",
-        "//sw/device/tests:uart2_tx_rx_test",
         "//sw/device/tests:uart_smoketest",
+        "//sw/device/tests:uart_tx_rx_test",
         "//sw/device/tests/pmod:i2c_host_eeprom_test",
     ],
 )

diff --git a/sw/device/tests/uart_tx_rx_test.c b/sw/device/tests/uart_tx_rx_test.c
@@ -15,7 +15,9 @@
 #include "sw/device/lib/runtime/log.h"
 #include "sw/device/lib/testing/clkmgr_testutils.h"
 #include "sw/device/lib/testing/test_framework/check.h"
+#include "sw/device/lib/testing/test_framework/ottf_console.h"
 #include "sw/device/lib/testing/test_framework/ottf_main.h"
+#include "sw/device/lib/testing/test_framework/ottf_utils.h"
 #include "sw/device/lib/testing/test_framework/status.h"
 #include "sw/device/lib/testing/uart_testutils.h"
 
@@ -58,18 +60,23 @@ typedef enum uart_direction {
 /**
  * Indicates the UART instance under test.
  *
- * From the software / compiler's perspective, this is a constant (hence the
- * `const` qualifier). However, the external DV testbench finds this symbol's
+ * When running in `dv_sim`, the external DV testbench finds this symbol's
  * address and modifies it via backdoor, to test a different UART instance with
  * the same test SW image. Hence, we add the `volatile` keyword to prevent the
  * compiler from optimizing it out.
+ *
  * The `const` is needed to put it in the .rodata section, otherwise it gets
  * placed in .data section in the main SRAM. We cannot backdoor write anything
  * in SRAM at the start of the test because the CRT init code wipes it to 0s.
- * This constant remains unchanged for non-simulation environments (FPGAs and
- * silicon) where this constant must be changed at compile-time to each UART.
  */
-static volatile const uint8_t kUartIdx = UART_IDX;
+static volatile const uint8_t kUartIdxDv = 0xff;
+/**
+ * Outside of DV simulation environments, the `kUartIdx` symbol needs to be
+ * _non_ `const` so that we can modify it via OTTF commands. `kUartIdx` is used
+ * as the source of truth in the test but we copy the value from `kUartIdxDv`
+ * to here if it has been set.
+ */
+static volatile uint8_t kUartIdx = 0xff;
 
 /**
  * Indicates if ext_clk is used and what speed.
@@ -139,6 +146,10 @@ static volatile bool uart_irq_tx_empty_fired;
 static volatile bool exp_uart_irq_rx_overflow;
 static volatile bool uart_irq_rx_overflow_fired;
 
+enum {
+  kCommandTimeout = 5000000,  // microseconds
+};
+
 void update_uart_base_addr_and_irq_id(void) {
   switch (kUartIdx) {
     case 0:
@@ -496,11 +507,20 @@ void config_external_clock(const dif_clkmgr_t *clkmgr) {
       clkmgr_testutils_enable_external_clock_blocking(clkmgr, kUseLowSpeedSel));
 }
 
-OTTF_DEFINE_TEST_CONFIG(.console.type = kOttfConsoleSpiDevice,
-                        .console.base_addr = TOP_EARLGREY_SPI_DEVICE_BASE_ADDR,
-                        .console.test_may_clobber = false);
+OTTF_DEFINE_TEST_CONFIG(.enable_uart_flow_control = true);
 
 bool test_main(void) {
+  uart_pinmux_platform_id_t platform_id = UartPinmuxPlatformIdCount;
+  if (kDeviceType == kDeviceFpgaCw310) {
+    platform_id = UartPinmuxPlatformIdHyper310;
+  } else if (kDeviceType == kDeviceSimDV) {
+    platform_id = UartPinmuxPlatformIdDvsim;
+  } else if (kDeviceType == kDeviceSilicon) {
+    platform_id = UartPinmuxPlatformIdSilicon;
+  } else {
+    CHECK(false, "Unsupported platform %d", kDeviceType);
+  }
+
   mmio_region_t base_addr;
 
   base_addr = mmio_region_from_addr(TOP_EARLGREY_CLKMGR_AON_BASE_ADDR);
@@ -509,21 +529,23 @@ bool test_main(void) {
   base_addr = mmio_region_from_addr(TOP_EARLGREY_PINMUX_AON_BASE_ADDR);
   CHECK_DIF_OK(dif_pinmux_init(base_addr, &pinmux));
 
+  if (kUartIdxDv != 0xff) {
+    kUartIdx = kUartIdxDv;
+  } else {
+    OTTF_WAIT_FOR(kUartIdx != 0xff, kCommandTimeout);
+  }
+
+  // If we're testing UART0 we need to move the console to UART1.
+  if (kUartIdx == 0) {
+    CHECK_STATUS_OK(uart_testutils_select_pinmux(&pinmux, 1, platform_id,
+                                                 UartPinmuxChannelConsole));
+    ottf_console_configure_uart(TOP_EARLGREY_UART1_BASE_ADDR);
+  }
+
   update_uart_base_addr_and_irq_id();
 
   LOG_INFO("Test UART%d with base_addr: %08x", kUartIdx, uart_base_addr);
 
-  uart_pinmux_platform_id_t platform_id = UartPinmuxPlatformIdCount;
-  if (kDeviceType == kDeviceFpgaCw310) {
-    platform_id = UartPinmuxPlatformIdHyper310;
-  } else if (kDeviceType == kDeviceSimDV) {
-    platform_id = UartPinmuxPlatformIdDvsim;
-  } else if (kDeviceType == kDeviceSilicon) {
-    platform_id = UartPinmuxPlatformIdSilicon;
-  } else {
-    CHECK(false, "Unsupported platform %d", kDeviceType);
-  }
-
   // Attach the UART under test.
   CHECK_STATUS_OK(uart_testutils_select_pinmux(&pinmux, kUartIdx, platform_id,
                                                UartPinmuxChannelDut));

diff --git a/sw/host/opentitanlib/src/app/config/hyperdebug_chipwhisperer.json b/sw/host/opentitanlib/src/app/config/hyperdebug_chipwhisperer.json
@@ -246,6 +246,10 @@
     {
       "name": "console",
       "alias_of": "UART2"
+    },
+    {
+      "name": "dut",
+      "alias_of": "UART3"
     }
   ],
   "strappings": [

diff --git a/sw/host/opentitanlib/src/app/config/opentitan_cw310.json b/sw/host/opentitanlib/src/app/config/opentitan_cw310.json
@@ -51,6 +51,10 @@
     {
       "name": "console",
       "alias_of": "0"
+    },
+    {
+      "name": "dut",
+      "alias_of": "1"
     }
   ]
 }