[U-Boot] [PATCH 1/4 v4] watchdog: Implement generic watchdog_reset() version
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@denx.de Cc: Heiko Schocher hs@denx.de Cc: Tom Rini trini@konsulko.com Cc: Michal Simek michal.simek@xilinx.com Cc: "Marek Behún" marek.behun@nic.cz Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Maxim Sloyko maxims@google.com Cc: Erik van Luijk evanluijk@interact.nl Cc: Ryder Lee ryder.lee@mediatek.com Cc: Weijie Gao weijie.gao@mediatek.com Cc: Simon Glass sjg@chromium.org Cc: "Álvaro Fernández Rojas" noltari@gmail.com Cc: Philippe Reynes philippe.reynes@softathome.com Cc: Christophe Leroy christophe.leroy@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@denx.de smartweb: Heiko Schocher hs@denx.de ast2500: Maxim Sloyko maxims@google.com picosam9g45: Erik van Luijk evanluijk@interact.nl mt7623n_bpir2: Ryder Lee ryder.lee@mediatek.com Weijie Gao weijie.gao@mediatek.com mt7629_rfb: Ryder Lee ryder.lee@mediatek.com Weijie Gao weijie.gao@mediatek.com bitmain_antminer_s9: Michal Simek monstr@monstr.eu sandbox64: Simon Glass sjg@chromium.org sandbox: Simon Glass sjg@chromium.org comtrend_ct5361_ram: "Álvaro Fernández Rojas" noltari@gmail.com netgear_cg3100d_ram: "Álvaro Fernández Rojas" noltari@gmail.com bcm968380gerg_ram: Philippe Reynes philippe.reynes@softathome.com bcm963158_ram: Philippe Reynes philippe.reynes@softathome.com bcm968580xref_ram: Philippe Reynes philippe.reynes@softathome.com MCR3000: Christophe Leroy christophe.leroy@c-s.fr
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
Now that we have a generic DT property "timeout-sec" handling, the driver specific implementation can be dropped.
This patch also changes the timeout restriction to the min and max values (clipping). Before this patch, the value provided via "timeout-sec" was used if the parameter was too high or low. Now the driver specific min and max values are used instead.
Signed-off-by: Stefan Roese sr@denx.de Cc: Michal Simek michal.simek@xilinx.com --- v4: - No Change
v3: - Divide timeout in _start() by 1000 to get value in seconds
v2: - New patch
drivers/watchdog/cdns_wdt.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/watchdog/cdns_wdt.c b/drivers/watchdog/cdns_wdt.c index fc85fbcec2..6a608b6371 100644 --- a/drivers/watchdog/cdns_wdt.c +++ b/drivers/watchdog/cdns_wdt.c @@ -10,6 +10,7 @@ #include <dm.h> #include <wdt.h> #include <clk.h> +#include <div64.h> #include <linux/io.h>
DECLARE_GLOBAL_DATA_PTR; @@ -23,7 +24,6 @@ struct cdns_regs {
struct cdns_wdt_priv { bool rst; - u32 timeout; struct cdns_regs *regs; };
@@ -142,10 +142,10 @@ static int cdns_wdt_start(struct udevice *dev, u64 timeout, ulong flags) return -1; }
- if ((timeout < CDNS_WDT_MIN_TIMEOUT) || - (timeout > CDNS_WDT_MAX_TIMEOUT)) { - timeout = priv->timeout; - } + /* Calculate timeout in seconds and restrict to min and max value */ + do_div(timeout, 1000); + timeout = max_t(u64, timeout, CDNS_WDT_MIN_TIMEOUT); + timeout = min_t(u64, timeout, CDNS_WDT_MAX_TIMEOUT);
debug("%s: CLK_FREQ %ld, timeout %lld\n", __func__, clk_f, timeout);
@@ -235,12 +235,9 @@ static int cdns_wdt_ofdata_to_platdata(struct udevice *dev) if (IS_ERR(priv->regs)) return PTR_ERR(priv->regs);
- priv->timeout = dev_read_u32_default(dev, "timeout-sec", - CDNS_WDT_DEFAULT_TIMEOUT); - priv->rst = dev_read_bool(dev, "reset-on-timeout");
- debug("%s: timeout %d, reset %d\n", __func__, priv->timeout, priv->rst); + debug("%s: reset %d\n", __func__, priv->rst);
return 0; }
On 11. 04. 19 15:58, Stefan Roese wrote:
Now that we have a generic DT property "timeout-sec" handling, the driver specific implementation can be dropped.
This patch also changes the timeout restriction to the min and max values (clipping). Before this patch, the value provided via "timeout-sec" was used if the parameter was too high or low. Now the driver specific min and max values are used instead.
Signed-off-by: Stefan Roese sr@denx.de Cc: Michal Simek michal.simek@xilinx.com
v4:
- No Change
v3:
- Divide timeout in _start() by 1000 to get value in seconds
v2:
- New patch
drivers/watchdog/cdns_wdt.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/watchdog/cdns_wdt.c b/drivers/watchdog/cdns_wdt.c index fc85fbcec2..6a608b6371 100644 --- a/drivers/watchdog/cdns_wdt.c +++ b/drivers/watchdog/cdns_wdt.c @@ -10,6 +10,7 @@ #include <dm.h> #include <wdt.h> #include <clk.h> +#include <div64.h> #include <linux/io.h>
DECLARE_GLOBAL_DATA_PTR; @@ -23,7 +24,6 @@ struct cdns_regs {
struct cdns_wdt_priv { bool rst;
- u32 timeout; struct cdns_regs *regs;
};
@@ -142,10 +142,10 @@ static int cdns_wdt_start(struct udevice *dev, u64 timeout, ulong flags) return -1; }
- if ((timeout < CDNS_WDT_MIN_TIMEOUT) ||
(timeout > CDNS_WDT_MAX_TIMEOUT)) {timeout = priv->timeout;- }
/* Calculate timeout in seconds and restrict to min and max value */
do_div(timeout, 1000);
timeout = max_t(u64, timeout, CDNS_WDT_MIN_TIMEOUT);
timeout = min_t(u64, timeout, CDNS_WDT_MAX_TIMEOUT);
debug("%s: CLK_FREQ %ld, timeout %lld\n", __func__, clk_f, timeout);
@@ -235,12 +235,9 @@ static int cdns_wdt_ofdata_to_platdata(struct udevice *dev) if (IS_ERR(priv->regs)) return PTR_ERR(priv->regs);
priv->timeout = dev_read_u32_default(dev, "timeout-sec",
CDNS_WDT_DEFAULT_TIMEOUT);priv->rst = dev_read_bool(dev, "reset-on-timeout");
debug("%s: timeout %d, reset %d\n", __func__, priv->timeout, priv->rst);
debug("%s: reset %d\n", __func__, priv->rst);
return 0;
}
Reviewed-by: Michal Simek michal.simek@xilinx.com Tested-by: Michal Simek michal.simek@xilinx.com (on zcu100)
Thanks, Michal
With the generic watchdog driver now implemented, this patch removes some legacy stuff from the MPC8xx watchdog driver and its Kconfig integration. CONFIG_MPC8xx_WATCHDOG is completely removed and hw_watchdog_reset() is made static, as the watchdog will now get serviced via the DM infrastructure if enabled via CONFIG_WATCHDOG.
Signed-off-by: Stefan Roese sr@denx.de Cc: Christophe Leroy christophe.leroy@c-s.fr --- v4: - New patch
arch/powerpc/Kconfig | 2 +- arch/powerpc/cpu/mpc8xx/Kconfig | 6 +++--- drivers/watchdog/Kconfig | 1 - drivers/watchdog/Makefile | 2 +- drivers/watchdog/mpc8xx_wdt.c | 4 +--- 5 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index c727d9162c..0b1629ba62 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -35,7 +35,7 @@ config MPC8xx bool "MPC8xx" select BOARD_EARLY_INIT_F imply CMD_REGINFO - imply MPC8xx_WATCHDOG + imply WDT_MPC8xx
endchoice
diff --git a/arch/powerpc/cpu/mpc8xx/Kconfig b/arch/powerpc/cpu/mpc8xx/Kconfig index b0e90a0f20..3e8ea38529 100644 --- a/arch/powerpc/cpu/mpc8xx/Kconfig +++ b/arch/powerpc/cpu/mpc8xx/Kconfig @@ -25,9 +25,9 @@ config MPC885
endchoice
-config MPC8xx_WATCHDOG - bool "Watchdog" - select HW_WATCHDOG +#config MPC8xx_WATCHDOG +# bool "Watchdog" +# select HW_WATCHDOG
config 8xx_GCLK_FREQ int "CPU GCLK Frequency" diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig index 82080d3867..8b74a67bcc 100644 --- a/drivers/watchdog/Kconfig +++ b/drivers/watchdog/Kconfig @@ -150,7 +150,6 @@ config WDT_MT7621 config WDT_MPC8xx bool "MPC8xx watchdog timer support" depends on WDT && MPC8xx - select CONFIG_MPC8xx_WATCHDOG help Select this to enable mpc8xx watchdog timer
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile index d901240ad1..40b2f4bc66 100644 --- a/drivers/watchdog/Makefile +++ b/drivers/watchdog/Makefile @@ -24,6 +24,6 @@ obj-$(CONFIG_WDT_BCM6345) += bcm6345_wdt.o obj-$(CONFIG_BCM2835_WDT) += bcm2835_wdt.o obj-$(CONFIG_WDT_ORION) += orion_wdt.o obj-$(CONFIG_WDT_CDNS) += cdns_wdt.o -obj-$(CONFIG_MPC8xx_WATCHDOG) += mpc8xx_wdt.o +obj-$(CONFIG_WDT_MPC8xx) += mpc8xx_wdt.o obj-$(CONFIG_WDT_MT7621) += mt7621_wdt.o obj-$(CONFIG_WDT_MTK) += mtk_wdt.o diff --git a/drivers/watchdog/mpc8xx_wdt.c b/drivers/watchdog/mpc8xx_wdt.c index c24c2a9da6..675b62d8b3 100644 --- a/drivers/watchdog/mpc8xx_wdt.c +++ b/drivers/watchdog/mpc8xx_wdt.c @@ -10,7 +10,7 @@ #include <asm/cpm_8xx.h> #include <asm/io.h>
-void hw_watchdog_reset(void) +static void hw_watchdog_reset(void) { immap_t __iomem *immap = (immap_t __iomem *)CONFIG_SYS_IMMR;
@@ -18,7 +18,6 @@ void hw_watchdog_reset(void) out_be16(&immap->im_siu_conf.sc_swsr, 0xaa39); /* write magic2 */ }
-#ifdef CONFIG_WDT_MPC8xx static int mpc8xx_wdt_start(struct udevice *dev, u64 timeout, ulong flags) { immap_t __iomem *immap = (immap_t __iomem *)CONFIG_SYS_IMMR; @@ -66,4 +65,3 @@ U_BOOT_DRIVER(wdt_mpc8xx) = { .of_match = mpc8xx_wdt_ids, .ops = &mpc8xx_wdt_ops, }; -#endif /* CONFIG_WDT_MPC8xx */
With the 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).
The watchdog servicing is enabled via CONFIG_WATCHDOG.
Signed-off-by: Stefan Roese sr@denx.de Cc: Heiko Schocher hs@denx.de Cc: Eugen Hristev eugen.hristev@microchip.com --- v4: - New patch (as this at91 watchdog changes just hit mainline)
arch/arm/mach-at91/clock.c | 45 ------------------- arch/arm/mach-at91/include/mach/at91_wdt.h | 2 - .../gardena-smart-gateway-at91sam_defconfig | 2 - drivers/watchdog/at91sam9_wdt.c | 8 ---- 4 files changed, 57 deletions(-)
diff --git a/arch/arm/mach-at91/clock.c b/arch/arm/mach-at91/clock.c index 1d3df2c09d..8344daeb39 100644 --- a/arch/arm/mach-at91/clock.c +++ b/arch/arm/mach-at91/clock.c @@ -14,8 +14,6 @@
#define EN_UPLL_TIMEOUT 500
-static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL; - void at91_periph_clk_enable(int id) { struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC; @@ -123,46 +121,3 @@ void at91_pllicpr_init(u32 icpr)
writel(icpr, &pmc->pllicpr); } - -/* 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); - } -} - -int arch_early_init_r(void) -{ - struct at91_wdt_priv *priv; - - /* 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; - } - } - - priv = dev_get_priv(watchdog_dev); - if (!priv) { - printf("Watchdog: priv not available!\n"); - return 0; - } - - wdt_start(watchdog_dev, priv->timeout * 1000, 0); - printf("Watchdog: Started\n"); - - return 0; -} diff --git a/arch/arm/mach-at91/include/mach/at91_wdt.h b/arch/arm/mach-at91/include/mach/at91_wdt.h index a8fc73b3d1..8ef8e007d7 100644 --- a/arch/arm/mach-at91/include/mach/at91_wdt.h +++ b/arch/arm/mach-at91/include/mach/at91_wdt.h @@ -28,7 +28,6 @@ typedef struct at91_wdt { struct at91_wdt_priv { void __iomem *regs; u32 regval; - u32 timeout; };
#endif @@ -51,6 +50,5 @@ struct at91_wdt_priv {
/* Hardware timeout in seconds */ #define WDT_MAX_TIMEOUT 16 -#define WDT_DEFAULT_TIMEOUT 2
#endif diff --git a/configs/gardena-smart-gateway-at91sam_defconfig b/configs/gardena-smart-gateway-at91sam_defconfig index b395a5a175..956897dc7f 100644 --- a/configs/gardena-smart-gateway-at91sam_defconfig +++ b/configs/gardena-smart-gateway-at91sam_defconfig @@ -23,7 +23,6 @@ CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="console=ttyS0,115200 earlyprintk mtdparts=atmel_nand:256k(bootstrap)ro,768k(uboot)ro,256k(env_redundant),256k(env),512k(dtb),6M(kernel)ro,-(rootfs) rootfstype=ubifs ubi.mtd=6 root=ubi0:rootfs rw" CONFIG_SYS_CONSOLE_IS_IN_ENV=y CONFIG_SYS_CONSOLE_INFO_QUIET=y -CONFIG_ARCH_EARLY_INIT_R=y CONFIG_SPL_SYS_MALLOC_SIMPLE=y CONFIG_SPL_SEPARATE_BSS=y # CONFIG_TPL_BANNER_PRINT is not set @@ -76,7 +75,6 @@ CONFIG_TIMER=y CONFIG_SPL_TIMER=y CONFIG_ATMEL_PIT_TIMER=y # CONFIG_SYS_WHITE_ON_BLACK is not set -CONFIG_WATCHDOG=y CONFIG_WDT=y CONFIG_WDT_AT91=y # CONFIG_UBIFS_SILENCE_MSG is not set diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c index 000769d46d..48433cc158 100644 --- a/drivers/watchdog/at91sam9_wdt.c +++ b/drivers/watchdog/at91sam9_wdt.c @@ -107,14 +107,6 @@ static int at91_wdt_probe(struct udevice *dev) if (!priv->regs) return -EINVAL;
-#if CONFIG_IS_ENABLED(OF_CONTROL) - priv->timeout = dev_read_u32_default(dev, "timeout-sec", - WDT_DEFAULT_TIMEOUT); - debug("%s: timeout %d", __func__, priv->timeout); -#else - priv->timeout = WDT_DEFAULT_TIMEOUT; -#endif - debug("%s: Probing wdt%u\n", __func__, dev->seq);
return 0;
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 sr@denx.de Cc: Heiko Schocher hs@denx.de Cc: Tom Rini trini@konsulko.com Cc: Michal Simek michal.simek@xilinx.com Cc: "Marek Behún" marek.behun@nic.cz Cc: Daniel Schwierzeck daniel.schwierzeck@gmail.com Cc: Maxim Sloyko maxims@google.com Cc: Erik van Luijk evanluijk@interact.nl Cc: Ryder Lee ryder.lee@mediatek.com Cc: Weijie Gao weijie.gao@mediatek.com Cc: Simon Glass sjg@chromium.org Cc: "Álvaro Fernández Rojas" noltari@gmail.com Cc: Philippe Reynes philippe.reynes@softathome.com Cc: Christophe Leroy christophe.leroy@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@denx.de smartweb: Heiko Schocher hs@denx.de ast2500: Maxim Sloyko maxims@google.com picosam9g45: Erik van Luijk evanluijk@interact.nl mt7623n_bpir2: Ryder Lee ryder.lee@mediatek.com Weijie Gao weijie.gao@mediatek.com mt7629_rfb: Ryder Lee ryder.lee@mediatek.com Weijie Gao weijie.gao@mediatek.com bitmain_antminer_s9: Michal Simek monstr@monstr.eu sandbox64: Simon Glass sjg@chromium.org sandbox: Simon Glass sjg@chromium.org comtrend_ct5361_ram: "Álvaro Fernández Rojas" noltari@gmail.com netgear_cg3100d_ram: "Álvaro Fernández Rojas" noltari@gmail.com bcm968380gerg_ram: Philippe Reynes philippe.reynes@softathome.com bcm963158_ram: Philippe Reynes philippe.reynes@softathome.com bcm968580xref_ram: Philippe Reynes philippe.reynes@softathome.com MCR3000: Christophe Leroy christophe.leroy@c-s.fr
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 michal.simek@xilinx.com Tested-by: Michal Simek michal.simek@xilinx.com (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
On 12.04.19 10:22, Michal Simek wrote:
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.
<snip>
Reviewed-by: Michal Simek michal.simek@xilinx.com Tested-by: Michal Simek michal.simek@xilinx.com (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)
I also have thought about adding this state information to the message. I like your idea. To not "disturb" this series any more, I'll send a follow-up patch for this, once this patchset lands in mainline.
Thanks, Stefan
On 12. 04. 19 10:32, Stefan Roese wrote:
On 12.04.19 10:22, Michal Simek wrote:
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.
<snip>
Reviewed-by: Michal Simek michal.simek@xilinx.com Tested-by: Michal Simek michal.simek@xilinx.com (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)
I also have thought about adding this state information to the message. I like your idea. To not "disturb" this series any more, I'll send a follow-up patch for this, once this patchset lands in mainline.
follow up make sense.
Thanks, Michal
On 12.04.19 11:46, Michal Simek wrote:
On 12. 04. 19 10:32, Stefan Roese wrote:
On 12.04.19 10:22, Michal Simek wrote:
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.
<snip>
Reviewed-by: Michal Simek michal.simek@xilinx.com Tested-by: Michal Simek michal.simek@xilinx.com (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)
I also have thought about adding this state information to the message. I like your idea. To not "disturb" this series any more, I'll send a follow-up patch for this, once this patchset lands in mainline.
follow up make sense.
I need to do a v5, since we now have a board with watchdog support in SPL in mainline (x530) and I need to make a few changes to support this with the generic approach as well. I'll fold the "with/out servicing" message into this new version as well. Stay tuned...
Thanks, Stefan
Hi Stefan,
Just noticed something when updating our internal u-boot repo
On Fri, Apr 12, 2019 at 2:00 AM Stefan Roese sr@denx.de 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.
<snip>
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) {
If the counter wraps we stop tickling the watchdog and the board will reset. On our 32-bit arm boards this is only about 72 minutes so it got noticed in production testing. The fix we had in our board code was:
+#ifndef time_after +#define time_after(a,b) \ + ((long)((b) - (a)) < 0) +#endif
@@ -202,7 +208,7 @@ void watchdog_reset(void) now = timer_get_us();
/* Do not reset the watchdog too often */ - if (now > next_reset) { + if (time_after(now, next_reset)) { wdt_reset(watchdog_dev); next_reset = now + 1000; }
I'll send a proper patch (gmail will eat the whitespace in the inline diff).
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)
participants (3)
-
Chris Packham -
Michal Simek -
Stefan Roese