| Message ID | 1345503784-18559-1-git-send-email-troy.kisky@boundarydevices.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
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
On 8/20/2012 11:11 PM, Stefano Babic wrote: > On 21/08/2012 01:03, Troy Kisky wrote: > 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; > }; > -#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 > 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) > 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) > > 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 > So, you are saying CONFIG_ options don't belong in imx-regs.h, or you didn't notice I stuck it there, or both??? Troy
On 21/08/2012 19:51, Troy Kisky wrote: > On 8/20/2012 11:11 PM, Stefano Babic wrote: >> On 21/08/2012 01:03, Troy Kisky wrote: >> 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; >> }; >> -#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 >> 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) >> 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) >> >> 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 >> > So, you are saying CONFIG_ options don't belong in imx-regs.h, or > you didn't notice I stuck it there, or both??? I didn't notice, but CONFIG_ options do not belong to imx-regs.h. They must be define only inside the board configuration file. Best regards, Stefano
On 8/22/2012 12:22 AM, Stefano Babic wrote: > On 21/08/2012 19:51, Troy Kisky wrote: >> On 8/20/2012 11:11 PM, Stefano Babic wrote: >>> On 21/08/2012 01:03, Troy Kisky wrote: >>> >> So, you are saying CONFIG_ options don't belong in imx-regs.h, or >> you didn't notice I stuck it there, or both??? > I didn't notice, but CONFIG_ options do not belong to imx-regs.h. They > must be define only inside the board configuration file. > > Best regards, > Stefano > Would it be acceptable to put #if defined(CONFIG_MX31) || defined(CONFIG_MX35) || defined(CONFIG_MX51) \ || defined(CONFIG_MX53) || defined(CONFIG_MX6Q) #define CONFIG_IMX_WATCHDOG /* cpu_reset implemented in watchdog */ #endif at the bottom of config_cmd_default.h or do you have a better place in mind? Thanks Troy
On 8/22/2012 11:30 AM, Troy Kisky wrote: > On 8/22/2012 12:22 AM, Stefano Babic wrote: >> On 21/08/2012 19:51, Troy Kisky wrote: >>> On 8/20/2012 11:11 PM, Stefano Babic wrote: >>>> On 21/08/2012 01:03, Troy Kisky wrote: >>>> >>> So, you are saying CONFIG_ options don't belong in imx-regs.h, or >>> you didn't notice I stuck it there, or both??? >> I didn't notice, but CONFIG_ options do not belong to imx-regs.h. They >> must be define only inside the board configuration file. >> >> Best regards, >> Stefano >> > Would it be acceptable to put > #if defined(CONFIG_MX31) || defined(CONFIG_MX35) || > defined(CONFIG_MX51) \ > || defined(CONFIG_MX53) || defined(CONFIG_MX6Q) > #define CONFIG_IMX_WATCHDOG /* cpu_reset implemented in watchdog */ > #endif > > > at the bottom of config_cmd_default.h or do you have a better place in > mind? > > Thanks > Troy > or perhaps COBJS-$(CONFIG_MX31) += imx_watchdog.o COBJS-$(CONFIG_MX35) += imx_watchdog.o COBJS-$(CONFIG_MX51) += imx_watchdog.o COBJS-$(CONFIG_MX53) += imx_watchdog.o COBJS-$(CONFIG_MX6Q) += imx_watchdog.o Thanks Troy
On 22/08/2012 20:30, Troy Kisky wrote: > On 8/22/2012 12:22 AM, Stefano Babic wrote: >> On 21/08/2012 19:51, Troy Kisky wrote: >>> On 8/20/2012 11:11 PM, Stefano Babic wrote: >>>> On 21/08/2012 01:03, Troy Kisky wrote: >>>> Hi Troy, >>> So, you are saying CONFIG_ options don't belong in imx-regs.h, or >>> you didn't notice I stuck it there, or both??? >> I didn't notice, but CONFIG_ options do not belong to imx-regs.h. They >> must be define only inside the board configuration file. >> >> Best regards, >> Stefano >> > Would it be acceptable to put > #if defined(CONFIG_MX31) || defined(CONFIG_MX35) || defined(CONFIG_MX51) \ > || defined(CONFIG_MX53) || defined(CONFIG_MX6Q) > #define CONFIG_IMX_WATCHDOG /* cpu_reset implemented in watchdog */ > #endif > > > at the bottom of config_cmd_default.h or do you have a better place in > mind? > I do not think this will be accepted by Wolfgang. config_cmd_default.h is a general file and should not have any reference to SOC specific details. We have discussed recently how we can put common code across CPU boundaries. I mean, code generic for all i.MXs (as in this case) that is then replicated under cpu/arm1136, cpu/arm926ejs,... Maybe we can start a new thread and ask Albert (I am already asking him..) what he thinks about a arch/arm/cpu/plat-imx. Best regards, Stefano Babic
On 23/08/2012 11:19, Stefano Babic wrote: Hi Troy, > We have discussed recently how we can put common code across CPU > boundaries. I mean, code generic for all i.MXs (as in this case) that is > then replicated under cpu/arm1136, cpu/arm926ejs,... > It seems we are now blocked without exactly know what to do...I am making a proposal to move shared files in a common place. I will send a patch for it in some minutes. If we agree, it could be a place where you can put the reset code. Regards, Stefano
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 + */ + } +} diff --git a/include/configs/mx31pdk.h b/include/configs/mx31pdk.h index 7634de7..6c287a7 100644 --- a/include/configs/mx31pdk.h +++ b/include/configs/mx31pdk.h @@ -63,6 +63,7 @@ #define CONFIG_MXC_UART #define CONFIG_MXC_UART_BASE UART1_BASE #define CONFIG_HW_WATCHDOG +#define CONFIG_IMX_HW_WATCHDOG #define CONFIG_MXC_GPIO #define CONFIG_HARD_SPI diff --git a/include/configs/qong.h b/include/configs/qong.h index e824e17..cdde1cb 100644 --- a/include/configs/qong.h +++ b/include/configs/qong.h @@ -54,6 +54,7 @@ #define CONFIG_MXC_GPIO #define CONFIG_HW_WATCHDOG +#define CONFIG_IMX_HW_WATCHDOG #define CONFIG_MXC_SPI #define CONFIG_DEFAULT_SPI_BUS 1 diff --git a/include/watchdog.h b/include/watchdog.h index b959914..328a30b 100644 --- a/include/watchdog.h +++ b/include/watchdog.h @@ -35,6 +35,7 @@ * Hardware watchdog */ #ifdef CONFIG_HW_WATCHDOG + void hw_watchdog_init(void); #if defined(__ASSEMBLY__) #define WATCHDOG_RESET bl hw_watchdog_reset #else
Use a common watchdog driver for all these cpus. Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> --- 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!! --- arch/arm/cpu/arm1136/mx31/timer.c | 39 ----------------- arch/arm/cpu/arm1136/mx35/generic.c | 6 --- arch/arm/cpu/armv7/imx-common/cpu.c | 5 --- arch/arm/include/asm/arch-mx31/clock.h | 2 - arch/arm/include/asm/arch-mx31/imx-regs.h | 15 +------ arch/arm/include/asm/arch-mx35/imx-regs.h | 12 +----- arch/arm/include/asm/arch-mx5/imx-regs.h | 11 +---- arch/arm/include/asm/arch-mx6/imx-regs.h | 1 + board/davedenx/qong/qong.c | 9 +--- board/freescale/mx31pdk/mx31pdk.c | 9 +--- board/hale/tt01/tt01.c | 2 +- doc/README.watchdog | 33 +++++++++++++++ drivers/watchdog/Makefile | 1 + drivers/watchdog/imx_watchdog.c | 66 +++++++++++++++++++++++++++++ include/configs/mx31pdk.h | 1 + include/configs/qong.h | 1 + include/watchdog.h | 1 + 17 files changed, 112 insertions(+), 102 deletions(-) create mode 100644 doc/README.watchdog create mode 100644 drivers/watchdog/imx_watchdog.c