On 9/23/20 6:45 PM, Michael Walle wrote:
> Let the user choose between three different behaviours of the watchdog:
> (1) Keep the watchdog disabled
> (2) Supervise u-boot
> (3) Supervise u-boot and the operating systen (default)
>
> Option (2) will disable the watchdog right before handing control to the
> operating system. This is useful when the OS is not aware of the
> watchdog. Option (3) doesn't disable the watchdog and assumes the OS
> will continue servicing.
>
> Signed-off-by: Michael Walle <mich...@walle.cc>
> ---
> cmd/boot.c | 7 +++++++
> cmd/bootefi.c | 7 +++++++
> cmd/efidebug.c | 7 +++++++
> common/board_r.c | 2 +-
> common/bootm_os.c | 5 +++++
> common/spl/spl.c | 6 +++---
> drivers/watchdog/Kconfig | 24 ++++++++++++++++++++++++
> drivers/watchdog/wdt-uclass.c | 33 +++++++++++++++++++++++++--------
> include/wdt.h | 17 +++++++++++++++++
> 9 files changed, 96 insertions(+), 12 deletions(-)
>
> diff --git a/cmd/boot.c b/cmd/boot.c
> index 36aba22b30..2412410371 100644
> --- a/cmd/boot.c
> +++ b/cmd/boot.c
> @@ -10,6 +10,7 @@
> #include <common.h>
> #include <command.h>
> #include <net.h>
> +#include <wdt.h>
>
> #ifdef CONFIG_CMD_GO
>
> @@ -33,6 +34,9 @@ static int do_go(struct cmd_tbl *cmdtp, int flag, int argc,
> char *const argv[])
>
> printf ("## Starting application at 0x%08lX ...\n", addr);
>
> + if (IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT))
> + stop_watchdog();
> +
> /*
> * pass address parameter as argv[0] (aka command name),
> * and all remaining args
> @@ -40,6 +44,9 @@ static int do_go(struct cmd_tbl *cmdtp, int flag, int argc,
> char *const argv[])
> rc = do_go_exec ((void *)addr, argc - 1, argv + 1);
> if (rc != 0) rcode = 1;
>
> + if (IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT))
> + start_watchdog();
> +
> printf ("## Application terminated, rc = 0x%lX\n", rc);
> return rcode;
> }
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 40d5ef2b3a..21f6650174 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -24,6 +24,7 @@
> #include <memalign.h>
> #include <asm-generic/sections.h>
> #include <linux/linkage.h>
> +#include <wdt.h>
>
> DECLARE_GLOBAL_DATA_PTR;
>
> @@ -320,6 +321,9 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle,
> void *load_options)
> efi_uintn_t exit_data_size = 0;
> u16 *exit_data = NULL;
>
> + if (IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT))
> + stop_watchdog();
> +
> /* Call our payload! */
> ret = EFI_CALL(efi_start_image(handle, &exit_data_size, &exit_data));
> if (ret != EFI_SUCCESS) {
> @@ -333,6 +337,9 @@ static efi_status_t do_bootefi_exec(efi_handle_t handle,
> void *load_options)
>
> efi_restore_gd();
>
> + if (IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT))
> + start_watchdog();
> +
> free(load_options);
>
> return ret;
> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> index 9874838b00..5fa0cd1df7 100644
> --- a/cmd/efidebug.c
> +++ b/cmd/efidebug.c
> @@ -16,6 +16,7 @@
> #include <mapmem.h>
> #include <search.h>
> #include <linux/ctype.h>
> +#include <wdt.h>
>
> #define BS systab.boottime
>
> @@ -1131,6 +1132,9 @@ static int do_efi_test_bootmgr(struct cmd_tbl *cmdtp,
> int flag,
> ret = efi_bootmgr_load(&image, &load_options);
> printf("efi_bootmgr_load() returned: %ld\n", ret & ~EFI_ERROR_MASK);
>
> + if (IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT))
> + stop_watchdog();
> +
> /* We call efi_start_image() even if error for test purpose. */
> ret = EFI_CALL(efi_start_image(image, &exit_data_size, &exit_data));
> printf("efi_start_image() returned: %ld\n", ret & ~EFI_ERROR_MASK);
> @@ -1139,6 +1143,9 @@ static int do_efi_test_bootmgr(struct cmd_tbl *cmdtp,
> int flag,
>
> efi_restore_gd();
>
> + if (IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT))
> + start_watchdog();
> +
> free(load_options);
> return CMD_RET_SUCCESS;
> }
> diff --git a/common/board_r.c b/common/board_r.c
> index 9b2fec701a..6734d3ad25 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -731,7 +731,7 @@ static init_fnc_t init_sequence_r[] = {
> stdio_init_tables,
> serial_initialize,
> initr_announce,
> -#if CONFIG_IS_ENABLED(WDT)
> +#if !IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_NOTHING)
> initr_watchdog,
This does not compile on odroid_c2_defconfig.
aarch64-linux-gnu-ld.bfd:
common/built-in.o:(.data.init_sequence_r+0x88): undefined reference to
`initr_watchdog'
make: *** [Makefile:1753: u-boot] Error 1
Best regards
Heinrich
> #endif
> INIT_FUNC_WATCHDOG_RESET
> diff --git a/common/bootm_os.c b/common/bootm_os.c
> index e9aaddf3e6..2b57a4e02c 100644
> --- a/common/bootm_os.c
> +++ b/common/bootm_os.c
> @@ -19,6 +19,7 @@
> #include <mapmem.h>
> #include <vxworks.h>
> #include <tee/optee.h>
> +#include <wdt.h>
>
> DECLARE_GLOBAL_DATA_PTR;
>
> @@ -612,6 +613,10 @@ int boot_selected_os(int argc, char *const argv[], int
> state,
> {
> arch_preboot_os();
> board_preboot_os();
> +
> + if (IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT))
> + stop_watchdog();
> +
> boot_fn(state, argc, argv, images);
>
> /* Stand-alone may return when 'autostart' is 'no' */
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index 4840d1d367..63c47c739c 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -639,9 +639,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
> spl_board_init();
> #endif
>
> -#if defined(CONFIG_SPL_WATCHDOG_SUPPORT) && CONFIG_IS_ENABLED(WDT)
> - initr_watchdog();
> -#endif
> + if (IS_ENABLED(CONFIG_SPL_WATCHDOG_SUPPORT) &&
> + !IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_NOTHING))
> + initr_watchdog();
>
> if (IS_ENABLED(CONFIG_SPL_OS_BOOT) || CONFIG_IS_ENABLED(HANDOFF) ||
> IS_ENABLED(CONFIG_SPL_ATF))
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 4532a40e45..861a0e012d 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1,5 +1,29 @@
> menu "Watchdog Timer Support"
>
> +choice
> + prompt "Watchdog behavior"
> + default WATCHDOG_SUPERVISE_OS
> + depends on WDT
> +
> +config WATCHDOG_SUPERVISE_NOTHING
> + bool "Supervise nothing"
> + help
> + No watchdog will be started.
> +
> +config WATCHDOG_SUPERVISE_U_BOOT
> + bool "Supervise U-Boot only"
> + help
> + Upon U-Boot startup the first watchdog will be started automatically
> + and stopped as soon as an operating system is booted.
> +
> +config WATCHDOG_SUPERVISE_OS
> + bool "Supervise U-boot and operating system"
> + help
> + Upon U-Boot startup the first watchdog will be started automatically
> + and kept running even after booting the operating system.
> + Be aware, that the operating system needs to service the watchdog!
> +endchoice
> +
> config WATCHDOG
> bool "Enable U-Boot watchdog reset"
> depends on !HW_WATCHDOG
> diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c
> index e632f077f3..ba3f8dde11 100644
> --- a/drivers/watchdog/wdt-uclass.c
> +++ b/drivers/watchdog/wdt-uclass.c
> @@ -22,11 +22,10 @@ DECLARE_GLOBAL_DATA_PTR;
> * hw_margin_ms property.
> */
> static ulong reset_period = 1000;
> +static ulong wdt_timeout = WATCHDOG_TIMEOUT_SECS;
>
> int initr_watchdog(void)
> {
> - u32 timeout = WATCHDOG_TIMEOUT_SECS;
> -
> /*
> * Init watchdog: This will call the probe function of the
> * watchdog driver, enabling the use of the device
> @@ -42,21 +41,39 @@ int initr_watchdog(void)
> }
>
> if (CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)) {
> - timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec",
> - WATCHDOG_TIMEOUT_SECS);
> + wdt_timeout = dev_read_u32_default(gd->watchdog_dev,
> + "timeout-sec",
> + WATCHDOG_TIMEOUT_SECS);
> reset_period = dev_read_u32_default(gd->watchdog_dev,
> "hw_margin_ms",
> 4 * reset_period) / 4;
> }
>
> - wdt_start(gd->watchdog_dev, timeout * 1000, 0);
> - gd->flags |= GD_FLG_WDT_READY;
> - printf("WDT: Started with%s servicing (%ds timeout)\n",
> - IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out", timeout);
> + start_watchdog();
> + printf("WDT: Started with%s servicing (supervising %s, %lus
> timeout)\n",
> + IS_ENABLED(CONFIG_WATCHDOG) ? "" : "out",
> + IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT) ? "U-Boot" : "OS",
> + wdt_timeout);
>
> return 0;
> }
>
> +void start_watchdog(void)
> +{
> + if (gd->watchdog_dev) {
> + wdt_start(gd->watchdog_dev, wdt_timeout * 1000, 0);
> + gd->flags |= GD_FLG_WDT_READY;
> + }
> +}
> +
> +void stop_watchdog(void)
> +{
> + if (gd->watchdog_dev) {
> + wdt_stop(gd->watchdog_dev);
> + gd->flags &= ~GD_FLG_WDT_READY;
> + }
> +}
> +
> int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags)
> {
> const struct wdt_ops *ops = device_get_ops(dev);
> diff --git a/include/wdt.h b/include/wdt.h
> index bc242c2eb2..d261cd0dd6 100644
> --- a/include/wdt.h
> +++ b/include/wdt.h
> @@ -105,6 +105,23 @@ struct wdt_ops {
> int (*expire_now)(struct udevice *dev, ulong flags);
> };
>
> +/*
> + * Initialize and start the global watchdog timer.
> + *
> + * @return: 0 if OK, -ve on error
> + */
> int initr_watchdog(void);
>
> +/*
> + * (Re)start the global watchdog timer. Used after returning from an EFI or
> + * application image.
> + */
> +void start_watchdog(void);
> +
> +/*
> + * Stop the global watchdog timer. Used before handing control to an EFI
> image
> + * or operating system.
> + */
> +void stop_watchdog(void);
> +
> #endif /* _WDT_H_ */
>
