[PATCH 2/2] watchdog: add watchdog behavior configuration

[Michael Walle]

Am 2020-09-23 19:28, schrieb Tom Rini:
> On Wed, Sep 23, 2020 at 07:21:57PM +0200, Heinrich Schuchardt wrote:
>> 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 <michael at 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
> 
> I think a pre-req for this will be to have re-run the migration of
> CONFIG_WATCHDOG / CONFIG_WDT from config headers and in to defconfigs.
> I'm not 100% sure that's what's causing the breakage here (migration
> isn't done yet) but I could see that being a problem.

No, I actually messed up my patch in the last minute (I did a kernel-ci
run in parallel, though).

#if !IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_NOTHING)
	initr_watchdog,
#endif

must be

#if IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_U_BOOT) || 
IS_ENABLED(CONFIG_WATCHDOG_SUPERVISE_OS)
	initr_watchdog,
#endif

-michael