On 21/08/2012 01:03, Troy Kisky wrote:
> Use a common watchdog driver for all these cpus.
> 
> Signed-off-by: Troy Kisky <troy.ki...@boundarydevices.com>
> 
> ---

Hi Troy,

> v2:   add README.watchdog
>       include mx31/mx35 watchdogs
>       move to drivers/watchdog
> 
> Please test on a mx31 and mx35 board.
> qong and mx31pdk would be best!!
> ---

Yes, this should be done. I can do on a qong.

> diff --git a/arch/arm/cpu/arm1136/mx31/timer.c 
> b/arch/arm/cpu/arm1136/mx31/timer.c
> index 72081a8..d6c52d7 100644
> --- a/arch/arm/cpu/arm1136/mx31/timer.c
> +++ b/arch/arm/cpu/arm1136/mx31/timer.c
> @@ -161,42 +161,3 @@ ulong get_tbclk(void)
>  {
>       return CONFIG_MX31_CLK32;
>  }
> -
> -void reset_cpu(ulong addr)
> -{
> -     struct wdog_regs *wdog = (struct wdog_regs *)WDOG_BASE;
> -     wdog->wcr = WDOG_ENABLE;
> -     while (1)
> -             ;
> -}
> -
> -#ifdef CONFIG_HW_WATCHDOG
> -void mxc_hw_watchdog_enable(void)
> -{
> -     struct wdog_regs *wdog = (struct wdog_regs *)WDOG_BASE;
> -     u16 secs;
> -
> -     /*
> -      * The timer watchdog can be set between
> -      * 0.5 and 128 Seconds. If not defined
> -      * in configuration file, sets 64 Seconds
> -      */
> -#ifdef CONFIG_SYS_WD_TIMER_SECS
> -     secs = (CONFIG_SYS_WD_TIMER_SECS << 1) & 0xFF;
> -     if (!secs) secs = 1;
> -#else
> -     secs = 64;
> -#endif
> -     setbits_le16(&wdog->wcr, (secs << WDOG_WT_SHIFT) | WDOG_ENABLE
> -                                                      | WDOG_WDZST);
> -}
> -
> -
> -void mxc_hw_watchdog_reset(void)
> -{
> -     struct wdog_regs *wdog = (struct wdog_regs *)WDOG_BASE;
> -
> -     writew(0x5555, &wdog->wsr);
> -     writew(0xAAAA, &wdog->wsr);
> -}
> -#endif
> diff --git a/arch/arm/cpu/arm1136/mx35/generic.c 
> b/arch/arm/cpu/arm1136/mx35/generic.c
> index ea1f730..1af870c 100644
> --- a/arch/arm/cpu/arm1136/mx35/generic.c
> +++ b/arch/arm/cpu/arm1136/mx35/generic.c
> @@ -495,9 +495,3 @@ int get_clocks(void)
>  #endif
>       return 0;
>  }
> -
> -void reset_cpu(ulong addr)
> -{
> -     struct wdog_regs *wdog = (struct wdog_regs *)WDOG_BASE_ADDR;
> -     writew(4, &wdog->wcr);
> -}
> diff --git a/arch/arm/cpu/armv7/imx-common/cpu.c 
> b/arch/arm/cpu/armv7/imx-common/cpu.c
> index fa1d468..ebded87 100644
> --- a/arch/arm/cpu/armv7/imx-common/cpu.c
> +++ b/arch/arm/cpu/armv7/imx-common/cpu.c
> @@ -122,11 +122,6 @@ int cpu_mmc_init(bd_t *bis)
>  }
>  #endif
>  
> -void reset_cpu(ulong addr)
> -{
> -     __raw_writew(4, WDOG1_BASE_ADDR);
> -}
> -
>  u32 get_ahb_clk(void)
>  {
>       struct mxc_ccm_reg *imx_ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR;
> diff --git a/arch/arm/include/asm/arch-mx31/clock.h 
> b/arch/arm/include/asm/arch-mx31/clock.h
> index 852c19c..6cb2611 100644
> --- a/arch/arm/include/asm/arch-mx31/clock.h
> +++ b/arch/arm/include/asm/arch-mx31/clock.h
> @@ -43,7 +43,5 @@ extern void mx31_set_gpr(enum iomux_gp_func gp, char en);
>  void mx31_uart1_hw_init(void);
>  void mx31_uart2_hw_init(void);
>  void mx31_spi2_hw_init(void);
> -void mxc_hw_watchdog_enable(void);
> -void mxc_hw_watchdog_reset(void);
>  
>  #endif /* __ASM_ARCH_CLOCK_H */
> diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h 
> b/arch/arm/include/asm/arch-mx31/imx-regs.h
> index bba37ac..594d613 100644
> --- a/arch/arm/include/asm/arch-mx31/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx31/imx-regs.h
> @@ -68,17 +68,6 @@ struct cspi_regs {
>       u32 test;
>  };
>  
> -/* Watchdog Timer (WDOG) registers */
> -#define WDOG_ENABLE  (1 << 2)
> -#define WDOG_WT_SHIFT        8
> -#define WDOG_WDZST   (1 << 0)
> -
> -struct wdog_regs {
> -     u16 wcr;        /* Control */
> -     u16 wsr;        /* Service */
> -     u16 wrsr;       /* Reset Status */
> -};
> -
>  /* IIM Control Registers */
>  struct iim_regs {
>       u32 iim_stat;
> @@ -673,8 +662,8 @@ struct esdc_regs {
>  
>  #define ARM_PPMRR            0x40000015
>  
> -#define WDOG_BASE            0x53FDC000
> -
> +#define WDOG1_BASE_ADDR              0x53FDC000
> +#define CONFIG_IMX_WATCHDOG  /* cpu_reset implemented in watchdog */
>  /*
>   * GPIO
>   */
> diff --git a/arch/arm/include/asm/arch-mx35/imx-regs.h 
> b/arch/arm/include/asm/arch-mx35/imx-regs.h
> index b18b984..45b331f 100644
> --- a/arch/arm/include/asm/arch-mx35/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx35/imx-regs.h
> @@ -76,7 +76,8 @@
>  #define GPIO2_BASE_ADDR              0x53FD0000
>  #define SDMA_BASE_ADDR               0x53FD4000
>  #define RTC_BASE_ADDR                0x53FD8000
> -#define WDOG_BASE_ADDR               0x53FDC000
> +#define WDOG1_BASE_ADDR              0x53FDC000
> +#define CONFIG_IMX_WATCHDOG  /* cpu_reset implemented in watchdog */
>  #define PWM_BASE_ADDR                0x53FE0000
>  #define RTIC_BASE_ADDR               0x53FEC000
>  #define IIM_BASE_ADDR                0x53FF0000
> @@ -312,15 +313,6 @@ struct cspi_regs {
>       u32 test;
>  };
>  
> -/* Watchdog Timer (WDOG) registers */
> -struct wdog_regs {
> -     u16 wcr;        /* Control */
> -     u16 wsr;        /* Service */
> -     u16 wrsr;       /* Reset Status */
> -     u16 wicr;       /* Interrupt Control */
> -     u16 wmcr;       /* Misc Control */
> -};
> -
>  struct esdc_regs {
>       u32     esdctl0;
>       u32     esdcfg0;
> diff --git a/arch/arm/include/asm/arch-mx5/imx-regs.h 
> b/arch/arm/include/asm/arch-mx5/imx-regs.h
> index c53465f..caac574 100644
> --- a/arch/arm/include/asm/arch-mx5/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx5/imx-regs.h
> @@ -78,6 +78,7 @@
>  #define GPIO4_BASE_ADDR              (AIPS1_BASE_ADDR + 0x00090000)
>  #define KPP_BASE_ADDR                (AIPS1_BASE_ADDR + 0x00094000)
>  #define WDOG1_BASE_ADDR              (AIPS1_BASE_ADDR + 0x00098000)
> +#define CONFIG_IMX_WATCHDOG  /* cpu_reset implemented in watchdog */
>  #define WDOG2_BASE_ADDR              (AIPS1_BASE_ADDR + 0x0009C000)
>  #define GPT1_BASE_ADDR               (AIPS1_BASE_ADDR + 0x000A0000)
>  #define SRTC_BASE_ADDR               (AIPS1_BASE_ADDR + 0x000A4000)
> @@ -216,16 +217,6 @@
>   */
>  #define WBED         1
>  
> -/*
> - * WEIM WCR
> - */
> -#define BCM          1
> -#define GBCD(x)              (((x) & 0x3) << 1)
> -#define INTEN                (1 << 4)
> -#define INTPOL               (1 << 5)
> -#define WDOG_EN              (1 << 8)
> -#define WDOG_LIMIT(x)        (((x) & 0x3) << 9)
> -
>  #define CS0_128                                      0
>  #define CS0_64M_CS1_64M                              1
>  #define CS0_64M_CS1_32M_CS2_32M                      2
> diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h 
> b/arch/arm/include/asm/arch-mx6/imx-regs.h
> index dacb9ea..0f59567 100644
> --- a/arch/arm/include/asm/arch-mx6/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h
> @@ -115,6 +115,7 @@
>  #define GPIO7_BASE_ADDR             (AIPS1_OFF_BASE_ADDR + 0x34000)
>  #define KPP_BASE_ADDR               (AIPS1_OFF_BASE_ADDR + 0x38000)
>  #define WDOG1_BASE_ADDR             (AIPS1_OFF_BASE_ADDR + 0x3C000)
> +#define CONFIG_IMX_WATCHDOG  /* cpu_reset implemented in watchdog */
>  #define WDOG2_BASE_ADDR             (AIPS1_OFF_BASE_ADDR + 0x40000)
>  #define ANATOP_BASE_ADDR            (AIPS1_OFF_BASE_ADDR + 0x48000)
>  #define USB_PHY0_BASE_ADDR          (AIPS1_OFF_BASE_ADDR + 0x49000)
> diff --git a/board/davedenx/qong/qong.c b/board/davedenx/qong/qong.c
> index c41f11d..d45b25c 100644
> --- a/board/davedenx/qong/qong.c
> +++ b/board/davedenx/qong/qong.c
> @@ -36,13 +36,6 @@
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> -#ifdef CONFIG_HW_WATCHDOG
> -void hw_watchdog_reset(void)
> -{
> -     mxc_hw_watchdog_reset();
> -}
> -#endif
> -
>  int dram_init(void)
>  {
>       /* dram_init must store complete ramsize in gd->ram_size */
> @@ -182,7 +175,7 @@ int board_late_init(void)
>       pmic_reg_write(p, REG_INT_STATUS1, RTCRSTI);
>  
>  #ifdef CONFIG_HW_WATCHDOG
> -     mxc_hw_watchdog_enable();
> +     hw_watchdog_init();
>  #endif
>  
>       return 0;
> diff --git a/board/freescale/mx31pdk/mx31pdk.c 
> b/board/freescale/mx31pdk/mx31pdk.c
> index 9f8bc53..54ac961 100644
> --- a/board/freescale/mx31pdk/mx31pdk.c
> +++ b/board/freescale/mx31pdk/mx31pdk.c
> @@ -35,13 +35,6 @@
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> -#ifdef CONFIG_HW_WATCHDOG
> -void hw_watchdog_reset(void)
> -{
> -     mxc_hw_watchdog_reset();
> -}
> -#endif
> -
>  int dram_init(void)
>  {
>       /* dram_init must store complete ramsize in gd->ram_size */
> @@ -92,7 +85,7 @@ int board_late_init(void)
>       pmic_reg_write(p, REG_POWER_CTL0, val | COINCHEN);
>       pmic_reg_write(p, REG_INT_STATUS1, RTCRSTI);
>  #ifdef CONFIG_HW_WATCHDOG
> -     mxc_hw_watchdog_enable();
> +     hw_watchdog_init();
>  #endif
>       return 0;
>  }
> diff --git a/board/hale/tt01/tt01.c b/board/hale/tt01/tt01.c
> index 02e75ed..137c7e9 100644
> --- a/board/hale/tt01/tt01.c
> +++ b/board/hale/tt01/tt01.c
> @@ -178,7 +178,7 @@ int board_init(void)
>  int board_late_init(void)
>  {
>  #ifdef CONFIG_HW_WATCHDOG
> -     mxc_hw_watchdog_enable();
> +     hw_watchdog_init();
>  #endif
>  
>       return 0;
> diff --git a/doc/README.watchdog b/doc/README.watchdog
> new file mode 100644
> index 0000000..e4e9bd1
> --- /dev/null
> +++ b/doc/README.watchdog
> @@ -0,0 +1,33 @@
> +Watchdog driver general info
> +
> +CONFIG_HW_WATCHDOG
> +     This enables hw_watchdog_reset to be called during various loops,
> +     including waiting for a character on a serial port. But it
> +     does not also call hw_watchdog_init. Boards which want this
> +     enabled must call this function in their board file. This split
> +     is useful because some rom's enable the watchdog when downloading
> +     new code, so it must be serviced, but the board would rather it
> +     was off. And, it cannot always be turned off once on.
> +
> +CONFIG_WATCHDOG_TIMEOUT_MSECS
> +     Can be used to change the timeout for i.mx31/35/5x/6x.
> +     If not given, will default to maximum timeout. This would
> +     be 128000 msec for i.mx31/35/5x/6x.
> +
> +CONFIG_AT91SAM9_WATCHDOG
> +     Available for AT91SAM9 to service the watchdog.
> +
> +CONFIG_FTWDT010_WATCHDOG
> +     Available for FTWDT010 to service the watchdog.
> +
> +CONFIG_FTWDT010_HW_TIMEOUT
> +     Can be used to change the timeout for FTWDT010.
> +
> +CONFIG_IMX_WATCHDOG
> +     Compiles imx_watchdog.c. This is automatically set for i.mx31/35/5x/6x
> +     boards because the file implements reset_cpu.
> +
> +CONFIG_IMX_HW_WATCHDOG
> +     Available for i.mx31/35/5x/6x to service the watchdog. This is not
> +     automatically set because some boards (vision2) still need to define
> +     their own hw_watchdog_reset routine.
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 5579bf2..71e3c88 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -27,6 +27,7 @@ LIB := $(obj)libwatchdog.o
>  
>  COBJS-$(CONFIG_AT91SAM9_WATCHDOG) += at91sam9_wdt.o
>  COBJS-$(CONFIG_FTWDT010_WATCHDOG) += ftwdt010_wdt.o
> +COBJS-$(CONFIG_IMX_WATCHDOG) += imx_watchdog.o
>  
>  COBJS        := $(COBJS-y)
>  SRCS := $(COBJS:.o=.c)
> diff --git a/drivers/watchdog/imx_watchdog.c b/drivers/watchdog/imx_watchdog.c
> new file mode 100644
> index 0000000..3e5ea6d
> --- /dev/null
> +++ b/drivers/watchdog/imx_watchdog.c
> @@ -0,0 +1,66 @@
> +/*
> + * watchdog.c - driver for i.mx on-chip watchdog
> + *
> + * Licensed under the GPL-2 or later.
> + */
> +
> +#include <common.h>
> +#include <asm/io.h>
> +#include <watchdog.h>
> +#include <asm/arch/imx-regs.h>
> +
> +struct watchdog_regs {
> +     u16     wcr;    /* Control */
> +     u16     wsr;    /* Service */
> +     u16     wrsr;   /* Reset Status */
> +};
> +
> +#define WCR_WDZST    0x01
> +#define WCR_WDBG     0x02
> +#define WCR_WDE              0x04    /* WDOG enable */
> +#define WCR_WDT              0x08
> +#define WCR_WDW              0x80
> +#define SET_WCR_WT(x)        (x << 8)
> +
> +#ifdef CONFIG_IMX_HW_WATCHDOG
> +void hw_watchdog_reset(void)
> +{
> +     struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
> +
> +     writew(0x5555, &wdog->wsr);
> +     writew(0xaaaa, &wdog->wsr);
> +}
> +
> +void hw_watchdog_init(void)
> +{
> +     struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
> +     u16 timeout;
> +
> +     /*
> +      * The timer watchdog can be set between
> +      * 0.5 and 128 Seconds. If not defined
> +      * in configuration file, sets 128 Seconds
> +      */
> +#ifndef CONFIG_WATCHDOG_TIMEOUT_MSECS
> +#define CONFIG_WATCHDOG_TIMEOUT_MSECS 128000
> +#endif
> +     timeout = (CONFIG_WATCHDOG_TIMEOUT_MSECS / 500) - 1;
> +     writew(WCR_WDZST | WCR_WDBG | WCR_WDE | WCR_WDT |
> +             WCR_WDW | SET_WCR_WT(timeout), &wdog->wcr);
> +     hw_watchdog_reset();
> +}
> +#endif
> +
> +void reset_cpu(ulong addr)
> +{
> +     struct watchdog_regs *wdog = (struct watchdog_regs *)WDOG1_BASE_ADDR;
> +
> +     writew(WCR_WDE, &wdog->wcr);
> +     writew(0x5555, &wdog->wsr);
> +     writew(0xaaaa, &wdog->wsr);     /* load minimum 1/2 second timeout */
> +     while (1) {
> +             /*
> +              * spin for .5 seconds before reset
> +              */
> +     }
> +}

I have only an issue with your patch. You move the reset_cpu (good idea
so we can factorize it) inside imx_watchdog.c, but then it depends on
CONFIG_IMX_WATCHDOG. If it is not set, the file is not compiled and
there is no reset_cpu. This constraints all boards to activate it, but
this is not what we want.

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=====================================================================
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to