[U-Boot] [PATCH 0/2]: Watchdog support from the command line

[U-Boot] [PATCH 0/2]: Watchdog support from the command line

[Simon Kagstrom]

Hi!

These two patches add a generic watchdog CLI command and a driver for
the watchdog on Marvell Kirkwood that uses it.

The command usage is

  watchdog - Watchdog commands

  Usage:
  watchdog <timeout>	- start the watchdog with `timeout' seconds timeout
  watchdog off		- stop the watchdog (can't be done on all boards)

// Simon

[U-Boot] [PATCH 1/2]: common: Add a watchdog CLI command

[Simon Kagstrom]

A watchdog command to enable the watchdog with a timeout or disable it
can sometimes be useful. Add that. This also adds a common API for
enabling or disabling watchdogs. The API is simple:

        void watchdog_enable(unsigned int timeout);
        void watchdog_disable(void);

disabling the watchdog might or might not be possible depending on the
hardware, and the timeout range can also vary in the same way.

Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---
 common/Makefile       |    1 +
 common/cmd_watchdog.c |   61 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/watchdog.h    |    9 +++++++
 3 files changed, 71 insertions(+), 0 deletions(-)
 create mode 100644 common/cmd_watchdog.c

diff --git a/common/Makefile b/common/Makefile
index 3781738..f14ba0e 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -160,6 +160,7 @@ COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o
 COBJS-$(CONFIG_MODEM_SUPPORT) += modem.o
 COBJS-$(CONFIG_UPDATE_TFTP) += update.o
 COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o
+COBJS-$(CONFIG_CMD_WATCHDOG) += cmd_watchdog.o
 
 
 COBJS	:= $(sort $(COBJS-y))
diff --git a/common/cmd_watchdog.c b/common/cmd_watchdog.c
new file mode 100644
index 0000000..d26be4f
--- /dev/null
+++ b/common/cmd_watchdog.c
@@ -0,0 +1,61 @@
+/*
+ * (C) Copyright 2009
+ * Net Insight <www.netinsight.net>
+ * Written-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
+ *
+ * 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., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ */
+
+#include <common.h>
+#include <watchdog.h>
+
+static int do_watchdog(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
+{
+	const char *cmd;
+	char *endp;
+	int timeout;
+
+	/* need one argument */
+	if (argc != 2)
+		goto usage;
+
+	cmd = argv[1];
+
+	if (strcmp(cmd, "off") == 0) {
+		watchdog_disable();
+		return 0;
+	}
+	timeout = simple_strtoul(cmd, &endp, 0);
+	if (endp == cmd)
+		goto usage;
+	if (timeout < 0)
+		goto usage;
+
+	/* Everything fine, enable the watchdog */
+	watchdog_enable(timeout);
+
+	return 0;
+usage:
+	cmd_usage(cmdtp);
+	return 1;
+}
+
+U_BOOT_CMD(
+	watchdog, 2, 0,	do_watchdog,
+	"Watchdog commands",
+	"<timeout>	- start the watchdog with `timeout' seconds timeout\n"
+	"watchdog off		- stop the watchdog (can't be done on all boards)\n"
+);
diff --git a/include/watchdog.h b/include/watchdog.h
index 9265be9..953cf61 100644
--- a/include/watchdog.h
+++ b/include/watchdog.h
@@ -70,6 +70,15 @@
 	#endif /* CONFIG_WATCHDOG && !__ASSEMBLY__ */
 #endif /* CONFIG_HW_WATCHDOG */
 
+#if defined(CONFIG_WATCHDOG) || defined(CONFIG_HW_WATCHDOG)
+extern void watchdog_enable(unsigned int timeout_secs);
+
+extern void watchdog_disable(void);
+#else
+static inline void watchdog_enable(unsigned int timeout_secs) { }
+static inline void watchdog_disable(void) { }
+#endif
+
 /*
  * Prototypes from $(CPU)/cpu.c.
  */
-- 
1.6.0.4

[U-Boot] [PATCH 1/2]: common: Add a watchdog CLI command

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Simon Kagstrom [mailto:simon.kagstrom at netinsight.net] 
> Sent: Wednesday, October 28, 2009 7:44 PM
> To: U-Boot ML; Prafulla Wadaskar
> Subject: [PATCH 1/2]: common: Add a watchdog CLI command
> 
> A watchdog command to enable the watchdog with a timeout or disable it
> can sometimes be useful. Add that. This also adds a common API for
> enabling or disabling watchdogs. The API is simple:
> 
>         void watchdog_enable(unsigned int timeout);
>         void watchdog_disable(void);
> 
> disabling the watchdog might or might not be possible depending on the
> hardware, and the timeout range can also vary in the same way.
> 
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> ---
>  common/Makefile       |    1 +
>  common/cmd_watchdog.c |   61 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/watchdog.h    |    9 +++++++
>  3 files changed, 71 insertions(+), 0 deletions(-)
>  create mode 100644 common/cmd_watchdog.c
> 
> diff --git a/common/Makefile b/common/Makefile
> index 3781738..f14ba0e 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -160,6 +160,7 @@ COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o
>  COBJS-$(CONFIG_MODEM_SUPPORT) += modem.o
>  COBJS-$(CONFIG_UPDATE_TFTP) += update.o
>  COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o
> +COBJS-$(CONFIG_CMD_WATCHDOG) += cmd_watchdog.o
>  
>  
>  COBJS	:= $(sort $(COBJS-y))
> diff --git a/common/cmd_watchdog.c b/common/cmd_watchdog.c
> new file mode 100644
> index 0000000..d26be4f
> --- /dev/null
> +++ b/common/cmd_watchdog.c
> @@ -0,0 +1,61 @@
> +/*
> + * (C) Copyright 2009
> + * Net Insight <www.netinsight.net>
> + * Written-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> + *
> + * 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., 51 Franklin Street, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + */
> +
> +#include <common.h>
> +#include <watchdog.h>
> +
> +static int do_watchdog(cmd_tbl_t *cmdtp, int flag, int argc, 
> char *argv[])
> +{
> +	const char *cmd;
> +	char *endp;
> +	int timeout;
> +
> +	/* need one argument */
> +	if (argc != 2)
> +		goto usage;
> +
> +	cmd = argv[1];
> +
> +	if (strcmp(cmd, "off") == 0) {
> +		watchdog_disable();
> +		return 0;
> +	}
> +	timeout = simple_strtoul(cmd, &endp, 0);
> +	if (endp == cmd)
> +		goto usage;
> +	if (timeout < 0)
> +		goto usage;

How about passing zero value here, will it be a correct input for watchdog_enable?
 
> +
> +	/* Everything fine, enable the watchdog */
> +	watchdog_enable(timeout);

Can we check for some error code here from lower layer and dump some error message?
For ex. Specified timeout value may be invalid for specific h/w

> +
> +	return 0;
> +usage:
> +	cmd_usage(cmdtp);
> +	return 1;
> +}
> +
> +U_BOOT_CMD(
> +	watchdog, 2, 0,	do_watchdog,
> +	"Watchdog commands",
> +	"<timeout>	- start the watchdog with `timeout' 
> seconds timeout\n"
> +	"watchdog off		- stop the watchdog (can't be 
> done on all boards)\n"
> +);
> diff --git a/include/watchdog.h b/include/watchdog.h
> index 9265be9..953cf61 100644
> --- a/include/watchdog.h
> +++ b/include/watchdog.h
> @@ -70,6 +70,15 @@
>  	#endif /* CONFIG_WATCHDOG && !__ASSEMBLY__ */
>  #endif /* CONFIG_HW_WATCHDOG */
>  
> +#if defined(CONFIG_WATCHDOG) || defined(CONFIG_HW_WATCHDOG)
> +extern void watchdog_enable(unsigned int timeout_secs);
> +
> +extern void watchdog_disable(void);
> +#else
> +static inline void watchdog_enable(unsigned int timeout_secs) { }
> +static inline void watchdog_disable(void) { }
> +#endif
> +
>  /*
>   * Prototypes from $(CPU)/cpu.c.
>   */

What does this means?

Regards..
Prafulla . .

> -- 
> 1.6.0.4
> 
> 

[U-Boot] [PATCH 1/2]: common: Add a watchdog CLI command

[Simon Kagstrom]

On Wed, 28 Oct 2009 07:29:35 -0700
Prafulla Wadaskar <prafulla@marvell.com> wrote:

> > +static int do_watchdog(cmd_tbl_t *cmdtp, int flag, int argc, 
> >
> > +	if (timeout < 0)
> > +		goto usage;
> 
> How about passing zero value here, will it be a correct input for watchdog_enable?

Good point, I'll update the patch.

> > +
> > +	/* Everything fine, enable the watchdog */
> > +	watchdog_enable(timeout);
> 
> Can we check for some error code here from lower layer and dump some error message?
> For ex. Specified timeout value may be invalid for specific h/w

We could, but I'd like to keep the interface simple. Basically: tell
the hardware driver to enable the watchdog "as good as possible", and
then the hardware will enable a watchdog that will timeout "sometime".

This is hardly an end-user issue anyway: he/she will test the board
properly to find a good timeout value anyway, and I believe the
interface can be kept simple. I just like it since it makes it simple
to enable the watchdog where you like it in boot scripts etc.

> > +#if defined(CONFIG_WATCHDOG) || defined(CONFIG_HW_WATCHDOG)
> > +extern void watchdog_enable(unsigned int timeout_secs);
> > +
> > +extern void watchdog_disable(void);
> > +#else
> > +static inline void watchdog_enable(unsigned int timeout_secs) { }
> > +static inline void watchdog_disable(void) { }
> > +#endif
> > +
>
> What does this means?

It was just a way of making the interface calls valid (but empty) if
the watchdog support isn't there. The idea is to avoid #ifdefs in the
code (like for WATCHDOG_RESET).

// Simon

[U-Boot] [PATCH 1/2]: common: Add a watchdog CLI command

[Wolfgang Denk]

Dear Simon Kagstrom,

In message <20091028154925.1dd1a8a9@marrow.netinsight.se> you wrote:
>
> > Can we check for some error code here from lower layer and dump some error message?
> > For ex. Specified timeout value may be invalid for specific h/w
> 
> We could, but I'd like to keep the interface simple. Basically: tell

Well, but error checking and sending respective information to the
user is essential.

> the hardware driver to enable the watchdog "as good as possible", and
> then the hardware will enable a watchdog that will timeout "sometime".

Um... No. This sounds horrible to me.

The driver should do _exactly_ what the user asks for, or raise an
error.

> This is hardly an end-user issue anyway: he/she will test the board
> properly to find a good timeout value anyway, and I believe the
> interface can be kept simple. I just like it since it makes it simple
> to enable the watchdog where you like it in boot scripts etc.

Well, if you have anything on your board which is actually worth the
name watchdog then this whole command will be moot.

A _real_ watchdog is automatically active after reset, and cannot be
disabled by any software.

A somewhat reasonable watchdog can be configured exactly once (for
example, the MPC8xx processors use a write-once register for this
purpose).

A "watchdog" that can be disabled by software is just a toy and
neither worth the money nor the effort ;-)


> > > +#if defined(CONFIG_WATCHDOG) || defined(CONFIG_HW_WATCHDOG)
> > > +extern void watchdog_enable(unsigned int timeout_secs);
> > > +
> > > +extern void watchdog_disable(void);
> > > +#else
> > > +static inline void watchdog_enable(unsigned int timeout_secs) { }
> > > +static inline void watchdog_disable(void) { }
> > > +#endif
> > > +
> >
> > What does this means?
> 
> It was just a way of making the interface calls valid (but empty) if
> the watchdog support isn't there. The idea is to avoid #ifdefs in the
> code (like for WATCHDOG_RESET).

Please use weak functions for this purpose.

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
Anyone who doesn't believe in miracles is not a realist.
                                                   - David Ben Gurion

[U-Boot] [PATCH 2/2]: arm:kirkwood: Add hardware watchdog support for Marvell Kirkwood boards

[Simon Kagstrom]

Initialize by calling the generic API watchdog_enable() with the number
of seconds for the watchdog to timeout. It's not possible to disable the
watchdog once it's on.

Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---
ChangeLog:

 v2: (Some of the comments from Prafulla)
   * Use readl/writel
   * Rename WATCHDOG_CNTR -> WATCHDOG_TMR

 cpu/arm926ejs/kirkwood/timer.c |   37 +++++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/cpu/arm926ejs/kirkwood/timer.c b/cpu/arm926ejs/kirkwood/timer.c
index 817ff42..797ab04 100644
--- a/cpu/arm926ejs/kirkwood/timer.c
+++ b/cpu/arm926ejs/kirkwood/timer.c
@@ -23,8 +23,10 @@
 
 #include <common.h>
 #include <asm/arch/kirkwood.h>
+#include <watchdog.h>
 
 #define UBOOT_CNTR	0	/* counter to use for uboot timer */
+#define WATCHDOG_TMR	2
 
 /* Timer reload and current value registers */
 struct kwtmr_val {
@@ -166,3 +168,38 @@ int timer_init(void)
 
 	return 0;
 }
+
+#if defined(CONFIG_HW_WATCHDOG)
+static unsigned long watchdog_timeout = 5;
+void hw_watchdog_reset(void)
+{
+	u32 time = CONFIG_SYS_TCLK * watchdog_timeout;
+
+	writel(time, CNTMR_VAL_REG(WATCHDOG_TMR));
+}
+
+void watchdog_enable(unsigned int timeout_secs)
+{
+	struct kwcpu_registers *cpureg =
+		(struct kwcpu_registers *)KW_CPU_REG_BASE;
+	u32 rstoutn_mask;
+	u32 cntmrctrl;
+
+	watchdog_timeout = timeout_secs;
+	/* Enable CPU reset if watchdog expires */
+	rstoutn_mask = readl(cpureg->rstoutn_mask);
+	writel(rstoutn_mask |= WATCHDOG_TMR, &cpureg->rstoutn_mask);
+	hw_watchdog_reset();
+
+	/* Enable the watchdog */
+	cntmrctrl = readl(CNTMR_CTRL_REG);
+	cntmrctrl |= CTCR_ARM_TIMER_EN(WATCHDOG_TMR);
+	cntmrctrl |= CTCR_ARM_TIMER_AUTO_EN(WATCHDOG_TMR);
+	writel(cntmrctrl, CNTMR_CTRL_REG);
+}
+
+void watchdog_disable(void)
+{
+	/* Can't be done */
+}
+#endif
-- 
1.6.0.4

[U-Boot] [PATCH 2/2]: arm:kirkwood: Add hardware watchdog support for Marvell Kirkwood boards

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Simon Kagstrom [mailto:simon.kagstrom at netinsight.net] 
> Sent: Wednesday, October 28, 2009 7:46 PM
> To: U-Boot ML; Prafulla Wadaskar
> Subject: [PATCH 2/2]: arm:kirkwood: Add hardware watchdog 
> support for Marvell Kirkwood boards
> 
> Initialize by calling the generic API watchdog_enable() with 
> the number
> of seconds for the watchdog to timeout. It's not possible to 
> disable the
> watchdog once it's on.
> 
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> ---
> ChangeLog:
> 
>  v2: (Some of the comments from Prafulla)
>    * Use readl/writel
>    * Rename WATCHDOG_CNTR -> WATCHDOG_TMR
> 
>  cpu/arm926ejs/kirkwood/timer.c |   37 
> +++++++++++++++++++++++++++++++++++++
>  1 files changed, 37 insertions(+), 0 deletions(-)
> 
> diff --git a/cpu/arm926ejs/kirkwood/timer.c 
> b/cpu/arm926ejs/kirkwood/timer.c
> index 817ff42..797ab04 100644
> --- a/cpu/arm926ejs/kirkwood/timer.c
> +++ b/cpu/arm926ejs/kirkwood/timer.c
> @@ -23,8 +23,10 @@
>  
>  #include <common.h>
>  #include <asm/arch/kirkwood.h>
> +#include <watchdog.h>
>  
>  #define UBOOT_CNTR	0	/* counter to use for uboot timer */
> +#define WATCHDOG_TMR	2
>  
>  /* Timer reload and current value registers */
>  struct kwtmr_val {
> @@ -166,3 +168,38 @@ int timer_init(void)
>  
>  	return 0;
>  }
> +
> +#if defined(CONFIG_HW_WATCHDOG)
> +static unsigned long watchdog_timeout = 5;
> +void hw_watchdog_reset(void)
> +{
> +	u32 time = CONFIG_SYS_TCLK * watchdog_timeout;
> +
> +	writel(time, CNTMR_VAL_REG(WATCHDOG_TMR));
> +}
> +
> +void watchdog_enable(unsigned int timeout_secs)
> +{
> +	struct kwcpu_registers *cpureg =
> +		(struct kwcpu_registers *)KW_CPU_REG_BASE;
> +	u32 rstoutn_mask;
> +	u32 cntmrctrl;
> +
> +	watchdog_timeout = timeout_secs;
> +	/* Enable CPU reset if watchdog expires */
> +	rstoutn_mask = readl(cpureg->rstoutn_mask);
> +	writel(rstoutn_mask |= WATCHDOG_TMR, &cpureg->rstoutn_mask);
> +	hw_watchdog_reset();
> +
> +	/* Enable the watchdog */
> +	cntmrctrl = readl(CNTMR_CTRL_REG);
> +	cntmrctrl |= CTCR_ARM_TIMER_EN(WATCHDOG_TMR);
> +	cntmrctrl |= CTCR_ARM_TIMER_AUTO_EN(WATCHDOG_TMR);
> +	writel(cntmrctrl, CNTMR_CTRL_REG);
> +}
> +
> +void watchdog_disable(void)
> +{
> +	/* Can't be done */

you can disable CPU reset if watchdog expires to achieve this functionality
Or reseting bit 4 in CPU timer control reg

Regards..
Prafulla . .

> +}
> +#endif
> -- 
> 1.6.0.4
> 
> 

[U-Boot] [PATCH v2 0/2]: Watchdog support from the command line

[Simon Kagstrom]

Hi!

These two patches add a generic watchdog CLI command and a driver for
the watchdog on Marvell Kirkwood that uses it.

The command usage is

  watchdog - Watchdog commands

  Usage:
  watchdog <timeout>	- start the watchdog with `timeout' seconds timeout


ChangeLog:
  v2: Adapt according to comments from Prafulla and Wolfgang
    * Passing zero as timeout is invalid (Prafulla)
    * Add return value from watchdog_enable(), negative means failure (Prafulla, Wolfgang)
    * Remove watchdog_disable() (Wolfgang)
    * Use weak default function for watchdog_enable() (Wolfgang)
    * Provide friendly and helpful printouts when invalid parameters are
      passed to the CLI command

// Simon

[U-Boot] [PATCH v2 1/2]: common: Add a watchdog CLI command

[Simon Kagstrom]

A watchdog command to enable the watchdog with a timeout from the CLI
can sometimes be useful. Add that. This also adds a common API for
enabling watchdogs. The API is simple:

        int watchdog_enable(unsigned int timeout);

the timeout range vary depending on hardware, and the driver should
return a negative value if the call failed.

Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---
ChangeLog:
 v2:
    * Passing zero as timeout is invalid (Prafulla)
    * Add return value from watchdog_enable(), negative means failure (Prafulla, Wolfgang)
    * Remove watchdog_disable() (Wolfgang)
    * Use weak default function for watchdog_enable() (Wolfgang)
    * Provide friendly and helpful printouts when invalid parameters are
      passed to the CLI command

 common/Makefile       |    1 +
 common/cmd_watchdog.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++++
 common/main.c         |    7 +++++
 include/watchdog.h    |    2 +
 4 files changed, 72 insertions(+), 0 deletions(-)
 create mode 100644 common/cmd_watchdog.c

diff --git a/common/Makefile b/common/Makefile
index 3781738..f14ba0e 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -160,6 +160,7 @@ COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o
 COBJS-$(CONFIG_MODEM_SUPPORT) += modem.o
 COBJS-$(CONFIG_UPDATE_TFTP) += update.o
 COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o
+COBJS-$(CONFIG_CMD_WATCHDOG) += cmd_watchdog.o
 
 
 COBJS	:= $(sort $(COBJS-y))
diff --git a/common/cmd_watchdog.c b/common/cmd_watchdog.c
new file mode 100644
index 0000000..ca1a8fd
--- /dev/null
+++ b/common/cmd_watchdog.c
@@ -0,0 +1,62 @@
+/*
+ * (C) Copyright 2009
+ * Net Insight <www.netinsight.net>
+ * Written-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
+ *
+ * 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., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ */
+
+#include <common.h>
+#include <watchdog.h>
+
+static int do_watchdog(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
+{
+	const char *cmd;
+	char *endp;
+	unsigned long timeout;
+
+	/* need one argument */
+	if (argc != 2)
+		goto usage;
+
+	cmd = argv[1];
+	timeout = simple_strtoul(cmd, &endp, 0);
+	if (endp == cmd) {
+		printf("Error: Could not convert `%s' to a number\n\n", cmd);
+		goto usage;
+	}
+	if (timeout < 1) {
+		printf("Error: zero timeouts are invalid\n\n");
+		goto usage;
+	}
+
+	/* Everything fine, enable the watchdog */
+	if (watchdog_enable(timeout) < 0) {
+		printf("Error: Could not enable watchdog, check timeout parameter\n\n");
+		goto usage;
+	}
+
+	return 0;
+usage:
+	cmd_usage(cmdtp);
+	return 1;
+}
+
+U_BOOT_CMD(
+	watchdog, 2, 0,	do_watchdog,
+	"Watchdog commands",
+	"<timeout>	- start the watchdog with `timeout' seconds timeout\n"
+);
diff --git a/common/main.c b/common/main.c
index 10d8904..47e867b 100644
--- a/common/main.c
+++ b/common/main.c
@@ -1446,3 +1446,10 @@ int do_run (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 	return 0;
 }
 #endif
+
+
+inline int __watchdog_enable(unsigned int timeout_secs)
+{
+	return -1;
+}
+int watchdog_enable(unsigned int timeout_secs) __attribute__((weak, alias("__watchdog_enable")));
diff --git a/include/watchdog.h b/include/watchdog.h
index 9265be9..74c2bda 100644
--- a/include/watchdog.h
+++ b/include/watchdog.h
@@ -70,6 +70,8 @@
 	#endif /* CONFIG_WATCHDOG && !__ASSEMBLY__ */
 #endif /* CONFIG_HW_WATCHDOG */
 
+extern int watchdog_enable(unsigned int timeout_secs);
+
 /*
  * Prototypes from $(CPU)/cpu.c.
  */
-- 
1.6.0.4

[U-Boot] [PATCH v2 1/2]: common: Add a watchdog CLI command

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Simon Kagstrom [mailto:simon.kagstrom at netinsight.net] 
> Sent: Thursday, October 29, 2009 1:39 PM
> To: U-Boot ML; Prafulla Wadaskar; Wolfgang Denk
> Subject: [PATCH v2 1/2]: common: Add a watchdog CLI command
> 
> A watchdog command to enable the watchdog with a timeout from the CLI
> can sometimes be useful. Add that. This also adds a common API for
> enabling watchdogs. The API is simple:
> 
>         int watchdog_enable(unsigned int timeout);
> 
> the timeout range vary depending on hardware, and the driver should
> return a negative value if the call failed.
> 
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> ---
> ChangeLog:
>  v2:
>     * Passing zero as timeout is invalid (Prafulla)
>     * Add return value from watchdog_enable(), negative means 
> failure (Prafulla, Wolfgang)
>     * Remove watchdog_disable() (Wolfgang)
>     * Use weak default function for watchdog_enable() (Wolfgang)
>     * Provide friendly and helpful printouts when invalid 
> parameters are
>       passed to the CLI command
> 
...snip..

> index 10d8904..47e867b 100644
> --- a/common/main.c
> +++ b/common/main.c
> @@ -1446,3 +1446,10 @@ int do_run (cmd_tbl_t * cmdtp, int 
> flag, int argc, char *argv[])
>  	return 0;
>  }
>  #endif
> +
> +

One small cosmetic change,
Additional line inserted here, pls remove it.
Otherwise ack.

Regards..
Prafulla . .

> +inline int __watchdog_enable(unsigned int timeout_secs)
> +{
> +	return -1;
> +}
> +int watchdog_enable(unsigned int timeout_secs) 
> __attribute__((weak, alias("__watchdog_enable")));
> diff --git a/include/watchdog.h b/include/watchdog.h
> index 9265be9..74c2bda 100644
> --- a/include/watchdog.h
> +++ b/include/watchdog.h
> @@ -70,6 +70,8 @@
>  	#endif /* CONFIG_WATCHDOG && !__ASSEMBLY__ */
>  #endif /* CONFIG_HW_WATCHDOG */
>  
> +extern int watchdog_enable(unsigned int timeout_secs);
> +
>  /*
>   * Prototypes from $(CPU)/cpu.c.
>   */
> -- 
> 1.6.0.4
> 
> 

[U-Boot] [PATCH v2 1/2]: common: Add a watchdog CLI command

[Simon Kagstrom]

(Ping!)

On Thu, 29 Oct 2009 09:09:23 +0100
Simon Kagstrom <simon.kagstrom@netinsight.net> wrote:

> A watchdog command to enable the watchdog with a timeout from the CLI
> can sometimes be useful. Add that. This also adds a common API for
> enabling watchdogs. The API is simple:
> 
>         int watchdog_enable(unsigned int timeout);
> 
> the timeout range vary depending on hardware, and the driver should
> return a negative value if the call failed.

Wolfgang: Do you have any additional comments on this patch?

// Simon

> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> ---
> ChangeLog:
>  v2:
>     * Passing zero as timeout is invalid (Prafulla)
>     * Add return value from watchdog_enable(), negative means failure (Prafulla, Wolfgang)
>     * Remove watchdog_disable() (Wolfgang)
>     * Use weak default function for watchdog_enable() (Wolfgang)
>     * Provide friendly and helpful printouts when invalid parameters are
>       passed to the CLI command
> 
>  common/Makefile       |    1 +
>  common/cmd_watchdog.c |   62 +++++++++++++++++++++++++++++++++++++++++++++++++
>  common/main.c         |    7 +++++
>  include/watchdog.h    |    2 +
>  4 files changed, 72 insertions(+), 0 deletions(-)
>  create mode 100644 common/cmd_watchdog.c
> 
> diff --git a/common/Makefile b/common/Makefile
> index 3781738..f14ba0e 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -160,6 +160,7 @@ COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o
>  COBJS-$(CONFIG_MODEM_SUPPORT) += modem.o
>  COBJS-$(CONFIG_UPDATE_TFTP) += update.o
>  COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o
> +COBJS-$(CONFIG_CMD_WATCHDOG) += cmd_watchdog.o
>  
>  
>  COBJS	:= $(sort $(COBJS-y))
> diff --git a/common/cmd_watchdog.c b/common/cmd_watchdog.c
> new file mode 100644
> index 0000000..ca1a8fd
> --- /dev/null
> +++ b/common/cmd_watchdog.c
> @@ -0,0 +1,62 @@
> +/*
> + * (C) Copyright 2009
> + * Net Insight <www.netinsight.net>
> + * Written-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> + *
> + * 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., 51 Franklin Street, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + */
> +
> +#include <common.h>
> +#include <watchdog.h>
> +
> +static int do_watchdog(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
> +{
> +	const char *cmd;
> +	char *endp;
> +	unsigned long timeout;
> +
> +	/* need one argument */
> +	if (argc != 2)
> +		goto usage;
> +
> +	cmd = argv[1];
> +	timeout = simple_strtoul(cmd, &endp, 0);
> +	if (endp == cmd) {
> +		printf("Error: Could not convert `%s' to a number\n\n", cmd);
> +		goto usage;
> +	}
> +	if (timeout < 1) {
> +		printf("Error: zero timeouts are invalid\n\n");
> +		goto usage;
> +	}
> +
> +	/* Everything fine, enable the watchdog */
> +	if (watchdog_enable(timeout) < 0) {
> +		printf("Error: Could not enable watchdog, check timeout parameter\n\n");
> +		goto usage;
> +	}
> +
> +	return 0;
> +usage:
> +	cmd_usage(cmdtp);
> +	return 1;
> +}
> +
> +U_BOOT_CMD(
> +	watchdog, 2, 0,	do_watchdog,
> +	"Watchdog commands",
> +	"<timeout>	- start the watchdog with `timeout' seconds timeout\n"
> +);
> diff --git a/common/main.c b/common/main.c
> index 10d8904..47e867b 100644
> --- a/common/main.c
> +++ b/common/main.c
> @@ -1446,3 +1446,10 @@ int do_run (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
>  	return 0;
>  }
>  #endif
> +
> +
> +inline int __watchdog_enable(unsigned int timeout_secs)
> +{
> +	return -1;
> +}
> +int watchdog_enable(unsigned int timeout_secs) __attribute__((weak, alias("__watchdog_enable")));
> diff --git a/include/watchdog.h b/include/watchdog.h
> index 9265be9..74c2bda 100644
> --- a/include/watchdog.h
> +++ b/include/watchdog.h
> @@ -70,6 +70,8 @@
>  	#endif /* CONFIG_WATCHDOG && !__ASSEMBLY__ */
>  #endif /* CONFIG_HW_WATCHDOG */
>  
> +extern int watchdog_enable(unsigned int timeout_secs);
> +
>  /*
>   * Prototypes from $(CPU)/cpu.c.
>   */

[U-Boot] [PATCH v2 1/2]: common: Add a watchdog CLI command

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Simon Kagstrom [mailto:simon.kagstrom at netinsight.net] 
> Sent: Friday, November 06, 2009 2:58 PM
> To: U-Boot ML; Wolfgang Denk
> Cc: Prafulla Wadaskar
> Subject: Re: [PATCH v2 1/2]: common: Add a watchdog CLI command
> 
> (Ping!)
> 
> On Thu, 29 Oct 2009 09:09:23 +0100
> Simon Kagstrom <simon.kagstrom@netinsight.net> wrote:
> 
> > A watchdog command to enable the watchdog with a timeout 
> from the CLI
> > can sometimes be useful. Add that. This also adds a common API for
> > enabling watchdogs. The API is simple:
> > 
> >         int watchdog_enable(unsigned int timeout);
> > 
> > the timeout range vary depending on hardware, and the driver should
> > return a negative value if the call failed.
> 
> Wolfgang: Do you have any additional comments on this patch?
> 
> // Simon
> 
> > Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> > ---
> > ChangeLog:
> >  v2:
> >     * Passing zero as timeout is invalid (Prafulla)
> >     * Add return value from watchdog_enable(), negative 
> means failure (Prafulla, Wolfgang)
> >     * Remove watchdog_disable() (Wolfgang)
> >     * Use weak default function for watchdog_enable() (Wolfgang)
> >     * Provide friendly and helpful printouts when invalid 
> parameters are
> >       passed to the CLI command
> > 
> >  common/Makefile       |    1 +
> >  common/cmd_watchdog.c |   62 
> +++++++++++++++++++++++++++++++++++++++++++++++++
> >  common/main.c         |    7 +++++
> >  include/watchdog.h    |    2 +
> >  4 files changed, 72 insertions(+), 0 deletions(-)
> >  create mode 100644 common/cmd_watchdog.c
> > 
> > diff --git a/common/Makefile b/common/Makefile
> > index 3781738..f14ba0e 100644
> > --- a/common/Makefile
> > +++ b/common/Makefile
> > @@ -160,6 +160,7 @@ COBJS-$(CONFIG_LYNXKDI) += lynxkdi.o
> >  COBJS-$(CONFIG_MODEM_SUPPORT) += modem.o
> >  COBJS-$(CONFIG_UPDATE_TFTP) += update.o
> >  COBJS-$(CONFIG_USB_KEYBOARD) += usb_kbd.o
> > +COBJS-$(CONFIG_CMD_WATCHDOG) += cmd_watchdog.o
> >  
> >  
> >  COBJS	:= $(sort $(COBJS-y))
> > diff --git a/common/cmd_watchdog.c b/common/cmd_watchdog.c
> > new file mode 100644
> > index 0000000..ca1a8fd
> > --- /dev/null
> > +++ b/common/cmd_watchdog.c
> > @@ -0,0 +1,62 @@
> > +/*
> > + * (C) Copyright 2009
> > + * Net Insight <www.netinsight.net>
> > + * Written-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> > + *
> > + * 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., 51 Franklin Street, Fifth Floor, Boston,
> > + * MA 02110-1301 USA
> > + */
> > +
> > +#include <common.h>
> > +#include <watchdog.h>
> > +
> > +static int do_watchdog(cmd_tbl_t *cmdtp, int flag, int 
> argc, char *argv[])
> > +{
> > +	const char *cmd;
> > +	char *endp;
> > +	unsigned long timeout;
> > +
> > +	/* need one argument */
> > +	if (argc != 2)
> > +		goto usage;
> > +
> > +	cmd = argv[1];
> > +	timeout = simple_strtoul(cmd, &endp, 0);
> > +	if (endp == cmd) {
> > +		printf("Error: Could not convert `%s' to a 
> number\n\n", cmd);
> > +		goto usage;
> > +	}
> > +	if (timeout < 1) {
> > +		printf("Error: zero timeouts are invalid\n\n");
> > +		goto usage;
> > +	}
> > +
> > +	/* Everything fine, enable the watchdog */
> > +	if (watchdog_enable(timeout) < 0) {
> > +		printf("Error: Could not enable watchdog, check 
> timeout parameter\n\n");
> > +		goto usage;
> > +	}
> > +
> > +	return 0;
> > +usage:
> > +	cmd_usage(cmdtp);
> > +	return 1;
> > +}
> > +
> > +U_BOOT_CMD(
> > +	watchdog, 2, 0,	do_watchdog,
> > +	"Watchdog commands",
> > +	"<timeout>	- start the watchdog with `timeout' 
> seconds timeout\n"
> > +);
> > diff --git a/common/main.c b/common/main.c
> > index 10d8904..47e867b 100644
> > --- a/common/main.c
> > +++ b/common/main.c
> > @@ -1446,3 +1446,10 @@ int do_run (cmd_tbl_t * cmdtp, int 
> flag, int argc, char *argv[])
> >  	return 0;
> >  }
> >  #endif
> > +
> > +

Apart from this additional line
It is okay for me,
If you can provide v3 for this it is good.

I am waiting for Tom/wolfgang's flag so that I can pull it.

Regards..
Prafulla . .

> > +inline int __watchdog_enable(unsigned int timeout_secs)
> > +{
> > +	return -1;
> > +}
> > +int watchdog_enable(unsigned int timeout_secs) 
> __attribute__((weak, alias("__watchdog_enable")));
> > diff --git a/include/watchdog.h b/include/watchdog.h
> > index 9265be9..74c2bda 100644
> > --- a/include/watchdog.h
> > +++ b/include/watchdog.h
> > @@ -70,6 +70,8 @@
> >  	#endif /* CONFIG_WATCHDOG && !__ASSEMBLY__ */
> >  #endif /* CONFIG_HW_WATCHDOG */
> >  
> > +extern int watchdog_enable(unsigned int timeout_secs);
> > +
> >  /*
> >   * Prototypes from $(CPU)/cpu.c.
> >   */
> 
> 

[U-Boot] [PATCH v2 1/2]: common: Add a watchdog CLI command

[Mike Frysinger]

On Friday 06 November 2009 04:28:23 Simon Kagstrom wrote:
> (Ping!)
> 
> On Thu, 29 Oct 2009 09:09:23 +0100
> 
> Simon Kagstrom <simon.kagstrom@netinsight.net> wrote:
> > A watchdog command to enable the watchdog with a timeout from the CLI
> > can sometimes be useful. Add that. This also adds a common API for
> > enabling watchdogs. The API is simple:
> >
> >         int watchdog_enable(unsigned int timeout);
> >
> > the timeout range vary depending on hardware, and the driver should
> > return a negative value if the call failed.
> 
> Wolfgang: Do you have any additional comments on this patch?

here's one: dont quote the entire e-mail when you dont need to.  you force 
people to scan the entire thing looking for possible inline comments.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091106/85b90f82/attachment.pgp 

[U-Boot] [PATCH v2 2/2]: arm:kirkwood: Add hardware watchdog support for Marvell Kirkwood boards

[Simon Kagstrom]

Initialize by calling the generic API watchdog_enable() with the number
of seconds for the watchdog to timeout. It's not possible to disable the
watchdog once it's on.

Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
---
ChangeLog:
  v2:
     * Remove watchdog_disable()
     * Add check to see that the maximum timeout supported by the hardware
       is not exceeded

 cpu/arm926ejs/kirkwood/timer.c |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/cpu/arm926ejs/kirkwood/timer.c b/cpu/arm926ejs/kirkwood/timer.c
index 817ff42..69a5cb7 100644
--- a/cpu/arm926ejs/kirkwood/timer.c
+++ b/cpu/arm926ejs/kirkwood/timer.c
@@ -23,8 +23,10 @@
 
 #include <common.h>
 #include <asm/arch/kirkwood.h>
+#include <watchdog.h>
 
 #define UBOOT_CNTR	0	/* counter to use for uboot timer */
+#define WATCHDOG_TMR	2
 
 /* Timer reload and current value registers */
 struct kwtmr_val {
@@ -166,3 +168,39 @@ int timer_init(void)
 
 	return 0;
 }
+
+#if defined(CONFIG_HW_WATCHDOG)
+static unsigned long watchdog_timeout = 5;
+void hw_watchdog_reset(void)
+{
+	u32 time = CONFIG_SYS_TCLK * watchdog_timeout;
+
+	writel(time, CNTMR_VAL_REG(WATCHDOG_TMR));
+}
+
+#define MAX_TIMEOUT (0xffffffff / CONFIG_SYS_TCLK)
+int watchdog_enable(unsigned int timeout_secs)
+{
+	struct kwcpu_registers *cpureg =
+		(struct kwcpu_registers *)KW_CPU_REG_BASE;
+	u32 rstoutn_mask;
+	u32 cntmrctrl;
+
+	if (timeout_secs > MAX_TIMEOUT)
+		return -1;
+
+	watchdog_timeout = timeout_secs;
+	/* Enable CPU reset if watchdog expires */
+	rstoutn_mask = readl(cpureg->rstoutn_mask);
+	writel(rstoutn_mask |= 2, &cpureg->rstoutn_mask);
+	hw_watchdog_reset();
+
+	/* Enable the watchdog */
+	cntmrctrl = readl(CNTMR_CTRL_REG);
+	cntmrctrl |= CTCR_ARM_TIMER_EN(WATCHDOG_TMR);
+	cntmrctrl |= CTCR_ARM_TIMER_AUTO_EN(WATCHDOG_TMR);
+	writel(cntmrctrl, CNTMR_CTRL_REG);
+
+	return 0;
+}
+#endif
-- 
1.6.0.4

[U-Boot] [PATCH v2 2/2]: arm:kirkwood: Add hardware watchdog support for Marvell Kirkwood boards

[Prafulla Wadaskar]

 

> -----Original Message-----
> From: Simon Kagstrom [mailto:simon.kagstrom at netinsight.net] 
> Sent: Thursday, October 29, 2009 1:41 PM
> To: U-Boot ML; Prafulla Wadaskar; Wolfgang Denk
> Subject: [PATCH v2 2/2]: arm:kirkwood: Add hardware watchdog 
> support for Marvell Kirkwood boards
> 
> Initialize by calling the generic API watchdog_enable() with 
> the number
> of seconds for the watchdog to timeout. It's not possible to 
> disable the
> watchdog once it's on.
> 
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> ---
> ChangeLog:
>   v2:
>      * Remove watchdog_disable()
>      * Add check to see that the maximum timeout supported by 
> the hardware
>        is not exceeded
> 
>  cpu/arm926ejs/kirkwood/timer.c |   38 
> ++++++++++++++++++++++++++++++++++++++
>  1 files changed, 38 insertions(+), 0 deletions(-)
> 
> diff --git a/cpu/arm926ejs/kirkwood/timer.c 
> b/cpu/arm926ejs/kirkwood/timer.c
> index 817ff42..69a5cb7 100644
> --- a/cpu/arm926ejs/kirkwood/timer.c
> +++ b/cpu/arm926ejs/kirkwood/timer.c
> @@ -23,8 +23,10 @@
>  
>  #include <common.h>
>  #include <asm/arch/kirkwood.h>
> +#include <watchdog.h>
>  
>  #define UBOOT_CNTR	0	/* counter to use for uboot timer */
> +#define WATCHDOG_TMR	2
>  
>  /* Timer reload and current value registers */
>  struct kwtmr_val {
> @@ -166,3 +168,39 @@ int timer_init(void)
>  
>  	return 0;
>  }
> +
> +#if defined(CONFIG_HW_WATCHDOG)
> +static unsigned long watchdog_timeout = 5;
> +void hw_watchdog_reset(void)
> +{
> +	u32 time = CONFIG_SYS_TCLK * watchdog_timeout;
> +
> +	writel(time, CNTMR_VAL_REG(WATCHDOG_TMR));
> +}
> +
> +#define MAX_TIMEOUT (0xffffffff / CONFIG_SYS_TCLK)
> +int watchdog_enable(unsigned int timeout_secs)
> +{
> +	struct kwcpu_registers *cpureg =
> +		(struct kwcpu_registers *)KW_CPU_REG_BASE;
> +	u32 rstoutn_mask;
> +	u32 cntmrctrl;
> +
> +	if (timeout_secs > MAX_TIMEOUT)
> +		return -1;
> +
> +	watchdog_timeout = timeout_secs;
> +	/* Enable CPU reset if watchdog expires */
> +	rstoutn_mask = readl(cpureg->rstoutn_mask);
> +	writel(rstoutn_mask |= 2, &cpureg->rstoutn_mask);
> +	hw_watchdog_reset();
> +
> +	/* Enable the watchdog */
> +	cntmrctrl = readl(CNTMR_CTRL_REG);
> +	cntmrctrl |= CTCR_ARM_TIMER_EN(WATCHDOG_TMR);
> +	cntmrctrl |= CTCR_ARM_TIMER_AUTO_EN(WATCHDOG_TMR);
> +	writel(cntmrctrl, CNTMR_CTRL_REG);
> +
> +	return 0;
> +}
> +#endif
> -- 
> 1.6.0.4

Ack
Regards..
Prafulla . .

> 
>