Currently the CMSDK APB watchdog tests target an specialized version
of the device (luminaris using the lm3s811evb machine) that prevents
the development of tests for the more generic device documented in:

https://developer.arm.com/documentation/ddi0479/d/apb-components/apb-watchdog/programmers-model

This patch allows the execution of the watchdog tests in an MPS2
machine (when applicable) which uses the generic version of the CMSDK
APB watchdog.

Finally the rules for compiling the test have to change because it is
possible not to have CONFIG_STELLARIS (required for the lm3s811evb
machine) while still having CONFIG_CMSDK_APB_WATCHDOG and the test
will fail. Due to the addition of the MPS2 machine CONFIG_MPS2
becomes also a dependency for the test compilation.

Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com>
Reviewed-by: Stephen Longfield <slongfield@google.com>
---
 tests/qtest/cmsdk-apb-watchdog-test.c | 115 +++++++++++++++++++-------
 tests/qtest/meson.build               |   3 +-
 2 files changed, 86 insertions(+), 32 deletions(-)

diff --git a/tests/qtest/cmsdk-apb-watchdog-test.c b/tests/qtest/cmsdk-apb-watchdog-test.c
index 00b5dbbc81..fe535a553c 100644
--- a/tests/qtest/cmsdk-apb-watchdog-test.c
+++ b/tests/qtest/cmsdk-apb-watchdog-test.c
@@ -15,14 +15,12 @@
  */
 
 #include "qemu/osdep.h"
+#include "exec/hwaddr.h"
 #include "qemu/bitops.h"
 #include "libqtest-single.h"
 
-/*
- * lm3s811evb watchdog; at board startup this runs at 200MHz / 16 == 12.5MHz,
- * which is 80ns per tick.
- */
 #define WDOG_BASE 0x40000000
+#define WDOG_BASE_MPS2 0x40008000
 
 #define WDOGLOAD 0
 #define WDOGVALUE 4
@@ -37,39 +35,87 @@
 #define SYSDIV_SHIFT 23
 #define SYSDIV_LENGTH 4
 
-static void test_watchdog(void)
+#define WDOGLOAD_DEFAULT 0xFFFFFFFF
+#define WDOGVALUE_DEFAULT 0xFFFFFFFF
+
+typedef struct CMSDKAPBWatchdogTestArgs {
+    int64_t tick;
+    hwaddr wdog_base;
+    const char *machine;
+} CMSDKAPBWatchdogTestArgs;
+
+enum {
+    MACHINE_LM3S811EVB,
+    MACHINE_MPS2_AN385,
+};
+
+/*
+ * lm3s811evb watchdog; at board startup this runs at 200MHz / 16 == 12.5MHz,
+ * which is 80ns per tick.
+ *
+ * IoTKit/ARMSSE dualtimer; driven at 25MHz in mps2-an385, so 40ns per tick
+ */
+static const CMSDKAPBWatchdogTestArgs machine_info[] = {
+    [MACHINE_LM3S811EVB] = {
+        .tick = 80,
+        .wdog_base = WDOG_BASE,
+        .machine = "lm3s811evb",
+    },
+    [MACHINE_MPS2_AN385] = {
+        .tick = 40,
+        .wdog_base = WDOG_BASE_MPS2,
+        .machine = "mps2-an385",
+    },
+};
+
+static void test_watchdog(const void *ptr)
 {
-    g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 0);
+    const CMSDKAPBWatchdogTestArgs *args = ptr;
+    hwaddr wdog_base = args->wdog_base;
+    int64_t tick = args->tick;
+    g_autofree gchar *cmdline = g_strdup_printf("-machine %s", args->machine);
+    qtest_start(cmdline);
 
-    writel(WDOG_BASE + WDOGCONTROL, 1);
-    writel(WDOG_BASE + WDOGLOAD, 1000);
+    g_assert_cmpuint(readl(wdog_base + WDOGRIS), ==, 0);
+
+    writel(wdog_base + WDOGCONTROL, 1);
+    writel(wdog_base + WDOGLOAD, 1000);
 
     /* Step to just past the 500th tick */
-    clock_step(500 * 80 + 1);
-    g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 0);
-    g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 500);
+    clock_step(500 * tick + 1);
+    g_assert_cmpuint(readl(wdog_base + WDOGRIS), ==, 0);
+    g_assert_cmpuint(readl(wdog_base + WDOGVALUE), ==, 500);
 
     /* Just past the 1000th tick: timer should have fired */
-    clock_step(500 * 80);
-    g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 1);
-    g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 0);
+    clock_step(500 * tick);
+    g_assert_cmpuint(readl(wdog_base + WDOGRIS), ==, 1);
+    g_assert_cmpuint(readl(wdog_base + WDOGVALUE), ==, 0);
 
     /* VALUE reloads at following tick */
-    clock_step(80);
-    g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 1000);
+    clock_step(tick);
+    g_assert_cmpuint(readl(wdog_base + WDOGVALUE), ==, 1000);
 
     /* Writing any value to WDOGINTCLR clears the interrupt and reloads */
-    clock_step(500 * 80);
-    g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 500);
-    g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 1);
-    writel(WDOG_BASE + WDOGINTCLR, 0);
-    g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 1000);
-    g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 0);
+    clock_step(500 * tick);
+    g_assert_cmpuint(readl(wdog_base + WDOGVALUE), ==, 500);
+    g_assert_cmpuint(readl(wdog_base + WDOGRIS), ==, 1);
+    writel(wdog_base + WDOGINTCLR, 0);
+    g_assert_cmpuint(readl(wdog_base + WDOGVALUE), ==, 1000);
+    g_assert_cmpuint(readl(wdog_base + WDOGRIS), ==, 0);
+
+    qtest_end();
 }
 
-static void test_clock_change(void)
+/*
+ * This test can only be executed in the stellaris board since it relies on a
+ * component of the board to change the clocking parameters of the watchdog.
+ */
+static void test_clock_change(const void *ptr)
 {
     uint32_t rcc;
+    const CMSDKAPBWatchdogTestArgs *args = ptr;
+    g_autofree gchar *cmdline = g_strdup_printf("-machine %s", args->machine);
+    qtest_start(cmdline);
 
     /*
      * Test that writing to the stellaris board's RCC register to
@@ -109,6 +155,8 @@ static void test_clock_change(void)
     writel(WDOG_BASE + WDOGINTCLR, 0);
     g_assert_cmpuint(readl(WDOG_BASE + WDOGVALUE), ==, 1000);
     g_assert_cmpuint(readl(WDOG_BASE + WDOGRIS), ==, 0);
+
+    qtest_end();
 }
 
 int main(int argc, char **argv)
@@ -116,16 +164,21 @@ int main(int argc, char **argv)
     int r;
 
     g_test_init(&argc, &argv, NULL);
-
-    qtest_start("-machine lm3s811evb");
-
-    qtest_add_func("/cmsdk-apb-watchdog/watchdog", test_watchdog);
-    qtest_add_func("/cmsdk-apb-watchdog/watchdog_clock_change",
-                   test_clock_change);
+    g_test_set_nonfatal_assertions();
+
+    if (qtest_has_machine(machine_info[MACHINE_LM3S811EVB].machine)) {
+        qtest_add_data_func("/cmsdk-apb-watchdog/watchdog",
+                            &machine_info[MACHINE_LM3S811EVB], test_watchdog);
+        qtest_add_data_func("/cmsdk-apb-watchdog/watchdog_clock_change",
+                            &machine_info[MACHINE_LM3S811EVB],
+                            test_clock_change);
+    }
+    if (qtest_has_machine(machine_info[MACHINE_MPS2_AN385].machine)) {
+        qtest_add_data_func("/cmsdk-apb-watchdog/watchdog_mps2",
+                            &machine_info[MACHINE_MPS2_AN385], test_watchdog);
+    }
 
     r = g_test_run();
 
-    qtest_end();
-
     return r;
 }
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index aa93e98418..f2f35367ae 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -227,7 +227,8 @@ qtests_arm = \
   (config_all_devices.has_key('CONFIG_MPS2') ? ['sse-timer-test'] : []) + \
   (config_all_devices.has_key('CONFIG_CMSDK_APB_DUALTIMER') ? ['cmsdk-apb-dualtimer-test'] : []) + \
   (config_all_devices.has_key('CONFIG_CMSDK_APB_TIMER') ? ['cmsdk-apb-timer-test'] : []) + \
-  (config_all_devices.has_key('CONFIG_CMSDK_APB_WATCHDOG') ? ['cmsdk-apb-watchdog-test'] : []) + \
+  (config_all_devices.has_key('CONFIG_STELLARIS') or
+   config_all_devices.has_key('CONFIG_MPS2') ? ['cmsdk-apb-watchdog-test'] : []) + \
   (config_all_devices.has_key('CONFIG_PFLASH_CFI02') and
    config_all_devices.has_key('CONFIG_MUSICPAL') ? ['pflash-cfi02-test'] : []) + \
   (config_all_devices.has_key('CONFIG_ASPEED_SOC') ? qtests_aspeed : []) + \
-- 
2.47.0.277.g8800431eea-goog

On Fri, 8 Nov 2024 at 19:10, Roque Arcudia Hernandez <roqueh@google.com> wrote:
>
> Currently the CMSDK APB watchdog tests target an specialized version
> of the device (luminaris using the lm3s811evb machine) that prevents
> the development of tests for the more generic device documented in:
>
> https://developer.arm.com/documentation/ddi0479/d/apb-components/apb-watchdog/programmers-model
>
> This patch allows the execution of the watchdog tests in an MPS2
> machine (when applicable) which uses the generic version of the CMSDK
> APB watchdog.
>
> Finally the rules for compiling the test have to change because it is
> possible not to have CONFIG_STELLARIS (required for the lm3s811evb
> machine) while still having CONFIG_CMSDK_APB_WATCHDOG and the test
> will fail. Due to the addition of the MPS2 machine CONFIG_MPS2
> becomes also a dependency for the test compilation.
>
> Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com>
> Reviewed-by: Stephen Longfield <slongfield@google.com>


This refactoring mostly looks good.

> @@ -116,16 +164,21 @@ int main(int argc, char **argv)
>      int r;
>
>      g_test_init(&argc, &argv, NULL);
> -
> -    qtest_start("-machine lm3s811evb");
> -
> -    qtest_add_func("/cmsdk-apb-watchdog/watchdog", test_watchdog);
> -    qtest_add_func("/cmsdk-apb-watchdog/watchdog_clock_change",
> -                   test_clock_change);
> +    g_test_set_nonfatal_assertions();

But this addition of g_test_set_nonfatal_assertions() is
not related to the refactoring and not mentioned in the
commit message.

thanks
-- PMM

I'll edit the g_test_set_nonfatal_assertions in a separate patch
explaining the reasoning behind it in version 2.

On Thu, Nov 14, 2024 at 4:55 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 8 Nov 2024 at 19:10, Roque Arcudia Hernandez <roqueh@google.com> wrote:
> >
> > Currently the CMSDK APB watchdog tests target an specialized version
> > of the device (luminaris using the lm3s811evb machine) that prevents
> > the development of tests for the more generic device documented in:
> >
> > https://developer.arm.com/documentation/ddi0479/d/apb-components/apb-watchdog/programmers-model
> >
> > This patch allows the execution of the watchdog tests in an MPS2
> > machine (when applicable) which uses the generic version of the CMSDK
> > APB watchdog.
> >
> > Finally the rules for compiling the test have to change because it is
> > possible not to have CONFIG_STELLARIS (required for the lm3s811evb
> > machine) while still having CONFIG_CMSDK_APB_WATCHDOG and the test
> > will fail. Due to the addition of the MPS2 machine CONFIG_MPS2
> > becomes also a dependency for the test compilation.
> >
> > Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com>
> > Reviewed-by: Stephen Longfield <slongfield@google.com>
>
>
> This refactoring mostly looks good.
>
> > @@ -116,16 +164,21 @@ int main(int argc, char **argv)
> >      int r;
> >
> >      g_test_init(&argc, &argv, NULL);
> > -
> > -    qtest_start("-machine lm3s811evb");
> > -
> > -    qtest_add_func("/cmsdk-apb-watchdog/watchdog", test_watchdog);
> > -    qtest_add_func("/cmsdk-apb-watchdog/watchdog_clock_change",
> > -                   test_clock_change);
> > +    g_test_set_nonfatal_assertions();
>
> But this addition of g_test_set_nonfatal_assertions() is
> not related to the refactoring and not mentioned in the
> commit message.
>
> thanks
> -- PMM