On 11. 04. 19 15:58, Stefan Roese wrote: > This patch tries to implement a generic watchdog_reset() function that > can be used by all boards that want to service the watchdog device in > U-Boot. This watchdog servicing is enabled via CONFIG_WATCHDOG. > > Without this approach, new boards or platforms needed to implement a > board specific version of this functionality, mostly copy'ing the same > code over and over again into their board or platforms code base. > > With this new generic function, the scattered other functions are now > removed to be replaced by the generic one. The new version also enables > the configuration of the watchdog timeout via the DT "timeout-sec" > property (if enabled via CONFIG_OF_CONTROL). > > This patch also adds a new flag to the GD flags, to flag that the > watchdog is ready to use and adds the pointer to the watchdog device > to the GD. This enables us to remove the global "watchdog_dev" > variable, which was prone to cause problems because of its potentially > very early use in watchdog_reset(), even before the BSS is cleared. > > Signed-off-by: Stefan Roese <[email protected]> > Cc: Heiko Schocher <[email protected]> > Cc: Tom Rini <[email protected]> > Cc: Michal Simek <[email protected]> > Cc: "Marek Behún" <[email protected]> > Cc: Daniel Schwierzeck <[email protected]> > Cc: Maxim Sloyko <[email protected]> > Cc: Erik van Luijk <[email protected]> > Cc: Ryder Lee <[email protected]> > Cc: Weijie Gao <[email protected]> > Cc: Simon Glass <[email protected]> > Cc: "Álvaro Fernández Rojas" <[email protected]> > Cc: Philippe Reynes <[email protected]> > Cc: Christophe Leroy <[email protected]> > --- > This patch depends on the following patch: > > [1] watchdog: Handle SPL build with watchdog disabled > https://patchwork.ozlabs.org/patch/1074098/ > > We have a few boards that have CONFIG_WDT (DM watchdog driver) enabled > and CONFIG_WATCHDOG (U-Boot watchdog servicing) disabled. This might > lead to a watchdog reboot, if the board does not boot fast enough into > some OS (like Linux) which services the watchdog. With this patch > now, the watchdog is started on all those boards (if found via the DT) > and also serviced as this patch also "implies" CONFIG_WATCHDOG. If > this is not desired, then please send a patch to disable > CONFIG_WATCHDOG for this board. Here the list of those boards with > their maintainers: > > taurus: Heiko Schocher <[email protected]> > smartweb: Heiko Schocher <[email protected]> > ast2500: Maxim Sloyko <[email protected]> > picosam9g45: Erik van Luijk <[email protected]> > mt7623n_bpir2: Ryder Lee <[email protected]> Weijie Gao > <[email protected]> > mt7629_rfb: Ryder Lee <[email protected]> Weijie Gao > <[email protected]> > bitmain_antminer_s9: Michal Simek <[email protected]> > sandbox64: Simon Glass <[email protected]> > sandbox: Simon Glass <[email protected]> > comtrend_ct5361_ram: "Álvaro Fernández Rojas" <[email protected]> > netgear_cg3100d_ram: "Álvaro Fernández Rojas" <[email protected]> > bcm968380gerg_ram: Philippe Reynes <[email protected]> > bcm963158_ram: Philippe Reynes <[email protected]> > bcm968580xref_ram: Philippe Reynes <[email protected]> > MCR3000: Christophe Leroy <[email protected]> > > Thanks, > Stefan > > v4: > - No change > > v3: > - Use already available CONFIG_WATCHDOG_TIMEOUT_MSECS option vs the > newly introduced WDT_DEFAULT_TIMEOUT default timeout value (if not > provided via DT property). > - Guard initr_watchdog by CONFIG_WDT instead of CONFIG_WATCHDOG && > CONFIG_WDT as suggested by Michal. This makes it possible to enable > and start the U-Boot watchdog without enabling the U-Boot watchdog > servicing. > - Add imply CONFIG_WATCHDOG to CONFIG_WDT, as this restores the > current behaviour to use the U-Boot watchdog servicing feature, if > the DM watchdog is enabled (CONFIG_WDT). If this is not desired, > CONFIG_WATCHDOG can be disabled in the board defconfig. > - Remove CONFIG_WATCHDOG from include/configs/turris_omnia.h > > v2: > - Rename watchdog_start() to initr_watchdog() and move it to board_r.c. > This way its only called once, after the DM subsystem has bound all > watchdog drivers. This enables the use of the currently implemented > logic of using the correct watchdog in U-Boot (via alias etc). > > arch/mips/mach-mt7620/cpu.c | 36 ---------------- > board/CZ.NIC/turris_mox/turris_mox.c | 30 -------------- > board/CZ.NIC/turris_omnia/turris_omnia.c | 35 ---------------- > .../microblaze-generic/microblaze-generic.c | 40 ------------------ > board/xilinx/zynq/board.c | 39 ------------------ > board/xilinx/zynqmp/zynqmp.c | 39 ------------------ > common/board_r.c | 41 +++++++++++++++++++ > drivers/watchdog/Kconfig | 1 + > drivers/watchdog/wdt-uclass.c | 26 ++++++++++++ > include/asm-generic/global_data.h | 4 ++ > include/configs/turris_omnia.h | 5 --- > 11 files changed, 72 insertions(+), 224 deletions(-) > > diff --git a/arch/mips/mach-mt7620/cpu.c b/arch/mips/mach-mt7620/cpu.c > index fe74f26a54..fcd0484a6d 100644 > --- a/arch/mips/mach-mt7620/cpu.c > +++ b/arch/mips/mach-mt7620/cpu.c > @@ -69,28 +69,6 @@ int print_cpuinfo(void) > return 0; > } > > -#ifdef CONFIG_WATCHDOG > -static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; > - > -/* Called by macro WATCHDOG_RESET */ > -void watchdog_reset(void) > -{ > - static ulong next_reset; > - ulong now; > - > - if (!watchdog_dev) > - return; > - > - now = get_timer(0); > - > - /* Do not reset the watchdog too often */ > - if (now > next_reset) { > - next_reset = now + 1000; /* reset every 1000ms */ > - wdt_reset(watchdog_dev); > - } > -} > -#endif > - > int arch_misc_init(void) > { > /* > @@ -103,19 +81,5 @@ int arch_misc_init(void) > flush_dcache_range(gd->bd->bi_memstart, > gd->bd->bi_memstart + gd->ram_size - 1); > > -#ifdef CONFIG_WATCHDOG > - /* Init watchdog */ > - if (uclass_get_device_by_seq(UCLASS_WDT, 0, &watchdog_dev)) { > - debug("Watchdog: Not found by seq!\n"); > - if (uclass_get_device(UCLASS_WDT, 0, &watchdog_dev)) { > - puts("Watchdog: Not found!\n"); > - return 0; > - } > - } > - > - wdt_start(watchdog_dev, 60000, 0); /* 60 seconds */ > - printf("Watchdog: Started\n"); > -#endif > - > return 0; > } > diff --git a/board/CZ.NIC/turris_mox/turris_mox.c > b/board/CZ.NIC/turris_mox/turris_mox.c > index 96cb9c7e5c..8a4872343b 100644 > --- a/board/CZ.NIC/turris_mox/turris_mox.c > +++ b/board/CZ.NIC/turris_mox/turris_mox.c > @@ -119,41 +119,11 @@ int board_fix_fdt(void *blob) > } > #endif > > -#ifdef CONFIG_WDT_ARMADA_37XX > -static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; > - > -void watchdog_reset(void) > -{ > - static ulong next_reset; > - ulong now; > - > - if (!watchdog_dev) > - return; > - > - now = timer_get_us(); > - > - /* Do not reset the watchdog too often */ > - if (now > next_reset) { > - wdt_reset(watchdog_dev); > - next_reset = now + 100000; > - } > -} > -#endif > - > int board_init(void) > { > /* address of boot parameters */ > gd->bd->bi_boot_params = CONFIG_SYS_SDRAM_BASE + 0x100; > > -#ifdef CONFIG_WDT_ARMADA_37XX > - if (uclass_get_device(UCLASS_WDT, 0, &watchdog_dev)) { > - printf("Cannot find Armada 3720 watchdog!\n"); > - } else { > - printf("Enabling Armada 3720 watchdog (3 minutes timeout).\n"); > - wdt_start(watchdog_dev, 180000, 0); > - } > -#endif > - > return 0; > } > > diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c > b/board/CZ.NIC/turris_omnia/turris_omnia.c > index c7f6479a0c..8101cfd88a 100644 > --- a/board/CZ.NIC/turris_omnia/turris_omnia.c > +++ b/board/CZ.NIC/turris_omnia/turris_omnia.c > @@ -364,25 +364,12 @@ static bool disable_mcu_watchdog(void) > } > #endif > > -#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT_ORION) > -static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; > -#endif > - > int board_init(void) > { > /* adress of boot parameters */ > gd->bd->bi_boot_params = mvebu_sdram_bar(0) + 0x100; > > #ifndef CONFIG_SPL_BUILD > -# ifdef CONFIG_WDT_ORION > - if (uclass_get_device(UCLASS_WDT, 0, &watchdog_dev)) { > - puts("Cannot find Armada 385 watchdog!\n"); > - } else { > - puts("Enabling Armada 385 watchdog.\n"); > - wdt_start(watchdog_dev, (u32) 25000000 * 120, 0); > - } > -# endif > - > if (disable_mcu_watchdog()) > puts("Disabled MCU startup watchdog.\n"); > > @@ -392,28 +379,6 @@ int board_init(void) > return 0; > } > > -#ifdef CONFIG_WATCHDOG > -/* Called by macro WATCHDOG_RESET */ > -void watchdog_reset(void) > -{ > -# if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT_ORION) > - static ulong next_reset = 0; > - ulong now; > - > - if (!watchdog_dev) > - return; > - > - now = timer_get_us(); > - > - /* Do not reset the watchdog too often */ > - if (now > next_reset) { > - wdt_reset(watchdog_dev); > - next_reset = now + 1000; > - } > -# endif > -} > -#endif > - > int board_late_init(void) > { > #ifndef CONFIG_SPL_BUILD > diff --git a/board/xilinx/microblaze-generic/microblaze-generic.c > b/board/xilinx/microblaze-generic/microblaze-generic.c > index 28c9efa3a2..ba82292e35 100644 > --- a/board/xilinx/microblaze-generic/microblaze-generic.c > +++ b/board/xilinx/microblaze-generic/microblaze-generic.c > @@ -24,10 +24,6 @@ > > DECLARE_GLOBAL_DATA_PTR; > > -#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) > -static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; > -#endif /* !CONFIG_SPL_BUILD && CONFIG_WDT */ > - > ulong ram_base; > > int dram_init_banksize(void) > @@ -43,44 +39,8 @@ int dram_init(void) > return 0; > }; > > -#ifdef CONFIG_WDT > -/* Called by macro WATCHDOG_RESET */ > -void watchdog_reset(void) > -{ > -#if !defined(CONFIG_SPL_BUILD) > - ulong now; > - static ulong next_reset; > - > - if (!watchdog_dev) > - return; > - > - now = timer_get_us(); > - > - /* Do not reset the watchdog too often */ > - if (now > next_reset) { > - wdt_reset(watchdog_dev); > - next_reset = now + 1000; > - } > -#endif /* !CONFIG_SPL_BUILD */ > -} > -#endif /* CONFIG_WDT */ > - > int board_late_init(void) > { > -#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) > - watchdog_dev = NULL; > - > - if (uclass_get_device_by_seq(UCLASS_WDT, 0, &watchdog_dev)) { > - debug("Watchdog: Not found by seq!\n"); > - if (uclass_get_device(UCLASS_WDT, 0, &watchdog_dev)) { > - puts("Watchdog: Not found!\n"); > - return 0; > - } > - } > - > - wdt_start(watchdog_dev, 0, 0); > - puts("Watchdog: Started\n"); > -#endif /* !CONFIG_SPL_BUILD && CONFIG_WDT */ > #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_SYSRESET_MICROBLAZE) > int ret; > > diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c > index ea26aad16f..6857f2c0b8 100644 > --- a/board/xilinx/zynq/board.c > +++ b/board/xilinx/zynq/board.c > @@ -18,10 +18,6 @@ > > DECLARE_GLOBAL_DATA_PTR; > > -#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) > -static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; > -#endif > - > #if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_BOARD_EARLY_INIT_F) > int board_early_init_f(void) > { > @@ -31,19 +27,6 @@ int board_early_init_f(void) > > int board_init(void) > { > -#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) > - if (uclass_get_device_by_seq(UCLASS_WDT, 0, &watchdog_dev)) { > - debug("Watchdog: Not found by seq!\n"); > - if (uclass_get_device(UCLASS_WDT, 0, &watchdog_dev)) { > - puts("Watchdog: Not found!\n"); > - return 0; > - } > - } > - > - wdt_start(watchdog_dev, 0, 0); > - puts("Watchdog: Started\n"); > -# endif > - > return 0; > } > > @@ -127,25 +110,3 @@ int dram_init(void) > return 0; > } > #endif > - > -#if defined(CONFIG_WATCHDOG) > -/* Called by macro WATCHDOG_RESET */ > -void watchdog_reset(void) > -{ > -# if !defined(CONFIG_SPL_BUILD) > - static ulong next_reset; > - ulong now; > - > - if (!watchdog_dev) > - return; > - > - now = timer_get_us(); > - > - /* Do not reset the watchdog too often */ > - if (now > next_reset) { > - wdt_reset(watchdog_dev); > - next_reset = now + 1000; > - } > -# endif > -} > -#endif > diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c > index db27247850..1c12891b82 100644 > --- a/board/xilinx/zynqmp/zynqmp.c > +++ b/board/xilinx/zynqmp/zynqmp.c > @@ -24,10 +24,6 @@ > > DECLARE_GLOBAL_DATA_PTR; > > -#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) > -static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; > -#endif > - > #if defined(CONFIG_FPGA) && defined(CONFIG_FPGA_ZYNQMPPL) && \ > !defined(CONFIG_SPL_BUILD) > static xilinx_desc zynqmppl = XILINX_ZYNQMP_DESC; > @@ -340,44 +336,9 @@ int board_init(void) > } > #endif > > -#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) > - if (uclass_get_device_by_seq(UCLASS_WDT, 0, &watchdog_dev)) { > - debug("Watchdog: Not found by seq!\n"); > - if (uclass_get_device(UCLASS_WDT, 0, &watchdog_dev)) { > - puts("Watchdog: Not found!\n"); > - return 0; > - } > - } > - > - wdt_start(watchdog_dev, 0, 0); > - puts("Watchdog: Started\n"); > -#endif > - > return 0; > } > > -#ifdef CONFIG_WATCHDOG > -/* Called by macro WATCHDOG_RESET */ > -void watchdog_reset(void) > -{ > -# if !defined(CONFIG_SPL_BUILD) > - static ulong next_reset; > - ulong now; > - > - if (!watchdog_dev) > - return; > - > - now = timer_get_us(); > - > - /* Do not reset the watchdog too often */ > - if (now > next_reset) { > - wdt_reset(watchdog_dev); > - next_reset = now + 1000; > - } > -# endif > -} > -#endif > - > int board_early_init_r(void) > { > u32 val; > diff --git a/common/board_r.c b/common/board_r.c > index 472987d5d5..d457b6715f 100644 > --- a/common/board_r.c > +++ b/common/board_r.c > @@ -48,6 +48,7 @@ > #include <linux/compiler.h> > #include <linux/err.h> > #include <efi_loader.h> > +#include <wdt.h> > > DECLARE_GLOBAL_DATA_PTR; > > @@ -621,6 +622,43 @@ static int initr_bedbug(void) > } > #endif > > +#if defined(CONFIG_WDT) > +#ifndef CONFIG_WATCHDOG_TIMEOUT_MSECS > +#define CONFIG_WATCHDOG_TIMEOUT_MSECS (60 * 1000) > +#endif > +#define WATCHDOG_TIMEOUT_SECS (CONFIG_WATCHDOG_TIMEOUT_MSECS / 1000) > + > +static 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 > + */ > + if (uclass_get_device_by_seq(UCLASS_WDT, 0, > + (struct udevice **)&gd->watchdog_dev)) { > + debug("WDT: Not found by seq!\n"); > + if (uclass_get_device(UCLASS_WDT, 0, > + (struct udevice **)&gd->watchdog_dev)) { > + printf("WDT: Not found!\n"); > + return 0; > + } > + } > + > + if (CONFIG_IS_ENABLED(OF_CONTROL)) { > + timeout = dev_read_u32_default(gd->watchdog_dev, "timeout-sec", > + WATCHDOG_TIMEOUT_SECS); > + } > + > + wdt_start(gd->watchdog_dev, timeout * 1000, 0); > + gd->flags |= GD_FLG_WDT_READY; > + printf("WDT: Started (%ds timeout)\n", timeout); > + > + return 0; > +} > +#endif > + > static int run_main_loop(void) > { > #ifdef CONFIG_SANDBOX > @@ -670,6 +708,9 @@ static init_fnc_t init_sequence_r[] = { > #ifdef CONFIG_DM > initr_dm, > #endif > +#if defined(CONFIG_WDT) > + initr_watchdog, > +#endif > #if defined(CONFIG_ARM) || defined(CONFIG_NDS32) || defined(CONFIG_RISCV) || > \ > defined(CONFIG_SANDBOX) > board_init, /* Setup chipselects */ > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig > index 34e78beb2a..82080d3867 100644 > --- a/drivers/watchdog/Kconfig > +++ b/drivers/watchdog/Kconfig > @@ -51,6 +51,7 @@ config ULP_WATCHDOG > config WDT > bool "Enable driver model for watchdog timer drivers" > depends on DM > + imply WATCHDOG > help > Enable driver model for watchdog timer. At the moment the API > is very simple and only supports four operations: > diff --git a/drivers/watchdog/wdt-uclass.c b/drivers/watchdog/wdt-uclass.c > index 23b7e3360d..bbfac4f0f9 100644 > --- a/drivers/watchdog/wdt-uclass.c > +++ b/drivers/watchdog/wdt-uclass.c > @@ -10,6 +10,8 @@ > #include <dm/device-internal.h> > #include <dm/lists.h> > > +DECLARE_GLOBAL_DATA_PTR; > + > int wdt_start(struct udevice *dev, u64 timeout_ms, ulong flags) > { > const struct wdt_ops *ops = device_get_ops(dev); > @@ -63,6 +65,30 @@ int wdt_expire_now(struct udevice *dev, ulong flags) > return ret; > } > > +#if defined(CONFIG_WATCHDOG) > +/* > + * Called by macro WATCHDOG_RESET. This function be called *very* early, > + * so we need to make sure, that the watchdog driver is ready before using > + * it in this function. > + */ > +void watchdog_reset(void) > +{ > + static ulong next_reset; > + ulong now; > + > + /* Exit if GD is not ready or watchdog is not initialized yet */ > + if (!gd || !(gd->flags & GD_FLG_WDT_READY)) > + return; > + > + /* Do not reset the watchdog too often */ > + now = get_timer(0); > + if (now > next_reset) { > + next_reset = now + 1000; /* reset every 1000ms */ > + wdt_reset(gd->watchdog_dev); > + } > +} > +#endif > + > static int wdt_post_bind(struct udevice *dev) > { > #if defined(CONFIG_NEEDS_MANUAL_RELOC) > diff --git a/include/asm-generic/global_data.h > b/include/asm-generic/global_data.h > index 78dcf40bff..d16f50f677 100644 > --- a/include/asm-generic/global_data.h > +++ b/include/asm-generic/global_data.h > @@ -133,6 +133,9 @@ typedef struct global_data { > struct spl_handoff *spl_handoff; > # endif > #endif > +#if defined(CONFIG_WDT) > + struct udevice *watchdog_dev; > +#endif > } gd_t; > #endif > > @@ -161,5 +164,6 @@ typedef struct global_data { > #define GD_FLG_ENV_DEFAULT 0x02000 /* Default variable flag */ > #define GD_FLG_SPL_EARLY_INIT 0x04000 /* Early SPL init is done > */ > #define GD_FLG_LOG_READY 0x08000 /* Log system is ready for use */ > +#define GD_FLG_WDT_READY 0x10000 /* Watchdog is ready for use */ > > #endif /* __ASM_GENERIC_GBL_DATA_H */ > diff --git a/include/configs/turris_omnia.h b/include/configs/turris_omnia.h > index 038f6398eb..78e3cf2ff4 100644 > --- a/include/configs/turris_omnia.h > +++ b/include/configs/turris_omnia.h > @@ -29,11 +29,6 @@ > #define CONFIG_SPL_I2C_MUX > #define CONFIG_SYS_I2C_MVTWSI > > -/* Watchdog support */ > -#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT_ORION) > -# define CONFIG_WATCHDOG > -#endif > - > /* SPI NOR flash default params, used by sf commands */ > #define CONFIG_SPI_FLASH_SPANSION > >
Reviewed-by: Michal Simek <[email protected]> Tested-by: Michal Simek <[email protected]> (on zcu100) Maybe at some point we should extend this message to also state that it is not serviced. Something like: WDT: Started without servicing (10s timeout) Thanks, Michal _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
