On 21.03.19 11:23, eugen.hris...@microchip.com wrote:
On 19.03.2019 17:56, Stefan Roese wrote:
External E-Mail
This patch enables and starts the watchdog on the AT91 platform if
configured. Currently the WD timeout is configured to 16 seconds,
which is the longest value for this timer.
Signed-off-by: Stefan Roese <s...@denx.de>
Cc: Heiko Schocher <h...@denx.de>
Cc: Andreas Bießmann <andr...@biessmann.org>
Cc: Eugen Hristev <eugen.hris...@microchip.com>
---
arch/arm/mach-at91/clock.c | 41 ++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/arch/arm/mach-at91/clock.c b/arch/arm/mach-at91/clock.c
index 64cbc3d1ed..2d442d0092 100644
--- a/arch/arm/mach-at91/clock.c
+++ b/arch/arm/mach-at91/clock.c
@@ -5,6 +5,8 @@
*/
#include <common.h>
+#include <dm.h>
+#include <wdt.h>
#include <asm/io.h>
#include <asm/arch/hardware.h>
#include <asm/arch/at91_pmc.h>
@@ -118,3 +120,42 @@ void at91_pllicpr_init(u32 icpr)
writel(icpr, &pmc->pllicpr);
}
+
+#if defined(CONFIG_WATCHDOG) && !defined(CONFIG_SPL_BUILD)
Hi Stefan,
Does this mean that for CONFIG_SPL_BUILD, this functions won't exist,
thus the SPL cannot use the watchdog ?
For example, configuring CONFIG_SPL_WATCHDOG_SUPPORT=y
makes SPL build fail :
drivers/built-in.o: In function `atmel_nand_pmecc_write_page':
/home/eugen/u-boot-denx/drivers/mtd/nand/raw/atmel_nand.c:592: undefined
reference to `watchdog_reset'
drivers/built-in.o: In function `atmel_nand_pmecc_read_page':
/home/eugen/u-boot-denx/drivers/mtd/nand/raw/atmel_nand.c:552: undefined
reference to `watchdog_reset'
drivers/built-in.o: In function `pmecc_err_location':
/home/eugen/u-boot-denx/drivers/mtd/nand/raw/atmel_nand.c:416: undefined
reference to `watchdog_reset'
scripts/Makefile.spl:384: recipe for target 'spl/u-boot-spl' failed
Let me see, if I can change this so that this code is
available if selected in SPL as well. Even though arch_misc_init()
will not be called, so the watchdog will not be started.
But the code looks cleaner without this #ifdef and if you agree, I
will send v2 soon with this change.
+static struct udevice *watchdog_dev;
+
+/* 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_misc_init(void)
+{
+ /* 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, 16000, 0); /* 16 seconds is max */
+ printf("Watchdog: Started\n");
+
Any reason why you use printf and puts and debug in the same function ?
Would expect to see debug everywhere except some fatal error that should
be printed all the time.
Good catch. This is copy-pasted from other very similar implementations.
I personally find this last status message "Watchdog: Started" quite
useful and would like to keep it. All others might be changed to debug().
Thanks,
Stefan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
