[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 at 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 at 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 at 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.
> >   */
> 
>