Re: [U-Boot] [PATCH V2 1/1] mx31/mx35/mx51/mx53/mx6: add watchdog
On 21/08/2012 01:03, Troy Kisky wrote:
> Use a common watchdog driver for all these cpus.
>
> Signed-off-by: Troy Kisky
>
> ---
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(0x, &wdog->wsr);
> - writew(0x, &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_SHIFT8
> -#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_PPMRR0x4015
>
> -#define WDOG_BASE0x53FDC000
> -
> +#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 0x53FD
> #define SDMA_BASE_ADDR 0x53FD4000
> #define RTC_BASE_ADDR0x53FD8000
> -#define WDOG_BASE_ADDR 0x53FDC000
> +#define WDOG1_BASE_ADDR 0x53FDC000
> +#define CONFIG_IMX_WATCHDOG /* cpu_reset implemented in watchdog */
> #define PWM_BASE_ADDR0x53FE
> #define RTIC_BASE_ADDR 0x53FEC000
> #define IIM_BASE_ADDR0x53FF
> @@ -312,15 +313,6 @@ struct cspi_regs {
> u32 test;
> };
>
> -/* Watchdog Timer (WDOG) registers */
> -struct wdog_regs {
> - u16 wcr;/* Control */
> - u16 wsr;/* Ser
Re: [U-Boot] [PATCH V2 1/1] mx31/mx35/mx51/mx53/mx6: add watchdog
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_ADDR0x53FDC000
+#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 0x53FD
#define SDMA_BASE_ADDR0x53FD4000
#define RTC_BASE_ADDR 0x53FD8000
-#define WDOG_BASE_ADDR 0x53FDC000
+#define WDOG1_BASE_ADDR0x53FDC000
+#define CONFIG_IMX_WATCHDOG/* cpu_reset implemented in watchdog */
#define PWM_BASE_ADDR 0x53FE
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 + 0x0009)
#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 + 0x000A)
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 + 0x4)
#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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/1] mx31/mx35/mx51/mx53/mx6: add watchdog
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_BASE0x53FDC000
>> -
>> +#define WDOG1_BASE_ADDR0x53FDC000
>> +#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_ADDR0x53FD
>> #define SDMA_BASE_ADDR0x53FD4000
>> #define RTC_BASE_ADDR0x53FD8000
>> -#define WDOG_BASE_ADDR0x53FDC000
>> +#define WDOG1_BASE_ADDR0x53FDC000
>> +#define CONFIG_IMX_WATCHDOG/* cpu_reset implemented in watchdog */
>> #define PWM_BASE_ADDR0x53FE
>> 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 + 0x0009)
>> #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 + 0x000A)
>> 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 + 0x4)
>> #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
--
=
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
Re: [U-Boot] [PATCH V2 1/1] mx31/mx35/mx51/mx53/mx6: add watchdog
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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/1] mx31/mx35/mx51/mx53/mx6: add watchdog
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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2 1/1] mx31/mx35/mx51/mx53/mx6: add watchdog
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 -- = 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
Re: [U-Boot] [PATCH V2 1/1] mx31/mx35/mx51/mx53/mx6: add watchdog
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 -- = 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
