[PATCH] watchdog: add nowayout helpers to Watchdog Timer Driver Kernel API

[PATCH] watchdog: add nowayout helpers to Watchdog Timer Driver Kernel API

[Wim Van Sebroeck]

Add two nowayout helpers for the Watchdog Timer Driver Kernel API.

Signed-off-by: Wim Van Sebroeck <wim@iguana.be>
---
diff --git a/Documentation/watchdog/00-INDEX b/Documentation/watchdog/00-INDEX
index fc51128..fc9082a 100644
--- a/Documentation/watchdog/00-INDEX
+++ b/Documentation/watchdog/00-INDEX
@@ -1,5 +1,7 @@
 00-INDEX
 	- this file.
+convert_drivers_to_kernel_api.txt
+	- how-to for converting old watchdog drivers to the new kernel API.
 hpwdt.txt
 	- information on the HP iLO2 NMI watchdog
 pcwd-watchdog.txt
diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 4f7c894..1c443fe 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -1,6 +1,6 @@
 The Linux WatchDog Timer Driver Core kernel API.
 ===============================================
-Last reviewed: 22-Jul-2011
+Last reviewed: 29-Nov-2011
 
 Wim Van Sebroeck <wim@iguana.be>
 
@@ -142,6 +142,14 @@ bit-operations. The status bits that are defined are:
 * WDOG_NO_WAY_OUT: this bit stores the nowayout setting for the watchdog.
   If this bit is set then the watchdog timer will not be able to stop.
 
+To set the WDOG_NO_WAY_OUT status bit (before registering your watchdog
+timer device) you can either:
+* set it statically in your watchdog_device struct with
+	.status = WATCHDOG_NOWAYOUT_INIT_STATUS,
+  (this will set the value the same as CONFIG_WATCHDOG_NOWAYOUT) or
+* use the following helper function:
+static inline void watchdog_set_nowayout(struct watchdog_device *wdd, int nowayout)
+
 Note: The WatchDog Timer Driver Core supports the magic close feature and
 the nowayout feature. To use the magic close feature you must set the
 WDIOF_MAGICCLOSE bit in the options field of the watchdog's info structure.
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 111843f..43ba5b3 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -53,11 +53,7 @@ struct watchdog_info {
 
 #ifdef __KERNEL__
 
-#ifdef CONFIG_WATCHDOG_NOWAYOUT
-#define WATCHDOG_NOWAYOUT	1
-#else
-#define WATCHDOG_NOWAYOUT	0
-#endif
+#include <linux/bitops.h>
 
 struct watchdog_ops;
 struct watchdog_device;
@@ -122,6 +118,21 @@ struct watchdog_device {
 #define WDOG_NO_WAY_OUT		3	/* Is 'nowayout' feature set ? */
 };
 
+#ifdef CONFIG_WATCHDOG_NOWAYOUT
+#define WATCHDOG_NOWAYOUT		1
+#define WATCHDOG_NOWAYOUT_INIT_STATUS	(1 << WDOG_NO_WAY_OUT)
+#else
+#define WATCHDOG_NOWAYOUT		0
+#define WATCHDOG_NOWAYOUT_INIT_STATUS	0
+#endif
+
+/* Use the following function to set the nowayout feature */
+static inline void watchdog_set_nowayout(struct watchdog_device *wdd, int nowayout)
+{
+	if (nowayout)
+		set_bit(WDOG_NO_WAY_OUT, &wdd->status);
+}
+
 /* Use the following functions to manipulate watchdog driver specific data */
 static inline void watchdog_set_drvdata(struct watchdog_device *wdd, void *data)
 {

Re: [PATCH] watchdog: add nowayout helpers to Watchdog Timer Driver Kernel API

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 2029 bytes --]

On Tue, Nov 29, 2011 at 12:14:30PM +0100, Wim Van Sebroeck wrote:
> Add two nowayout helpers for the Watchdog Timer Driver Kernel API.
> 
> Signed-off-by: Wim Van Sebroeck <wim@iguana.be>
> ---
> diff --git a/Documentation/watchdog/00-INDEX b/Documentation/watchdog/00-INDEX
> index fc51128..fc9082a 100644
> --- a/Documentation/watchdog/00-INDEX
> +++ b/Documentation/watchdog/00-INDEX
> @@ -1,5 +1,7 @@
>  00-INDEX
>  	- this file.
> +convert_drivers_to_kernel_api.txt
> +	- how-to for converting old watchdog drivers to the new kernel API.

Since the howto needs a NOWAYOUT-update as well, I suggest I'll send a
patch for that and take care of the missing index-entry there?

> @@ -142,6 +142,14 @@ bit-operations. The status bits that are defined are:
>  * WDOG_NO_WAY_OUT: this bit stores the nowayout setting for the watchdog.
>    If this bit is set then the watchdog timer will not be able to stop.
>  
> +To set the WDOG_NO_WAY_OUT status bit (before registering your watchdog
> +timer device) you can either:
> +* set it statically in your watchdog_device struct with
> +	.status = WATCHDOG_NOWAYOUT_INIT_STATUS,
> +  (this will set the value the same as CONFIG_WATCHDOG_NOWAYOUT) or
> +* use the following helper function:
> +static inline void watchdog_set_nowayout(struct watchdog_device *wdd, int nowayout)
> +

I think one level of indentation would be helpful here so it visually
belongs to the WDOG_NO_WAY_OUT paragraph?

> +/* Use the following function to set the nowayout feature */
> +static inline void watchdog_set_nowayout(struct watchdog_device *wdd, int nowayout)
> +{
> +	if (nowayout)
> +		set_bit(WDOG_NO_WAY_OUT, &wdd->status);
> +}
> +

Other than that:

Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>

So, shall I fix the s3c-driver based on that or will you do?

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] watchdog: add nowayout helpers to Watchdog Timer Driver Kernel API

[Wim Van Sebroeck]

Hi Marc, Wolfram,

> Add two nowayout helpers for the Watchdog Timer Driver Kernel API.
> 
> Signed-off-by: Wim Van Sebroeck <wim@iguana.be>

And this would give following patch for your drivers (see below).

Kind regards,
Wim.
---
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index a79e384..4bc3744 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -378,6 +378,8 @@ static int __devinit s3c2410wdt_probe(struct platform_device *pdev)
 							"cannot start\n");
 	}
 
+	watchdog_set_nowayout(&s3c2410_wdd, nowayout);
+
 	ret = watchdog_register_device(&s3c2410_wdd);
 	if (ret) {
 		dev_err(dev, "cannot register watchdog (%d)\n", ret);
diff --git a/drivers/watchdog/wm831x_wdt.c b/drivers/watchdog/wm831x_wdt.c
index beb3ad2..6cd1ba4 100644
--- a/drivers/watchdog/wm831x_wdt.c
+++ b/drivers/watchdog/wm831x_wdt.c
@@ -213,11 +213,9 @@ static int __devinit wm831x_wdt_probe(struct platform_device *pdev)
 
 	wm831x_wdt->info = &wm831x_wdt_info;
 	wm831x_wdt->ops = &wm831x_wdt_ops;
+	watchdog_set_nowayout(wm831x_wdt, nowayout);
 	watchdog_set_drvdata(wm831x_wdt, driver_data);
 
-	if (nowayout)
-		wm831x_wdt->status |= WDOG_NO_WAY_OUT;
-
 	reg = wm831x_reg_read(wm831x, WM831X_WATCHDOG);
 	reg &= WM831X_WDOG_TO_MASK;
 	for (i = 0; i < ARRAY_SIZE(wm831x_wdt_cfgs); i++)

Re: [PATCH] watchdog: add nowayout helpers to Watchdog Timer Driver Kernel API

[Mark Brown]

On Tue, Nov 29, 2011 at 12:47:44PM +0100, Wim Van Sebroeck wrote:
> Hi Marc, Wolfram,

Wim, please take more care with people's names.  You routinely get my
name wrong and you addressed Axel Lin as Alex earlier as well.

> And this would give following patch for your drivers (see below).

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH] watchdog: add nowayout helpers to Watchdog Timer Driver Kernel API

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 435 bytes --]

On Tue, Nov 29, 2011 at 12:47:44PM +0100, Wim Van Sebroeck wrote:
> Hi Marc, Wolfram,
> 
> > Add two nowayout helpers for the Watchdog Timer Driver Kernel API.
> > 
> > Signed-off-by: Wim Van Sebroeck <wim@iguana.be>

Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] watchdog: add nowayout helpers to Watchdog Timer Driver Kernel API

[Wim Van Sebroeck]

Hi Mark,

> Wim, please take more care with people's names.  You routinely get my
> name wrong and you addressed Axel Lin as Alex earlier as well.

You are right. Sorry about that.

Kind regards,
Wim.

Re: [PATCH] watchdog: add nowayout helpers to Watchdog Timer Driver Kernel API

[Wim Van Sebroeck]

Hi Wolfram,

> > diff --git a/Documentation/watchdog/00-INDEX b/Documentation/watchdog/00-INDEX
> > index fc51128..fc9082a 100644
> > --- a/Documentation/watchdog/00-INDEX
> > +++ b/Documentation/watchdog/00-INDEX
> > @@ -1,5 +1,7 @@
> >  00-INDEX
> >  	- this file.
> > +convert_drivers_to_kernel_api.txt
> > +	- how-to for converting old watchdog drivers to the new kernel API.
> 
> Since the howto needs a NOWAYOUT-update as well, I suggest I'll send a
> patch for that and take care of the missing index-entry there?

Ok.

> > @@ -142,6 +142,14 @@ bit-operations. The status bits that are defined are:
> >  * WDOG_NO_WAY_OUT: this bit stores the nowayout setting for the watchdog.
> >    If this bit is set then the watchdog timer will not be able to stop.
> >  
> > +To set the WDOG_NO_WAY_OUT status bit (before registering your watchdog
> > +timer device) you can either:
> > +* set it statically in your watchdog_device struct with
> > +	.status = WATCHDOG_NOWAYOUT_INIT_STATUS,
> > +  (this will set the value the same as CONFIG_WATCHDOG_NOWAYOUT) or
> > +* use the following helper function:
> > +static inline void watchdog_set_nowayout(struct watchdog_device *wdd, int nowayout)
> > +
> 
> I think one level of indentation would be helpful here so it visually
> belongs to the WDOG_NO_WAY_OUT paragraph?

Will fix that.

> > +/* Use the following function to set the nowayout feature */
> > +static inline void watchdog_set_nowayout(struct watchdog_device *wdd, int nowayout)
> > +{
> > +	if (nowayout)
> > +		set_bit(WDOG_NO_WAY_OUT, &wdd->status);
> > +}
> > +
> 
> Other than that:
> 
> Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
> 
> So, shall I fix the s3c-driver based on that or will you do?

Will included the fix in the final patch.

Kind regards,
Wim.

[PATCH v2] watchdog: add nowayout helpers to Watchdog Timer Driver Kernel API

[Wim Van Sebroeck]

Hi All,

The patch like it is in the linux-next branch of the watchdog tree since a couple of minutes.

Kind regards,
Wim.

commit 3d17ee59111dd2311e5904294246d5b848342550
Author: Wim Van Sebroeck <wim@iguana.be>
Date:   Tue Nov 29 16:24:16 2011 +0100

    watchdog: add nowayout helpers to Watchdog Timer Driver Kernel API
    
    Add two nowayout helpers for the Watchdog Timer Driver Kernel API.
    And apply this to the already converted drivers.
    Note: s3c2410_wdt lost the nowayout feature during the conversion.
    
    Reviewed-by: Wolfram Sang <w.sang@pengutronix.de>
    Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
    Signed-off-by: Wim Van Sebroeck <wim@iguana.be>

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 4f7c894..4b93c28 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -1,6 +1,6 @@
 The Linux WatchDog Timer Driver Core kernel API.
 ===============================================
-Last reviewed: 22-Jul-2011
+Last reviewed: 29-Nov-2011
 
 Wim Van Sebroeck <wim@iguana.be>
 
@@ -142,6 +142,14 @@ bit-operations. The status bits that are defined are:
 * WDOG_NO_WAY_OUT: this bit stores the nowayout setting for the watchdog.
   If this bit is set then the watchdog timer will not be able to stop.
 
+  To set the WDOG_NO_WAY_OUT status bit (before registering your watchdog
+  timer device) you can either:
+  * set it statically in your watchdog_device struct with
+	.status = WATCHDOG_NOWAYOUT_INIT_STATUS,
+    (this will set the value the same as CONFIG_WATCHDOG_NOWAYOUT) or
+  * use the following helper function:
+  static inline void watchdog_set_nowayout(struct watchdog_device *wdd, int nowayout)
+
 Note: The WatchDog Timer Driver Core supports the magic close feature and
 the nowayout feature. To use the magic close feature you must set the
 WDIOF_MAGICCLOSE bit in the options field of the watchdog's info structure.
diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index a79e384..4bc3744 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -378,6 +378,8 @@ static int __devinit s3c2410wdt_probe(struct platform_device *pdev)
 							"cannot start\n");
 	}
 
+	watchdog_set_nowayout(&s3c2410_wdd, nowayout);
+
 	ret = watchdog_register_device(&s3c2410_wdd);
 	if (ret) {
 		dev_err(dev, "cannot register watchdog (%d)\n", ret);
diff --git a/drivers/watchdog/wm831x_wdt.c b/drivers/watchdog/wm831x_wdt.c
index beb3ad2..6cd1ba4 100644
--- a/drivers/watchdog/wm831x_wdt.c
+++ b/drivers/watchdog/wm831x_wdt.c
@@ -213,11 +213,9 @@ static int __devinit wm831x_wdt_probe(struct platform_device *pdev)
 
 	wm831x_wdt->info = &wm831x_wdt_info;
 	wm831x_wdt->ops = &wm831x_wdt_ops;
+	watchdog_set_nowayout(wm831x_wdt, nowayout);
 	watchdog_set_drvdata(wm831x_wdt, driver_data);
 
-	if (nowayout)
-		wm831x_wdt->status |= WDOG_NO_WAY_OUT;
-
 	reg = wm831x_reg_read(wm831x, WM831X_WATCHDOG);
 	reg &= WM831X_WDOG_TO_MASK;
 	for (i = 0; i < ARRAY_SIZE(wm831x_wdt_cfgs); i++)
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 111843f..43ba5b3 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -53,11 +53,7 @@ struct watchdog_info {
 
 #ifdef __KERNEL__
 
-#ifdef CONFIG_WATCHDOG_NOWAYOUT
-#define WATCHDOG_NOWAYOUT	1
-#else
-#define WATCHDOG_NOWAYOUT	0
-#endif
+#include <linux/bitops.h>
 
 struct watchdog_ops;
 struct watchdog_device;
@@ -122,6 +118,21 @@ struct watchdog_device {
 #define WDOG_NO_WAY_OUT		3	/* Is 'nowayout' feature set ? */
 };
 
+#ifdef CONFIG_WATCHDOG_NOWAYOUT
+#define WATCHDOG_NOWAYOUT		1
+#define WATCHDOG_NOWAYOUT_INIT_STATUS	(1 << WDOG_NO_WAY_OUT)
+#else
+#define WATCHDOG_NOWAYOUT		0
+#define WATCHDOG_NOWAYOUT_INIT_STATUS	0
+#endif
+
+/* Use the following function to set the nowayout feature */
+static inline void watchdog_set_nowayout(struct watchdog_device *wdd, int nowayout)
+{
+	if (nowayout)
+		set_bit(WDOG_NO_WAY_OUT, &wdd->status);
+}
+
 /* Use the following functions to manipulate watchdog driver specific data */
 static inline void watchdog_set_drvdata(struct watchdog_device *wdd, void *data)
 {