|
From: | Thomas Huth |
Subject: | Re: [PATCH v3 3/3] tests/qtest: sifive-e-aon-watchdog-test.c : Add QTest of watchdog of sifive_e |
Date: | Wed, 30 Nov 2022 12:29:31 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0 |
On 30/11/2022 02.54, Tommy Wu wrote:
Add some simple tests of the watchdog timer in the always-on domain device of HiFive 1 rev b. Signed-off-by: Tommy Wu <tommy.wu@sifive.com> ---
...
diff --git a/tests/qtest/sifive-e-aon-watchdog-test.c b/tests/qtest/sifive-e-aon-watchdog-test.c new file mode 100644 index 0000000000..1f454c266a --- /dev/null +++ b/tests/qtest/sifive-e-aon-watchdog-test.c @@ -0,0 +1,650 @@
I'd suggest to add at least add a SPDX identifier so that people know which license this file has. A short comment on what is being tested here would also be nice (though it is already quite obvious from the file name).
+#include "qemu/osdep.h" +#include "qemu/timer.h" +#include "qemu/bitops.h" +#include "libqtest-single.h"
Note that libqtest-single.h restricts your code to testing with one QEMU binary. If you want to write code that is a little bit more future-proof (e.g. if some of the functions should be used with migration testing later etc.), I'd suggest to get along without libqtest-single.h and pass the QTestState around as a parameter of the functions.
[...]
+static void test_wdogcfg(void) +{ + uint32_t tmp_cfg; + + test_init(); + + tmp_cfg = readl(WDOG_BASE + WDOGCFG); + writel(WDOG_BASE + WDOGCFG, 0xFFFFFFFF); + g_assert(readl(WDOG_BASE + WDOGCFG) == tmp_cfg); + + writel(WDOG_BASE + WDOGKEY, SIFIVE_E_AON_WDOGKEY); + writel(WDOG_BASE + WDOGCFG, 0xFFFFFFFF); + g_assert(0xFFFFFFFF == readl(WDOG_BASE + WDOGCFG)); + + tmp_cfg = readl(WDOG_BASE + WDOGCFG); + g_assert(15 == + FIELD_EX32(tmp_cfg, + AON_WDT_WDOGCFG, + SCALE)); + g_assert(1 == + FIELD_EX32(tmp_cfg, + AON_WDT_WDOGCFG, + RSTEN)); + g_assert(1 == + FIELD_EX32(tmp_cfg, + AON_WDT_WDOGCFG, + ZEROCMP)); + g_assert(1 == + FIELD_EX32(tmp_cfg, + AON_WDT_WDOGCFG, + EN_ALWAYS)); + g_assert(1 == + FIELD_EX32(tmp_cfg, + AON_WDT_WDOGCFG, + EN_CORE_AWAKE)); + g_assert(1 == + FIELD_EX32(tmp_cfg, + AON_WDT_WDOGCFG, + IP0));
Just a matter of taste, but at least I would prefer the statements on one line as long as they still fit into 80 columns.
Thomas
[Prev in Thread] | Current Thread | [Next in Thread] |