[U-Boot] [PATCH v2 2/3] arm: zynq: Wire watchdog internals

[Lukasz Majewski <lukma@denx.de>]

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 at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180228/85e5f340/attachment.sig>