[U-Boot] [PATCH 0/2] at91sam9: add watchdog support

[U-Boot] [PATCH 0/2] at91sam9: add watchdog support

[Jean-Christophe PLAGNIOL-VILLARD]

	This patch series will introduce the AT91SAM9 watchdog support

	but becare aware that the watchdog can only be activate once so
	you need to activate in the bootloader and in the bootstrap
	if you use it

Best Regards,
J.

[U-Boot] [PATCH 1/2] at91sam9: add watchdog support

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 Makefile                             |    1 +
 doc/README.at91                      |    9 ++++
 drivers/serial/atmel_usart.c         |    4 +-
 drivers/watchdog/Makefile            |   46 ++++++++++++++++++++
 drivers/watchdog/at91sam9_wdt.c      |   79 ++++++++++++++++++++++++++++++++++
 include/asm-arm/arch-at91/at91_wdt.h |   38 ++++++++++++++++
 6 files changed, 176 insertions(+), 1 deletions(-)
 create mode 100644 drivers/watchdog/Makefile
 create mode 100644 drivers/watchdog/at91sam9_wdt.c
 create mode 100644 include/asm-arm/arch-at91/at91_wdt.h

diff --git a/Makefile b/Makefile
index 4a3a488..c3f089d 100644
--- a/Makefile
+++ b/Makefile
@@ -266,6 +266,7 @@ LIBS += drivers/rtc/librtc.a
 LIBS += drivers/serial/libserial.a
 LIBS += drivers/usb/libusb.a
 LIBS += drivers/video/libvideo.a
+LIBS += drivers/watchdog/libwatchdog.a
 LIBS += common/libcommon.a
 LIBS += libfdt/libfdt.a
 LIBS += api/libapi.a
diff --git a/doc/README.at91 b/doc/README.at91
index 4e3928a..1ef3b2a 100644
--- a/doc/README.at91
+++ b/doc/README.at91
@@ -2,6 +2,7 @@ Atmel AT91 Evaluation kits
 
 http://atmel.com/dyn/products/tools.asp?family_id=605#1443
 
+I. Board mapping & boot media
 ------------------------------------------------------------------------------
 AT91SAM9260EK & AT91SAM9XEEK
 ------------------------------------------------------------------------------
@@ -86,3 +87,11 @@ Environment variables
 		make at91sam9263ek_config		- use data flash (spi cs0) (default)
 		make at91sam9263ek_nandflash_config	- use nand flash
 		make at91sam9263ek_dataflash_cs0_config	- use data flash (spi cs0)
+
+II. Watchdog support
+
+	The watchdog can only be activate once so you need to activate in the
+	bootloader and in the bootstrap if you use it
+
+	The you can activate with
+	CONFIG_AT91SAM9_WATCHDOG and CONFIG_HW_WATCHDOG
diff --git a/drivers/serial/atmel_usart.c b/drivers/serial/atmel_usart.c
index f3b146c..f50552a 100644
--- a/drivers/serial/atmel_usart.c
+++ b/drivers/serial/atmel_usart.c
@@ -16,6 +16,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #include <common.h>
+#include <watchdog.h>
 
 #include <asm/io.h>
 #include <asm/arch/clk.h>
@@ -87,7 +88,8 @@ void serial_puts(const char *s)
 
 int serial_getc(void)
 {
-	while (!(usart3_readl(CSR) & USART3_BIT(RXRDY))) ;
+	while (!(usart3_readl(CSR) & USART3_BIT(RXRDY)))
+		 WATCHDOG_RESET();
 	return usart3_readl(RHR);
 }
 
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
new file mode 100644
index 0000000..200968d
--- /dev/null
+++ b/drivers/watchdog/Makefile
@@ -0,0 +1,46 @@
+#
+# (C) Copyright 2008
+# Wolfgang Denk, DENX Software Engineering, wd at denx.de.
+#
+# See file CREDITS for list of people who contributed to this
+# project.
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License as
+# published by the Free Software Foundation; either version 2 of
+# the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+# MA 02111-1307 USA
+#
+
+include $(TOPDIR)/config.mk
+
+LIB	:= $(obj)libwatchdog.a
+
+COBJS-$(CONFIG_AT91SAM9_WATCHDOG) += at91sam9_wdt.o
+
+COBJS	:= $(COBJS-y)
+SRCS	:= $(COBJS:.o=.c)
+OBJS	:= $(addprefix $(obj),$(COBJS))
+
+all:	$(LIB)
+
+$(LIB):	$(obj).depend $(OBJS)
+	$(AR) $(ARFLAGS) $@ $(OBJS)
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
new file mode 100644
index 0000000..5bb8b77
--- /dev/null
+++ b/drivers/watchdog/at91sam9_wdt.c
@@ -0,0 +1,79 @@
+/*
+ * [origin: Linux kernel drivers/watchdog/at91sam9_wdt.c]
+ *
+ * Watchdog driver for Atmel AT91SAM9x processors.
+ *
+ * Copyright (C) 2008 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
+ * Copyright (C) 2008 Renaud CERRATO r.cerrato at til-technologies.fr
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ */
+
+/*
+ * The Watchdog Timer Mode Register can be only written to once. If the
+ * timeout need to be set from U-Boot, be sure that the bootstrap doesn't
+ * write to this register. Inform Linux to it too
+ */
+
+#include <common.h>
+#include <watchdog.h>
+#include <asm/arch/hardware.h>
+#include <asm/arch/io.h>
+#include <asm/arch/at91_wdt.h>
+
+/*
+ * AT91SAM9 watchdog runs a 12bit counter @ 256Hz,
+ * use this to convert a watchdog
+ * value from/to milliseconds.
+ */
+#define ms_to_ticks(t)	(((t << 8) / 1000) - 1)
+#define ticks_to_ms(t)	(((t + 1) * 1000) >> 8)
+
+/* Hardware timeout in seconds */
+#define WDT_HW_TIMEOUT 2
+
+/*
+ * Set the watchdog time interval in 1/256Hz (write-once)
+ * Counter is 12 bit.
+ */
+static int at91_wdt_settimeout(unsigned int timeout)
+{
+	unsigned int reg;
+	unsigned int mr;
+
+	/* Check if disabled */
+	mr = at91_sys_read(AT91_WDT_MR);
+	if (mr & AT91_WDT_WDDIS) {
+		printf("sorry, watchdog is disabled\n");
+		return -1;
+	}
+
+	/*
+	 * All counting occurs at SLOW_CLOCK / 128 = 256 Hz
+	 *
+	 * Since WDV is a 12-bit counter, the maximum period is
+	 * 4096 / 256 = 16 seconds.
+	 */
+	reg = AT91_WDT_WDRSTEN	/* causes watchdog reset */
+		/* | AT91_WDT_WDRPROC	causes processor reset only */
+		| AT91_WDT_WDDBGHLT		/* disabled in debug mode */
+		| AT91_WDT_WDD			/* restart at any time */
+		| (timeout & AT91_WDT_WDV);	/* timer value */
+	at91_sys_write(AT91_WDT_MR, reg);
+
+	return 0;
+}
+
+void hw_watchdog_reset(void)
+{
+	at91_sys_write(AT91_WDT_CR, AT91_WDT_KEY | AT91_WDT_WDRSTT);
+}
+
+void hw_watchdog_init(void)
+{
+	/* 16 seconds timer, resets enabled */
+	at91_wdt_settimeout(ms_to_ticks(WDT_HW_TIMEOUT * 1000));
+}
diff --git a/include/asm-arm/arch-at91/at91_wdt.h b/include/asm-arm/arch-at91/at91_wdt.h
new file mode 100644
index 0000000..7e18537
--- /dev/null
+++ b/include/asm-arm/arch-at91/at91_wdt.h
@@ -0,0 +1,38 @@
+/*
+ * [origin: Linux kernel arch/arm/mach-at91/include/mach/at91_wdt.h]
+ *
+ * Copyright (C) 2008 Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
+ * Copyright (C) 2007 Andrew Victor
+ * Copyright (C) 2007 Atmel Corporation.
+ *
+ * Watchdog Timer (WDT) - System peripherals regsters.
+ * Based on AT91SAM9261 datasheet revision D.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef AT91_WDT_H
+#define AT91_WDT_H
+
+#define AT91_WDT_CR		(AT91_WDT + 0x00)	/* Watchdog Control Register */
+#define		AT91_WDT_WDRSTT		(1    << 0)		/* Restart */
+#define		AT91_WDT_KEY		(0xa5 << 24)		/* KEY Password */
+
+#define AT91_WDT_MR		(AT91_WDT + 0x04)	/* Watchdog Mode Register */
+#define		AT91_WDT_WDV		(0xfff << 0)		/* Counter Value */
+#define		AT91_WDT_WDFIEN		(1     << 12)		/* Fault Interrupt Enable */
+#define		AT91_WDT_WDRSTEN	(1     << 13)		/* Reset Processor */
+#define		AT91_WDT_WDRPROC	(1     << 14)		/* Timer Restart */
+#define		AT91_WDT_WDDIS		(1     << 15)		/* Watchdog Disable */
+#define		AT91_WDT_WDD		(0xfff << 16)		/* Delta Value */
+#define		AT91_WDT_WDDBGHLT	(1     << 28)		/* Debug Halt */
+#define		AT91_WDT_WDIDLEHLT	(1     << 29)		/* Idle Halt */
+
+#define AT91_WDT_SR		(AT91_WDT + 0x08)	/* Watchdog Status Register */
+#define		AT91_WDT_WDUNF		(1 << 0)		/* Watchdog Underflow */
+#define		AT91_WDT_WDERR		(1 << 1)		/* Watchdog Error */
+
+#endif
-- 
1.5.6.5

[U-Boot] [PATCH 2/2] at91sam9263ek: active watchdog support via at91sam9263ek_wdt_config

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 Makefile                        |    5 +++++
 include/configs/at91sam9263ek.h |    5 +++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index c3f089d..2fb0c42 100644
--- a/Makefile
+++ b/Makefile
@@ -2663,6 +2663,7 @@ at91sam9261ek_config	:	unconfig
 at91sam9263ek_nandflash_config \
 at91sam9263ek_dataflash_config \
 at91sam9263ek_dataflash_cs0_config \
+at91sam9263ek_wdt_config \
 at91sam9263ek_config	:	unconfig
 	@mkdir -p $(obj)include
 	@if [ "$(findstring _nandflash,$@)" ] ; then \
@@ -2672,6 +2673,10 @@ at91sam9263ek_config	:	unconfig
 		echo "#define CONFIG_SYS_USE_DATAFLASH 1"	>>$(obj)include/config.h ; \
 		$(XECHO) "... with environment variable in SPI DATAFLASH CS0" ; \
 	fi;
+	@if [ "$(findstring _wdt_, $@)" ] ; then \
+		echo "#define CONFIG_AT91SAM9_WATCHDOG" >> $(obj)include/config.h ; \
+		$(XECHO) "... configured with watchdog active"; \
+	fi
 	@$(MKCONFIG) -a at91sam9263ek arm arm926ejs at91sam9263ek atmel at91
 
 at91sam9rlek_nandflash_config \
diff --git a/include/configs/at91sam9263ek.h b/include/configs/at91sam9263ek.h
index d9ebc87..080ba8b 100644
--- a/include/configs/at91sam9263ek.h
+++ b/include/configs/at91sam9263ek.h
@@ -203,4 +203,9 @@
 #error CONFIG_USE_IRQ not supported
 #endif
 
+/* WATCHDOG */
+#ifdef CONFIG_AT91SAM9_WATCHDOG
+#define CONFIG_HW_WATCHDOG			/* watchdog */
+#endif
+
 #endif
-- 
1.5.6.5

[U-Boot] [PATCH 2/2] at91sam9263ek: active watchdog support via at91sam9263ek_wdt_config

[Wolfgang Denk]

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <1232623377-26351-3-git-send-email-plagnioj@jcrosoft.com> you wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  Makefile                        |    5 +++++
>  include/configs/at91sam9263ek.h |    5 +++++
>  2 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index c3f089d..2fb0c42 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2663,6 +2663,7 @@ at91sam9261ek_config	:	unconfig
>  at91sam9263ek_nandflash_config \
>  at91sam9263ek_dataflash_config \
>  at91sam9263ek_dataflash_cs0_config \
> +at91sam9263ek_wdt_config \
>  at91sam9263ek_config	:	unconfig
>  	@mkdir -p $(obj)include
>  	@if [ "$(findstring _nandflash,$@)" ] ; then \
> @@ -2672,6 +2673,10 @@ at91sam9263ek_config	:	unconfig
>  		echo "#define CONFIG_SYS_USE_DATAFLASH 1"	>>$(obj)include/config.h ; \
>  		$(XECHO) "... with environment variable in SPI DATAFLASH CS0" ; \
>  	fi;
> +	@if [ "$(findstring _wdt_, $@)" ] ; then \
> +		echo "#define CONFIG_AT91SAM9_WATCHDOG" >> $(obj)include/config.h ; \
> +		$(XECHO) "... configured with watchdog active"; \
> +	fi
>  	@$(MKCONFIG) -a at91sam9263ek arm arm926ejs at91sam9263ek atmel at91

Sorry for the late reply - but this makes no sense to me.

CONFIG_ settings belong into the board config file, where a user
expects to find user configurable settings.

Why would the use of the watchdog be restricted like here? Why would
it be not possible to use it in combination with USE_NANDFLASH or with
USE_DATAFLASH?

NAK.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
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
Marriage is the sole cause of divorce.

[U-Boot] [PATCH 1/2] at91sam9: add watchdog support

[Wolfgang Denk]

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <1232623377-26351-2-git-send-email-plagnioj@jcrosoft.com> you wrote:
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  Makefile                             |    1 +
>  doc/README.at91                      |    9 ++++
>  drivers/serial/atmel_usart.c         |    4 +-
>  drivers/watchdog/Makefile            |   46 ++++++++++++++++++++
>  drivers/watchdog/at91sam9_wdt.c      |   79 ++++++++++++++++++++++++++++++++++
>  include/asm-arm/arch-at91/at91_wdt.h |   38 ++++++++++++++++
>  6 files changed, 176 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/watchdog/Makefile
>  create mode 100644 drivers/watchdog/at91sam9_wdt.c
>  create mode 100644 include/asm-arm/arch-at91/at91_wdt.h
> 
> diff --git a/Makefile b/Makefile
> index 4a3a488..c3f089d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -266,6 +266,7 @@ LIBS += drivers/rtc/librtc.a
>  LIBS += drivers/serial/libserial.a
>  LIBS += drivers/usb/libusb.a
>  LIBS += drivers/video/libvideo.a
> +LIBS += drivers/watchdog/libwatchdog.a
>  LIBS += common/libcommon.a
>  LIBS += libfdt/libfdt.a
>  LIBS += api/libapi.a
> diff --git a/doc/README.at91 b/doc/README.at91
> index 4e3928a..1ef3b2a 100644
> --- a/doc/README.at91
> +++ b/doc/README.at91
> @@ -2,6 +2,7 @@ Atmel AT91 Evaluation kits
>  
>  http://atmel.com/dyn/products/tools.asp?family_id=605#1443
>  
> +I. Board mapping & boot media
>  ------------------------------------------------------------------------------
>  AT91SAM9260EK & AT91SAM9XEEK
>  ------------------------------------------------------------------------------
> @@ -86,3 +87,11 @@ Environment variables
>  		make at91sam9263ek_config		- use data flash (spi cs0) (default)
>  		make at91sam9263ek_nandflash_config	- use nand flash
>  		make at91sam9263ek_dataflash_cs0_config	- use data flash (spi cs0)
> +
> +II. Watchdog support
> +
> +	The watchdog can only be activate once so you need to activate in the
> +	bootloader and in the bootstrap if you use it

This sentence makes no sense to me.

If I can activate it only once, then why do you claim I have to
activate it twice ("in the bootloader [1] and in the bootstrap [2]").

And what exaxctly is "the bootstrap"?

Also note a spelling error: Please write "...can only be activated".

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
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
You Don't Have To Be 'Damned' To Work Here, But It Helps!!!
                                             - Terry Pratchett, _Eric_

[U-Boot] [PATCH 1/2] at91sam9: add watchdog support

[Jean-Christophe PLAGNIOL-VILLARD]

On 00:17 Mon 09 Mar     , Wolfgang Denk wrote:
> Dear Jean-Christophe PLAGNIOL-VILLARD,
> 
> In message <1232623377-26351-2-git-send-email-plagnioj@jcrosoft.com> you wrote:
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > ---
> >  Makefile                             |    1 +
> >  doc/README.at91                      |    9 ++++
> >  drivers/serial/atmel_usart.c         |    4 +-
> >  drivers/watchdog/Makefile            |   46 ++++++++++++++++++++
> >  drivers/watchdog/at91sam9_wdt.c      |   79 ++++++++++++++++++++++++++++++++++
> >  include/asm-arm/arch-at91/at91_wdt.h |   38 ++++++++++++++++
> >  6 files changed, 176 insertions(+), 1 deletions(-)
> >  create mode 100644 drivers/watchdog/Makefile
> >  create mode 100644 drivers/watchdog/at91sam9_wdt.c
> >  create mode 100644 include/asm-arm/arch-at91/at91_wdt.h
> > 
> > diff --git a/Makefile b/Makefile
> > index 4a3a488..c3f089d 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -266,6 +266,7 @@ LIBS += drivers/rtc/librtc.a
> >  LIBS += drivers/serial/libserial.a
> >  LIBS += drivers/usb/libusb.a
> >  LIBS += drivers/video/libvideo.a
> > +LIBS += drivers/watchdog/libwatchdog.a
> >  LIBS += common/libcommon.a
> >  LIBS += libfdt/libfdt.a
> >  LIBS += api/libapi.a
> > diff --git a/doc/README.at91 b/doc/README.at91
> > index 4e3928a..1ef3b2a 100644
> > --- a/doc/README.at91
> > +++ b/doc/README.at91
> > @@ -2,6 +2,7 @@ Atmel AT91 Evaluation kits
> >  
> >  http://atmel.com/dyn/products/tools.asp?family_id=605#1443
> >  
> > +I. Board mapping & boot media
> >  ------------------------------------------------------------------------------
> >  AT91SAM9260EK & AT91SAM9XEEK
> >  ------------------------------------------------------------------------------
> > @@ -86,3 +87,11 @@ Environment variables
> >  		make at91sam9263ek_config		- use data flash (spi cs0) (default)
> >  		make at91sam9263ek_nandflash_config	- use nand flash
> >  		make at91sam9263ek_dataflash_cs0_config	- use data flash (spi cs0)
> > +
> > +II. Watchdog support
> > +
> > +	The watchdog can only be activate once so you need to activate in the
> > +	bootloader and in the bootstrap if you use it
> 
> This sentence makes no sense to me.
> 
> If I can activate it only once, then why do you claim I have to
> activate it twice ("in the bootloader [1] and in the bootstrap [2]").
you must activate it support in the twice
> 
> And what exaxctly is "the bootstrap"?
U-Boot can be the First or the Second level bootloader on AT91SAM9 or CAP9
In this second case the Bootstrap is the first bootloader (when booting on
dataflash as example)

Best Regards,
J.

[U-Boot] [PATCH 2/2] at91sam9263ek: active watchdog support via at91sam9263ek_wdt_config

[Jean-Christophe PLAGNIOL-VILLARD]

On 00:15 Mon 09 Mar     , Wolfgang Denk wrote:
> Dear Jean-Christophe PLAGNIOL-VILLARD,
> 
> In message <1232623377-26351-3-git-send-email-plagnioj@jcrosoft.com> you wrote:
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > ---
> >  Makefile                        |    5 +++++
> >  include/configs/at91sam9263ek.h |    5 +++++
> >  2 files changed, 10 insertions(+), 0 deletions(-)
> > 
> > diff --git a/Makefile b/Makefile
> > index c3f089d..2fb0c42 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -2663,6 +2663,7 @@ at91sam9261ek_config	:	unconfig
> >  at91sam9263ek_nandflash_config \
> >  at91sam9263ek_dataflash_config \
> >  at91sam9263ek_dataflash_cs0_config \
> > +at91sam9263ek_wdt_config \
> >  at91sam9263ek_config	:	unconfig
> >  	@mkdir -p $(obj)include
> >  	@if [ "$(findstring _nandflash,$@)" ] ; then \
> > @@ -2672,6 +2673,10 @@ at91sam9263ek_config	:	unconfig
> >  		echo "#define CONFIG_SYS_USE_DATAFLASH 1"	>>$(obj)include/config.h ; \
> >  		$(XECHO) "... with environment variable in SPI DATAFLASH CS0" ; \
> >  	fi;
> > +	@if [ "$(findstring _wdt_, $@)" ] ; then \
> > +		echo "#define CONFIG_AT91SAM9_WATCHDOG" >> $(obj)include/config.h ; \
> > +		$(XECHO) "... configured with watchdog active"; \
> > +	fi
> >  	@$(MKCONFIG) -a at91sam9263ek arm arm926ejs at91sam9263ek atmel at91
> 
> Sorry for the late reply - but this makes no sense to me.
> 
> CONFIG_ settings belong into the board config file, where a user
> expects to find user configurable settings.
> 
> Why would the use of the watchdog be restricted like here? Why would
> it be not possible to use it in combination with USE_NANDFLASH or with
> USE_DATAFLASH?

It's possible but do we really need to add 4 new configs?

or just one and a doc how to use it?

Best Regards,
J.

[U-Boot] [PATCH 1/2] at91sam9: add watchdog support

[Wolfgang Denk]

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <20090309083209.GC24237@game.jcrosoft.org> you wrote:
>
> > > +	The watchdog can only be activate once so you need to activate in the
> > > +	bootloader and in the bootstrap if you use it
> > 
> > This sentence makes no sense to me.
> > 
> > If I can activate it only once, then why do you claim I have to
> > activate it twice ("in the bootloader [1] and in the bootstrap [2]").
> you must activate it support in the twice

Then you should write that in clear words in the documentation.

> > And what exaxctly is "the bootstrap"?
> U-Boot can be the First or the Second level bootloader on AT91SAM9 or CAP9
> In this second case the Bootstrap is the first bootloader (when booting on
> dataflash as example)

Please add an explanation for such terminology. You might want to
refer to the terms already used for NAND booting etc.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
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
Single tasking: Just Say No.

[U-Boot] [PATCH 2/2] at91sam9263ek: active watchdog support via at91sam9263ek_wdt_config

[Wolfgang Denk]

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <20090309083946.GG24237@game.jcrosoft.org> you wrote:
>
> > > @@ -2663,6 +2663,7 @@ at91sam9261ek_config	:	unconfig
> > >  at91sam9263ek_nandflash_config \
> > >  at91sam9263ek_dataflash_config \
> > >  at91sam9263ek_dataflash_cs0_config \
> > > +at91sam9263ek_wdt_config \
> > >  at91sam9263ek_config	:	unconfig
> > >  	@mkdir -p $(obj)include
...
> > Sorry for the late reply - but this makes no sense to me.
> > 
> > CONFIG_ settings belong into the board config file, where a user
> > expects to find user configurable settings.
> > 
> > Why would the use of the watchdog be restricted like here? Why would
> > it be not possible to use it in combination with USE_NANDFLASH or with
> > USE_DATAFLASH?
> 
> It's possible but do we really need to add 4 new configs?

I did not say that I'm a big fan of things like USE_NANDFLASH and
USE_DATAFLASH and the like. I did not invent this code.

> or just one and a doc how to use it?

I don't see how flash device usage and watchdog usage are related or
how you could handle these in one config option. that doesn't make
sense to me.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
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
Death, when unnecessary, is a tragic thing.
	-- Flint, "Requiem for Methuselah", stardate 5843.7

[U-Boot] [PATCH 2/2] at91sam9263ek: active watchdog support via at91sam9263ek_wdt_config

[Giulio Benetti]

I think that with this patch at91 watchdog is enough, it reduces code that
is not yet used and I think it HAS NOT to be used(no wdt init).

Signed-off-by: giulio.benetti at micronovasrl.com

diff -urpN b/drivers/watchdog/at91sam9_wdt.c
a/drivers/watchdog/at91sam9_wdt.c
--- b/drivers/watchdog/at91sam9_wdt.c   2009-06-14 21:30:39.000000000 +0200
+++ a/drivers/watchdog/at91sam9_wdt.c   2009-06-18 17:26:26.000000000 +0200
@@ -5,6 +5,7 @@
  *
  * Copyright (C) 2008 Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com>
  * Copyright (C) 2008 Renaud CERRATO r.cerrato at til-technologies.fr
+ * Giulio Benetti <giulio.benetti@micronovasrl.com>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -24,56 +25,7 @@
 #include <asm/arch/io.h>
 #include <asm/arch/at91_wdt.h>
 
-/*
- * AT91SAM9 watchdog runs a 12bit counter @ 256Hz,
- * use this to convert a watchdog
- * value from/to milliseconds.
- */
-#define ms_to_ticks(t) (((t << 8) / 1000) - 1)
-#define ticks_to_ms(t) (((t + 1) * 1000) >> 8)
-
-/* Hardware timeout in seconds */
-#define WDT_HW_TIMEOUT 2
-
-/*
- * Set the watchdog time interval in 1/256Hz (write-once)
- * Counter is 12 bit.
- */
-static int at91_wdt_settimeout(unsigned int timeout)
-{
-       unsigned int reg;
-       unsigned int mr;
-
-       /* Check if disabled */
-       mr = at91_sys_read(AT91_WDT_MR);
-       if (mr & AT91_WDT_WDDIS) {
-               printf("sorry, watchdog is disabled\n");
-               return -1;
-       }
-
-       /*
-        * All counting occurs at SLOW_CLOCK / 128 = 256 Hz
-        *
-        * Since WDV is a 12-bit counter, the maximum period is
-        * 4096 / 256 = 16 seconds.
-        */
-       reg = AT91_WDT_WDRSTEN  /* causes watchdog reset */
-               /* | AT91_WDT_WDRPROC   causes processor reset only */
-               | AT91_WDT_WDDBGHLT             /* disabled in debug mode */
-               | AT91_WDT_WDD                  /* restart at any time */
-               | (timeout & AT91_WDT_WDV);     /* timer value */
-       at91_sys_write(AT91_WDT_MR, reg);
-
-       return 0;
-}
-
 void hw_watchdog_reset(void)
 {
        at91_sys_write(AT91_WDT_CR, AT91_WDT_KEY | AT91_WDT_WDRSTT);
 }
-
-void hw_watchdog_init(void)
-{
-       /* 16 seconds timer, resets enabled */
-       at91_wdt_settimeout(ms_to_ticks(WDT_HW_TIMEOUT * 1000));
-}


-- 
Giulio Benetti
R&D
Micronova srl

[U-Boot] [PATCH 2/2] at91sam9263ek: active watchdog support via at91sam9263ek_wdt_config

[Alessandro Rubini]

>> But you have to delete hw_watchdog_init().
>> If you set there MR then linux won't be able to do it.
>> The patch needs only the WATCHDOG_RESET() and nothing more.

I agree.

> you need to update linux, the watchdog can be enable in u-boot
> but as it can only be disable once if u-boot or any other bootloader init
> we can not disable it
> so in this case linux will just have to buy time to the watchdog

On the sam926x processors It can only be confitured once. So Giulio
is right: u-boot shouldn't configure it, just kick it any now and then.

When a client asked to used the watchdog on the 9260 I chose not to
disable it in at91boot nor in u-boot. Then Linux (i.e., the final
user) could do whatever was fit: either disable it, enable it with a
faster timeout or leave it alone (at 13s or whatever the default is)
until some application sets a policy.

I don't like this hardware choice of "configure once only":
never-disable makes sense, but never-enable doesn't, in my
opinion.  But based on that the right thing for u-boot is not
configuring it a all, or we loose flexibility in later stages.

/alessandro

[U-Boot] [PATCH 2/2] at91sam9263ek: active watchdog support via at91sam9263ek_wdt_config

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:38 Mon 15 Jun     , Giulio Benetti wrote:
> Jean-Christophe PLAGNIOL-VILLARD wrote:
> 
> > On 12:52 Sat 13 Jun     , Giulio Benetti wrote:
> >> In this way the linux driver won't work(the one by Renaud CERRATO).
> >> Because it expects MR register not to be set, or to be set enabled.
> > it's work we have test it
> > you need to enable it in the at91bootstrap or in your lowlevel board
> > config
> >> 
> >> After I don't agree refreshing watchdog inside atmel_usart.c
> > you MUST do it otherwise you board will be not usable at all
> > simple exammple
> > you stop the boot and get a prompt if you do not type any command you
> > reboot it's not what we expect
> >> If u-boot hangs there instead of execute bootcmd, then the machine lies
> >> there forever.
> >> In my opinion we should put WATCHDOG_RESET() inside main_loop and nand
> >> write and read.
> >> What do you think?
> > NACK for the main_loop
> > 
> > nand read write ok
> > 
> > but if we excecute the bootcmd we have finished and jump to the os
> > so it's over for u-boot
> > 
> > the OS will have to care about the watchdog
> > 
> > Best Regards,
> > J.
> 
> Agree with you.
> But you have to delete hw_watchdog_init().
> If you set there MR then linux won't be able to do it.
> The patch needs only the WATCHDOG_RESET() and nothing more.
you need to update linux, the watchdog can be enable in u-boot
but as it can only be disable once if u-boot or any other bootloader init
we can not disable it
so in this case linux will just have to buy time to the watchdog

Best Regards,
J.

[U-Boot] [PATCH 2/2] at91sam9263ek: active watchdog support via at91sam9263ek_wdt_config

[Giulio Benetti]

Jean-Christophe PLAGNIOL-VILLARD wrote:

> On 12:52 Sat 13 Jun     , Giulio Benetti wrote:
>> In this way the linux driver won't work(the one by Renaud CERRATO).
>> Because it expects MR register not to be set, or to be set enabled.
> it's work we have test it
> you need to enable it in the at91bootstrap or in your lowlevel board
> config
>> 
>> After I don't agree refreshing watchdog inside atmel_usart.c
> you MUST do it otherwise you board will be not usable at all
> simple exammple
> you stop the boot and get a prompt if you do not type any command you
> reboot it's not what we expect
>> If u-boot hangs there instead of execute bootcmd, then the machine lies
>> there forever.
>> In my opinion we should put WATCHDOG_RESET() inside main_loop and nand
>> write and read.
>> What do you think?
> NACK for the main_loop
> 
> nand read write ok
> 
> but if we excecute the bootcmd we have finished and jump to the os
> so it's over for u-boot
> 
> the OS will have to care about the watchdog
> 
> Best Regards,
> J.

Agree with you.
But you have to delete hw_watchdog_init().
If you set there MR then linux won't be able to do it.
The patch needs only the WATCHDOG_RESET() and nothing more.

-- 
Giulio Benetti
R&D
Micronova srl

[U-Boot] [PATCH 2/2] at91sam9263ek: active watchdog support via at91sam9263ek_wdt_config

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:52 Sat 13 Jun     , Giulio Benetti wrote:
> In this way the linux driver won't work(the one by Renaud CERRATO).
> Because it expects MR register not to be set, or to be set enabled.
it's work we have test it
you need to enable it in the at91bootstrap or in your lowlevel board config
> 
> After I don't agree refreshing watchdog inside atmel_usart.c
you MUST do it otherwise you board will be not usable at all
simple exammple
you stop the boot and get a prompt if you do not type any command you reboot
it's not what we expect
> If u-boot hangs there instead of execute bootcmd, then the machine lies
> there forever.
> In my opinion we should put WATCHDOG_RESET() inside main_loop and nand write
> and read.
> What do you think?
NACK for the main_loop

nand read write ok

but if we excecute the bootcmd we have finished and jump to the os
so it's over for u-boot

the OS will have to care about the watchdog

Best Regards,
J.

[U-Boot] [PATCH 2/2] at91sam9263ek: active watchdog support via at91sam9263ek_wdt_config

[Giulio Benetti]

In this way the linux driver won't work(the one by Renaud CERRATO).
Because it expects MR register not to be set, or to be set enabled.

After I don't agree refreshing watchdog inside atmel_usart.c
If u-boot hangs there instead of execute bootcmd, then the machine lies
there forever.
In my opinion we should put WATCHDOG_RESET() inside main_loop and nand write
and read.
What do you think?

Kind regards

-- 
Giulio Benetti
R&D
Micronova srl

[U-Boot] [PATCH 2/2] at91sam9263ek: active watchdog support via at91sam9263ek_wdt_config

[Wolfgang Denk]

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <1233505762-23290-2-git-send-email-plagnioj@jcrosoft.com> you wrote:
> 
>  II. Watchdog support
>  
> -	The watchdog wan only be activate once so you need to activate in the
> +	The watchdog can only be activate once so you need to activate in the
>  	bootloader and in the bootstrap if you use it

Also please squash this into the patch where you add that text.
Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
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
We're all sorry for the other guy when he loses his job to a machine.
But when it comes to your job -- that's different. And it always will
be different.
	-- McCoy, "The Ultimate Computer", stardate 4729.4

[U-Boot] [PATCH 2/2] at91sam9263ek: active watchdog support via at91sam9263ek_wdt_config

[Jean-Christophe PLAGNIOL-VILLARD]

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 Makefile                        |    5 +++++
 doc/README.at91                 |    2 +-
 include/configs/at91sam9263ek.h |    5 +++++
 3 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 3e61417..cc3a22e 100644
--- a/Makefile
+++ b/Makefile
@@ -2680,6 +2680,7 @@ at91sam9261ek_config	:	unconfig
 at91sam9263ek_nandflash_config \
 at91sam9263ek_dataflash_config \
 at91sam9263ek_dataflash_cs0_config \
+at91sam9263ek_wdt_config \
 at91sam9263ek_config	:	unconfig
 	@mkdir -p $(obj)include
 	@if [ "$(findstring _nandflash,$@)" ] ; then \
@@ -2689,6 +2690,10 @@ at91sam9263ek_config	:	unconfig
 		echo "#define CONFIG_SYS_USE_DATAFLASH 1"	>>$(obj)include/config.h ; \
 		$(XECHO) "... with environment variable in SPI DATAFLASH CS0" ; \
 	fi;
+	@if [ "$(findstring _wdt_, $@)" ] ; then \
+		echo "#define CONFIG_AT91SAM9_WATCHDOG" >> $(obj)include/config.h ; \
+		$(XECHO) "... configured with watchdog active"; \
+	fi
 	@$(MKCONFIG) -a at91sam9263ek arm arm926ejs at91sam9263ek atmel at91
 
 at91sam9rlek_nandflash_config \
diff --git a/doc/README.at91 b/doc/README.at91
index c883c7c..1ef3b2a 100644
--- a/doc/README.at91
+++ b/doc/README.at91
@@ -90,7 +90,7 @@ Environment variables
 
 II. Watchdog support
 
-	The watchdog wan only be activate once so you need to activate in the
+	The watchdog can only be activate once so you need to activate in the
 	bootloader and in the bootstrap if you use it
 
 	The you can activate with
diff --git a/include/configs/at91sam9263ek.h b/include/configs/at91sam9263ek.h
index 09b871a..e28903c 100644
--- a/include/configs/at91sam9263ek.h
+++ b/include/configs/at91sam9263ek.h
@@ -201,4 +201,9 @@
 #error CONFIG_USE_IRQ not supported
 #endif
 
+/* WATCHDOG */
+#ifdef CONFIG_AT91SAM9_WATCHDOG
+#define CONFIG_HW_WATCHDOG			/* watchdog */
+#endif
+
 #endif
-- 
1.5.6.5