[PATCH 0/2] sandbox: enable testing SYSRESET_WATCHDOG
With CONFIG_SYSRESET_WATCHDOG=y the sandbox can use a watchdog based system reset. To make this work calling sandbox_wdt_expire_now() must lead to a reset.
Further we need CONFIG_WDT_GPIO=n to actually call the right watchdog. Adjust the watchdog test to avoid a build error.
With this change we can test the development suggested in
[PATCH 0/4] Improved sysreset/watchdog uclass integration https://lists.denx.de/pipermail/u-boot/2021-August/458656.html
Heinrich Schuchardt (2): test/dm: fix watchdog test sandbox: fix sandbox_wdt_expire_now()
drivers/watchdog/sandbox_wdt.c | 1 + test/dm/wdt.c | 4 ++++ 2 files changed, 5 insertions(+)
Avoid a build failure for CONFIG_WDT_GPIO=n.
We need this setting to test watchdog based system reset.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- test/dm/wdt.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/test/dm/wdt.c b/test/dm/wdt.c index ee615f0e14..07a8eb0e7a 100644 --- a/test/dm/wdt.c +++ b/test/dm/wdt.c @@ -44,6 +44,7 @@ static int dm_test_wdt_base(struct unit_test_state *uts) } DM_TEST(dm_test_wdt_base, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+#ifdef CONFIG_WDT_GPIO static int dm_test_wdt_gpio(struct unit_test_state *uts) { /* @@ -75,6 +76,7 @@ static int dm_test_wdt_gpio(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_wdt_gpio, UT_TESTF_SCAN_FDT); +#endif
static int dm_test_wdt_watchdog_reset(struct unit_test_state *uts) { @@ -86,9 +88,11 @@ static int dm_test_wdt_watchdog_reset(struct unit_test_state *uts) uint reset_count; int val;
+#ifdef CONFIG_WDT_GPIO ut_assertok(uclass_get_device_by_driver(UCLASS_WDT, DM_DRIVER_GET(wdt_gpio), &gpio_wdt)); ut_assertnonnull(gpio_wdt); +#endif ut_assertok(uclass_get_device_by_driver(UCLASS_WDT, DM_DRIVER_GET(wdt_sandbox), &sandbox_wdt)); ut_assertnonnull(sandbox_wdt);
On Thu, 28 Oct 2021 at 04:16, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Avoid a build failure for CONFIG_WDT_GPIO=n.
We need this setting to test watchdog based system reset.
watchdog-based
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
test/dm/wdt.c | 4 ++++ 1 file changed, 4 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/test/dm/wdt.c b/test/dm/wdt.c index ee615f0e14..07a8eb0e7a 100644 --- a/test/dm/wdt.c +++ b/test/dm/wdt.c @@ -44,6 +44,7 @@ static int dm_test_wdt_base(struct unit_test_state *uts) } DM_TEST(dm_test_wdt_base, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+#ifdef CONFIG_WDT_GPIO static int dm_test_wdt_gpio(struct unit_test_state *uts) { /* @@ -75,6 +76,7 @@ static int dm_test_wdt_gpio(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_wdt_gpio, UT_TESTF_SCAN_FDT); +#endif
static int dm_test_wdt_watchdog_reset(struct unit_test_state *uts) { @@ -86,9 +88,11 @@ static int dm_test_wdt_watchdog_reset(struct unit_test_state *uts) uint reset_count; int val;
+#ifdef CONFIG_WDT_GPIO
The #ifdef is unfortunate but I believe it is needed due to DM_DRIVER_GET. You could get by name perhaps, or just get the first device?
ut_assertok(uclass_get_device_by_driver(UCLASS_WDT, DM_DRIVER_GET(wdt_gpio), &gpio_wdt)); ut_assertnonnull(gpio_wdt);+#endif ut_assertok(uclass_get_device_by_driver(UCLASS_WDT, DM_DRIVER_GET(wdt_sandbox), &sandbox_wdt)); ut_assertnonnull(sandbox_wdt); -- 2.32.0
Regards, Simon
Am 1. November 2021 00:46:49 MEZ schrieb Simon Glass sjg@chromium.org:
On Thu, 28 Oct 2021 at 04:16, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Avoid a build failure for CONFIG_WDT_GPIO=n.
We need this setting to test watchdog based system reset.
watchdog-based
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
test/dm/wdt.c | 4 ++++ 1 file changed, 4 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/test/dm/wdt.c b/test/dm/wdt.c index ee615f0e14..07a8eb0e7a 100644 --- a/test/dm/wdt.c +++ b/test/dm/wdt.c @@ -44,6 +44,7 @@ static int dm_test_wdt_base(struct unit_test_state *uts) } DM_TEST(dm_test_wdt_base, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+#ifdef CONFIG_WDT_GPIO static int dm_test_wdt_gpio(struct unit_test_state *uts) { /* @@ -75,6 +76,7 @@ static int dm_test_wdt_gpio(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_wdt_gpio, UT_TESTF_SCAN_FDT); +#endif
static int dm_test_wdt_watchdog_reset(struct unit_test_state *uts) { @@ -86,9 +88,11 @@ static int dm_test_wdt_watchdog_reset(struct unit_test_state *uts) uint reset_count; int val;
+#ifdef CONFIG_WDT_GPIO
The #ifdef is unfortunate but I believe it is needed due to DM_DRIVER_GET. You could get by name perhaps, or just get the first device?
Thanks for reviewing
We could move the GPIO test to a separate file and make the decision in Makefile. This would avoid #ifdef.
Best regards Heinrich
ut_assertok(uclass_get_device_by_driver(UCLASS_WDT, DM_DRIVER_GET(wdt_gpio), &gpio_wdt)); ut_assertnonnull(gpio_wdt);+#endif ut_assertok(uclass_get_device_by_driver(UCLASS_WDT, DM_DRIVER_GET(wdt_sandbox), &sandbox_wdt)); ut_assertnonnull(sandbox_wdt); -- 2.32.0
Regards, Simon
On 11/1/21 06:21, Heinrich Schuchardt wrote:
Am 1. November 2021 00:46:49 MEZ schrieb Simon Glass sjg@chromium.org:
On Thu, 28 Oct 2021 at 04:16, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Avoid a build failure for CONFIG_WDT_GPIO=n.
We need this setting to test watchdog based system reset.
watchdog-based
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
test/dm/wdt.c | 4 ++++ 1 file changed, 4 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/test/dm/wdt.c b/test/dm/wdt.c index ee615f0e14..07a8eb0e7a 100644 --- a/test/dm/wdt.c +++ b/test/dm/wdt.c @@ -44,6 +44,7 @@ static int dm_test_wdt_base(struct unit_test_state *uts) } DM_TEST(dm_test_wdt_base, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+#ifdef CONFIG_WDT_GPIO static int dm_test_wdt_gpio(struct unit_test_state *uts) { /* @@ -75,6 +76,7 @@ static int dm_test_wdt_gpio(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_wdt_gpio, UT_TESTF_SCAN_FDT); +#endif
static int dm_test_wdt_watchdog_reset(struct unit_test_state *uts) { @@ -86,9 +88,11 @@ static int dm_test_wdt_watchdog_reset(struct unit_test_state *uts) uint reset_count; int val;
+#ifdef CONFIG_WDT_GPIO
The #ifdef is unfortunate but I believe it is needed due to DM_DRIVER_GET. You could get by name perhaps, or just get the first device?
Thanks for reviewing
We could move the GPIO test to a separate file and make the decision in Makefile. This would avoid #ifdef.
Best regards Heinrich
The WDT test can only be successful if both the GPIO and the SANDBOX watchdog are enabled. So what we need is:
diff --git a/test/dm/Makefile b/test/dm/Makefile index 7de013f636..548649f8e8 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -106,6 +106,8 @@ obj-$(CONFIG_TIMER) += timer.o obj-$(CONFIG_DM_USB) += usb.o obj-$(CONFIG_DM_VIDEO) += video.o obj-$(CONFIG_VIRTIO_SANDBOX) += virtio.o -obj-$(CONFIG_WDT) += wdt.o +ifeq ($(CONFIG_WDT_GPIO)$(CONFIG_WDT_SANDBOX),yy) +obj-y += wdt.o +endif endif endif # !SPL
Best regards
Heinrich
ut_assertok(uclass_get_device_by_driver(UCLASS_WDT, DM_DRIVER_GET(wdt_gpio), &gpio_wdt)); ut_assertnonnull(gpio_wdt);+#endif ut_assertok(uclass_get_device_by_driver(UCLASS_WDT, DM_DRIVER_GET(wdt_sandbox), &sandbox_wdt)); ut_assertnonnull(sandbox_wdt); -- 2.32.0
Regards, Simon
Hi Heinrich,
On Sun, 31 Oct 2021 at 23:21, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 1. November 2021 00:46:49 MEZ schrieb Simon Glass sjg@chromium.org:
On Thu, 28 Oct 2021 at 04:16, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
Avoid a build failure for CONFIG_WDT_GPIO=n.
We need this setting to test watchdog based system reset.
watchdog-based
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
test/dm/wdt.c | 4 ++++ 1 file changed, 4 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/test/dm/wdt.c b/test/dm/wdt.c index ee615f0e14..07a8eb0e7a 100644 --- a/test/dm/wdt.c +++ b/test/dm/wdt.c @@ -44,6 +44,7 @@ static int dm_test_wdt_base(struct unit_test_state *uts) } DM_TEST(dm_test_wdt_base, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+#ifdef CONFIG_WDT_GPIO static int dm_test_wdt_gpio(struct unit_test_state *uts) { /* @@ -75,6 +76,7 @@ static int dm_test_wdt_gpio(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_wdt_gpio, UT_TESTF_SCAN_FDT); +#endif
static int dm_test_wdt_watchdog_reset(struct unit_test_state *uts) { @@ -86,9 +88,11 @@ static int dm_test_wdt_watchdog_reset(struct unit_test_state *uts) uint reset_count; int val;
+#ifdef CONFIG_WDT_GPIO
The #ifdef is unfortunate but I believe it is needed due to DM_DRIVER_GET. You could get by name perhaps, or just get the first device?
Thanks for reviewing
We could move the GPIO test to a separate file and make the decision in Makefile. This would avoid #ifdef.
OK, but I don't think it is worth it. We can keep the #ifdef
Regards, Simon
Best regards Heinrich
ut_assertok(uclass_get_device_by_driver(UCLASS_WDT, DM_DRIVER_GET(wdt_gpio), &gpio_wdt)); ut_assertnonnull(gpio_wdt);+#endif ut_assertok(uclass_get_device_by_driver(UCLASS_WDT, DM_DRIVER_GET(wdt_sandbox), &sandbox_wdt)); ut_assertnonnull(sandbox_wdt); -- 2.32.0
Regards, Simon
With CONFIG_SYSRESET_WATCHDOG=y the sandbox can use a watchdog based system reset.
To make this work calling sandbox_wdt_expire_now() must lead to a reset.
With this change we can test the development suggested in
[PATCH 0/4] Improved sysreset/watchdog uclass integration https://lists.denx.de/pipermail/u-boot/2021-August/458656.html
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- drivers/watchdog/sandbox_wdt.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/watchdog/sandbox_wdt.c b/drivers/watchdog/sandbox_wdt.c index e05d82789f..535614f04d 100644 --- a/drivers/watchdog/sandbox_wdt.c +++ b/drivers/watchdog/sandbox_wdt.c @@ -39,6 +39,7 @@ static int sandbox_wdt_reset(struct udevice *dev) static int sandbox_wdt_expire_now(struct udevice *dev, ulong flags) { sandbox_wdt_start(dev, 1, flags); + sandbox_reset();
return 0; }
On Thu, 28 Oct 2021 at 04:16, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
With CONFIG_SYSRESET_WATCHDOG=y the sandbox can use a watchdog based system reset.
To make this work calling sandbox_wdt_expire_now() must lead to a reset.
With this change we can test the development suggested in
[PATCH 0/4] Improved sysreset/watchdog uclass integration https://lists.denx.de/pipermail/u-boot/2021-August/458656.html
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
drivers/watchdog/sandbox_wdt.c | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Simon Glass sjg@chromium.org
watchdog-based
participants (3)
-
Heinrich Schuchardt -
Heinrich Schuchardt -
Simon Glass