[U-Boot] [PATCH 06/12 v2] arm: at91: Enable watchdog support

[Heiko Schocher]

Hello Stefan,

Am 26.03.2019 um 13:16 schrieb Stefan Roese:
> 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 <sr at denx.de>
> Cc: Heiko Schocher <hs at denx.de>
> Cc: Andreas Bießmann <andreas at biessmann.org>
> Cc: Eugen Hristev <eugen.hristev at microchip.com>
> ---
> v2:
> - Remove #ifdef to enable compilation also in SPL version
> 
>   arch/arm/mach-at91/clock.c | 39 ++++++++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
> 
> diff --git a/arch/arm/mach-at91/clock.c b/arch/arm/mach-at91/clock.c
> index 64cbc3d1ed..e3513f3473 100644
> --- a/arch/arm/mach-at91/clock.c
> +++ b/arch/arm/mach-at91/clock.c
> @@ -5,12 +5,16 @@
>    */
>   
>   #include <common.h>
> +#include <dm.h>
> +#include <wdt.h>
>   #include <asm/io.h>
>   #include <asm/arch/hardware.h>
>   #include <asm/arch/at91_pmc.h>
>   
>   #define EN_UPLL_TIMEOUT		500
>   
> +static struct udevice *watchdog_dev;

This does not work for me on the taurus Board!

This variable sits in the BSS, which is not set to 0 before
U-Boot is relocated.

On the taurus board I see:

System.map:
21040930 b next_reset.9546
21040934 b watchdog_dev
21040938 b data.8182

hexudmp u-boot.bin:
(TEXT_BASE 0x21000000)

00040920  00 00 00 00 00 00 00 00  01 00 00 00 20 00 00 21  |............ ..!|
00040930  17 00 00 00 24 00 00 21  17 00 00 00 28 00 00 21  |....$..!....(..!|
00040940  17 00 00 00 2c 00 00 21  17 00 00 00 30 00 00 21  |....,..!....0..!|

And I see before relocation 0x21000024 for *watchdog_dev ...
which leads in failure of the check "if (!watchdog_dev)"
and cpu accesses wrong addresses in the end ...

As discussed offline moving watchdog_dev to data section helps:

diff --git a/arch/arm/mach-at91/clock.c b/arch/arm/mach-at91/clock.c
index e3513f3473..45e5f2fb57 100644
--- a/arch/arm/mach-at91/clock.c
+++ b/arch/arm/mach-at91/clock.c
@@ -13,7 +13,7 @@

  #define EN_UPLL_TIMEOUT                500

-static struct udevice *watchdog_dev;
+static struct udevice *watchdog_dev __attribute__((section(".data"))) = NULL;

  void at91_periph_clk_enable(int id)
  {


The big question is, how many places do we have in code, where we access
BSS before relocation ?

May we better clear BSS very early (at last may possible on arm)?

> +
>   void at91_periph_clk_enable(int id)
>   {
>   	struct at91_pmc *pmc = (struct at91_pmc *)ATMEL_BASE_PMC;
> @@ -118,3 +122,38 @@ void at91_pllicpr_init(u32 icpr)
>   
>   	writel(icpr, &pmc->pllicpr);
>   }
> +
> +/* 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 */

Here we have now a fix wdt timeout for all at91 based boards ...

We should use the value from DTS.

Beside of this, wdt is now running fine again on the taurus board!

Thanks for this work.

> +	printf("Watchdog: Started\n");
> +
> +	return 0;
> +}
> 

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de