From b8a6988e7978443ad7544528a622ac578d6f421d Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Fri, 24 Jun 2022 14:04:28 +0200 Subject: [PATCH 1/2] drivers/wdt: add periph_wdt_auto_start for early watchdog Add an option to enable the watchdog early to detect hangs during initialisation. --- drivers/Makefile.dep | 4 ++++ drivers/include/periph/wdt.h | 17 +++++++++++++++++ drivers/periph_common/init.c | 5 +++++ 3 files changed, 26 insertions(+) diff --git a/drivers/Makefile.dep b/drivers/Makefile.dep index a190ffeb82..0d3495102f 100644 --- a/drivers/Makefile.dep +++ b/drivers/Makefile.dep @@ -254,6 +254,10 @@ ifneq (,$(filter periph_timer_periodic,$(USEMODULE))) FEATURES_REQUIRED += periph_timer endif +ifneq (,$(filter periph_wdt_auto_start,$(USEMODULE))) + FEATURES_REQUIRED += periph_wdt +endif + ifneq (,$(filter-out netdev_default netdev_new_api netdev_legacy_api, $(filter netdev_%,$(USEMODULE)))) USEMODULE += netdev # Don't register netdevs if there is only a single one of them diff --git a/drivers/include/periph/wdt.h b/drivers/include/periph/wdt.h index 2f30c5b158..fee8d4297e 100644 --- a/drivers/include/periph/wdt.h +++ b/drivers/include/periph/wdt.h @@ -230,6 +230,23 @@ extern "C" { #define WDT_HAS_INIT (0) #endif +/** + * @brief If `periph_wdt_auto_start` is used, this will be the lower bound + * of when the WDT can be kicked. + */ +#ifndef CONFIG_PERIPH_WDT_WIN_MIN_MS +#define CONFIG_PERIPH_WDT_WIN_MIN_MS (0) +#endif + +/** + * @brief If `periph_wdt_auto_start` is used, this will be the max period + * after which the WDT must be kicked or else it will reboot the + * system. + */ +#ifndef CONFIG_PERIPH_WDT_WIN_MAX_MS +#define CONFIG_PERIPH_WDT_WIN_MAX_MS (1024) +#endif + /** * @brief Start watchdog timer */ diff --git a/drivers/periph_common/init.c b/drivers/periph_common/init.c index cc7242fd7f..47b28314e3 100644 --- a/drivers/periph_common/init.c +++ b/drivers/periph_common/init.c @@ -102,6 +102,11 @@ void periph_init(void) #if defined(MODULE_PERIPH_INIT_WDT) && WDT_HAS_INIT wdt_init(); + + if (IS_ACTIVE(MODULE_PERIPH_WDT_AUTO_START)) { + wdt_setup_reboot(CONFIG_PERIPH_WDT_WIN_MIN_MS, CONFIG_PERIPH_WDT_WIN_MAX_MS); + wdt_start(); + } #endif #if defined(MODULE_PERIPH_INIT_PTP) From 80fe4e5f2698b9006a5a73df7a4aff5b9089203f Mon Sep 17 00:00:00 2001 From: Benjamin Valentin Date: Fri, 24 Jun 2022 15:01:10 +0200 Subject: [PATCH 2/2] sys/auto_init: add auto_init_wdt_{event, thread} modules --- drivers/include/periph/wdt.h | 35 +++++++++++- makefiles/pseudomodules.inc.mk | 2 + sys/Makefile.dep | 17 ++++++ sys/auto_init/Makefile | 8 +++ sys/auto_init/include/auto_init_priorities.h | 12 ++++ sys/auto_init/wdt_event/Makefile | 3 + sys/auto_init/wdt_event/wdt.c | 47 ++++++++++++++++ sys/auto_init/wdt_thread/Makefile | 3 + sys/auto_init/wdt_thread/wdt.c | 58 ++++++++++++++++++++ 9 files changed, 184 insertions(+), 1 deletion(-) create mode 100644 sys/auto_init/wdt_event/Makefile create mode 100644 sys/auto_init/wdt_event/wdt.c create mode 100644 sys/auto_init/wdt_thread/Makefile create mode 100644 sys/auto_init/wdt_thread/wdt.c diff --git a/drivers/include/periph/wdt.h b/drivers/include/periph/wdt.h index fee8d4297e..70d51eb96b 100644 --- a/drivers/include/periph/wdt.h +++ b/drivers/include/periph/wdt.h @@ -157,9 +157,42 @@ * Makefile add: * * ~~~~~~~~~~~~~~~~~~~~~~~~ {.mk} - * MODULE += periph_wdt_cb + * USEMODULE += periph_wdt_cb * ~~~~~~~~~~~~~~~~~~~~~~~~ * + * WDT Auto-Start + * ============== + * + * It is possible to enable the Watchdog in early boot, before application startup: + * + * ~~~~~~~~~~~~~~~~~~~~~~~~ {.mk} + * USEMODULE += periph_wdt_auto_start + * ~~~~~~~~~~~~~~~~~~~~~~~~ + * + * The watchdog will automatically be initialized with the parameters + * @ref CONFIG_PERIPH_WDT_WIN_MIN_MS and @ref CONFIG_PERIPH_WDT_WIN_MAX_MS + * + * It is also possible to automatically kick the watchdog. + * This is a very non-invasive way of using the watchdog, but it is also very + * weak as it can only detect situations where low-priority threads are + * starved from execution and may even trigger wrongly in situations where the + * system just experiences high load, but would otherwise have recovered on it's own. + * + * If you want to enable it anyway, select this module: + * + * ~~~~~~~~~~~~~~~~~~~~~~~~ {.mk} + * USEMODULE += auto_init_wdt_thread + * ~~~~~~~~~~~~~~~~~~~~~~~~ + * + * If you are using an event thread, you can also use the watchdog to ensure that events + * are processed in time. + * To do so, add + * + * ~~~~~~~~~~~~~~~~~~~~~~~~ {.mk} + * USEMODULE += auto_init_wdt_event + * ~~~~~~~~~~~~~~~~~~~~~~~~ + * + * * @{ * * @file wdt.h diff --git a/makefiles/pseudomodules.inc.mk b/makefiles/pseudomodules.inc.mk index 18ae0c5f8c..ac9292dad0 100644 --- a/makefiles/pseudomodules.inc.mk +++ b/makefiles/pseudomodules.inc.mk @@ -589,6 +589,8 @@ NO_PSEUDOMODULES += auto_init_multimedia NO_PSEUDOMODULES += auto_init_security NO_PSEUDOMODULES += auto_init_usbus NO_PSEUDOMODULES += auto_init_screen +NO_PSEUDOMODULES += auto_init_wdt_event +NO_PSEUDOMODULES += auto_init_wdt_thread # Packages and drivers may also add modules to PSEUDOMODULES in their `Makefile.include`. diff --git a/sys/Makefile.dep b/sys/Makefile.dep index 1233470594..b1215f54a9 100644 --- a/sys/Makefile.dep +++ b/sys/Makefile.dep @@ -24,6 +24,23 @@ ifneq (,$(filter auto_init_gnrc_netif,$(USEMODULE))) USEMODULE += gnrc_netif_init_devs endif +ifneq (,$(filter auto_init_wdt_thread,$(USEMODULE))) + USEMODULE += periph_wdt_auto_start + USEMODULE += ztimer_msec + + # we can only have either auto_init_wdt_event or auto_init_wdt_thread + ifneq (,$(filter auto_init_wdt_event,$(USEMODULE))) + $(error auto_init_wdt_event and auto_init_wdt_thread are mutually exclusive) + endif +endif + +ifneq (,$(filter auto_init_wdt_event,$(USEMODULE))) + USEMODULE += event_periodic_callback + USEMODULE += event_thread + USEMODULE += periph_wdt_auto_start + USEMODULE += ztimer_msec +endif + ifneq (,$(filter auto_init_sock_dns,$(USEMODULE))) ifneq (,$(filter ipv4,$(USEMODULE))) USEMODULE += ipv4_addr diff --git a/sys/auto_init/Makefile b/sys/auto_init/Makefile index f1ac62ab8a..7f5b3e3b6f 100644 --- a/sys/auto_init/Makefile +++ b/sys/auto_init/Makefile @@ -30,4 +30,12 @@ ifneq (,$(filter auto_init_screen,$(USEMODULE))) DIRS += screen endif +ifneq (,$(filter auto_init_wdt_event,$(USEMODULE))) + DIRS += wdt_event +endif + +ifneq (,$(filter auto_init_wdt_thread,$(USEMODULE))) + DIRS += wdt_thread +endif + include $(RIOTBASE)/Makefile.base diff --git a/sys/auto_init/include/auto_init_priorities.h b/sys/auto_init/include/auto_init_priorities.h index be941ded7d..59f9c994f2 100644 --- a/sys/auto_init/include/auto_init_priorities.h +++ b/sys/auto_init/include/auto_init_priorities.h @@ -41,6 +41,12 @@ extern "C" { */ #define AUTO_INIT_PRIO_MOD_XTIMER 1030 #endif +#ifndef AUTO_INIT_PRIO_WDT_THREAD +/** + * @brief WDT priority + */ +#define AUTO_INIT_PRIO_WDT_THREAD 1035 +#endif #ifndef AUTO_INIT_PRIO_MOD_RANDOM /** * @brief RNG priority @@ -71,6 +77,12 @@ extern "C" { */ #define AUTO_INIT_PRIO_MOD_EVENT_THREAD 1080 #endif +#ifndef AUTO_INIT_PRIO_WDT_EVENT +/** + * @brief WDT event priority + */ +#define AUTO_INIT_PRIO_WDT_EVENT 1085 +#endif #ifndef AUTO_INIT_PRIO_MOD_SYS_BUS /** * @brief sys bus priority diff --git a/sys/auto_init/wdt_event/Makefile b/sys/auto_init/wdt_event/Makefile new file mode 100644 index 0000000000..f505661fbb --- /dev/null +++ b/sys/auto_init/wdt_event/Makefile @@ -0,0 +1,3 @@ +MODULE = auto_init_wdt_event + +include $(RIOTBASE)/Makefile.base diff --git a/sys/auto_init/wdt_event/wdt.c b/sys/auto_init/wdt_event/wdt.c new file mode 100644 index 0000000000..f4442ae861 --- /dev/null +++ b/sys/auto_init/wdt_event/wdt.c @@ -0,0 +1,47 @@ +/* + * Copyright (C) 2022 ML!PA Consulting GmbH + * + * This file is subject to the terms and conditions of the GNU Lesser + * General Public License v2.1. See the file LICENSE in the top level + * directory for more details. + */ + +/** + * @ingroup sys_auto_init + * @{ + * + * @file wdt.c + * @brief Watchdog Event + * + * @author Benjamin Valentin + * + * @} + */ + +#include "auto_init.h" +#include "auto_init_utils.h" +#include "auto_init_priorities.h" + +#include "architecture.h" +#include "event/periodic_callback.h" +#include "event/thread.h" +#include "periph/wdt.h" +#include "ztimer.h" + +static void _wdt_event_cb(void *ctx) +{ + (void)ctx; + wdt_kick(); +} + +static void auto_init_wdt_event(void) +{ + static event_periodic_callback_t wdt_event; + unsigned sleep_ms = (CONFIG_PERIPH_WDT_WIN_MIN_MS + CONFIG_PERIPH_WDT_WIN_MAX_MS) + / 2; + + event_periodic_callback_init(&wdt_event, ZTIMER_MSEC, EVENT_PRIO_LOWEST, _wdt_event_cb, NULL); + event_periodic_callback_start(&wdt_event, sleep_ms); +} + +AUTO_INIT(auto_init_wdt_event, AUTO_INIT_PRIO_WDT_EVENT); diff --git a/sys/auto_init/wdt_thread/Makefile b/sys/auto_init/wdt_thread/Makefile new file mode 100644 index 0000000000..4aa059f571 --- /dev/null +++ b/sys/auto_init/wdt_thread/Makefile @@ -0,0 +1,3 @@ +MODULE = auto_init_wdt_thread + +include $(RIOTBASE)/Makefile.base diff --git a/sys/auto_init/wdt_thread/wdt.c b/sys/auto_init/wdt_thread/wdt.c new file mode 100644 index 0000000000..36b8d15187 --- /dev/null +++ b/sys/auto_init/wdt_thread/wdt.c @@ -0,0 +1,58 @@ +/* + * Copyright (C) 2022 ML!PA Consulting GmbH + * + * This file is subject to the terms and conditions of the GNU Lesser + * General Public License v2.1. See the file LICENSE in the top level + * directory for more details. + */ + +/** + * @ingroup sys_auto_init + * @{ + * + * @file wdt.c + * @brief Watchdog Thread + * + * @author Benjamin Valentin + * + * @} + */ + +#include "auto_init.h" +#include "auto_init_utils.h" +#include "auto_init_priorities.h" + +#include "architecture.h" +#include "periph/wdt.h" +#include "ztimer.h" + +#ifndef WDT_THREAD_STACKSIZE +#if DEVELHELP +#define WDT_THREAD_STACKSIZE THREAD_STACKSIZE_SMALL +#else +#define WDT_THREAD_STACKSIZE THREAD_STACKSIZE_TINY +#endif +#endif + +static char WORD_ALIGNED wdt_stack[WDT_THREAD_STACKSIZE]; + +static void *_wdt_thread(void *ctx) +{ + (void)ctx; + unsigned sleep_ms = (CONFIG_PERIPH_WDT_WIN_MIN_MS + CONFIG_PERIPH_WDT_WIN_MAX_MS) + / 2; + while (1) { + ztimer_sleep(ZTIMER_MSEC, sleep_ms); + wdt_kick(); + } + + return NULL; +} + +static void auto_init_wdt_thread(void) +{ + thread_create(wdt_stack, sizeof(wdt_stack), THREAD_PRIORITY_MIN, + THREAD_CREATE_STACKTEST, _wdt_thread, NULL, "watchdog"); +} + +AUTO_INIT(auto_init_wdt_thread, AUTO_INIT_PRIO_WDT_THREAD);