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

[Simon Kagstrom <simon.kagstrom@netinsight.net>]

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