HI, 2018-02-28 9:55 GMT+01:00 Lukasz Majewski <[email protected]>:
> On Mon, 26 Feb 2018 10:09:55 +0100 > Michal Simek <[email protected]> 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 <[email protected]> > > --- > > > > 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(); > Unfortunately I didn't find a way how to do it with less code. If you see a way to optimize it please let me know. hw_watchdog_init is not using DM. I used similar solution which is used by turris omnia. Code needs to handle reference to watchdog_dev that's why the code is there. > > 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? > The reason is that it depends how exactly you want to use it. Also adding dependency on WATCHDOG not WDT is reasonable because you can just start watchdog in u-boot but never service it. It should enable you and option to boot till certain time or reboot. I have tested that this setting is working. I haven't tested if Linux cadence driver can handle it but that's different topic. Definitely please let me know if you see any issue with this patch. Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/ Maintainer of Linux kernel - Xilinx Zynq ARM architecture Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
