On 21.03.2019 14:00, Stefan Roese wrote: > External E-Mail > > > 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.
It should at least build :) > > But the code looks cleaner without this #ifdef and if you agree, I > will send v2 soon with this change. I did not have time to review all the patch series yet. So more reviews will follow > >> >>> +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(). I do not mind if you keep it Eugen > > Thanks, > Stefan _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot