[PATCH 36 v2] LEON3: GPTIMER timer watchdog driver

[Gedare Bloom]

Looking at this from the perspective of generalizing to a shared BSP
infrastructure I have a couple of nit-picky comments.

On Thu, Apr 19, 2012 at 9:21 AM, Daniel Hellstrom <daniel at gaisler.com> wrote:
> Last timer instance of GPTIMER is sometimes a watchdog timer that
> can reset the system on timer underflow.
>
> Signed-off-by: Daniel Hellstrom <daniel at gaisler.com>
> ---
>  c/src/lib/libbsp/sparc/leon3/Makefile.am        |    2 +
>  c/src/lib/libbsp/sparc/leon3/include/watchdog.h |   49 ++++++++++++
>  c/src/lib/libbsp/sparc/leon3/preinstall.am      |    4 +
>  c/src/lib/libbsp/sparc/leon3/timer/watchdog.c   |   90 +++++++++++++++++++++++
>  4 files changed, 145 insertions(+), 0 deletions(-)
>  create mode 100644 c/src/lib/libbsp/sparc/leon3/include/watchdog.h
>  create mode 100644 c/src/lib/libbsp/sparc/leon3/timer/watchdog.c
>
> diff --git a/c/src/lib/libbsp/sparc/leon3/Makefile.am b/c/src/lib/libbsp/sparc/leon3/Makefile.am
> index cc25993..09d8b8e 100644
> --- a/c/src/lib/libbsp/sparc/leon3/Makefile.am
> +++ b/c/src/lib/libbsp/sparc/leon3/Makefile.am
> @@ -115,6 +115,8 @@ libbsp_a_SOURCES += ../../sparc/shared/i2c/i2cmst.c
>
>  # timer
>  libbsp_a_SOURCES += timer/timer.c
> +libbsp_a_SOURCES += timer/watchdog.c
> +include_HEADERS += include/watchdog.h
This new file perhaps should be named bspwatchdog.h

>
>  if HAS_SMP
>  libbsp_a_SOURCES += smp/getcpuid.c
> diff --git a/c/src/lib/libbsp/sparc/leon3/include/watchdog.h b/c/src/lib/libbsp/sparc/leon3/include/watchdog.h
> new file mode 100644
> index 0000000..8110efd
> --- /dev/null
> +++ b/c/src/lib/libbsp/sparc/leon3/include/watchdog.h
> @@ -0,0 +1,49 @@
> +/*  watchdog.h
> + *
> + *  The LEON3 BSP timer watch-dog interface
> + *
> + *  COPYRIGHT (c) 2012
> + *  Aeroflex Gaisler
> + *
> + *  The license and distribution terms for this file may be
> + *  found in the file LICENSE in this distribution or at
> + *  http://www.rtems.com/license/LICENSE.
> + */
> +
> +#ifndef __WATCHDOG_H__
> +#define __WATCHDOG_H__
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/* Initialize BSP watchdog routines. Returns number of watchdog timers found.
> + * Currently only one is supported.
> + */
> +int bsp_watchdog_init(void);
Please name bsp_watchdog_initialize() -- for shared code we prefer to
avoid abbreviations.

> +
> +/* Reload watchdog (last timer on the first GPTIMER core), all systems does not
> + * feature a watchdog, it is expected that if this function is called the
> + * user knows that there is a watchdog available.
> + *
> + * The prescaler is normally set to number of MHz of system, this is to
> + * make the system clock tick be stable.
> + *
> + * Arguments
> + *  watchdog       - Always 0 for now
> + *  reload_value   - Number of timer clocks (after prescaler) to count before
> + *                   watchdog is woken.
> + */
> +void bsp_watchdog_reload(int watchdog, unsigned int reload_value);
> +
> +/* Stop watchdog timer */
> +void bsp_watchdog_stop(int watchdog);
> +
> +/* Use watchdog0 timer to reset the system */
> +void bsp_watchdog_system_reset(void);
Why not pass the watchdog number to use in this function also?

> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> diff --git a/c/src/lib/libbsp/sparc/leon3/preinstall.am b/c/src/lib/libbsp/sparc/leon3/preinstall.am
> index dffbb5a..b55cf35 100644
> --- a/c/src/lib/libbsp/sparc/leon3/preinstall.am
> +++ b/c/src/lib/libbsp/sparc/leon3/preinstall.am
> @@ -149,3 +149,7 @@ $(PROJECT_INCLUDE)/i2cmst.h: ../../sparc/shared/include/i2cmst.h $(PROJECT_INCLU
>        $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/i2cmst.h
>  PREINSTALL_FILES += $(PROJECT_INCLUDE)/i2cmst.h
>
> +$(PROJECT_INCLUDE)/watchdog.h: include/watchdog.h $(PROJECT_INCLUDE)/$(dirstamp)
> +       $(INSTALL_DATA) $< $(PROJECT_INCLUDE)/watchdog.h
> +PREINSTALL_FILES += $(PROJECT_INCLUDE)/watchdog.h
> +
> diff --git a/c/src/lib/libbsp/sparc/leon3/timer/watchdog.c b/c/src/lib/libbsp/sparc/leon3/timer/watchdog.c
> new file mode 100644
> index 0000000..b09ab9c
> --- /dev/null
> +++ b/c/src/lib/libbsp/sparc/leon3/timer/watchdog.c
> @@ -0,0 +1,90 @@
> +/*  GPTIMER Watchdog timer routines. On some systems the first GPTIMER
> + *  core's last Timer instance underflow signal is connected to system
> + *  reset.
> + *
> + *  COPYRIGHT (c) 2012
> + *  Aeroflex Gaisler
> + *
> + *  The license and distribution terms for this file may be
> + *  found in the file LICENSE in this distribution or at
> + *  http://www.rtems.com/license/LICENSE.
> + */
> +
> +#include <bsp.h>
> +#include <watchdog.h>
> +#include <grlib.h>
> +
> +extern volatile struct gptimer_regs *LEON3_Timer_Regs;
> +
> +struct gptimer_watchdog_priv {
> +  struct gptimer_regs *regs;
> +  struct gptimer_timer_regs *timer;
> +  int timerno;
> +};
> +
> +struct gptimer_watchdog_priv bsp_watchdogs[1];
> +int bsp_watchdog_count = 0;
> +
> +int bsp_watchdog_init(void)
> +{
> +  int timercnt;
> +
> +  if (!LEON3_Timer_Regs)
> +    return 0;
> +
> +  /* Get Watchdogs in system, this is implemented for one GPTIMER core
> +   * only.
> +   *
> +   * First watchdog is a special case, we can get the first timer core by
> +   * looking at LEON3_Timer_Regs, the watchdog within a timer core is
> +   * always the last timer. Unfortunately we can not know it the watchdog
> +   * functionality is available or not, we assume that it is if we
> +   * reached this function.
> +   */
> +  bsp_watchdogs[0].regs = (struct gptimer_regs *)LEON3_Timer_Regs;
> +
> +  /* Find Timer that has watchdog functionality */
> +  timercnt = bsp_watchdogs[0].regs->cfg & 0x7;
> +  if (timercnt < 2) /* First timer system clock timer */
> +    return 0;
> +
> +  bsp_watchdogs[0].timerno = timercnt - 1;
> +  bsp_watchdogs[0].timer = &bsp_watchdogs[0].regs->timer[bsp_watchdogs[0].timerno];
> +
> +  bsp_watchdog_count = 1;
Can we get rid of this global var?

> +  return bsp_watchdog_count;
> +}
> +
> +void bsp_watchdog_reload(int watchdog, unsigned int reload_value)
> +{
> +  if (bsp_watchdog_count == 0)
> +    bsp_watchdog_init();
> +
> +  if (bsp_watchdog_count <= watchdog)
> +    return;
> +
> +  /* Kick watchdog, and clear interrupt pending bit */
> +  bsp_watchdogs[watchdog].timer->reload = reload_value;
> +  bsp_watchdogs[watchdog].timer->ctrl =
> +    (LEON3_GPTIMER_LD | LEON3_GPTIMER_EN) |
> +    (bsp_watchdogs[watchdog].timer->ctrl & ~(1<<4));
> +}
> +
> +void bsp_watchdog_stop(int watchdog)
> +{
> +  if (bsp_watchdog_count == 0)
> +    bsp_watchdog_init();
> +
> +  if (bsp_watchdog_count <= watchdog)
> +    return;
> +
> +  /* Stop watchdog timer */
> +  bsp_watchdogs[watchdog].timer->ctrl = 0;
> +}
> +
> +/* Use watchdog timer to reset system */
> +void bsp_watchdog_system_reset(void)
> +{
> +  sparc_disable_interrupts();
> +  bsp_watchdog_reload(0, 1);
> +}
> --
> 1.7.0.4
>