[PATCH 00/10] Convert the IPMI watchdog to use the watchdog

[PATCH 00/10] Convert the IPMI watchdog to use the watchdog

[minyard]

This has been discussed before, and I've been working on it.  I think
this is the minimum that can be done to keep the IPMI watchdog
functionality exactly the same and use the watchdog subsystem.

The first patch is something that I thought might be a bug, and is not
directly related to the changes.

The IPMI watchdog has the following capabilities that need extensions to
the watchdog subsystem:

* The ability to provide read data on the device when a pretimeout
  occurs.
* The ability to dynamically set the timeout and pretimeout with module
  parameters.
* The ability to start the watchdog as soon as the driver starts.

I have no idea if anyone is using the capabilities, but they are there,
unfortunately.

A patch later in the series adds the ability to modify the watchdog time
on a reboot.  This was specifically requested in the past, so I know it
was at least used in the past.  The IPMI watchdog driver can do this
itself, but it's simple to add to the watchdog framework and gets rid of
some redundant code.

This leaves the driver code in the ipmi directory.  After these changes
it could be moved, but it doesn't matter much to me.

Thanks,

-corey

[PATCH 01/10] watchdog: Ignore stop_on_reboot if no stop function

[minyard]

From: Corey Minyard <cminyard@mvista.com>

The reboot notifier unconditionally calls the stop function on the
watchdog, which would result in a crash if the watchdog didn't have a
stop function.  So check at register time to see if there is a stop
function, and don't do stop_on_reboot if it is NULL.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/watchdog/watchdog_core.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index 423844757812..03943a34e9fb 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -260,10 +260,16 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
 
 	/* Module parameter to force watchdog policy on reboot. */
 	if (stop_on_reboot != -1) {
-		if (stop_on_reboot)
-			set_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
-		else
+		if (stop_on_reboot) {
+			if (!wdd->ops->stop) {
+				pr_err("watchdog%d: stop_on_reboot set, but no stop function.  Ignoring stop_on_reboot.\n", wdd->id);
+				clear_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
+			} else {
+				set_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
+			}
+		} else {
 			clear_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
+		}
 	}
 
 	if (test_bit(WDOG_STOP_ON_REBOOT, &wdd->status)) {
-- 
2.17.1

[PATCH 02/10] watchdog: Add read capability

[minyard]

From: Corey Minyard <cminyard@mvista.com>

Allow read, poll, and fasync calls on the watchdog device to be passed
to the driver.  This is so the IPMI driver can be moved over to the
watchdog framework, as it has the ability to have a read return when
data comes in on the watchdog.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/watchdog/watchdog_dev.c | 45 +++++++++++++++++++++++++++++++++
 include/linux/watchdog.h        |  8 ++++++
 2 files changed, 53 insertions(+)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 7e4cd34a8c20..45a0a4fe731d 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -698,6 +698,48 @@ static ssize_t watchdog_write(struct file *file, const char __user *data,
 	return len;
 }
 
+/*
+ *	watchdog_read: Pass a read on to the device if it accepts it
+ *	@file: file handle to the device
+ *	@buf: the buffer to read into
+ *	@count: the size of buf in bytes
+ *      @ppos: pointer to the file offset
+ *
+ *	The watchdog API defines a common set of functions for all watchdogs
+ *	according to their available features.
+ */
+
+static ssize_t watchdog_read(struct file *file, char __user *buf,
+			     size_t count, loff_t *ppos)
+{
+	struct watchdog_core_data *wd_data = file->private_data;
+	struct watchdog_device *wdd = wd_data->wdd;
+
+	if (!wdd->ops->read)
+		return -EINVAL;
+	return wdd->ops->read(wdd, file, buf, count, ppos);
+}
+
+static __poll_t watchdog_poll(struct file *file, poll_table *wait)
+{
+	struct watchdog_core_data *wd_data = file->private_data;
+	struct watchdog_device *wdd = wd_data->wdd;
+
+	if (!wdd->ops->poll)
+		return DEFAULT_POLLMASK;
+	return wdd->ops->poll(wdd, file, wait);
+}
+
+static int watchdog_fasync(int fd, struct file *file, int on)
+{
+	struct watchdog_core_data *wd_data = file->private_data;
+	struct watchdog_device *wdd = wd_data->wdd;
+
+	if (!wdd->ops->fasync)
+		return 0;
+	return wdd->ops->fasync(wdd, fd, file, on);
+}
+
 /*
  *	watchdog_ioctl: handle the different ioctl's for the watchdog device.
  *	@file: file handle to the device
@@ -951,6 +993,9 @@ static int watchdog_release(struct inode *inode, struct file *file)
 static const struct file_operations watchdog_fops = {
 	.owner		= THIS_MODULE,
 	.write		= watchdog_write,
+	.read		= watchdog_read,
+	.poll		= watchdog_poll,
+	.fasync		= watchdog_fasync,
 	.unlocked_ioctl	= watchdog_ioctl,
 	.compat_ioctl	= compat_ptr_ioctl,
 	.open		= watchdog_open,
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 1464ce6ffa31..36f99c8c973e 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -15,6 +15,7 @@
 #include <linux/device.h>
 #include <linux/kernel.h>
 #include <linux/notifier.h>
+#include <linux/poll.h>
 #include <uapi/linux/watchdog.h>
 
 struct watchdog_ops;
@@ -34,6 +35,9 @@ struct watchdog_governor;
  * @get_timeleft:The routine that gets the time left before a reset (in seconds).
  * @restart:	The routine for restarting the machine.
  * @ioctl:	The routines that handles extra ioctl calls.
+ * @read:	Call this is not NULL and a read comes in on the watchdog dev.
+ * @poll:	Call this is not NULL and a poll comes in on the watchdog dev.
+ * @fasync:	Call this is not NULL and a fasync comes in on the watchdog dev.
  *
  * The watchdog_ops structure contains a list of low-level operations
  * that control a watchdog device. It also contains the module that owns
@@ -53,6 +57,10 @@ struct watchdog_ops {
 	unsigned int (*get_timeleft)(struct watchdog_device *);
 	int (*restart)(struct watchdog_device *, unsigned long, void *);
 	long (*ioctl)(struct watchdog_device *, unsigned int, unsigned long);
+	ssize_t (*read)(struct watchdog_device *, struct file *, char __user *,
+			size_t, loff_t *);
+	__poll_t (*poll)(struct watchdog_device *, struct file *, poll_table *);
+	int (*fasync)(struct watchdog_device *, int, struct file *, int);
 };
 
 /** struct watchdog_device - The structure that defines a watchdog device
-- 
2.17.1

[PATCH 03/10] watchdog: Add documentation for read capability

[minyard]

From: Corey Minyard <cminyard@mvista.com>

Document the read, poll, and async operations.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 Documentation/watchdog/watchdog-kernel-api.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/watchdog/watchdog-kernel-api.rst b/Documentation/watchdog/watchdog-kernel-api.rst
index 068a55ee0d4a..aa15a2d35397 100644
--- a/Documentation/watchdog/watchdog-kernel-api.rst
+++ b/Documentation/watchdog/watchdog-kernel-api.rst
@@ -132,6 +132,10 @@ The list of watchdog operations is defined as::
 	unsigned int (*get_timeleft)(struct watchdog_device *);
 	int (*restart)(struct watchdog_device *);
 	long (*ioctl)(struct watchdog_device *, unsigned int, unsigned long);
+	ssize_t (*read)(struct watchdog_device *, struct file *, char __user *,
+			size_t, loff_t *);
+	__poll_t (*poll)(struct watchdog_device *, struct file *, poll_table *);
+	int (*fasync)(struct watchdog_device *, int, struct file *, int);
   };
 
 It is important that you first define the module owner of the watchdog timer
@@ -235,6 +239,14 @@ they are supported. These optional routines/operations are:
   our own internal ioctl call handling. This routine should return -ENOIOCTLCMD
   if a command is not supported. The parameters that are passed to the ioctl
   call are: watchdog_device, cmd and arg.
+* read: If a read call comes in on the watchdog device, call this function.
+  This allows a watchdog to provide data to the user.
+* poll: If a poll call comes in on the watchdog device, call this.  This
+  allows the user to do select/poll waiting on the device to have read data
+  ready.
+* fasync: If a fasync call comes in on the watchdog, call this.  This
+  allows the user to do async operations waiting for read data from
+  the device.
 
 The status bits should (preferably) be set with the set_bit and clear_bit alike
 bit-operations. The status bits that are defined are:
-- 
2.17.1

[PATCH 04/10] watchdog: Add functions to set the timeout and pretimeout

[minyard]

From: Corey Minyard <cminyard@mvista.com>

If the watchdog device wants to set it's pretimeout (say from module
parameters), this lets them do that.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/watchdog/watchdog_dev.c | 54 +++++++++++++++++++++++++++++----
 include/linux/watchdog.h        |  4 +++
 2 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 45a0a4fe731d..6c423aed3f3c 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -364,14 +364,14 @@ static unsigned int watchdog_get_status(struct watchdog_device *wdd)
 }
 
 /*
- *	watchdog_set_timeout: set the watchdog timer timeout
+ *	_watchdog_set_timeout: set the watchdog timer timeout
  *	@wdd: the watchdog device to set the timeout for
  *	@timeout: timeout to set in seconds
  *
  *	The caller must hold wd_data->lock.
  */
 
-static int watchdog_set_timeout(struct watchdog_device *wdd,
+static int _watchdog_set_timeout(struct watchdog_device *wdd,
 							unsigned int timeout)
 {
 	int err = 0;
@@ -396,14 +396,35 @@ static int watchdog_set_timeout(struct watchdog_device *wdd,
 	return err;
 }
 
+/*
+ *	watchdog_set_timeout: set the watchdog timer timeout
+ *	@wdd: the watchdog device to set the timeout for
+ *	@timeout: timeout to set in seconds
+ */
+
+int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int timeout)
+{
+	int err = 0;
+
+	if (!wdd->wd_data)
+		return -ENODEV;
+
+	mutex_lock(&wdd->wd_data->lock);
+	err = _watchdog_set_timeout(wdd, timeout);
+	mutex_unlock(&wdd->wd_data->lock);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(watchdog_set_timeout);
+
 /*
  *	watchdog_set_pretimeout: set the watchdog timer pretimeout
  *	@wdd: the watchdog device to set the timeout for
  *	@timeout: pretimeout to set in seconds
  */
 
-static int watchdog_set_pretimeout(struct watchdog_device *wdd,
-				   unsigned int timeout)
+static int _watchdog_set_pretimeout(struct watchdog_device *wdd,
+				    unsigned int timeout)
 {
 	int err = 0;
 
@@ -421,6 +442,27 @@ static int watchdog_set_pretimeout(struct watchdog_device *wdd,
 	return err;
 }
 
+/*
+ *	watchdog_set_pretimeout: set the watchdog timer pretimeout
+ *	@wdd: the watchdog device to set the timeout for
+ *	@timeout: pretimeout to set in seconds
+ */
+
+int watchdog_set_pretimeout(struct watchdog_device *wdd, unsigned int timeout)
+{
+	int err = 0;
+
+	if (!wdd->wd_data)
+		return -ENODEV;
+
+	mutex_lock(&wdd->wd_data->lock);
+	err = _watchdog_set_pretimeout(wdd, timeout);
+	mutex_unlock(&wdd->wd_data->lock);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(watchdog_set_pretimeout);
+
 /*
  *	watchdog_get_timeleft: wrapper to get the time left before a reboot
  *	@wdd: the watchdog device to get the remaining time from
@@ -809,7 +851,7 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
 			err = -EFAULT;
 			break;
 		}
-		err = watchdog_set_timeout(wdd, val);
+		err = _watchdog_set_timeout(wdd, val);
 		if (err < 0)
 			break;
 		/* If the watchdog is active then we send a keepalive ping
@@ -838,7 +880,7 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
 			err = -EFAULT;
 			break;
 		}
-		err = watchdog_set_pretimeout(wdd, val);
+		err = _watchdog_set_pretimeout(wdd, val);
 		break;
 	case WDIOC_GETPRETIMEOUT:
 		err = put_user(wdd->pretimeout, p);
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 36f99c8c973e..95396b644a9b 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -201,6 +201,10 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
 	return wdd->driver_data;
 }
 
+/* Allow the driver to set the timeout and pretimeout. */
+int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int timeout);
+int watchdog_set_pretimeout(struct watchdog_device *wdd, unsigned int timeout);
+
 /* Use the following functions to report watchdog pretimeout event */
 #if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV)
 void watchdog_notify_pretimeout(struct watchdog_device *wdd);
-- 
2.17.1

[PATCH 05/10] watchdog: Export an interface to start the watchdog

[minyard]

From: Corey Minyard <cminyard@mvista.com>

This way a watchdog driver can start itself.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/watchdog/watchdog_dev.c | 30 ++++++++++++++++++++++++++----
 include/linux/watchdog.h        |  3 +++
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 6c423aed3f3c..752358df1606 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -253,7 +253,7 @@ static enum hrtimer_restart watchdog_timer_expired(struct hrtimer *timer)
 }
 
 /*
- *	watchdog_start: wrapper to start the watchdog.
+ *	_watchdog_start: wrapper to start the watchdog.
  *	@wdd: the watchdog device to start
  *
  *	The caller must hold wd_data->lock.
@@ -263,7 +263,7 @@ static enum hrtimer_restart watchdog_timer_expired(struct hrtimer *timer)
  *	failure.
  */
 
-static int watchdog_start(struct watchdog_device *wdd)
+static int _watchdog_start(struct watchdog_device *wdd)
 {
 	struct watchdog_core_data *wd_data = wdd->wd_data;
 	ktime_t started_at;
@@ -289,6 +289,28 @@ static int watchdog_start(struct watchdog_device *wdd)
 	return err;
 }
 
+/*
+ *	watchdog_start: External interface to start the watchdog.
+ *	@wdd: the watchdog device to start
+ *
+ *	Start the watchdog if it is not active and mark it active.
+ *	This function returns zero on success or a negative errno code for
+ *	failure.
+ */
+
+int watchdog_start(struct watchdog_device *wdd)
+{
+	struct watchdog_core_data *wd_data = wdd->wd_data;
+	int err;
+
+	mutex_lock(&wd_data->lock);
+	err = _watchdog_start(wdd);
+	mutex_unlock(&wd_data->lock);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(watchdog_start);
+
 /*
  *	watchdog_stop: wrapper to stop the watchdog.
  *	@wdd: the watchdog device to stop
@@ -837,7 +859,7 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
 				break;
 		}
 		if (val & WDIOS_ENABLECARD)
-			err = watchdog_start(wdd);
+			err = _watchdog_start(wdd);
 		break;
 	case WDIOC_KEEPALIVE:
 		if (!(wdd->info->options & WDIOF_KEEPALIVEPING)) {
@@ -935,7 +957,7 @@ static int watchdog_open(struct inode *inode, struct file *file)
 		goto out_clear;
 	}
 
-	err = watchdog_start(wdd);
+	err = _watchdog_start(wdd);
 	if (err < 0)
 		goto out_mod;
 
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 95396b644a9b..1eefae61215d 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -205,6 +205,9 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
 int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int timeout);
 int watchdog_set_pretimeout(struct watchdog_device *wdd, unsigned int timeout);
 
+/* Allow the driver to start the watchdog. */
+int watchdog_start(struct watchdog_device *wdd);
+
 /* Use the following functions to report watchdog pretimeout event */
 #if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV)
 void watchdog_notify_pretimeout(struct watchdog_device *wdd);
-- 
2.17.1

[PATCH 06/10] ipmi:watchdog: Convert over to the watchdog framework

[minyard]

From: Corey Minyard <cminyard@mvista.com>

Use the standard watchdog framework for the IPMI watchdog.  This reduces
complexity and should avoid issues with interactions between the IPMI
watchdog and other watchdogs.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_watchdog.c | 381 ++++++++++++------------------
 1 file changed, 153 insertions(+), 228 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
index 55986e10a124..9265a5145691 100644
--- a/drivers/char/ipmi/ipmi_watchdog.c
+++ b/drivers/char/ipmi/ipmi_watchdog.c
@@ -19,7 +19,6 @@
 #include <linux/ipmi_smi.h>
 #include <linux/mutex.h>
 #include <linux/watchdog.h>
-#include <linux/miscdevice.h>
 #include <linux/init.h>
 #include <linux/completion.h>
 #include <linux/kdebug.h>
@@ -122,7 +121,11 @@
 
 #define IPMI_WDOG_TIMER_NOT_INIT_RESP	0x80
 
+/* Forward declaration. */
+static struct watchdog_device ipmi_wdd;
+
 static DEFINE_MUTEX(ipmi_watchdog_mutex);
+
 static bool nowayout = WATCHDOG_NOWAYOUT;
 
 static struct ipmi_user *watchdog_user;
@@ -154,7 +157,6 @@ static char data_to_read;
 static DECLARE_WAIT_QUEUE_HEAD(read_q);
 static struct fasync_struct *fasync_q;
 static atomic_t pretimeout_since_last_heartbeat;
-static char expect_close;
 
 static int ifnum_to_use = -1;
 
@@ -163,7 +165,6 @@ static int ifnum_to_use = -1;
 #define IPMI_SET_TIMEOUT_HB_IF_NECESSARY	1
 #define IPMI_SET_TIMEOUT_FORCE_HB		2
 
-static int ipmi_set_timeout(int do_heartbeat);
 static void ipmi_register_watchdog(int ipmi_intf);
 static void ipmi_unregister_watchdog(int ipmi_intf);
 
@@ -175,19 +176,24 @@ static int start_now;
 
 static int set_param_timeout(const char *val, const struct kernel_param *kp)
 {
-	char *endp;
-	int  l;
+	unsigned long l;
 	int  rv = 0;
+	int *ptr;
 
 	if (!val)
 		return -EINVAL;
-	l = simple_strtoul(val, &endp, 0);
-	if (endp == val)
-		return -EINVAL;
+	rv = kstrtoul(val, 0, &l);
+	if (rv)
+		return rv;
 
-	*((int *)kp->arg) = l;
-	if (watchdog_user)
-		rv = ipmi_set_timeout(IPMI_SET_TIMEOUT_HB_IF_NECESSARY);
+	ptr = (int *) kp->arg;
+	*ptr = l;
+	if (watchdog_user) {
+		if (ptr == &pretimeout)
+			rv = watchdog_set_pretimeout(&ipmi_wdd, l);
+		else if (ptr == &timeout)
+			rv = watchdog_set_timeout(&ipmi_wdd, l);
+	}
 
 	return rv;
 }
@@ -223,7 +229,7 @@ static int set_param_str(const char *val, const struct kernel_param *kp)
 
 	check_parms();
 	if (watchdog_user)
-		rv = ipmi_set_timeout(IPMI_SET_TIMEOUT_HB_IF_NECESSARY);
+		rv = watchdog_set_timeout(&ipmi_wdd, ipmi_wdd.timeout);
 
  out:
 	return rv;
@@ -266,6 +272,38 @@ static const struct kernel_param_ops param_ops_str = {
 	.get = get_param_str,
 };
 
+static int set_param_nowayout(const char *val, const struct kernel_param *kp)
+{
+	unsigned long l;
+	int rv;
+
+	if (!val)
+		return -EINVAL;
+	if (val[0] == 'Y' || val[0] == 'y')
+		l = true;
+	else if (val[0] == 'N' || val[0] == 'n')
+		l = false;
+	else {
+		rv = kstrtoul(val, 0, &l);
+		if (rv)
+			return rv;
+	}
+
+	nowayout = l;
+	if (nowayout)
+		set_bit(WDOG_NO_WAY_OUT, &ipmi_wdd.status);
+	else
+		clear_bit(WDOG_NO_WAY_OUT, &ipmi_wdd.status);
+
+	return 0;
+}
+
+static const struct kernel_param_ops param_ops_nowayout = {
+	.set = set_param_nowayout,
+	.get = param_get_bool,
+};
+#define param_check_nowayout param_check_bool
+
 module_param(ifnum_to_use, wdog_ifnum, 0644);
 MODULE_PARM_DESC(ifnum_to_use, "The interface number to use for the watchdog "
 		 "timer.  Setting to -1 defaults to the first registered "
@@ -296,24 +334,13 @@ module_param(start_now, int, 0444);
 MODULE_PARM_DESC(start_now, "Set to 1 to start the watchdog as"
 		 "soon as the driver is loaded.");
 
-module_param(nowayout, bool, 0644);
+module_param(nowayout, nowayout, 0644);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started "
 		 "(default=CONFIG_WATCHDOG_NOWAYOUT)");
 
 /* Default state of the timer. */
 static unsigned char ipmi_watchdog_state = WDOG_TIMEOUT_NONE;
 
-/* Is someone using the watchdog?  Only one user is allowed. */
-static unsigned long ipmi_wdog_open;
-
-/*
- * If set to 1, the heartbeat command will set the state to reset and
- * start the timer.  The timer doesn't normally run when the driver is
- * first opened until the heartbeat is set the first time, this
- * variable is used to accomplish this.
- */
-static int ipmi_start_timer_on_heartbeat;
-
 /* IPMI version of the BMC. */
 static unsigned char ipmi_version_major;
 static unsigned char ipmi_version_minor;
@@ -326,7 +353,7 @@ static int testing_nmi;
 static int nmi_handler_registered;
 #endif
 
-static int __ipmi_heartbeat(void);
+static int __ipmi_heartbeat(struct watchdog_device *wdd);
 
 /*
  * We use a mutex to make sure that only one thing can send a set a
@@ -352,7 +379,8 @@ static struct ipmi_recv_msg recv_msg = {
 	.done = msg_free_recv
 };
 
-static int __ipmi_set_timeout(struct ipmi_smi_msg  *smi_msg,
+static int __ipmi_set_timeout(struct watchdog_device *wdd,
+			      struct ipmi_smi_msg  *smi_msg,
 			      struct ipmi_recv_msg *recv_msg,
 			      int                  *send_heartbeat_now)
 {
@@ -380,15 +408,16 @@ static int __ipmi_set_timeout(struct ipmi_smi_msg  *smi_msg,
 
 	data[1] = 0;
 	WDOG_SET_TIMEOUT_ACT(data[1], ipmi_watchdog_state);
-	if ((pretimeout > 0) && (ipmi_watchdog_state != WDOG_TIMEOUT_NONE)) {
-	    WDOG_SET_PRETIMEOUT_ACT(data[1], preaction_val);
-	    data[2] = pretimeout;
+	if ((wdd->pretimeout > 0) &&
+	    (ipmi_watchdog_state != WDOG_TIMEOUT_NONE)) {
+		WDOG_SET_PRETIMEOUT_ACT(data[1], preaction_val);
+		data[2] = wdd->pretimeout;
 	} else {
 	    WDOG_SET_PRETIMEOUT_ACT(data[1], WDOG_PRETIMEOUT_NONE);
 	    data[2] = 0; /* No pretimeout. */
 	}
 	data[3] = 0;
-	WDOG_SET_TIMEOUT(data[4], data[5], timeout);
+	WDOG_SET_TIMEOUT(data[4], data[5], wdd->timeout);
 
 	addr.addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE;
 	addr.channel = IPMI_BMC_CHANNEL;
@@ -414,7 +443,7 @@ static int __ipmi_set_timeout(struct ipmi_smi_msg  *smi_msg,
 	return rv;
 }
 
-static int _ipmi_set_timeout(int do_heartbeat)
+static int _ipmi_set_timeout(struct watchdog_device *wdd, int do_heartbeat)
 {
 	int send_heartbeat_now;
 	int rv;
@@ -424,8 +453,7 @@ static int _ipmi_set_timeout(int do_heartbeat)
 
 	atomic_set(&msg_tofree, 2);
 
-	rv = __ipmi_set_timeout(&smi_msg,
-				&recv_msg,
+	rv = __ipmi_set_timeout(wdd, &smi_msg, &recv_msg,
 				&send_heartbeat_now);
 	if (rv)
 		return rv;
@@ -435,17 +463,17 @@ static int _ipmi_set_timeout(int do_heartbeat)
 	if ((do_heartbeat == IPMI_SET_TIMEOUT_FORCE_HB)
 		|| ((send_heartbeat_now)
 		    && (do_heartbeat == IPMI_SET_TIMEOUT_HB_IF_NECESSARY)))
-		rv = __ipmi_heartbeat();
+		rv = __ipmi_heartbeat(wdd);
 
 	return rv;
 }
 
-static int ipmi_set_timeout(int do_heartbeat)
+static int ipmi_set_timeout(struct watchdog_device *wdd, int do_heartbeat)
 {
 	int rv;
 
 	mutex_lock(&ipmi_watchdog_mutex);
-	rv = _ipmi_set_timeout(do_heartbeat);
+	rv = _ipmi_set_timeout(wdd, do_heartbeat);
 	mutex_unlock(&ipmi_watchdog_mutex);
 
 	return rv;
@@ -525,9 +553,8 @@ static void panic_halt_ipmi_set_timeout(void)
 	while (atomic_read(&panic_done_count) != 0)
 		ipmi_poll_interface(watchdog_user);
 	atomic_add(1, &panic_done_count);
-	rv = __ipmi_set_timeout(&panic_halt_smi_msg,
-				&panic_halt_recv_msg,
-				&send_heartbeat_now);
+	rv = __ipmi_set_timeout(&ipmi_wdd, &panic_halt_smi_msg,
+				&panic_halt_recv_msg, &send_heartbeat_now);
 	if (rv) {
 		atomic_sub(1, &panic_done_count);
 		pr_warn("Unable to extend the watchdog timeout\n");
@@ -539,7 +566,7 @@ static void panic_halt_ipmi_set_timeout(void)
 		ipmi_poll_interface(watchdog_user);
 }
 
-static int __ipmi_heartbeat(void)
+static int __ipmi_heartbeat(struct watchdog_device *wdd)
 {
 	struct kernel_ipmi_msg msg;
 	int rv;
@@ -596,7 +623,7 @@ static int __ipmi_heartbeat(void)
 		 * in this process, so must say no heartbeat to avoid a
 		 * deadlock on this mutex
 		 */
-		rv = _ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB);
+		rv = _ipmi_set_timeout(wdd, IPMI_SET_TIMEOUT_NO_HB);
 		if (rv) {
 			pr_err("Unable to send the command to set the watchdog's settings, giving up\n");
 			goto out;
@@ -617,165 +644,87 @@ static int __ipmi_heartbeat(void)
 	return rv;
 }
 
-static int _ipmi_heartbeat(void)
+static int _ipmi_heartbeat(struct watchdog_device *wdd)
 {
 	int rv;
 
 	if (!watchdog_user)
 		return -ENODEV;
 
-	if (ipmi_start_timer_on_heartbeat) {
-		ipmi_start_timer_on_heartbeat = 0;
-		ipmi_watchdog_state = action_val;
-		rv = _ipmi_set_timeout(IPMI_SET_TIMEOUT_FORCE_HB);
-	} else if (atomic_cmpxchg(&pretimeout_since_last_heartbeat, 1, 0)) {
-		/*
-		 * A pretimeout occurred, make sure we set the timeout.
-		 * We don't want to set the action, though, we want to
-		 * leave that alone (thus it can't be combined with the
-		 * above operation.
-		 */
-		rv = _ipmi_set_timeout(IPMI_SET_TIMEOUT_HB_IF_NECESSARY);
+	if (atomic_cmpxchg(&pretimeout_since_last_heartbeat, 1, 0)) {
+		/* A pretimeout occurred, make sure we set the timeout. */
+		rv = _ipmi_set_timeout(wdd, IPMI_SET_TIMEOUT_HB_IF_NECESSARY);
 	} else {
-		rv = __ipmi_heartbeat();
+		rv = __ipmi_heartbeat(wdd);
 	}
 
 	return rv;
 }
 
-static int ipmi_heartbeat(void)
+static int ipmi_wdog_set_timeout(struct watchdog_device *wdd,
+				 unsigned int timeout)
 {
 	int rv;
 
 	mutex_lock(&ipmi_watchdog_mutex);
-	rv = _ipmi_heartbeat();
+	wdd->timeout = timeout;
+	rv = _ipmi_set_timeout(wdd, IPMI_SET_TIMEOUT_HB_IF_NECESSARY);
 	mutex_unlock(&ipmi_watchdog_mutex);
 
 	return rv;
 }
 
-static struct watchdog_info ident = {
-	.options	= 0,	/* WDIOF_SETTIMEOUT, */
-	.firmware_version = 1,
-	.identity	= "IPMI"
-};
-
-static int ipmi_ioctl(struct file *file,
-		      unsigned int cmd, unsigned long arg)
+static int ipmi_wdog_set_pretimeout(struct watchdog_device *wdd,
+				    unsigned int pretimeout)
 {
-	void __user *argp = (void __user *)arg;
-	int i;
-	int val;
-
-	switch (cmd) {
-	case WDIOC_GETSUPPORT:
-		i = copy_to_user(argp, &ident, sizeof(ident));
-		return i ? -EFAULT : 0;
-
-	case WDIOC_SETTIMEOUT:
-		i = copy_from_user(&val, argp, sizeof(int));
-		if (i)
-			return -EFAULT;
-		timeout = val;
-		return _ipmi_set_timeout(IPMI_SET_TIMEOUT_HB_IF_NECESSARY);
-
-	case WDIOC_GETTIMEOUT:
-		i = copy_to_user(argp, &timeout, sizeof(timeout));
-		if (i)
-			return -EFAULT;
-		return 0;
-
-	case WDIOC_SETPRETIMEOUT:
-		i = copy_from_user(&val, argp, sizeof(int));
-		if (i)
-			return -EFAULT;
-		pretimeout = val;
-		return _ipmi_set_timeout(IPMI_SET_TIMEOUT_HB_IF_NECESSARY);
-
-	case WDIOC_GETPRETIMEOUT:
-		i = copy_to_user(argp, &pretimeout, sizeof(pretimeout));
-		if (i)
-			return -EFAULT;
-		return 0;
-
-	case WDIOC_KEEPALIVE:
-		return _ipmi_heartbeat();
-
-	case WDIOC_SETOPTIONS:
-		i = copy_from_user(&val, argp, sizeof(int));
-		if (i)
-			return -EFAULT;
-		if (val & WDIOS_DISABLECARD) {
-			ipmi_watchdog_state = WDOG_TIMEOUT_NONE;
-			_ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB);
-			ipmi_start_timer_on_heartbeat = 0;
-		}
-
-		if (val & WDIOS_ENABLECARD) {
-			ipmi_watchdog_state = action_val;
-			_ipmi_set_timeout(IPMI_SET_TIMEOUT_FORCE_HB);
-		}
-		return 0;
+	int rv;
 
-	case WDIOC_GETSTATUS:
-		val = 0;
-		i = copy_to_user(argp, &val, sizeof(val));
-		if (i)
-			return -EFAULT;
-		return 0;
+	mutex_lock(&ipmi_watchdog_mutex);
+	wdd->pretimeout = pretimeout;
+	rv = _ipmi_set_timeout(wdd, IPMI_SET_TIMEOUT_HB_IF_NECESSARY);
+	mutex_unlock(&ipmi_watchdog_mutex);
 
-	default:
-		return -ENOIOCTLCMD;
-	}
+	return rv;
 }
 
-static long ipmi_unlocked_ioctl(struct file *file,
-				unsigned int cmd,
-				unsigned long arg)
+static int ipmi_wdog_start(struct watchdog_device *wdd)
 {
-	int ret;
+	int rv;
 
 	mutex_lock(&ipmi_watchdog_mutex);
-	ret = ipmi_ioctl(file, cmd, arg);
+	ipmi_watchdog_state = action_val;
+	rv = _ipmi_set_timeout(wdd, IPMI_SET_TIMEOUT_FORCE_HB);
 	mutex_unlock(&ipmi_watchdog_mutex);
 
-	return ret;
+	return rv;
 }
 
-static ssize_t ipmi_write(struct file *file,
-			  const char  __user *buf,
-			  size_t      len,
-			  loff_t      *ppos)
+static int ipmi_wdog_ping(struct watchdog_device *wdd)
 {
 	int rv;
 
-	if (len) {
-		if (!nowayout) {
-			size_t i;
+	mutex_lock(&ipmi_watchdog_mutex);
+	rv = _ipmi_heartbeat(wdd);
+	mutex_unlock(&ipmi_watchdog_mutex);
 
-			/* In case it was set long ago */
-			expect_close = 0;
+	return rv;
+}
 
-			for (i = 0; i != len; i++) {
-				char c;
+static int ipmi_wdog_stop(struct watchdog_device *wdd)
+{
+	mutex_lock(&ipmi_watchdog_mutex);
+	ipmi_watchdog_state = WDOG_TIMEOUT_NONE;
+	_ipmi_set_timeout(wdd, IPMI_SET_TIMEOUT_NO_HB);
+	mutex_unlock(&ipmi_watchdog_mutex);
 
-				if (get_user(c, buf + i))
-					return -EFAULT;
-				if (c == 'V')
-					expect_close = 42;
-			}
-		}
-		rv = ipmi_heartbeat();
-		if (rv)
-			return rv;
-	}
-	return len;
+	return 0;
 }
 
-static ssize_t ipmi_read(struct file *file,
-			 char        __user *buf,
-			 size_t      count,
-			 loff_t      *ppos)
+static ssize_t ipmi_wdog_read(struct watchdog_device *wdd,
+			      struct file *file,
+			      char        __user *buf,
+			      size_t      count,
+			      loff_t      *ppos)
 {
 	int          rv = 0;
 	wait_queue_entry_t wait;
@@ -824,27 +773,8 @@ static ssize_t ipmi_read(struct file *file,
 	return rv;
 }
 
-static int ipmi_open(struct inode *ino, struct file *filep)
-{
-	switch (iminor(ino)) {
-	case WATCHDOG_MINOR:
-		if (test_and_set_bit(0, &ipmi_wdog_open))
-			return -EBUSY;
-
-
-		/*
-		 * Don't start the timer now, let it start on the
-		 * first heartbeat.
-		 */
-		ipmi_start_timer_on_heartbeat = 1;
-		return stream_open(ino, filep);
-
-	default:
-		return (-ENODEV);
-	}
-}
-
-static __poll_t ipmi_poll(struct file *file, poll_table *wait)
+static __poll_t ipmi_wdog_poll(struct watchdog_device *wdd,
+			       struct file *file, poll_table *wait)
 {
 	__poll_t mask = 0;
 
@@ -858,7 +788,8 @@ static __poll_t ipmi_poll(struct file *file, poll_table *wait)
 	return mask;
 }
 
-static int ipmi_fasync(int fd, struct file *file, int on)
+static int ipmi_wdog_fasync(struct watchdog_device *wdd,
+			    int fd, struct file *file, int on)
 {
 	int result;
 
@@ -867,43 +798,31 @@ static int ipmi_fasync(int fd, struct file *file, int on)
 	return (result);
 }
 
-static int ipmi_close(struct inode *ino, struct file *filep)
-{
-	if (iminor(ino) == WATCHDOG_MINOR) {
-		if (expect_close == 42) {
-			mutex_lock(&ipmi_watchdog_mutex);
-			ipmi_watchdog_state = WDOG_TIMEOUT_NONE;
-			_ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB);
-			mutex_unlock(&ipmi_watchdog_mutex);
-		} else {
-			pr_crit("Unexpected close, not stopping watchdog!\n");
-			ipmi_heartbeat();
-		}
-		clear_bit(0, &ipmi_wdog_open);
-	}
-
-	expect_close = 0;
-
-	return 0;
-}
+static const struct watchdog_ops ipmi_wdog_ops = {
+	.start		= ipmi_wdog_start,
+	.stop		= ipmi_wdog_stop,
+	.set_timeout	= ipmi_wdog_set_timeout,
+	.set_pretimeout	= ipmi_wdog_set_pretimeout,
+	.ping		= ipmi_wdog_ping,
+	.read		= ipmi_wdog_read,
+	.poll		= ipmi_wdog_poll,
+	.fasync		= ipmi_wdog_fasync,
+};
 
-static const struct file_operations ipmi_wdog_fops = {
-	.owner   = THIS_MODULE,
-	.read    = ipmi_read,
-	.poll    = ipmi_poll,
-	.write   = ipmi_write,
-	.unlocked_ioctl = ipmi_unlocked_ioctl,
-	.compat_ioctl	= compat_ptr_ioctl,
-	.open    = ipmi_open,
-	.release = ipmi_close,
-	.fasync  = ipmi_fasync,
-	.llseek  = no_llseek,
+static struct watchdog_info ipmi_wdog_info = {
+	.options	= (WDIOF_SETTIMEOUT | WDIOF_PRETIMEOUT |
+			   WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE),
+	.firmware_version = 1,
+	.identity	= "IPMI"
 };
 
-static struct miscdevice ipmi_wdog_miscdev = {
-	.minor		= WATCHDOG_MINOR,
-	.name		= "watchdog",
-	.fops		= &ipmi_wdog_fops
+static struct watchdog_device ipmi_wdd = {
+	.ops = &ipmi_wdog_ops,
+	.info = &ipmi_wdog_info,
+	.min_timeout = 1,
+	.max_timeout = 6553,
+	.min_hw_heartbeat_ms = 1,
+	.max_hw_heartbeat_ms = 65535,
 };
 
 static void ipmi_wdog_msg_handler(struct ipmi_recv_msg *msg,
@@ -944,7 +863,7 @@ static void ipmi_wdog_pretimeout_handler(void *handler_data)
 	atomic_set(&pretimeout_since_last_heartbeat, 1);
 }
 
-static void ipmi_wdog_panic_handler(void *user_data)
+static void ipmi_wdog_panic_handler(void *handler_data)
 {
 	static int panic_event_handled;
 
@@ -998,11 +917,21 @@ static void ipmi_register_watchdog(int ipmi_intf)
 		ipmi_version_minor = 0;
 	}
 
-	rv = misc_register(&ipmi_wdog_miscdev);
+	ipmi_wdd.pretimeout = pretimeout;
+
+	rv = watchdog_init_timeout(&ipmi_wdd, timeout, NULL);
 	if (rv < 0) {
 		ipmi_destroy_user(watchdog_user);
 		watchdog_user = NULL;
-		pr_crit("Unable to register misc device\n");
+		pr_crit("Unable to initialize the IPMI watchdog device\n");
+	}
+	watchdog_set_nowayout(&ipmi_wdd, nowayout);
+
+	rv = watchdog_register_device(&ipmi_wdd);
+	if (rv) {
+		ipmi_destroy_user(watchdog_user);
+		watchdog_user = NULL;
+		pr_crit("Unable to register the IPMI watchdog device\n");
 	}
 
 #ifdef HAVE_DIE_NMI
@@ -1022,7 +951,7 @@ static void ipmi_register_watchdog(int ipmi_intf)
 
 		testing_nmi = 1;
 
-		rv = ipmi_set_timeout(IPMI_SET_TIMEOUT_FORCE_HB);
+		rv = ipmi_set_timeout(&ipmi_wdd, IPMI_SET_TIMEOUT_FORCE_HB);
 		if (rv) {
 			pr_warn("Error starting timer to test NMI: 0x%x.  The NMI pretimeout will likely not work\n",
 				rv);
@@ -1047,13 +976,12 @@ static void ipmi_register_watchdog(int ipmi_intf)
 	if ((start_now) && (rv == 0)) {
 		/* Run from startup, so start the timer now. */
 		start_now = 0; /* Disable this function after first startup. */
-		ipmi_watchdog_state = action_val;
-		ipmi_set_timeout(IPMI_SET_TIMEOUT_FORCE_HB);
+		watchdog_start(&ipmi_wdd);
 		pr_info("Starting now!\n");
 	} else {
 		/* Stop the timer now. */
 		ipmi_watchdog_state = WDOG_TIMEOUT_NONE;
-		ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB);
+		_ipmi_set_timeout(&ipmi_wdd, IPMI_SET_TIMEOUT_NO_HB);
 	}
 }
 
@@ -1069,7 +997,7 @@ static void ipmi_unregister_watchdog(int ipmi_intf)
 		return;
 
 	/* Make sure no one can call us any more. */
-	misc_deregister(&ipmi_wdog_miscdev);
+	watchdog_unregister_device(&ipmi_wdd);
 
 	watchdog_user = NULL;
 
@@ -1088,9 +1016,6 @@ static void ipmi_unregister_watchdog(int ipmi_intf)
 	if (rv)
 		pr_warn("error unlinking from IPMI: %d\n",  rv);
 
-	/* If it comes back, restart it properly. */
-	ipmi_start_timer_on_heartbeat = 1;
-
 	mutex_unlock(&ipmi_watchdog_mutex);
 }
 
@@ -1147,7 +1072,7 @@ static int wdog_reboot_handler(struct notifier_block *this,
 		if (code == SYS_POWER_OFF || code == SYS_HALT) {
 			/* Disable the WDT if we are shutting down. */
 			ipmi_watchdog_state = WDOG_TIMEOUT_NONE;
-			ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB);
+			ipmi_set_timeout(&ipmi_wdd, IPMI_SET_TIMEOUT_NO_HB);
 		} else if (ipmi_watchdog_state != WDOG_TIMEOUT_NONE) {
 			/* Set a long timer to let the reboot happen or
 			   reset if it hangs, but only if the watchdog
@@ -1156,7 +1081,7 @@ static int wdog_reboot_handler(struct notifier_block *this,
 				timeout = 120;
 			pretimeout = 0;
 			ipmi_watchdog_state = WDOG_TIMEOUT_RESET;
-			ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB);
+			ipmi_set_timeout(&ipmi_wdd, IPMI_SET_TIMEOUT_NO_HB);
 		}
 	}
 	return NOTIFY_OK;
-- 
2.17.1

[PATCH 07/10] ipmi:watchdog: Allow the reboot timeout to be specified

[minyard]

From: Corey Minyard <cminyard@mvista.com>

It was fixed at 120, allow that to be changed.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_watchdog.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
index 9265a5145691..6e0c9faa6e6a 100644
--- a/drivers/char/ipmi/ipmi_watchdog.c
+++ b/drivers/char/ipmi/ipmi_watchdog.c
@@ -140,6 +140,9 @@ static int pretimeout;
 /* Default timeout to set on panic */
 static int panic_wdt_timeout = 255;
 
+/* Default timeout to set on reboot */
+static int reboot_wdt_timeout = 120;
+
 /* Default action is to reset the board on a timeout. */
 static unsigned char action_val = WDOG_TIMEOUT_RESET;
 
@@ -318,6 +321,9 @@ MODULE_PARM_DESC(pretimeout, "Pretimeout value in seconds.");
 module_param(panic_wdt_timeout, timeout, 0644);
 MODULE_PARM_DESC(panic_wdt_timeout, "Timeout value on kernel panic in seconds.");
 
+module_param(reboot_wdt_timeout, timeout, 0644);
+MODULE_PARM_DESC(reboot_wdt_timeout, "Timeout value on a reboot in seconds.");
+
 module_param_cb(action, &param_ops_str, action_op, 0644);
 MODULE_PARM_DESC(action, "Timeout action. One of: "
 		 "reset, none, power_cycle, power_off.");
@@ -1077,8 +1083,8 @@ static int wdog_reboot_handler(struct notifier_block *this,
 			/* Set a long timer to let the reboot happen or
 			   reset if it hangs, but only if the watchdog
 			   timer was already running. */
-			if (timeout < 120)
-				timeout = 120;
+			if (timeout < reboot_wdt_timeout)
+				timeout = reboot_wdt_timeout;
 			pretimeout = 0;
 			ipmi_watchdog_state = WDOG_TIMEOUT_RESET;
 			ipmi_set_timeout(&ipmi_wdd, IPMI_SET_TIMEOUT_NO_HB);
-- 
2.17.1

[PATCH 08/10] watchdog: Add a way to extend the timeout on a reboot

[minyard]

From: Corey Minyard <cminyard@mvista.com>

If reboot_timeout is set in the watchdog device struct, set the timeout
to that value on a reboot.  This way more time can be given for a reboot
to complete.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/watchdog/watchdog_core.c | 2 ++
 include/linux/watchdog.h         | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index 03943a34e9fb..5792f9bca645 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -165,6 +165,8 @@ static int watchdog_reboot_notifier(struct notifier_block *nb,
 			if (ret)
 				return NOTIFY_BAD;
 		}
+	} else if (wdd->reboot_timeout) {
+		watchdog_set_timeout(wdd, wdd->reboot_timeout);
 	}
 
 	return NOTIFY_DONE;
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 1eefae61215d..0fb57f29346c 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -92,6 +92,9 @@ struct watchdog_ops {
  * @status:	Field that contains the devices internal status bits.
  * @deferred:	Entry in wtd_deferred_reg_list which is used to
  *		register early initialized watchdogs.
+ * @reboot_timeout:
+ *		If non-zero, the timeout will be set to this value
+ *		on a reboot.  This lets a reboot be given more time.
  *
  * The watchdog_device structure contains all information about a
  * watchdog timer device.
@@ -125,6 +128,7 @@ struct watchdog_device {
 #define WDOG_HW_RUNNING		3	/* True if HW watchdog running */
 #define WDOG_STOP_ON_UNREGISTER	4	/* Should be stopped on unregister */
 	struct list_head deferred;
+	unsigned int reboot_timeout;
 };
 
 #define WATCHDOG_NOWAYOUT		IS_BUILTIN(CONFIG_WATCHDOG_NOWAYOUT)
-- 
2.17.1

[PATCH 09/10] ipmi:watchdog: Convert over to watchdog framework reboot handling

[minyard]

From: Corey Minyard <cminyard@mvista.com>

With the watchdog framework supporting a reboot timeout, we can switch
the IPMI driver over to use that and get rid of some redundance code.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_watchdog.c | 47 ++-----------------------------
 1 file changed, 3 insertions(+), 44 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
index 6e0c9faa6e6a..1ee884e6dcac 100644
--- a/drivers/char/ipmi/ipmi_watchdog.c
+++ b/drivers/char/ipmi/ipmi_watchdog.c
@@ -27,7 +27,6 @@
 #include <linux/uaccess.h>
 #include <linux/notifier.h>
 #include <linux/nmi.h>
-#include <linux/reboot.h>
 #include <linux/wait.h>
 #include <linux/poll.h>
 #include <linux/string.h>
@@ -140,9 +139,6 @@ static int pretimeout;
 /* Default timeout to set on panic */
 static int panic_wdt_timeout = 255;
 
-/* Default timeout to set on reboot */
-static int reboot_wdt_timeout = 120;
-
 /* Default action is to reset the board on a timeout. */
 static unsigned char action_val = WDOG_TIMEOUT_RESET;
 
@@ -321,7 +317,7 @@ MODULE_PARM_DESC(pretimeout, "Pretimeout value in seconds.");
 module_param(panic_wdt_timeout, timeout, 0644);
 MODULE_PARM_DESC(panic_wdt_timeout, "Timeout value on kernel panic in seconds.");
 
-module_param(reboot_wdt_timeout, timeout, 0644);
+module_param_named(reboot_wdt_timeout, ipmi_wdd.reboot_timeout, timeout, 0644);
 MODULE_PARM_DESC(reboot_wdt_timeout, "Timeout value on a reboot in seconds.");
 
 module_param_cb(action, &param_ops_str, action_op, 0644);
@@ -825,10 +821,12 @@ static struct watchdog_info ipmi_wdog_info = {
 static struct watchdog_device ipmi_wdd = {
 	.ops = &ipmi_wdog_ops,
 	.info = &ipmi_wdog_info,
+	.status = 1 << WDOG_STOP_ON_REBOOT,
 	.min_timeout = 1,
 	.max_timeout = 6553,
 	.min_hw_heartbeat_ms = 1,
 	.max_hw_heartbeat_ms = 65535,
+	.reboot_timeout = 120,
 };
 
 static void ipmi_wdog_msg_handler(struct ipmi_recv_msg *msg,
@@ -1065,40 +1063,6 @@ ipmi_nmi(unsigned int val, struct pt_regs *regs)
 }
 #endif
 
-static int wdog_reboot_handler(struct notifier_block *this,
-			       unsigned long         code,
-			       void                  *unused)
-{
-	static int reboot_event_handled;
-
-	if ((watchdog_user) && (!reboot_event_handled)) {
-		/* Make sure we only do this once. */
-		reboot_event_handled = 1;
-
-		if (code == SYS_POWER_OFF || code == SYS_HALT) {
-			/* Disable the WDT if we are shutting down. */
-			ipmi_watchdog_state = WDOG_TIMEOUT_NONE;
-			ipmi_set_timeout(&ipmi_wdd, IPMI_SET_TIMEOUT_NO_HB);
-		} else if (ipmi_watchdog_state != WDOG_TIMEOUT_NONE) {
-			/* Set a long timer to let the reboot happen or
-			   reset if it hangs, but only if the watchdog
-			   timer was already running. */
-			if (timeout < reboot_wdt_timeout)
-				timeout = reboot_wdt_timeout;
-			pretimeout = 0;
-			ipmi_watchdog_state = WDOG_TIMEOUT_RESET;
-			ipmi_set_timeout(&ipmi_wdd, IPMI_SET_TIMEOUT_NO_HB);
-		}
-	}
-	return NOTIFY_OK;
-}
-
-static struct notifier_block wdog_reboot_notifier = {
-	.notifier_call	= wdog_reboot_handler,
-	.next		= NULL,
-	.priority	= 0
-};
-
 static void ipmi_new_smi(int if_num, struct device *device)
 {
 	ipmi_register_watchdog(if_num);
@@ -1232,15 +1196,12 @@ static int __init ipmi_wdog_init(void)
 
 	check_parms();
 
-	register_reboot_notifier(&wdog_reboot_notifier);
-
 	rv = ipmi_smi_watcher_register(&smi_watcher);
 	if (rv) {
 #ifdef HAVE_DIE_NMI
 		if (nmi_handler_registered)
 			unregister_nmi_handler(NMI_UNKNOWN, "ipmi");
 #endif
-		unregister_reboot_notifier(&wdog_reboot_notifier);
 		pr_warn("can't register smi watcher\n");
 		return rv;
 	}
@@ -1259,8 +1220,6 @@ static void __exit ipmi_wdog_exit(void)
 	if (nmi_handler_registered)
 		unregister_nmi_handler(NMI_UNKNOWN, "ipmi");
 #endif
-
-	unregister_reboot_notifier(&wdog_reboot_notifier);
 }
 module_exit(ipmi_wdog_exit);
 module_init(ipmi_wdog_init);
-- 
2.17.1

[PATCH 10/10] ipmi:watchdog: Add the op to get the current timeout

[minyard]

From: Corey Minyard <cminyard@mvista.com>

The function is there and it can be done, so add it to round out the
functionality.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/char/ipmi/ipmi_watchdog.c | 80 +++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/drivers/char/ipmi/ipmi_watchdog.c b/drivers/char/ipmi/ipmi_watchdog.c
index 1ee884e6dcac..e0dd820af3a9 100644
--- a/drivers/char/ipmi/ipmi_watchdog.c
+++ b/drivers/char/ipmi/ipmi_watchdog.c
@@ -722,6 +722,85 @@ static int ipmi_wdog_stop(struct watchdog_device *wdd)
 	return 0;
 }
 
+/*
+ * These are for fetching the current watchdog timeout.  We use a
+ * separate set so we don't block a time set when getting the time.
+ */
+static DEFINE_MUTEX(ipmi_wdog_get_mutex);
+static atomic_t get_msg_tofree = ATOMIC_INIT(0);
+static DECLARE_COMPLETION(get_msg_wait);
+static void get_msg_free_smi(struct ipmi_smi_msg *msg)
+{
+	if (atomic_dec_and_test(&get_msg_tofree))
+		complete(&get_msg_wait);
+}
+static void get_msg_free_recv(struct ipmi_recv_msg *msg)
+{
+	if (atomic_dec_and_test(&get_msg_tofree))
+		complete(&get_msg_wait);
+}
+static struct ipmi_smi_msg get_smi_msg = {
+	.done = get_msg_free_smi
+};
+static struct ipmi_recv_msg get_recv_msg = {
+	.done = get_msg_free_recv
+};
+
+static unsigned int ipmi_wdog_get_timeleft(struct watchdog_device *wdd)
+{
+	struct kernel_ipmi_msg msg;
+	struct ipmi_system_interface_addr addr;
+	unsigned char *data;
+	int rv;
+	unsigned int currtime = 0;
+
+	mutex_lock(&ipmi_wdog_get_mutex);
+
+	atomic_set(&get_msg_tofree, 2);
+
+	addr.addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE;
+	addr.channel = IPMI_BMC_CHANNEL;
+	addr.lun = 0;
+
+	msg.netfn = 0x06;
+	msg.cmd = IPMI_WDOG_GET_TIMER;
+	msg.data = NULL;
+	msg.data_len = 0;
+	rv = ipmi_request_supply_msgs(watchdog_user,
+				      (struct ipmi_addr *) &addr,
+				      0, &msg, NULL,
+				      &get_smi_msg, &get_recv_msg, 1);
+	if (rv) {
+		pr_warn("get timeout error: %d\n", rv);
+		goto out;
+	}
+
+	wait_for_completion(&get_msg_wait);
+
+	if (get_recv_msg.msg.data_len < 1) {
+		pr_warn("get timeout received zero length message\n");
+		goto out;
+	}
+
+	data = get_recv_msg.msg.data;
+	if (data[0] != 0) {
+		pr_warn("get timeout received error: 0x%x\n", data[0]);
+		goto out;
+	}
+
+	if (get_recv_msg.msg.data_len < 9) {
+		pr_warn("get timeout received message too small, expected 9, got %u\n",
+			get_recv_msg.msg.data_len);
+		goto out;
+	}
+
+	currtime = data[7] | data[8] << 8;
+	currtime /= 10;
+ out:
+	mutex_unlock(&ipmi_wdog_get_mutex);
+	return currtime;
+}
+
 static ssize_t ipmi_wdog_read(struct watchdog_device *wdd,
 			      struct file *file,
 			      char        __user *buf,
@@ -806,6 +885,7 @@ static const struct watchdog_ops ipmi_wdog_ops = {
 	.set_timeout	= ipmi_wdog_set_timeout,
 	.set_pretimeout	= ipmi_wdog_set_pretimeout,
 	.ping		= ipmi_wdog_ping,
+	.get_timeleft	= ipmi_wdog_get_timeleft,
 	.read		= ipmi_wdog_read,
 	.poll		= ipmi_wdog_poll,
 	.fasync		= ipmi_wdog_fasync,
-- 
2.17.1

Re: [PATCH 04/10] watchdog: Add functions to set the timeout and pretimeout

[Guenter Roeck]

On Sat, Jun 20, 2020 at 12:49:01PM -0500, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> If the watchdog device wants to set it's pretimeout (say from module
> parameters), this lets them do that.
> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>  drivers/watchdog/watchdog_dev.c | 54 +++++++++++++++++++++++++++++----
>  include/linux/watchdog.h        |  4 +++
>  2 files changed, 52 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index 45a0a4fe731d..6c423aed3f3c 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -364,14 +364,14 @@ static unsigned int watchdog_get_status(struct watchdog_device *wdd)
>  }
>  
>  /*
> - *	watchdog_set_timeout: set the watchdog timer timeout
> + *	_watchdog_set_timeout: set the watchdog timer timeout
>   *	@wdd: the watchdog device to set the timeout for
>   *	@timeout: timeout to set in seconds
>   *
>   *	The caller must hold wd_data->lock.
>   */
>  
> -static int watchdog_set_timeout(struct watchdog_device *wdd,
> +static int _watchdog_set_timeout(struct watchdog_device *wdd,
>  							unsigned int timeout)
>  {
>  	int err = 0;
> @@ -396,14 +396,35 @@ static int watchdog_set_timeout(struct watchdog_device *wdd,
>  	return err;
>  }
>  
> +/*
> + *	watchdog_set_timeout: set the watchdog timer timeout
> + *	@wdd: the watchdog device to set the timeout for
> + *	@timeout: timeout to set in seconds
> + */
> +
> +int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int timeout)
> +{
> +	int err = 0;
> +
> +	if (!wdd->wd_data)
> +		return -ENODEV;
> +
> +	mutex_lock(&wdd->wd_data->lock);
> +	err = _watchdog_set_timeout(wdd, timeout);
> +	mutex_unlock(&wdd->wd_data->lock);

These functions are expected to be called prior to registration. wd_data isn't
set at that time, and even if it was using the mutex would be unnecessary
since there is nothing to synchronize against.

Besides being unnecessary, this change isn't mentioned in the patch description.

Guenter

> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(watchdog_set_timeout);
> +
>  /*
>   *	watchdog_set_pretimeout: set the watchdog timer pretimeout
>   *	@wdd: the watchdog device to set the timeout for
>   *	@timeout: pretimeout to set in seconds
>   */
>  
> -static int watchdog_set_pretimeout(struct watchdog_device *wdd,
> -				   unsigned int timeout)
> +static int _watchdog_set_pretimeout(struct watchdog_device *wdd,
> +				    unsigned int timeout)
>  {
>  	int err = 0;
>  
> @@ -421,6 +442,27 @@ static int watchdog_set_pretimeout(struct watchdog_device *wdd,
>  	return err;
>  }
>  
> +/*
> + *	watchdog_set_pretimeout: set the watchdog timer pretimeout
> + *	@wdd: the watchdog device to set the timeout for
> + *	@timeout: pretimeout to set in seconds
> + */
> +
> +int watchdog_set_pretimeout(struct watchdog_device *wdd, unsigned int timeout)
> +{
> +	int err = 0;
> +
> +	if (!wdd->wd_data)
> +		return -ENODEV;
> +
> +	mutex_lock(&wdd->wd_data->lock);
> +	err = _watchdog_set_pretimeout(wdd, timeout);
> +	mutex_unlock(&wdd->wd_data->lock);
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(watchdog_set_pretimeout);
> +
>  /*
>   *	watchdog_get_timeleft: wrapper to get the time left before a reboot
>   *	@wdd: the watchdog device to get the remaining time from
> @@ -809,7 +851,7 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
>  			err = -EFAULT;
>  			break;
>  		}
> -		err = watchdog_set_timeout(wdd, val);
> +		err = _watchdog_set_timeout(wdd, val);
>  		if (err < 0)
>  			break;
>  		/* If the watchdog is active then we send a keepalive ping
> @@ -838,7 +880,7 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
>  			err = -EFAULT;
>  			break;
>  		}
> -		err = watchdog_set_pretimeout(wdd, val);
> +		err = _watchdog_set_pretimeout(wdd, val);
>  		break;
>  	case WDIOC_GETPRETIMEOUT:
>  		err = put_user(wdd->pretimeout, p);
> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> index 36f99c8c973e..95396b644a9b 100644
> --- a/include/linux/watchdog.h
> +++ b/include/linux/watchdog.h
> @@ -201,6 +201,10 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
>  	return wdd->driver_data;
>  }
>  
> +/* Allow the driver to set the timeout and pretimeout. */
> +int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int timeout);
> +int watchdog_set_pretimeout(struct watchdog_device *wdd, unsigned int timeout);
> +
>  /* Use the following functions to report watchdog pretimeout event */
>  #if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV)
>  void watchdog_notify_pretimeout(struct watchdog_device *wdd);
> -- 
> 2.17.1
> 

Re: [PATCH 05/10] watchdog: Export an interface to start the watchdog

[Guenter Roeck]

On Sat, Jun 20, 2020 at 12:49:02PM -0500, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> This way a watchdog driver can start itself.

The watchdog driver can start itself prior to registration if
it wants to, like many other watchdog drivers already do. All the
driver needs to do is to call its own start function.
I do not see the need for this API.

Guenter

> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>  drivers/watchdog/watchdog_dev.c | 30 ++++++++++++++++++++++++++----
>  include/linux/watchdog.h        |  3 +++
>  2 files changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index 6c423aed3f3c..752358df1606 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -253,7 +253,7 @@ static enum hrtimer_restart watchdog_timer_expired(struct hrtimer *timer)
>  }
>  
>  /*
> - *	watchdog_start: wrapper to start the watchdog.
> + *	_watchdog_start: wrapper to start the watchdog.
>   *	@wdd: the watchdog device to start
>   *
>   *	The caller must hold wd_data->lock.
> @@ -263,7 +263,7 @@ static enum hrtimer_restart watchdog_timer_expired(struct hrtimer *timer)
>   *	failure.
>   */
>  
> -static int watchdog_start(struct watchdog_device *wdd)
> +static int _watchdog_start(struct watchdog_device *wdd)
>  {
>  	struct watchdog_core_data *wd_data = wdd->wd_data;
>  	ktime_t started_at;
> @@ -289,6 +289,28 @@ static int watchdog_start(struct watchdog_device *wdd)
>  	return err;
>  }
>  
> +/*
> + *	watchdog_start: External interface to start the watchdog.
> + *	@wdd: the watchdog device to start
> + *
> + *	Start the watchdog if it is not active and mark it active.
> + *	This function returns zero on success or a negative errno code for
> + *	failure.
> + */
> +
> +int watchdog_start(struct watchdog_device *wdd)
> +{
> +	struct watchdog_core_data *wd_data = wdd->wd_data;
> +	int err;
> +
> +	mutex_lock(&wd_data->lock);
> +	err = _watchdog_start(wdd);
> +	mutex_unlock(&wd_data->lock);
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(watchdog_start);
> +
>  /*
>   *	watchdog_stop: wrapper to stop the watchdog.
>   *	@wdd: the watchdog device to stop
> @@ -837,7 +859,7 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
>  				break;
>  		}
>  		if (val & WDIOS_ENABLECARD)
> -			err = watchdog_start(wdd);
> +			err = _watchdog_start(wdd);
>  		break;
>  	case WDIOC_KEEPALIVE:
>  		if (!(wdd->info->options & WDIOF_KEEPALIVEPING)) {
> @@ -935,7 +957,7 @@ static int watchdog_open(struct inode *inode, struct file *file)
>  		goto out_clear;
>  	}
>  
> -	err = watchdog_start(wdd);
> +	err = _watchdog_start(wdd);
>  	if (err < 0)
>  		goto out_mod;
>  
> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> index 95396b644a9b..1eefae61215d 100644
> --- a/include/linux/watchdog.h
> +++ b/include/linux/watchdog.h
> @@ -205,6 +205,9 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
>  int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int timeout);
>  int watchdog_set_pretimeout(struct watchdog_device *wdd, unsigned int timeout);
>  
> +/* Allow the driver to start the watchdog. */
> +int watchdog_start(struct watchdog_device *wdd);
> +
>  /* Use the following functions to report watchdog pretimeout event */
>  #if IS_ENABLED(CONFIG_WATCHDOG_PRETIMEOUT_GOV)
>  void watchdog_notify_pretimeout(struct watchdog_device *wdd);
> -- 
> 2.17.1
> 

Re: [PATCH 03/10] watchdog: Add documentation for read capability

[Guenter Roeck]

On Sat, Jun 20, 2020 at 12:49:00PM -0500, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> Document the read, poll, and async operations.
> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>  Documentation/watchdog/watchdog-kernel-api.rst | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/watchdog/watchdog-kernel-api.rst b/Documentation/watchdog/watchdog-kernel-api.rst
> index 068a55ee0d4a..aa15a2d35397 100644
> --- a/Documentation/watchdog/watchdog-kernel-api.rst
> +++ b/Documentation/watchdog/watchdog-kernel-api.rst
> @@ -132,6 +132,10 @@ The list of watchdog operations is defined as::
>  	unsigned int (*get_timeleft)(struct watchdog_device *);
>  	int (*restart)(struct watchdog_device *);
>  	long (*ioctl)(struct watchdog_device *, unsigned int, unsigned long);
> +	ssize_t (*read)(struct watchdog_device *, struct file *, char __user *,
> +			size_t, loff_t *);
> +	__poll_t (*poll)(struct watchdog_device *, struct file *, poll_table *);
> +	int (*fasync)(struct watchdog_device *, int, struct file *, int);
>    };
>  
>  It is important that you first define the module owner of the watchdog timer
> @@ -235,6 +239,14 @@ they are supported. These optional routines/operations are:
>    our own internal ioctl call handling. This routine should return -ENOIOCTLCMD
>    if a command is not supported. The parameters that are passed to the ioctl
>    call are: watchdog_device, cmd and arg.
> +* read: If a read call comes in on the watchdog device, call this function.
> +  This allows a watchdog to provide data to the user.

I don't think this makes any sense. To be accepted, this would need to be
standardized. We can't have userspace read from a watchdog device without
knowing what is going to be returned.

Guenter

> +* poll: If a poll call comes in on the watchdog device, call this.  This
> +  allows the user to do select/poll waiting on the device to have read data
> +  ready.
> +* fasync: If a fasync call comes in on the watchdog, call this.  This
> +  allows the user to do async operations waiting for read data from
> +  the device.
>  
>  The status bits should (preferably) be set with the set_bit and clear_bit alike
>  bit-operations. The status bits that are defined are:
> -- 
> 2.17.1
> 

Re: [PATCH 01/10] watchdog: Ignore stop_on_reboot if no stop function

[Guenter Roeck]

On Sat, Jun 20, 2020 at 12:48:58PM -0500, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> The reboot notifier unconditionally calls the stop function on the
> watchdog, which would result in a crash if the watchdog didn't have a
> stop function.  So check at register time to see if there is a stop
> function, and don't do stop_on_reboot if it is NULL.
> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>  drivers/watchdog/watchdog_core.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
> index 423844757812..03943a34e9fb 100644
> --- a/drivers/watchdog/watchdog_core.c
> +++ b/drivers/watchdog/watchdog_core.c
> @@ -260,10 +260,16 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
>  
>  	/* Module parameter to force watchdog policy on reboot. */
>  	if (stop_on_reboot != -1) {
> -		if (stop_on_reboot)
> -			set_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
> -		else
> +		if (stop_on_reboot) {
> +			if (!wdd->ops->stop) {
> +				pr_err("watchdog%d: stop_on_reboot set, but no stop function.  Ignoring stop_on_reboot.\n", wdd->id);

This should be pr_notice(). It is not an error (otherwise I would expect
registration to abort). Also:

WARNING: line length of 133 exceeds 100 columns
#108: FILE: drivers/watchdog/watchdog_core.c:265:
+				pr_err("watchdog%d: stop_on_reboot set, but no stop function.  Ignoring stop_on_reboot.\n", wdd->id);


This patch that is independent from the rest of the series and should be
applied/handled independently.

Thanks,
Guenter

> +				clear_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
> +			} else {
> +				set_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
> +			}
> +		} else {
>  			clear_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
> +		}
>  	}
>  
>  	if (test_bit(WDOG_STOP_ON_REBOOT, &wdd->status)) {

Re: [PATCH 08/10] watchdog: Add a way to extend the timeout on a reboot

[Guenter Roeck]

On Sat, Jun 20, 2020 at 12:49:05PM -0500, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> If reboot_timeout is set in the watchdog device struct, set the timeout
> to that value on a reboot.  This way more time can be given for a reboot
> to complete.
> 
I think this should be aligned with watchdog_set_open_deadline(),
ie use a function to set the reboot timeout instead of adding it
to struct watchdog_device.

> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>  drivers/watchdog/watchdog_core.c | 2 ++
>  include/linux/watchdog.h         | 4 ++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
> index 03943a34e9fb..5792f9bca645 100644
> --- a/drivers/watchdog/watchdog_core.c
> +++ b/drivers/watchdog/watchdog_core.c
> @@ -165,6 +165,8 @@ static int watchdog_reboot_notifier(struct notifier_block *nb,
>  			if (ret)
>  				return NOTIFY_BAD;
>  		}
> +	} else if (wdd->reboot_timeout) {
> +		watchdog_set_timeout(wdd, wdd->reboot_timeout);

This has no practical impact; the code above checks for SYS_DOWN,
and for whatever reason SYS_DOWN has the same value as SYS_RESTART.
So the only value is for SYS_POWER_OFF, and that should arguably
be included in the first check (meaning we should probably remove
that check entirely, if anything).

Also, the reboot notifier is only called if WDOG_STOP_ON_REBOOT
is set, which contradicts the idea behind this change.

Guenter

>  	}
>  
>  	return NOTIFY_DONE;
> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> index 1eefae61215d..0fb57f29346c 100644
> --- a/include/linux/watchdog.h
> +++ b/include/linux/watchdog.h
> @@ -92,6 +92,9 @@ struct watchdog_ops {
>   * @status:	Field that contains the devices internal status bits.
>   * @deferred:	Entry in wtd_deferred_reg_list which is used to
>   *		register early initialized watchdogs.
> + * @reboot_timeout:
> + *		If non-zero, the timeout will be set to this value
> + *		on a reboot.  This lets a reboot be given more time.
>   *
>   * The watchdog_device structure contains all information about a
>   * watchdog timer device.
> @@ -125,6 +128,7 @@ struct watchdog_device {
>  #define WDOG_HW_RUNNING		3	/* True if HW watchdog running */
>  #define WDOG_STOP_ON_UNREGISTER	4	/* Should be stopped on unregister */
>  	struct list_head deferred;
> +	unsigned int reboot_timeout;
>  };
>  
>  #define WATCHDOG_NOWAYOUT		IS_BUILTIN(CONFIG_WATCHDOG_NOWAYOUT)