Generic Watchdog Timer Driver

commit 1959ea403c69af855f5cd13e5c9b33123d2137b2

Author: Wim Van Sebroeck<[email protected]>
Date:   Fri Jun 18 09:45:49 2010 +0000

watchdog: WatchDog Timer Driver Core - Part 5

This part add's the WDIOC_SETTIMEOUT and WDIOC_GETTIMEOUT ioctl functionality to the WatchDog Timer Driver Core framework.

Signed-off-by Alan Cox<[email protected]>

Signed-off-by Wim Van Sebroeck<[email protected]>

Documentation/watchdog/src/watchdog-with-timer-example.c +13 -1

index 6a4af47..f1d4f217 100644
@@ -110,18 +110,27 @@ static int wdt_stop(struct watchdog_device *wdd)
  	return 0;
  }
 
+static int wdt_set_timeout(struct watchdog_device *wdd, int new_timeout)
+{
+	if (new_timeout<  1)
+		return -EINVAL;
+	return 0;
+}
+
  /*
   * The watchdog kernel structures
   */
  static const struct watchdog_info wdt_info = {
  	.identity =	DRV_NAME,
-	.options =	WDIOF_KEEPALIVEPING,
+	.options =	WDIOF_SETTIMEOUT |
+			WDIOF_KEEPALIVEPING,
  };
 
  static const struct watchdog_ops wdt_ops = {
  	.start =	wdt_start,
  	.stop =		wdt_stop,
  	.ping =		wdt_ping,
+	.set_timeout =	wdt_set_timeout,
  };
 
  static struct watchdog_device wdt_dev = {

@@ -139,6 +148,9 @@ static int __devinit wdt_probe(struct platform_device *pdev)
 
  	/* Register other stuff */
 
+	/* Set watchdog_device parameters */
+	wdt_dev.timeout = timeout;
+
  	/* Register the watchdog timer device */
  	res = register_watchdogdevice(&wdt_dev);
  	if (res) {

Documentation/watchdog/watchdog-kernel-api.txt +9 -0

index 3de69e7..f15e8d4 100644
@@ -42,6 +42,7 @@ struct watchdog_device {
  	char *name;
  	const struct watchdog_info *info;
  	const struct watchdog_ops *ops;
+	int timeout;
  	int bootstatus;
  	long status;
  };

@@ -51,6 +52,7 @@ It contains following fields:
  * info: a pointer to a watchdog_info structure. This structure gives some
    additional information about the watchdog timer itself.
  * ops: a pointer to the list of watchdog operations that the watchdog supports.
+* timeout: the watchdog timer's timeout value (in seconds).
  * bootstatus: status of the device after booting (reported with watchdog
    WDIOF_* status bits).
  * status: this field contains a number of status bits that give extra

@@ -67,6 +69,7 @@ struct watchdog_ops {
  	/* optional operations */
  	int (*ping)(struct watchdog_device *);
  	int (*status)(struct watchdog_device *);
+	int (*set_timeout)(struct watchdog_device *, int);
  };
 
  Some operations are mandatory and some are optional. The mandatory operations

@@ -102,6 +105,12 @@ they are supported. These optional routines/operations are:
    info structure).
  * status: this routine checks the status of the watchdog timer device. The
    status of the device is reported with watchdog WDIOF_* status flags/bits.
+* set_timeout: this routine checks and changes the timeout of the watchdog
+  timer device. It returns 0 on success and an errno code on failure. On success
+  the timeout value of the watchdog_device will be changed to the value that
+  was just used to re-program the watchdog timer device.
+  (Note: the WDIOF_SETTIMEOUT needs to be set in the options field of the
+  watchdog's info structure).
 
  The status bits should (preferably) be set with the set_bit and clear_bit alike
  bit-operations. The status bit's that are defined are:

drivers/watchdog/core/watchdog_dev.c +0 -0

index 2bf4f67..b80c6e6 100644

drivers/watchdog/core/watchdog_dev.c +14 -0

+		err = wdd->ops->set_timeout(wdd, val);
+		if (err<  0)
+			return err;
+		wdd->timeout = val;
+		/* If the watchdog is active then we sent a keepalive ping
+		 * to make sure that the watchdog keep's running (and if
+		 * possible that it takes the new timeout) */
+		watchdog_ping(wdd);
+		/* Fall */
+	case WDIOC_GETTIMEOUT:
+		/* timeout == 0 means that we don't know the timeout */
+		if (wdd->timeout)
+			return put_user(wdd->timeout, p);
+		return -EOPNOTSUPP;
  	default:
  		return -ENOTTY;
  	}

include/linux/watchdog.h +2 -0

index 736814f..e35f51f 100644
@@ -70,6 +70,7 @@ struct watchdog_ops {
  	/* optional operations */
  	int (*ping)(struct watchdog_device *);
  	int (*status)(struct watchdog_device *);
+	int (*set_timeout)(struct watchdog_device *, int);
  };
 
  /* The structure that defines a watchdog device */

@@ -78,6 +79,7 @@ struct watchdog_device {
  	const struct watchdog_info *info;
  	const struct watchdog_ops *ops;
  	int bootstatus;
+	int timeout;
  	long status;
  #define WDOG_ACTIVE		0	/* is the watchdog running/active */
  #define WDOG_DEV_OPEN		1	/* is the watchdog opened via

Quoted 5 lines

drivers/watchdog/core/watchdog_dev.c +0 -0

index 2bf4f67..b80c6e6 100644

drivers/watchdog/core/watchdog_dev.c static long watchdog_ioctl(struct file *file, unsigned int cmd, · line 229

Quoted 9 lines

drivers/watchdog/core/watchdog_dev.c +6 -0

@@ -221,6 +221,26 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
  			return -EOPNOTSUPP;
  		watchdog_ping(wdd);
  		return 0;
+	case WDIOC_SETTIMEOUT:
+		if ((wdd->ops->set_timeout == NULL) ||
+		    !(wdd->info->options&  WDIOF_SETTIMEOUT))
+			return -EOPNOTSUPP;
+		if (get_user(val, p))
+			return -EFAULT;

Should we sanity check that val > 0 here? I realize that the driver would need to do it's own sanity checking, but a value of 0 or < 0 makes no sense to the core itself.

Maybe timeout is best expressed as u32 if this is the case.

commit 1959ea403c69af855f5cd13e5c9b33123d2137b2

Author: Wim Van Sebroeck <[email protected]>
Date:   Fri Jun 18 09:45:49 2010 +0000

watchdog: WatchDog Timer Driver Core - Part 5

This part add's the WDIOC_SETTIMEOUT and WDIOC_GETTIMEOUT ioctl functionality to the WatchDog Timer Driver Core framework.

Signed-off-by Alan Cox <[email protected]>

Signed-off-by Wim Van Sebroeck <[email protected]>

[snip]

Quoted 12 lines

commit 1959ea403c69af855f5cd13e5c9b33123d2137b2

Author: Wim Van Sebroeck <[email protected]>
Date:   Fri Jun 18 09:45:49 2010 +0000

watchdog: WatchDog Timer Driver Core - Part 5

This part add's the WDIOC_SETTIMEOUT and WDIOC_GETTIMEOUT ioctl functionality to the WatchDog Timer Driver Core framework.

Signed-off-by Alan Cox <[email protected]>

Signed-off-by Wim Van Sebroeck <[email protected]>

Quoted 1 line

[snip]

Any specific reason you can only specify seconds as timeout? There is no way to set a timeout of e.g. 500ms or 1500ms.
You can change this by using using a struct timeval for setting a timeout. What do you think?

2) Is there a real need for having 500ms or 1500ms? why would 1 or 2 seconds not be OK? (which are allready small timeouts for userspace anyway...)

On Thu, Mar 03, 2011 at 11:38:14AM +0100, Wim Van Sebroeck wrote:

Reflowed into 80 columns.

Quoted 3 lines

2) Is there a real need for having 500ms or 1500ms? why would 1 or 2 seconds not be OK? (which are allready small timeouts for userspace anyway...)

Yes, in embedded cases. There it's much easier to have good control over userspace such that you can meet the scheduling requirements.