[U-Boot] [PATCH] MX31: Disable watchdog during low-power modes

[U-Boot] [PATCH] MX31: Disable watchdog during low-power modes

[Fabio Estevam]

Turn on the watchdog WDZST bit so that watchdog timer does not count during low power modes.

Prior to applying this patch mx31pdk board got watchdog resets because when it booted in the Linux prompt
and there was no activity, the system entered into idle mode while watchdog timer was still active.

Fix this by disabling watchdog timer during idle mode.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 arch/arm/cpu/arm1136/mx31/timer.c         |    4 ++--
 arch/arm/include/asm/arch-mx31/imx-regs.h |    2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/arm1136/mx31/timer.c b/arch/arm/cpu/arm1136/mx31/timer.c
index c05a39d..7197108 100644
--- a/arch/arm/cpu/arm1136/mx31/timer.c
+++ b/arch/arm/cpu/arm1136/mx31/timer.c
@@ -173,8 +173,8 @@ void mxc_hw_watchdog_enable(void)
 #else
 	secs = 64;
 #endif
-	writew(readw(&wdog->wcr) | (secs << WDOG_WT_SHIFT) | WDOG_ENABLE,
-		&wdog->wcr);
+	writew(readw(&wdog->wcr) | (secs << WDOG_WT_SHIFT) | WDOG_ENABLE
+						| WDOG_WDZST, &wdog->wcr);
 }
 
 
diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h b/arch/arm/include/asm/arch-mx31/imx-regs.h
index 2064870..338ba4d 100644
--- a/arch/arm/include/asm/arch-mx31/imx-regs.h
+++ b/arch/arm/include/asm/arch-mx31/imx-regs.h
@@ -71,6 +71,8 @@ struct cspi_regs {
 /* Watchdog Timer (WDOG) registers */
 #define WDOG_ENABLE	(1 << 2)
 #define WDOG_WT_SHIFT	8
+#define WDOG_WDZST	1
+
 struct wdog_regs {
 	u16 wcr;	/* Control */
 	u16 wsr;	/* Service */
-- 
1.6.0.4

[U-Boot] [PATCH] MX31: Disable watchdog during low-power modes

[Marek Vasut]

On Monday, September 19, 2011 07:51:10 PM Fabio Estevam wrote:
> Turn on the watchdog WDZST bit so that watchdog timer does not count during
> low power modes.
> 
> Prior to applying this patch mx31pdk board got watchdog resets because when
> it booted in the Linux prompt and there was no activity, the system
> entered into idle mode while watchdog timer was still active.
> 
> Fix this by disabling watchdog timer during idle mode.
> 
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  arch/arm/cpu/arm1136/mx31/timer.c         |    4 ++--
>  arch/arm/include/asm/arch-mx31/imx-regs.h |    2 ++
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/cpu/arm1136/mx31/timer.c
> b/arch/arm/cpu/arm1136/mx31/timer.c index c05a39d..7197108 100644
> --- a/arch/arm/cpu/arm1136/mx31/timer.c
> +++ b/arch/arm/cpu/arm1136/mx31/timer.c
> @@ -173,8 +173,8 @@ void mxc_hw_watchdog_enable(void)
>  #else
>  	secs = 64;
>  #endif
> -	writew(readw(&wdog->wcr) | (secs << WDOG_WT_SHIFT) | WDOG_ENABLE,
> -		&wdog->wcr);
> +	writew(readw(&wdog->wcr) | (secs << WDOG_WT_SHIFT) | WDOG_ENABLE
> +						| WDOG_WDZST, &wdog->wcr);

Maybe
tmp = readw();
tmp |= ...
writel(tmp, ...);

?
>  }
> 
> 
> diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h
> b/arch/arm/include/asm/arch-mx31/imx-regs.h index 2064870..338ba4d 100644
> --- a/arch/arm/include/asm/arch-mx31/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx31/imx-regs.h
> @@ -71,6 +71,8 @@ struct cspi_regs {
>  /* Watchdog Timer (WDOG) registers */
>  #define WDOG_ENABLE	(1 << 2)
>  #define WDOG_WT_SHIFT	8
> +#define WDOG_WDZST	1

(1 << 0) ?

> +
>  struct wdog_regs {
>  	u16 wcr;	/* Control */
>  	u16 wsr;	/* Service */

[U-Boot] [PATCH] MX31: Disable watchdog during low-power modes

[Stefano Babic]

On 09/20/2011 06:16 AM, Marek Vasut wrote:
> On Monday, September 19, 2011 07:51:10 PM Fabio Estevam wrote:
>> Turn on the watchdog WDZST bit so that watchdog timer does not count during
>> low power modes.
>>
>> Prior to applying this patch mx31pdk board got watchdog resets because when
>> it booted in the Linux prompt and there was no activity, the system
>> entered into idle mode while watchdog timer was still active.
>>
>> Fix this by disabling watchdog timer during idle mode.
>>
>> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
>> ---
>>  arch/arm/cpu/arm1136/mx31/timer.c         |    4 ++--
>>  arch/arm/include/asm/arch-mx31/imx-regs.h |    2 ++
>>  2 files changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/cpu/arm1136/mx31/timer.c
>> b/arch/arm/cpu/arm1136/mx31/timer.c index c05a39d..7197108 100644
>> --- a/arch/arm/cpu/arm1136/mx31/timer.c
>> +++ b/arch/arm/cpu/arm1136/mx31/timer.c
>> @@ -173,8 +173,8 @@ void mxc_hw_watchdog_enable(void)
>>  #else
>>  	secs = 64;
>>  #endif
>> -	writew(readw(&wdog->wcr) | (secs << WDOG_WT_SHIFT) | WDOG_ENABLE,
>> -		&wdog->wcr);
>> +	writew(readw(&wdog->wcr) | (secs << WDOG_WT_SHIFT) | WDOG_ENABLE
>> +						| WDOG_WDZST, &wdog->wcr);
> 
> Maybe
> tmp = readw();
> tmp |= ...
> writel(tmp, ...);

Well, I do not see a big difference with your proposal - so it is only a
question of taste. Calling write with an embedded read as first
parameter is used often in u-boot.

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-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] MX31: Disable watchdog during low-power modes

[Marek Vasut]

On Tuesday, September 20, 2011 10:48:12 AM Stefano Babic wrote:
> On 09/20/2011 06:16 AM, Marek Vasut wrote:
> > On Monday, September 19, 2011 07:51:10 PM Fabio Estevam wrote:
> >> Turn on the watchdog WDZST bit so that watchdog timer does not count
> >> during low power modes.
> >> 
> >> Prior to applying this patch mx31pdk board got watchdog resets because
> >> when it booted in the Linux prompt and there was no activity, the
> >> system entered into idle mode while watchdog timer was still active.
> >> 
> >> Fix this by disabling watchdog timer during idle mode.
> >> 
> >> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> >> ---
> >> 
> >>  arch/arm/cpu/arm1136/mx31/timer.c         |    4 ++--
> >>  arch/arm/include/asm/arch-mx31/imx-regs.h |    2 ++
> >>  2 files changed, 4 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/arch/arm/cpu/arm1136/mx31/timer.c
> >> b/arch/arm/cpu/arm1136/mx31/timer.c index c05a39d..7197108 100644
> >> --- a/arch/arm/cpu/arm1136/mx31/timer.c
> >> +++ b/arch/arm/cpu/arm1136/mx31/timer.c
> >> @@ -173,8 +173,8 @@ void mxc_hw_watchdog_enable(void)
> >> 
> >>  #else
> >>  
> >>  	secs = 64;
> >>  
> >>  #endif
> >> 
> >> -	writew(readw(&wdog->wcr) | (secs << WDOG_WT_SHIFT) | WDOG_ENABLE,
> >> -		&wdog->wcr);
> >> +	writew(readw(&wdog->wcr) | (secs << WDOG_WT_SHIFT) | WDOG_ENABLE
> >> +						| WDOG_WDZST, &wdog->wcr);
> > 
> > Maybe
> > tmp = readw();
> > tmp |= ...
> > writel(tmp, ...);
> 
> Well, I do not see a big difference with your proposal - so it is only a
> question of taste. Calling write with an embedded read as first
> parameter is used often in u-boot.

It's a matter of taste and readability ... but then maybe use clrsetbits()?

> 
> Best regards,
> Stefano Babic

[U-Boot] [PATCH] MX31: Disable watchdog during low-power modes

[Fabio Estevam]

Hi Stefano and Marek,

On Tue, Sep 20, 2011 at 5:48 AM, Stefano Babic <sbabic@denx.de> wrote:
...
>> Maybe
>> tmp = readw();
>> tmp |= ...
>> writel(tmp, ...);
>
> Well, I do not see a big difference with your proposal - so it is only a
> question of taste. Calling write with an embedded read as first
> parameter is used often in u-boot.

Have we reached an agreement on whether I should change my original
patch or if it is fine as is?

If I need to change it, please let me know how you prefer it.

I am fine either way, but really want to stop my MX31PDK to get
watchdog resets all the time ;-)

Thanks,

Fabio Estevam

[U-Boot] [PATCH] MX31: Disable watchdog during low-power modes

[Stefano Babic]

On 09/21/2011 05:43 AM, Fabio Estevam wrote:
> Hi Stefano and Marek,

Hi Fabio,

> 
> On Tue, Sep 20, 2011 at 5:48 AM, Stefano Babic <sbabic@denx.de> wrote:
> ...
>>> Maybe
>>> tmp = readw();
>>> tmp |= ...
>>> writel(tmp, ...);
>>
>> Well, I do not see a big difference with your proposal - so it is only a
>> question of taste. Calling write with an embedded read as first
>> parameter is used often in u-boot.
> 
> Have we reached an agreement on whether I should change my original
> patch or if it is fine as is?

IMHO I can push your patch as it is. Your change uses the same style as
in the current file, setting only a further bit.

> 
> If I need to change it, please let me know how you prefer it.
> 
> I am fine either way, but really want to stop my MX31PDK to get
> watchdog resets all the time ;-)

I can understand...

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-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

[U-Boot] [PATCH] MX31: Disable watchdog during low-power modes

[Wolfgang Denk]

Dear Fabio Estevam,

In message <CAOMZO5Do_W8FX6Qyihy8_bwu6XPoCZ=VKOdU54GvpF7p=CkJBw@mail.gmail.com> you wrote:
> 
> If I need to change it, please let me know how you prefer it.

Please use clrsetbits() as Marek suggested.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Documentation is the castor oil of programming.
Managers know it must be good because the programmers hate it so much.