[U-Boot] [PATCH 2/2] arm: mvebu: x530: Enable watchdog in SPL and U-Boot

[Stefan Roese]

Hi Chris,

On 15.02.19 03:12, Chris Packham wrote:
> Enable the hardware watchdog to guard against system lock ups when
> running in the SPL or U-Boot. Stop the watchdog just before booting so
> that the OS.
> 
> Signed-off-by: Chris Packham <judge.packham at gmail.com>
> ---
> 
>   arch/arm/dts/armada-385-atl-x530-u-boot.dtsi |  4 ++
>   board/alliedtelesis/x530/x530.c              | 48 ++++++++++++++++++++
>   configs/x530_defconfig                       |  5 ++
>   3 files changed, 57 insertions(+)
> 
> diff --git a/arch/arm/dts/armada-385-atl-x530-u-boot.dtsi b/arch/arm/dts/armada-385-atl-x530-u-boot.dtsi
> index 7074a73537fa..79b694cb84bc 100644
> --- a/arch/arm/dts/armada-385-atl-x530-u-boot.dtsi
> +++ b/arch/arm/dts/armada-385-atl-x530-u-boot.dtsi
> @@ -11,3 +11,7 @@
>   &uart0 {
>   	u-boot,dm-pre-reloc;
>   };
> +
> +&watchdog {
> +	u-boot,dm-pre-reloc;
> +};
> diff --git a/board/alliedtelesis/x530/x530.c b/board/alliedtelesis/x530/x530.c
> index d7d1942fe686..1b22b6307cd2 100644
> --- a/board/alliedtelesis/x530/x530.c
> +++ b/board/alliedtelesis/x530/x530.c
> @@ -7,6 +7,7 @@
>   #include <command.h>
>   #include <dm.h>
>   #include <i2c.h>
> +#include <wdt.h>
>   #include <asm/gpio.h>
>   #include <linux/mbus.h>
>   #include <linux/io.h>
> @@ -24,6 +25,10 @@ DECLARE_GLOBAL_DATA_PTR;
>   #define CONFIG_NVS_LOCATION		0xf4800000
>   #define CONFIG_NVS_SIZE			(512 << 10)
>   
> +#ifdef CONFIG_WATCHDOG
> +static struct udevice *watchdog_dev;
> +#endif
> +
>   static struct serdes_map board_serdes_map[] = {
>   	{PEX0, SERDES_SPEED_5_GBPS, PEX_ROOT_COMPLEX_X1, 0, 0},
>   	{DEFAULT_SERDES, SERDES_SPEED_5_GBPS, SERDES_DEFAULT_MODE, 0, 0},
> @@ -75,6 +80,10 @@ struct mv_ddr_topology_map *mv_ddr_topology_map_get(void)
>   
>   int board_early_init_f(void)
>   {
> +#ifdef CONFIG_WATCHDOG
> +	watchdog_dev = NULL;
> +#endif
> +
>   	/* Configure MPP */
>   	writel(0x00001111, MVEBU_MPP_BASE + 0x00);
>   	writel(0x00000000, MVEBU_MPP_BASE + 0x04);
> @@ -88,6 +97,17 @@ int board_early_init_f(void)
>   	return 0;
>   }
>   
> +void spl_board_init(void)
> +{
> +#ifdef CONFIG_WATCHDOG
> +	int ret;
> +
> +	ret = uclass_get_device(UCLASS_WDT, 0, &watchdog_dev);
> +	if (!ret)
> +		wdt_start(watchdog_dev, 25000000ULL * 120, 0);

This is CONFIG_SYS_TCLK? Would it make sense to use this macro
instead here?

> +#endif
> +}
> +
>   int board_init(void)
>   {
>   	/* address of boot parameters */
> @@ -100,9 +120,37 @@ int board_init(void)
>   	/* DEV_READYn is not needed for NVS, ignore it when accessing CS1 */
>   	writel(0x00004001, MVEBU_DEV_BUS_BASE + 0xc8);
>   
> +	spl_board_init();
> +
>   	return 0;
>   }
>   
> +void arch_preboot_os(void)
> +{
> +#ifdef CONFIG_WATCHDOG
> +	wdt_stop(watchdog_dev);
> +#endif
> +}

So you are not using the WDT in the OS (Linux)?

> +
> +#ifdef CONFIG_WATCHDOG
> +void watchdog_reset(void)
> +{
> +	static ulong next_reset = 0;
> +	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

When I recently implemented the watchdog support the MT7688, I
wondered why there is so much code necessary, that each board
and platform needs to copy to get this real watchdog working.
We should definitely rework this at some time, so make this more
generic with less board specific code that could be shared.

You don't need to change this here. I just wanted to express my
thoughts, as I'm stumbling over this code again.

Other than this (and your commit text change):

Reviewed-by: Stefan Roese <sr at denx.de>

Thanks,
Stefan