* [PATCH 1/2] watchdog: imx2_wdt: Do not clear IMX2_WDT_WCR_WDE
@ 2011-09-19 18:23 Fabio Estevam
2011-09-19 18:24 ` [PATCH 2/2] watchdog: imx2_wdt: Disable watchdog during low-power modes Fabio Estevam
2011-09-21 13:25 ` [PATCH 1/2] watchdog: imx2_wdt: Do not clear IMX2_WDT_WCR_WDE Wolfram Sang
0 siblings, 2 replies; 6+ messages in thread
From: Fabio Estevam @ 2011-09-19 18:23 UTC (permalink / raw)
To: linux-watchdog; +Cc: kernel, wim, Fabio Estevam
i.MX reference manual states that software can only write "1" into the WDE bit.
Remove the line that sets 0 to this bit and while at it remove an unneeded extra
write to register WCR.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/watchdog/imx2_wdt.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index b8ef2c6..19df82d 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -89,13 +89,10 @@ static inline void imx2_wdt_setup(void)
val &= ~IMX2_WDT_WCR_WT;
/* Generate reset if WDOG times out */
val &= ~IMX2_WDT_WCR_WRE;
- /* Keep Watchdog Disabled */
- val &= ~IMX2_WDT_WCR_WDE;
+
/* Set the watchdog's Time-Out value */
val |= WDOG_SEC_TO_COUNT(imx2_wdt.timeout);
- __raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
-
/* enable the watchdog */
val |= IMX2_WDT_WCR_WDE;
__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
--
1.6.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] watchdog: imx2_wdt: Disable watchdog during low-power modes
2011-09-19 18:23 [PATCH 1/2] watchdog: imx2_wdt: Do not clear IMX2_WDT_WCR_WDE Fabio Estevam
@ 2011-09-19 18:24 ` Fabio Estevam
2011-09-21 13:27 ` Wolfram Sang
2011-09-21 13:25 ` [PATCH 1/2] watchdog: imx2_wdt: Do not clear IMX2_WDT_WCR_WDE Wolfram Sang
1 sibling, 1 reply; 6+ messages in thread
From: Fabio Estevam @ 2011-09-19 18:24 UTC (permalink / raw)
To: linux-watchdog; +Cc: kernel, wim, Fabio Estevam
Activate WDZST so that the watchdog timer does not count during low power modes.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/watchdog/imx2_wdt.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 19df82d..e1a9abb 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -41,6 +41,7 @@
#define IMX2_WDT_WCR_WT (0xFF << 8) /* -> Watchdog Timeout Field */
#define IMX2_WDT_WCR_WRE (1 << 3) /* -> WDOG Reset Enable */
#define IMX2_WDT_WCR_WDE (1 << 2) /* -> Watchdog Enable */
+#define IMX2_WDT_WCR_WDZST (1 << 0) /* -> Watchdog Low Power */
#define IMX2_WDT_WSR 0x02 /* Service Register */
#define IMX2_WDT_SEQ1 0x5555 /* -> service sequence 1 */
@@ -93,6 +94,9 @@ static inline void imx2_wdt_setup(void)
/* Set the watchdog's Time-Out value */
val |= WDOG_SEC_TO_COUNT(imx2_wdt.timeout);
+ /* Disable watchdog suring low power modes */
+ val |= IMX2_WDT_WCR_WDZST;
+
/* enable the watchdog */
val |= IMX2_WDT_WCR_WDE;
__raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
--
1.6.0.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] watchdog: imx2_wdt: Do not clear IMX2_WDT_WCR_WDE
2011-09-19 18:23 [PATCH 1/2] watchdog: imx2_wdt: Do not clear IMX2_WDT_WCR_WDE Fabio Estevam
2011-09-19 18:24 ` [PATCH 2/2] watchdog: imx2_wdt: Disable watchdog during low-power modes Fabio Estevam
@ 2011-09-21 13:25 ` Wolfram Sang
1 sibling, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2011-09-21 13:25 UTC (permalink / raw)
To: Fabio Estevam; +Cc: linux-watchdog, kernel, wim
[-- Attachment #1: Type: text/plain, Size: 1785 bytes --]
On Mon, Sep 19, 2011 at 03:23:59PM -0300, Fabio Estevam wrote:
> i.MX reference manual states that software can only write "1" into the WDE bit.
But it also says "Note: This bit can be set/reset in debug mode
(exception)."
> Remove the line that sets 0 to this bit and while at it remove an unneeded extra
> write to register WCR.
The MX31 manual says in 37.5.3.1:
"The Watchdog Control Register (WCR) field WT[7:0] must have a time-out
value written to it before the Watchdog can be enabled. The WDOG is
enabled by setting the Watchdog Enable (WDE) bit in the WCR."
I seem to recall that on some imx that two-folded write was indeed
necessary.
I'd say better safe than sorry and keep things as they are?
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> drivers/watchdog/imx2_wdt.c | 5 +----
> 1 files changed, 1 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
> index b8ef2c6..19df82d 100644
> --- a/drivers/watchdog/imx2_wdt.c
> +++ b/drivers/watchdog/imx2_wdt.c
> @@ -89,13 +89,10 @@ static inline void imx2_wdt_setup(void)
> val &= ~IMX2_WDT_WCR_WT;
> /* Generate reset if WDOG times out */
> val &= ~IMX2_WDT_WCR_WRE;
> - /* Keep Watchdog Disabled */
> - val &= ~IMX2_WDT_WCR_WDE;
> +
> /* Set the watchdog's Time-Out value */
> val |= WDOG_SEC_TO_COUNT(imx2_wdt.timeout);
>
> - __raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
> -
> /* enable the watchdog */
> val |= IMX2_WDT_WCR_WDE;
> __raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
> --
> 1.6.0.4
>
>
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] watchdog: imx2_wdt: Disable watchdog during low-power modes
2011-09-19 18:24 ` [PATCH 2/2] watchdog: imx2_wdt: Disable watchdog during low-power modes Fabio Estevam
@ 2011-09-21 13:27 ` Wolfram Sang
2011-11-09 16:58 ` Fabio Estevam
0 siblings, 1 reply; 6+ messages in thread
From: Wolfram Sang @ 2011-09-21 13:27 UTC (permalink / raw)
To: Fabio Estevam; +Cc: linux-watchdog, kernel, wim
[-- Attachment #1: Type: text/plain, Size: 1775 bytes --]
On Mon, Sep 19, 2011 at 03:24:00PM -0300, Fabio Estevam wrote:
> Activate WDZST so that the watchdog timer does not count during low power modes.
Should be a user-selectable option IMHO. Changing the default behaviour
may cause regressions.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> drivers/watchdog/imx2_wdt.c | 4 ++++
> 1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
> index 19df82d..e1a9abb 100644
> --- a/drivers/watchdog/imx2_wdt.c
> +++ b/drivers/watchdog/imx2_wdt.c
> @@ -41,6 +41,7 @@
> #define IMX2_WDT_WCR_WT (0xFF << 8) /* -> Watchdog Timeout Field */
> #define IMX2_WDT_WCR_WRE (1 << 3) /* -> WDOG Reset Enable */
> #define IMX2_WDT_WCR_WDE (1 << 2) /* -> Watchdog Enable */
> +#define IMX2_WDT_WCR_WDZST (1 << 0) /* -> Watchdog Low Power */
>
> #define IMX2_WDT_WSR 0x02 /* Service Register */
> #define IMX2_WDT_SEQ1 0x5555 /* -> service sequence 1 */
> @@ -93,6 +94,9 @@ static inline void imx2_wdt_setup(void)
> /* Set the watchdog's Time-Out value */
> val |= WDOG_SEC_TO_COUNT(imx2_wdt.timeout);
>
> + /* Disable watchdog suring low power modes */
> + val |= IMX2_WDT_WCR_WDZST;
> +
> /* enable the watchdog */
> val |= IMX2_WDT_WCR_WDE;
> __raw_writew(val, imx2_wdt.base + IMX2_WDT_WCR);
> --
> 1.6.0.4
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] watchdog: imx2_wdt: Disable watchdog during low-power modes
2011-09-21 13:27 ` Wolfram Sang
@ 2011-11-09 16:58 ` Fabio Estevam
2011-11-09 22:04 ` Wolfram Sang
0 siblings, 1 reply; 6+ messages in thread
From: Fabio Estevam @ 2011-11-09 16:58 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-watchdog, kernel, wim
Wolfram,
On 9/21/2011 10:27 AM, Wolfram Sang wrote:
> On Mon, Sep 19, 2011 at 03:24:00PM -0300, Fabio Estevam wrote:
>> Activate WDZST so that the watchdog timer does not count during low power modes.
>
> Should be a user-selectable option IMHO. Changing the default behaviour
> may cause regressions.
I can´t visualize a real system usage where the watchdog needs to be active in low power mode.
Which regressions exactly are you concerned about? Which platforms?
Regards,
Fabio Estevam
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] watchdog: imx2_wdt: Disable watchdog during low-power modes
2011-11-09 16:58 ` Fabio Estevam
@ 2011-11-09 22:04 ` Wolfram Sang
0 siblings, 0 replies; 6+ messages in thread
From: Wolfram Sang @ 2011-11-09 22:04 UTC (permalink / raw)
To: Fabio Estevam; +Cc: linux-watchdog, kernel, wim
[-- Attachment #1: Type: text/plain, Size: 982 bytes --]
> >> Activate WDZST so that the watchdog timer does not count during low power modes.
> >
> > Should be a user-selectable option IMHO. Changing the default behaviour
> > may cause regressions.
>
> I can´t visualize a real system usage where the watchdog needs to be active in low power mode.
>
> Which regressions exactly are you concerned about? Which platforms?
I don't have any concrete example, yet I see things I could not visualize
before daily ;)
My concern was that changing the default behaviour of a crucial device like a
watchdog driver should be done _very_ carefully. And here it could be avoided
by making this feature optional per platform_data, so I thought. If others
think this is over-engineered, so be it. I just didn't want this concern go
unnoticed.
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-11-09 22:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-19 18:23 [PATCH 1/2] watchdog: imx2_wdt: Do not clear IMX2_WDT_WCR_WDE Fabio Estevam
2011-09-19 18:24 ` [PATCH 2/2] watchdog: imx2_wdt: Disable watchdog during low-power modes Fabio Estevam
2011-09-21 13:27 ` Wolfram Sang
2011-11-09 16:58 ` Fabio Estevam
2011-11-09 22:04 ` Wolfram Sang
2011-09-21 13:25 ` [PATCH 1/2] watchdog: imx2_wdt: Do not clear IMX2_WDT_WCR_WDE Wolfram Sang
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.