linux-watchdog.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] watchdog: softdog: cleanups
@ 2016-05-25  6:37 Wolfram Sang
  2016-05-25  6:37 ` [PATCH 1/7] watchdog: softdog: remove obsolete comments Wolfram Sang
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Wolfram Sang @ 2016-05-25  6:37 UTC (permalink / raw)
  To: linux-watchdog; +Cc: linux-renesas-soc, Wolfram Sang

While working on another series where I need the softdog, this series already
came out of it. It gets the driver prepared for the interesting stuff (coming
later today hopefully) but is orthogonal from that.

Buildbot is happy, branch is here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/softdog_cleanup

Please review, comment, apply...

Thanks,

   Wolfram

Wolfram Sang (7):
  watchdog: softdog: remove obsolete comments
  watchdog: softdog: use watchdog core to init timeout value
  watchdog: softdog: consistently use softdog_ prefix
  watchdog: softdog: remove forward declaration
  watchdog: softdog: sort includes to avoid duplicates
  watchdog: softdog: drop superfluous set_timeout callback
  watchdog: softdog: improve coding style

 drivers/watchdog/softdog.c | 92 +++++++++++-----------------------------------
 1 file changed, 21 insertions(+), 71 deletions(-)

-- 
2.8.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/7] watchdog: softdog: remove obsolete comments
  2016-05-25  6:37 [PATCH 0/7] watchdog: softdog: cleanups Wolfram Sang
@ 2016-05-25  6:37 ` Wolfram Sang
  2016-05-25 17:30   ` Guenter Roeck
  2016-05-25  6:37 ` [PATCH 2/7] watchdog: softdog: use watchdog core to init timeout value Wolfram Sang
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Wolfram Sang @ 2016-05-25  6:37 UTC (permalink / raw)
  To: linux-watchdog; +Cc: linux-renesas-soc, Wolfram Sang

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

The history is obsolete, especially since we switched to watchdog
framework. The section markers also don't make sense anymore given
the small size of the driver.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/softdog.c | 33 ---------------------------------
 1 file changed, 33 deletions(-)

diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
index 99a06f9e393064..8bc0b164afc94b 100644
--- a/drivers/watchdog/softdog.c
+++ b/drivers/watchdog/softdog.c
@@ -17,23 +17,6 @@
  *
  *	Software only watchdog driver. Unlike its big brother the WDT501P
  *	driver this won't always recover a failed machine.
- *
- *  03/96: Angelo Haritsis <ah@doc.ic.ac.uk> :
- *	Modularised.
- *	Added soft_margin; use upon insmod to change the timer delay.
- *	NB: uses same minor as wdt (WATCHDOG_MINOR); we could use separate
- *	    minors.
- *
- *  19980911 Alan Cox
- *	Made SMP safe for 2.3.x
- *
- *  20011127 Joel Becker (jlbec@evilplan.org>
- *	Added soft_noboot; Allows testing the softdog trigger without
- *	requiring a recompile.
- *	Added WDIOC_GETTIMEOUT and WDIOC_SETTIMOUT.
- *
- *  20020530 Joel Becker <joel.becker@oracle.com>
- *	Added Matt Domsch's nowayout module option.
  */
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
@@ -71,19 +54,11 @@ module_param(soft_panic, int, 0);
 MODULE_PARM_DESC(soft_panic,
 	"Softdog action, set to 1 to panic, 0 to reboot (default=0)");
 
-/*
- *	Our timer
- */
-
 static void watchdog_fire(unsigned long);
 
 static struct timer_list watchdog_ticktock =
 		TIMER_INITIALIZER(watchdog_fire, 0, 0);
 
-/*
- *	If the timer expires..
- */
-
 static void watchdog_fire(unsigned long data)
 {
 	module_put(THIS_MODULE);
@@ -99,10 +74,6 @@ static void watchdog_fire(unsigned long data)
 	}
 }
 
-/*
- *	Softdog operations
- */
-
 static int softdog_ping(struct watchdog_device *w)
 {
 	if (!mod_timer(&watchdog_ticktock, jiffies+(w->timeout*HZ)))
@@ -124,10 +95,6 @@ static int softdog_set_timeout(struct watchdog_device *w, unsigned int t)
 	return 0;
 }
 
-/*
- *	Kernel Interfaces
- */
-
 static struct watchdog_info softdog_info = {
 	.identity = "Software Watchdog",
 	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/7] watchdog: softdog: use watchdog core to init timeout value
  2016-05-25  6:37 [PATCH 0/7] watchdog: softdog: cleanups Wolfram Sang
  2016-05-25  6:37 ` [PATCH 1/7] watchdog: softdog: remove obsolete comments Wolfram Sang
@ 2016-05-25  6:37 ` Wolfram Sang
  2016-05-25 17:30   ` Guenter Roeck
  2016-05-25  6:37 ` [PATCH 3/7] watchdog: softdog: consistently use softdog_ prefix Wolfram Sang
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Wolfram Sang @ 2016-05-25  6:37 UTC (permalink / raw)
  To: linux-watchdog; +Cc: linux-renesas-soc, Wolfram Sang

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Error string and comment say we fall back to a default, but in reality
we bailed out. Refactor the code to use the core helper which then
matches the described behaviour. While updating the init message anyhow,
shorten it while we are here; no need for versioning there as well and
the name is already given via pr_fmt.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/softdog.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
index 8bc0b164afc94b..a9ad27dd46502b 100644
--- a/drivers/watchdog/softdog.c
+++ b/drivers/watchdog/softdog.c
@@ -111,22 +111,15 @@ static struct watchdog_device softdog_dev = {
 	.info = &softdog_info,
 	.ops = &softdog_ops,
 	.min_timeout = 1,
-	.max_timeout = 0xFFFF
+	.max_timeout = 65535,
+	.timeout = TIMER_MARGIN,
 };
 
 static int __init watchdog_init(void)
 {
 	int ret;
 
-	/* Check that the soft_margin value is within it's range;
-	   if not reset to the default */
-	if (soft_margin < 1 || soft_margin > 65535) {
-		pr_info("soft_margin must be 0 < soft_margin < 65536, using %d\n",
-			TIMER_MARGIN);
-		return -EINVAL;
-	}
-	softdog_dev.timeout = soft_margin;
-
+	watchdog_init_timeout(&softdog_dev, soft_margin, NULL);
 	watchdog_set_nowayout(&softdog_dev, nowayout);
 	watchdog_stop_on_reboot(&softdog_dev);
 
@@ -134,8 +127,8 @@ static int __init watchdog_init(void)
 	if (ret)
 		return ret;
 
-	pr_info("Software Watchdog Timer: 0.08 initialized. soft_noboot=%d soft_margin=%d sec soft_panic=%d (nowayout=%d)\n",
-		soft_noboot, soft_margin, soft_panic, nowayout);
+	pr_info("initialized. soft_noboot=%d soft_margin=%d sec soft_panic=%d (nowayout=%d)\n",
+		soft_noboot, softdog_dev.timeout, soft_panic, nowayout);
 
 	return 0;
 }
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/7] watchdog: softdog: consistently use softdog_ prefix
  2016-05-25  6:37 [PATCH 0/7] watchdog: softdog: cleanups Wolfram Sang
  2016-05-25  6:37 ` [PATCH 1/7] watchdog: softdog: remove obsolete comments Wolfram Sang
  2016-05-25  6:37 ` [PATCH 2/7] watchdog: softdog: use watchdog core to init timeout value Wolfram Sang
@ 2016-05-25  6:37 ` Wolfram Sang
  2016-05-25 17:30   ` Guenter Roeck
  2016-05-25  6:37 ` [PATCH 4/7] watchdog: softdog: remove forward declaration Wolfram Sang
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Wolfram Sang @ 2016-05-25  6:37 UTC (permalink / raw)
  To: linux-watchdog; +Cc: linux-renesas-soc, Wolfram Sang

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

And move module_init/exit to the proper place while here.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/softdog.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
index a9ad27dd46502b..0a29f5a0833787 100644
--- a/drivers/watchdog/softdog.c
+++ b/drivers/watchdog/softdog.c
@@ -54,12 +54,12 @@ module_param(soft_panic, int, 0);
 MODULE_PARM_DESC(soft_panic,
 	"Softdog action, set to 1 to panic, 0 to reboot (default=0)");
 
-static void watchdog_fire(unsigned long);
+static void softdog_fire(unsigned long);
 
-static struct timer_list watchdog_ticktock =
-		TIMER_INITIALIZER(watchdog_fire, 0, 0);
+static struct timer_list softdog_ticktock =
+		TIMER_INITIALIZER(softdog_fire, 0, 0);
 
-static void watchdog_fire(unsigned long data)
+static void softdog_fire(unsigned long data)
 {
 	module_put(THIS_MODULE);
 	if (soft_noboot)
@@ -76,14 +76,14 @@ static void watchdog_fire(unsigned long data)
 
 static int softdog_ping(struct watchdog_device *w)
 {
-	if (!mod_timer(&watchdog_ticktock, jiffies+(w->timeout*HZ)))
+	if (!mod_timer(&softdog_ticktock, jiffies+(w->timeout*HZ)))
 		__module_get(THIS_MODULE);
 	return 0;
 }
 
 static int softdog_stop(struct watchdog_device *w)
 {
-	if (del_timer(&watchdog_ticktock))
+	if (del_timer(&softdog_ticktock))
 		module_put(THIS_MODULE);
 
 	return 0;
@@ -115,7 +115,7 @@ static struct watchdog_device softdog_dev = {
 	.timeout = TIMER_MARGIN,
 };
 
-static int __init watchdog_init(void)
+static int __init softdog_init(void)
 {
 	int ret;
 
@@ -132,14 +132,13 @@ static int __init watchdog_init(void)
 
 	return 0;
 }
+module_init(softdog_init);
 
-static void __exit watchdog_exit(void)
+static void __exit softdog_exit(void)
 {
 	watchdog_unregister_device(&softdog_dev);
 }
-
-module_init(watchdog_init);
-module_exit(watchdog_exit);
+module_exit(softdog_exit);
 
 MODULE_AUTHOR("Alan Cox");
 MODULE_DESCRIPTION("Software Watchdog Device Driver");
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/7] watchdog: softdog: remove forward declaration
  2016-05-25  6:37 [PATCH 0/7] watchdog: softdog: cleanups Wolfram Sang
                   ` (2 preceding siblings ...)
  2016-05-25  6:37 ` [PATCH 3/7] watchdog: softdog: consistently use softdog_ prefix Wolfram Sang
@ 2016-05-25  6:37 ` Wolfram Sang
  2016-05-25 17:31   ` Guenter Roeck
  2016-05-25  6:37 ` [PATCH 5/7] watchdog: softdog: sort includes to avoid duplicates Wolfram Sang
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Wolfram Sang @ 2016-05-25  6:37 UTC (permalink / raw)
  To: linux-watchdog; +Cc: linux-renesas-soc, Wolfram Sang

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/softdog.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
index 0a29f5a0833787..42faa3d424d5ca 100644
--- a/drivers/watchdog/softdog.c
+++ b/drivers/watchdog/softdog.c
@@ -54,11 +54,6 @@ module_param(soft_panic, int, 0);
 MODULE_PARM_DESC(soft_panic,
 	"Softdog action, set to 1 to panic, 0 to reboot (default=0)");
 
-static void softdog_fire(unsigned long);
-
-static struct timer_list softdog_ticktock =
-		TIMER_INITIALIZER(softdog_fire, 0, 0);
-
 static void softdog_fire(unsigned long data)
 {
 	module_put(THIS_MODULE);
@@ -74,6 +69,9 @@ static void softdog_fire(unsigned long data)
 	}
 }
 
+static struct timer_list softdog_ticktock =
+		TIMER_INITIALIZER(softdog_fire, 0, 0);
+
 static int softdog_ping(struct watchdog_device *w)
 {
 	if (!mod_timer(&softdog_ticktock, jiffies+(w->timeout*HZ)))
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/7] watchdog: softdog: sort includes to avoid duplicates
  2016-05-25  6:37 [PATCH 0/7] watchdog: softdog: cleanups Wolfram Sang
                   ` (3 preceding siblings ...)
  2016-05-25  6:37 ` [PATCH 4/7] watchdog: softdog: remove forward declaration Wolfram Sang
@ 2016-05-25  6:37 ` Wolfram Sang
  2016-05-25 17:31   ` Guenter Roeck
  2016-05-25  6:37 ` [PATCH 6/7] watchdog: softdog: drop superfluous set_timeout callback Wolfram Sang
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Wolfram Sang @ 2016-05-25  6:37 UTC (permalink / raw)
  To: linux-watchdog; +Cc: linux-renesas-soc, Wolfram Sang

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/softdog.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
index 42faa3d424d5ca..ab0e02fc81a276 100644
--- a/drivers/watchdog/softdog.c
+++ b/drivers/watchdog/softdog.c
@@ -21,15 +21,15 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/init.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
-#include <linux/types.h>
+#include <linux/reboot.h>
 #include <linux/timer.h>
+#include <linux/types.h>
 #include <linux/watchdog.h>
-#include <linux/reboot.h>
-#include <linux/init.h>
-#include <linux/jiffies.h>
-#include <linux/kernel.h>
 
 #define TIMER_MARGIN	60		/* Default is 60 seconds */
 static unsigned int soft_margin = TIMER_MARGIN;	/* in seconds */
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/7] watchdog: softdog: drop superfluous set_timeout callback
  2016-05-25  6:37 [PATCH 0/7] watchdog: softdog: cleanups Wolfram Sang
                   ` (4 preceding siblings ...)
  2016-05-25  6:37 ` [PATCH 5/7] watchdog: softdog: sort includes to avoid duplicates Wolfram Sang
@ 2016-05-25  6:37 ` Wolfram Sang
  2016-05-25 17:31   ` Guenter Roeck
  2016-05-25  6:37 ` [PATCH 7/7] watchdog: softdog: improve coding style Wolfram Sang
  2016-07-17 19:48 ` [PATCH 0/7] watchdog: softdog: cleanups Wim Van Sebroeck
  7 siblings, 1 reply; 16+ messages in thread
From: Wolfram Sang @ 2016-05-25  6:37 UTC (permalink / raw)
  To: linux-watchdog; +Cc: linux-renesas-soc, Wolfram Sang

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

If we leave set_timeout empty, the core will do exactly what is
implemented here anyway.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/softdog.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
index ab0e02fc81a276..5e3a30b99d4415 100644
--- a/drivers/watchdog/softdog.c
+++ b/drivers/watchdog/softdog.c
@@ -87,12 +87,6 @@ static int softdog_stop(struct watchdog_device *w)
 	return 0;
 }
 
-static int softdog_set_timeout(struct watchdog_device *w, unsigned int t)
-{
-	w->timeout = t;
-	return 0;
-}
-
 static struct watchdog_info softdog_info = {
 	.identity = "Software Watchdog",
 	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
@@ -102,7 +96,6 @@ static struct watchdog_ops softdog_ops = {
 	.owner = THIS_MODULE,
 	.start = softdog_ping,
 	.stop = softdog_stop,
-	.set_timeout = softdog_set_timeout,
 };
 
 static struct watchdog_device softdog_dev = {
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 7/7] watchdog: softdog: improve coding style
  2016-05-25  6:37 [PATCH 0/7] watchdog: softdog: cleanups Wolfram Sang
                   ` (5 preceding siblings ...)
  2016-05-25  6:37 ` [PATCH 6/7] watchdog: softdog: drop superfluous set_timeout callback Wolfram Sang
@ 2016-05-25  6:37 ` Wolfram Sang
  2016-05-25 17:31   ` Guenter Roeck
  2016-07-17 19:48 ` [PATCH 0/7] watchdog: softdog: cleanups Wim Van Sebroeck
  7 siblings, 1 reply; 16+ messages in thread
From: Wolfram Sang @ 2016-05-25  6:37 UTC (permalink / raw)
  To: linux-watchdog; +Cc: linux-renesas-soc, Wolfram Sang

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/watchdog/softdog.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
index 5e3a30b99d4415..b067edf246dff2 100644
--- a/drivers/watchdog/softdog.c
+++ b/drivers/watchdog/softdog.c
@@ -57,9 +57,9 @@ MODULE_PARM_DESC(soft_panic,
 static void softdog_fire(unsigned long data)
 {
 	module_put(THIS_MODULE);
-	if (soft_noboot)
+	if (soft_noboot) {
 		pr_crit("Triggered - Reboot ignored\n");
-	else if (soft_panic) {
+	} else if (soft_panic) {
 		pr_crit("Initiating panic\n");
 		panic("Software Watchdog Timer expired");
 	} else {
@@ -74,7 +74,7 @@ static struct timer_list softdog_ticktock =
 
 static int softdog_ping(struct watchdog_device *w)
 {
-	if (!mod_timer(&softdog_ticktock, jiffies+(w->timeout*HZ)))
+	if (!mod_timer(&softdog_ticktock, jiffies + (w->timeout * HZ)))
 		__module_get(THIS_MODULE);
 	return 0;
 }
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/7] watchdog: softdog: remove obsolete comments
  2016-05-25  6:37 ` [PATCH 1/7] watchdog: softdog: remove obsolete comments Wolfram Sang
@ 2016-05-25 17:30   ` Guenter Roeck
  0 siblings, 0 replies; 16+ messages in thread
From: Guenter Roeck @ 2016-05-25 17:30 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-watchdog, linux-renesas-soc, Wolfram Sang

On Wed, May 25, 2016 at 08:37:43AM +0200, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> The history is obsolete, especially since we switched to watchdog
> framework. The section markers also don't make sense anymore given
> the small size of the driver.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/softdog.c | 33 ---------------------------------
>  1 file changed, 33 deletions(-)
> 
> diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
> index 99a06f9e393064..8bc0b164afc94b 100644
> --- a/drivers/watchdog/softdog.c
> +++ b/drivers/watchdog/softdog.c
> @@ -17,23 +17,6 @@
>   *
>   *	Software only watchdog driver. Unlike its big brother the WDT501P
>   *	driver this won't always recover a failed machine.
> - *
> - *  03/96: Angelo Haritsis <ah@doc.ic.ac.uk> :
> - *	Modularised.
> - *	Added soft_margin; use upon insmod to change the timer delay.
> - *	NB: uses same minor as wdt (WATCHDOG_MINOR); we could use separate
> - *	    minors.
> - *
> - *  19980911 Alan Cox
> - *	Made SMP safe for 2.3.x
> - *
> - *  20011127 Joel Becker (jlbec@evilplan.org>
> - *	Added soft_noboot; Allows testing the softdog trigger without
> - *	requiring a recompile.
> - *	Added WDIOC_GETTIMEOUT and WDIOC_SETTIMOUT.
> - *
> - *  20020530 Joel Becker <joel.becker@oracle.com>
> - *	Added Matt Domsch's nowayout module option.
>   */
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> @@ -71,19 +54,11 @@ module_param(soft_panic, int, 0);
>  MODULE_PARM_DESC(soft_panic,
>  	"Softdog action, set to 1 to panic, 0 to reboot (default=0)");
>  
> -/*
> - *	Our timer
> - */
> -
>  static void watchdog_fire(unsigned long);
>  
>  static struct timer_list watchdog_ticktock =
>  		TIMER_INITIALIZER(watchdog_fire, 0, 0);
>  
> -/*
> - *	If the timer expires..
> - */
> -
>  static void watchdog_fire(unsigned long data)
>  {
>  	module_put(THIS_MODULE);
> @@ -99,10 +74,6 @@ static void watchdog_fire(unsigned long data)
>  	}
>  }
>  
> -/*
> - *	Softdog operations
> - */
> -
>  static int softdog_ping(struct watchdog_device *w)
>  {
>  	if (!mod_timer(&watchdog_ticktock, jiffies+(w->timeout*HZ)))
> @@ -124,10 +95,6 @@ static int softdog_set_timeout(struct watchdog_device *w, unsigned int t)
>  	return 0;
>  }
>  
> -/*
> - *	Kernel Interfaces
> - */
> -
>  static struct watchdog_info softdog_info = {
>  	.identity = "Software Watchdog",
>  	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> -- 
> 2.8.1
> 
> --
> 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] 16+ messages in thread

* Re: [PATCH 2/7] watchdog: softdog: use watchdog core to init timeout value
  2016-05-25  6:37 ` [PATCH 2/7] watchdog: softdog: use watchdog core to init timeout value Wolfram Sang
@ 2016-05-25 17:30   ` Guenter Roeck
  0 siblings, 0 replies; 16+ messages in thread
From: Guenter Roeck @ 2016-05-25 17:30 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-watchdog, linux-renesas-soc, Wolfram Sang

On Wed, May 25, 2016 at 08:37:44AM +0200, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Error string and comment say we fall back to a default, but in reality
> we bailed out. Refactor the code to use the core helper which then
> matches the described behaviour. While updating the init message anyhow,
> shorten it while we are here; no need for versioning there as well and
> the name is already given via pr_fmt.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/softdog.c | 17 +++++------------
>  1 file changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
> index 8bc0b164afc94b..a9ad27dd46502b 100644
> --- a/drivers/watchdog/softdog.c
> +++ b/drivers/watchdog/softdog.c
> @@ -111,22 +111,15 @@ static struct watchdog_device softdog_dev = {
>  	.info = &softdog_info,
>  	.ops = &softdog_ops,
>  	.min_timeout = 1,
> -	.max_timeout = 0xFFFF
> +	.max_timeout = 65535,
> +	.timeout = TIMER_MARGIN,
>  };
>  
>  static int __init watchdog_init(void)
>  {
>  	int ret;
>  
> -	/* Check that the soft_margin value is within it's range;
> -	   if not reset to the default */
> -	if (soft_margin < 1 || soft_margin > 65535) {
> -		pr_info("soft_margin must be 0 < soft_margin < 65536, using %d\n",
> -			TIMER_MARGIN);
> -		return -EINVAL;
> -	}
> -	softdog_dev.timeout = soft_margin;
> -
> +	watchdog_init_timeout(&softdog_dev, soft_margin, NULL);
>  	watchdog_set_nowayout(&softdog_dev, nowayout);
>  	watchdog_stop_on_reboot(&softdog_dev);
>  
> @@ -134,8 +127,8 @@ static int __init watchdog_init(void)
>  	if (ret)
>  		return ret;
>  
> -	pr_info("Software Watchdog Timer: 0.08 initialized. soft_noboot=%d soft_margin=%d sec soft_panic=%d (nowayout=%d)\n",
> -		soft_noboot, soft_margin, soft_panic, nowayout);
> +	pr_info("initialized. soft_noboot=%d soft_margin=%d sec soft_panic=%d (nowayout=%d)\n",
> +		soft_noboot, softdog_dev.timeout, soft_panic, nowayout);
>  
>  	return 0;
>  }
> -- 
> 2.8.1
> 
> --
> 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] 16+ messages in thread

* Re: [PATCH 3/7] watchdog: softdog: consistently use softdog_ prefix
  2016-05-25  6:37 ` [PATCH 3/7] watchdog: softdog: consistently use softdog_ prefix Wolfram Sang
@ 2016-05-25 17:30   ` Guenter Roeck
  0 siblings, 0 replies; 16+ messages in thread
From: Guenter Roeck @ 2016-05-25 17:30 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-watchdog, linux-renesas-soc, Wolfram Sang

On Wed, May 25, 2016 at 08:37:45AM +0200, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> And move module_init/exit to the proper place while here.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/softdog.c | 21 ++++++++++-----------
>  1 file changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
> index a9ad27dd46502b..0a29f5a0833787 100644
> --- a/drivers/watchdog/softdog.c
> +++ b/drivers/watchdog/softdog.c
> @@ -54,12 +54,12 @@ module_param(soft_panic, int, 0);
>  MODULE_PARM_DESC(soft_panic,
>  	"Softdog action, set to 1 to panic, 0 to reboot (default=0)");
>  
> -static void watchdog_fire(unsigned long);
> +static void softdog_fire(unsigned long);
>  
> -static struct timer_list watchdog_ticktock =
> -		TIMER_INITIALIZER(watchdog_fire, 0, 0);
> +static struct timer_list softdog_ticktock =
> +		TIMER_INITIALIZER(softdog_fire, 0, 0);
>  
> -static void watchdog_fire(unsigned long data)
> +static void softdog_fire(unsigned long data)
>  {
>  	module_put(THIS_MODULE);
>  	if (soft_noboot)
> @@ -76,14 +76,14 @@ static void watchdog_fire(unsigned long data)
>  
>  static int softdog_ping(struct watchdog_device *w)
>  {
> -	if (!mod_timer(&watchdog_ticktock, jiffies+(w->timeout*HZ)))
> +	if (!mod_timer(&softdog_ticktock, jiffies+(w->timeout*HZ)))
>  		__module_get(THIS_MODULE);
>  	return 0;
>  }
>  
>  static int softdog_stop(struct watchdog_device *w)
>  {
> -	if (del_timer(&watchdog_ticktock))
> +	if (del_timer(&softdog_ticktock))
>  		module_put(THIS_MODULE);
>  
>  	return 0;
> @@ -115,7 +115,7 @@ static struct watchdog_device softdog_dev = {
>  	.timeout = TIMER_MARGIN,
>  };
>  
> -static int __init watchdog_init(void)
> +static int __init softdog_init(void)
>  {
>  	int ret;
>  
> @@ -132,14 +132,13 @@ static int __init watchdog_init(void)
>  
>  	return 0;
>  }
> +module_init(softdog_init);
>  
> -static void __exit watchdog_exit(void)
> +static void __exit softdog_exit(void)
>  {
>  	watchdog_unregister_device(&softdog_dev);
>  }
> -
> -module_init(watchdog_init);
> -module_exit(watchdog_exit);
> +module_exit(softdog_exit);
>  
>  MODULE_AUTHOR("Alan Cox");
>  MODULE_DESCRIPTION("Software Watchdog Device Driver");
> -- 
> 2.8.1
> 
> --
> 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] 16+ messages in thread

* Re: [PATCH 4/7] watchdog: softdog: remove forward declaration
  2016-05-25  6:37 ` [PATCH 4/7] watchdog: softdog: remove forward declaration Wolfram Sang
@ 2016-05-25 17:31   ` Guenter Roeck
  0 siblings, 0 replies; 16+ messages in thread
From: Guenter Roeck @ 2016-05-25 17:31 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-watchdog, linux-renesas-soc, Wolfram Sang

On Wed, May 25, 2016 at 08:37:46AM +0200, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/softdog.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
> index 0a29f5a0833787..42faa3d424d5ca 100644
> --- a/drivers/watchdog/softdog.c
> +++ b/drivers/watchdog/softdog.c
> @@ -54,11 +54,6 @@ module_param(soft_panic, int, 0);
>  MODULE_PARM_DESC(soft_panic,
>  	"Softdog action, set to 1 to panic, 0 to reboot (default=0)");
>  
> -static void softdog_fire(unsigned long);
> -
> -static struct timer_list softdog_ticktock =
> -		TIMER_INITIALIZER(softdog_fire, 0, 0);
> -
>  static void softdog_fire(unsigned long data)
>  {
>  	module_put(THIS_MODULE);
> @@ -74,6 +69,9 @@ static void softdog_fire(unsigned long data)
>  	}
>  }
>  
> +static struct timer_list softdog_ticktock =
> +		TIMER_INITIALIZER(softdog_fire, 0, 0);
> +
>  static int softdog_ping(struct watchdog_device *w)
>  {
>  	if (!mod_timer(&softdog_ticktock, jiffies+(w->timeout*HZ)))
> -- 
> 2.8.1
> 
> --
> 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] 16+ messages in thread

* Re: [PATCH 5/7] watchdog: softdog: sort includes to avoid duplicates
  2016-05-25  6:37 ` [PATCH 5/7] watchdog: softdog: sort includes to avoid duplicates Wolfram Sang
@ 2016-05-25 17:31   ` Guenter Roeck
  0 siblings, 0 replies; 16+ messages in thread
From: Guenter Roeck @ 2016-05-25 17:31 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-watchdog, linux-renesas-soc, Wolfram Sang

On Wed, May 25, 2016 at 08:37:47AM +0200, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/softdog.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
> index 42faa3d424d5ca..ab0e02fc81a276 100644
> --- a/drivers/watchdog/softdog.c
> +++ b/drivers/watchdog/softdog.c
> @@ -21,15 +21,15 @@
>  
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
> +#include <linux/init.h>
> +#include <linux/jiffies.h>
> +#include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
> -#include <linux/types.h>
> +#include <linux/reboot.h>
>  #include <linux/timer.h>
> +#include <linux/types.h>
>  #include <linux/watchdog.h>
> -#include <linux/reboot.h>
> -#include <linux/init.h>
> -#include <linux/jiffies.h>
> -#include <linux/kernel.h>
>  
>  #define TIMER_MARGIN	60		/* Default is 60 seconds */
>  static unsigned int soft_margin = TIMER_MARGIN;	/* in seconds */
> -- 
> 2.8.1
> 
> --
> 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] 16+ messages in thread

* Re: [PATCH 6/7] watchdog: softdog: drop superfluous set_timeout callback
  2016-05-25  6:37 ` [PATCH 6/7] watchdog: softdog: drop superfluous set_timeout callback Wolfram Sang
@ 2016-05-25 17:31   ` Guenter Roeck
  0 siblings, 0 replies; 16+ messages in thread
From: Guenter Roeck @ 2016-05-25 17:31 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-watchdog, linux-renesas-soc, Wolfram Sang

On Wed, May 25, 2016 at 08:37:48AM +0200, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> If we leave set_timeout empty, the core will do exactly what is
> implemented here anyway.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/softdog.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
> index ab0e02fc81a276..5e3a30b99d4415 100644
> --- a/drivers/watchdog/softdog.c
> +++ b/drivers/watchdog/softdog.c
> @@ -87,12 +87,6 @@ static int softdog_stop(struct watchdog_device *w)
>  	return 0;
>  }
>  
> -static int softdog_set_timeout(struct watchdog_device *w, unsigned int t)
> -{
> -	w->timeout = t;
> -	return 0;
> -}
> -
>  static struct watchdog_info softdog_info = {
>  	.identity = "Software Watchdog",
>  	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> @@ -102,7 +96,6 @@ static struct watchdog_ops softdog_ops = {
>  	.owner = THIS_MODULE,
>  	.start = softdog_ping,
>  	.stop = softdog_stop,
> -	.set_timeout = softdog_set_timeout,
>  };
>  
>  static struct watchdog_device softdog_dev = {
> -- 
> 2.8.1
> 
> --
> 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] 16+ messages in thread

* Re: [PATCH 7/7] watchdog: softdog: improve coding style
  2016-05-25  6:37 ` [PATCH 7/7] watchdog: softdog: improve coding style Wolfram Sang
@ 2016-05-25 17:31   ` Guenter Roeck
  0 siblings, 0 replies; 16+ messages in thread
From: Guenter Roeck @ 2016-05-25 17:31 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-watchdog, linux-renesas-soc, Wolfram Sang

On Wed, May 25, 2016 at 08:37:49AM +0200, Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/softdog.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
> index 5e3a30b99d4415..b067edf246dff2 100644
> --- a/drivers/watchdog/softdog.c
> +++ b/drivers/watchdog/softdog.c
> @@ -57,9 +57,9 @@ MODULE_PARM_DESC(soft_panic,
>  static void softdog_fire(unsigned long data)
>  {
>  	module_put(THIS_MODULE);
> -	if (soft_noboot)
> +	if (soft_noboot) {
>  		pr_crit("Triggered - Reboot ignored\n");
> -	else if (soft_panic) {
> +	} else if (soft_panic) {
>  		pr_crit("Initiating panic\n");
>  		panic("Software Watchdog Timer expired");
>  	} else {
> @@ -74,7 +74,7 @@ static struct timer_list softdog_ticktock =
>  
>  static int softdog_ping(struct watchdog_device *w)
>  {
> -	if (!mod_timer(&softdog_ticktock, jiffies+(w->timeout*HZ)))
> +	if (!mod_timer(&softdog_ticktock, jiffies + (w->timeout * HZ)))
>  		__module_get(THIS_MODULE);
>  	return 0;
>  }
> -- 
> 2.8.1
> 
> --
> 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] 16+ messages in thread

* Re: [PATCH 0/7] watchdog: softdog: cleanups
  2016-05-25  6:37 [PATCH 0/7] watchdog: softdog: cleanups Wolfram Sang
                   ` (6 preceding siblings ...)
  2016-05-25  6:37 ` [PATCH 7/7] watchdog: softdog: improve coding style Wolfram Sang
@ 2016-07-17 19:48 ` Wim Van Sebroeck
  7 siblings, 0 replies; 16+ messages in thread
From: Wim Van Sebroeck @ 2016-07-17 19:48 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: linux-watchdog, linux-renesas-soc

Hi Wolfram,

> While working on another series where I need the softdog, this series already
> came out of it. It gets the driver prepared for the interesting stuff (coming
> later today hopefully) but is orthogonal from that.
> 
> Buildbot is happy, branch is here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/softdog_cleanup
> 
> Please review, comment, apply...
> 
> Thanks,
> 
>    Wolfram
> 
> Wolfram Sang (7):
>   watchdog: softdog: remove obsolete comments
>   watchdog: softdog: use watchdog core to init timeout value
>   watchdog: softdog: consistently use softdog_ prefix
>   watchdog: softdog: remove forward declaration
>   watchdog: softdog: sort includes to avoid duplicates
>   watchdog: softdog: drop superfluous set_timeout callback
>   watchdog: softdog: improve coding style
> 
>  drivers/watchdog/softdog.c | 92 +++++++++++-----------------------------------
>  1 file changed, 21 insertions(+), 71 deletions(-)
> 
> -- 
> 2.8.1
> 
> --
> 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

These patches have been added to linux-watchdog-next.

Kind regards,
Wim.


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-07-17 19:48 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-25  6:37 [PATCH 0/7] watchdog: softdog: cleanups Wolfram Sang
2016-05-25  6:37 ` [PATCH 1/7] watchdog: softdog: remove obsolete comments Wolfram Sang
2016-05-25 17:30   ` Guenter Roeck
2016-05-25  6:37 ` [PATCH 2/7] watchdog: softdog: use watchdog core to init timeout value Wolfram Sang
2016-05-25 17:30   ` Guenter Roeck
2016-05-25  6:37 ` [PATCH 3/7] watchdog: softdog: consistently use softdog_ prefix Wolfram Sang
2016-05-25 17:30   ` Guenter Roeck
2016-05-25  6:37 ` [PATCH 4/7] watchdog: softdog: remove forward declaration Wolfram Sang
2016-05-25 17:31   ` Guenter Roeck
2016-05-25  6:37 ` [PATCH 5/7] watchdog: softdog: sort includes to avoid duplicates Wolfram Sang
2016-05-25 17:31   ` Guenter Roeck
2016-05-25  6:37 ` [PATCH 6/7] watchdog: softdog: drop superfluous set_timeout callback Wolfram Sang
2016-05-25 17:31   ` Guenter Roeck
2016-05-25  6:37 ` [PATCH 7/7] watchdog: softdog: improve coding style Wolfram Sang
2016-05-25 17:31   ` Guenter Roeck
2016-07-17 19:48 ` [PATCH 0/7] watchdog: softdog: cleanups Wim Van Sebroeck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).