On Mon, 26 Feb 2018 10:09:55 +0100 Michal Simek michal.simek@xilinx.com wrote:
Watchdog is only enabled in full u-boot. Adoption for SPL should be also done because that's the right place where watchdog should be enabled.
Signed-off-by: Michal Simek michal.simek@xilinx.com
Changes in v2:
- Make watchdog_reset depends on CONFIG_WATCHDOG not WDT This will handle use cases where watchdog is cleared by OS
arch/arm/Kconfig | 1 + board/xilinx/zynq/board.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 2c52ff025a22..a66d04eadfcb 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -761,6 +761,7 @@ config ARCH_ZYNQ select SUPPORT_SPL select OF_CONTROL select SPL_BOARD_INIT if SPL
- select BOARD_EARLY_INIT_F if WDT select SPL_OF_CONTROL if SPL select DM select DM_ETH if NET
diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c index fb8eab07d768..838ac0f4c4ea 100644 --- a/board/xilinx/zynq/board.c +++ b/board/xilinx/zynq/board.c @@ -6,9 +6,11 @@ */
#include <common.h> +#include <dm/uclass.h> #include <fdtdec.h> #include <fpga.h> #include <mmc.h> +#include <wdt.h> #include <zynqpl.h> #include <asm/arch/hardware.h> #include <asm/arch/sys_proto.h> @@ -33,6 +35,22 @@ static xilinx_desc fpga045 = XILINX_XC7Z045_DESC(0x45); static xilinx_desc fpga100 = XILINX_XC7Z100_DESC(0x100); #endif
+#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT) +static struct udevice *watchdog_dev; +#endif
+#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_BOARD_EARLY_INIT_F) +int board_early_init_f(void) +{ +# if defined(CONFIG_WDT)
- /* bss is not cleared at time when watchdog_reset() is
called */
- watchdog_dev = NULL;
+# endif
- return 0;
+} +#endif
int board_init(void) { #if (defined(CONFIG_FPGA) && !defined(CONFIG_SPL_BUILD)) || \ @@ -75,6 +93,15 @@ int board_init(void) } #endif
+#if !defined(CONFIG_SPL_BUILD) && defined(CONFIG_WDT)
- if (uclass_get_device(UCLASS_WDT, 0, &watchdog_dev)) {
puts("Watchdog: Not found!\n");- } else {
wdt_start(watchdog_dev, 0, 0);puts("Watchdog: Started\n");- }
+# endif
#if (defined(CONFIG_FPGA) && !defined(CONFIG_SPL_BUILD)) || \ (defined(CONFIG_SPL_FPGA_SUPPORT) && defined(CONFIG_SPL_BUILD)) fpga_init(); @@ -164,3 +191,25 @@ 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
I do have two questions if you don't mind:
1. It seems like a lot of code added to a board file to provide WDT support. Normally we just call a few functions - like hw_watchdog_init(); WATCHDOG_RESET();
2. Is there any good reason for Watchdog_reset not being put into driver, being wrapped, so it would provide WATCHDOG_RESET() macro, which is used to refresh WDT in several places?
+} +#endif
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de