All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefano Babic <sbabic@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH V2 1/1] mx31/mx35/mx51/mx53/mx6: add watchdog
Date: Tue, 21 Aug 2012 08:11:37 +0200	[thread overview]
Message-ID: <50332699.6040701@denx.de> (raw)
In-Reply-To: <1345503784-18559-1-git-send-email-troy.kisky@boundarydevices.com>

On 21/08/2012 01:03, Troy Kisky wrote:
> Use a common watchdog driver for all these cpus.
> 
> Signed-off-by: Troy Kisky <troy.kisky@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: sbabic at denx.de
=====================================================================

  reply	other threads:[~2012-08-21  6:11 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-20 23:03 [U-Boot] [PATCH V2 1/1] mx31/mx35/mx51/mx53/mx6: add watchdog Troy Kisky
2012-08-21  6:11 ` Stefano Babic [this message]
2012-08-21 17:51   ` Troy Kisky
2012-08-22  7:22     ` Stefano Babic
2012-08-22 18:30       ` Troy Kisky
2012-08-22 18:37         ` Troy Kisky
2012-08-23  9:19         ` Stefano Babic
2012-09-01  7:59           ` Stefano Babic
2012-10-23  1:19             ` [U-Boot] [PATCH V3 " Troy Kisky
2012-10-25 10:56               ` Stefano Babic
2012-10-28 11:48               ` Stefano Babic
2013-01-11 23:26                 ` Troy Kisky
2013-01-13 10:43                   ` Stefano Babic

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50332699.6040701@denx.de \
    --to=sbabic@denx.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.