[U-Boot] [PATCH 1/2 v3] watchdog: Implement generic watchdog_reset() version

Stefan Roese sr at denx.de
Wed Apr 10 12:01:30 UTC 2019


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 <sr at denx.de>
Cc: Heiko Schocher <hs at denx.de>
Cc: Tom Rini <trini at konsulko.com>
Cc: Michal Simek <michal.simek at xilinx.com>
Cc: "Marek Behún" <marek.behun at nic.cz>
Cc: Daniel Schwierzeck <daniel.schwierzeck at gmail.com>
Cc: Maxim Sloyko <maxims at google.com>
Cc: Erik van Luijk <evanluijk at interact.nl>
Cc: Ryder Lee <ryder.lee at mediatek.com>
Cc: Weijie Gao <weijie.gao at mediatek.com>
Cc: Simon Glass <sjg at chromium.org>
Cc: "Álvaro Fernández Rojas" <noltari at gmail.com>
Cc: Philippe Reynes <philippe.reynes at softathome.com>
Cc: Christophe Leroy <christophe.leroy at c-s.fr>
---
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 <hs at denx.de>
smartweb: Heiko Schocher <hs at denx.de>
ast2500: Maxim Sloyko <maxims at google.com>
picosam9g45: Erik van Luijk <evanluijk at interact.nl>
mt7623n_bpir2: Ryder Lee <ryder.lee at mediatek.com> Weijie Gao <weijie.gao at mediatek.com>
mt7629_rfb: Ryder Lee <ryder.lee at mediatek.com> Weijie Gao <weijie.gao at mediatek.com>
bitmain_antminer_s9: Michal Simek <monstr at monstr.eu>
sandbox64: Simon Glass <sjg at chromium.org>
sandbox: Simon Glass <sjg at chromium.org>
comtrend_ct5361_ram: "Álvaro Fernández Rojas" <noltari at gmail.com>
netgear_cg3100d_ram: "Álvaro Fernández Rojas" <noltari at gmail.com>
bcm968380gerg_ram: Philippe Reynes <philippe.reynes at softathome.com>
bcm963158_ram: Philippe Reynes <philippe.reynes at softathome.com>
bcm968580xref_ram: Philippe Reynes <philippe.reynes at softathome.com>
MCR3000: Christophe Leroy <christophe.leroy at c-s.fr>

Thanks,
Stefan

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 115fc4551f..a0c32c6afd 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
 
-- 
2.21.0



More information about the U-Boot mailing list