Re: [PATCH 2/7] watchdog: add at91sam9 watchdog support

[Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>]

On 10:36 Mon 19 Nov     , Sascha Hauer wrote:
> 
> On Fri, Nov 16, 2012 at 06:55:40PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > with keep alive support
> > 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > ---
> >  drivers/watchdog/Kconfig        |    7 +++
> >  drivers/watchdog/Makefile       |    1 +
> >  drivers/watchdog/at91sam9_wdt.c |  131 +++++++++++++++++++++++++++++++++++++++
> >  drivers/watchdog/at91sam9_wdt.h |   38 ++++++++++++
> >  4 files changed, 177 insertions(+)
> >  create mode 100644 drivers/watchdog/at91sam9_wdt.c
> >  create mode 100644 drivers/watchdog/at91sam9_wdt.h
> > 
> > diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> > index 21480a1..5bd1083 100644
> > --- a/drivers/watchdog/Kconfig
> > +++ b/drivers/watchdog/Kconfig
> > @@ -7,6 +7,13 @@ menuconfig WATCHDOG
> >  
> >  if WATCHDOG
> >  
> > +config WATCHDOG_AT91SAM9X
> > +	tristate "AT91SAM9X / AT91CAP9 watchdog"
> > +	depends on ARCH_AT91
> > +	help
> > +	  Watchdog timer embedded into AT91SAM9X and AT91CAP9 chips. This will
> > +	  reboot your system when the timeout is reached.
> > +
> >  config WATCHDOG_MXS28
> >  	bool "i.MX28"
> >  	depends on ARCH_IMX28
> > diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> > index b29103b..4e863a5 100644
> > --- a/drivers/watchdog/Makefile
> > +++ b/drivers/watchdog/Makefile
> > @@ -1,2 +1,3 @@
> >  obj-$(CONFIG_WATCHDOG) += wd_core.o
> > +obj-$(CONFIG_WATCHDOG_AT91SAM9X) += at91sam9_wdt.o
> >  obj-$(CONFIG_WATCHDOG_MXS28) += im28wd.o
> > diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
> > new file mode 100644
> > index 0000000..203d83a
> > --- /dev/null
> > +++ b/drivers/watchdog/at91sam9_wdt.c
> > @@ -0,0 +1,131 @@
> > +/*
> > + * (c) 2012 Juergen Beisert <kernel@pengutronix.de>
> 
> Juergen Beisert?
> 
> > + *
> > + * 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.
> > + *
> > + * Note: this driver works for the i.MX28 SoC. It might work for the
> > + * i.MX23 Soc as well, but is not tested yet.
> 
> This might work on i.MX23?
> 
> > +
> > +static void at91sam9_wdt_keep_alive(struct watchdog *wdt)
> > +{
> > +	struct at91sam9_wdt *at91wdt = to_at91sam9_wdt(wdt);
> > +
> > +	wdt_write(at91wdt, AT91_WDT_CR, AT91_WDT_KEY | AT91_WDT_WDRSTT);
> > +}
> > +
> > +static int at91sam9_wdt_settimeout(struct watchdog *wdt, unsigned int timeout)
> > +{
> > +	struct at91sam9_wdt *at91wdt = to_at91sam9_wdt(wdt);
> > +	unsigned int reg;
> > +	unsigned int mr;
> > +
> > +	/* Check if disabled */
> > +	mr = wdt_read(at91wdt, AT91_WDT_MR);
> > +	if (mr & AT91_WDT_WDDIS) {
> > +		pr_err("sorry, watchdog is disabled\n");
> > +		return -EIO;
> > +	}
> > +
> > +	if (!timeout) {
> > +		wdt_write(at91wdt, AT91_WDT_MR, AT91_WDT_WDDIS);
> > +		return 0;
> > +	}
> > +
> > +	/*
> > +	 * 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 */
> > +	wdt_write(at91wdt, AT91_WDT_MR, reg);
> 
> This driver does not work like the watchdog API is supposed to work. It
> currently works in the way that the watchdog command calls the
> settimeout callback to keep the watchdog alive, hence we do not need
> an explicit keepalive callback. Whether this API is good is debatable,
> but this patch violates it and renders the watchdog command useless.
The watchdod commad just need to enable the watchdog/change the timeout or
diabled 

the rest need to be automatic

so we need to fix the API

because I can not add wd command in long nfs or tftp transfert

Best Regards,
J.

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox