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

Michael Walle
2020-09-24T06:53:37Z

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@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