Re: [PATCH] watchdog: add support for w83697hg chip

[Samuel Tardieu <sam@rfc1149.net>]

On  6/09, Pádraig Brady wrote:

| Looks good, thanks.

Thanks a lot Pádraig for your review.

| I've got a W83697H*F* here on a VIA motherboard,
| which is the same from a watchdog point of view.
| It is on port 0x2e though.
| Is 0x4e a good default?

I think we may find half of each (0x2e and 0x4e). I'm reluctant to let
the autodetection routine be the default. What if another peripheral is at
the other address, could bad things happen?

| Is W83697HG a good name?

HF would be equally good. I just named it after the watchdog I had in my
hardware. If there is a preference for HF, I can change it.

| Note I've CC'd Wim Van Sebroeck who is the watchdog tree maintainer.

Thank you.

| I noticed you didn't include the check that's in the
| W83627HF driver to reset the timeout if already running
| from the BIOS. This was because some BIOS set the timeout
| to 4 minutes for example, so when the driver was loaded
| and reset the mode to seconds, the machine rebooted
| before the init scripts could run and start the userspace
| watchdog daemon.

Note that the watchdog is enabled only when the device is open and is
signalled during the wdt_enable() routine just after switching the mode
to seconds. The opportunity for the device to reboot while we are in the
middle of open() will exist anyway and would be a very bad luck.

But I buy your argument in order to reduce the risks: setting the
counter to 0 before switching the mode (in wdt_enable()) would decrease
the possibility of bad luck happening :) But I don't think this is worth
a warning; just set the counter to 0 (not counting) before doing the
configuration is benign enough not to be signalled IMO.

Also, wdt_open() has a wdt_ping() just after the wdt_enable() and this
is superfluous, I remove it.

Here is an updated patch with the changes mentionned above. Don't
hesitate to comment on it or request other changes.



Winbond W83697HG watchdog timer

The Winbond SuperIO W83697HG includes a watchdog that can count from
1 to 255 seconds (or minutes). This drivers allows the seconds mode to
be used. It exposes a standard /dev/watchdog interface.

This chip is currently being used on motherboards designed by
VIA and Dedibox. It should be compatible with W83697HF chips as well
according to the datasheet but is untested on those.

By default, the module looks for a chip at I/O port 0x4e. The chip can
be configured to be at 0x2e on some motherboards, the address can be
chosen using the wdt_io module parameter. Using 0 will try to autodetect
the address.

Signed-off-by: Samuel Tardieu <sam@rfc1149.net>

diff -r b1d36669f98d drivers/char/watchdog/Kconfig
--- a/drivers/char/watchdog/Kconfig	Mon Sep 04 03:00:04 2006 +0000
+++ b/drivers/char/watchdog/Kconfig	Wed Sep 06 12:25:06 2006 +0200
@@ -371,6 +371,21 @@ config W83627HF_WDT
 
 	  Most people will say N.
 
+config W83697HG_WDT
+	tristate "W83697HG Watchdog Timer"
+	depends on WATCHDOG && X86
+	---help---
+	  This is the driver for the hardware watchdog on the W83697HG chipset
+	  as used in Dedibox/VIA motherboards (and likely others).
+	  This watchdog simply watches your kernel to make sure it doesn't
+	  freeze, and if it does, it reboots your computer after a certain
+	  amount of time.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called w83697hg_wdt.
+
+	  Most people will say N.
+
 config W83877F_WDT
 	tristate "W83877F (EMACS) Watchdog Timer"
 	depends on WATCHDOG && X86
diff -r b1d36669f98d drivers/char/watchdog/Makefile
--- a/drivers/char/watchdog/Makefile	Mon Sep 04 03:00:04 2006 +0000
+++ b/drivers/char/watchdog/Makefile	Wed Sep 06 12:25:06 2006 +0200
@@ -51,6 +51,7 @@ obj-$(CONFIG_SBC8360_WDT) += sbc8360.o
 obj-$(CONFIG_SBC8360_WDT) += sbc8360.o
 obj-$(CONFIG_CPU5_WDT) += cpu5wdt.o
 obj-$(CONFIG_W83627HF_WDT) += w83627hf_wdt.o
+obj-$(CONFIG_W83697HG_WDT) += w83697hg_wdt.o
 obj-$(CONFIG_W83877F_WDT) += w83877f_wdt.o
 obj-$(CONFIG_W83977F_WDT) += w83977f_wdt.o
 obj-$(CONFIG_MACHZ_WDT) += machzwd.o
diff -r b1d36669f98d drivers/char/watchdog/w83697hg_wdt.c
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/drivers/char/watchdog/w83697hg_wdt.c	Wed Sep 06 13:25:02 2006 +0200
@@ -0,0 +1,424 @@
+/*
+ *	w83697hg WDT driver
+ *
+ *      (c) Copyright 2006 Samuel Tardieu <sam@rfc1149.net>
+ *
+ *	Based on w83627hf_wdt which is based on wadvantechwdt.c
+ *      which is based on wdt.c.
+ *	Original copyright messages:
+ *
+ *	(c) Copyright 2003 Pádraig Brady <P@draigBrady.com>
+ *
+ *	(c) Copyright 2000-2001 Marek Michalkiewicz <marekm@linux.org.pl>
+ *
+ *	(c) Copyright 1996 Alan Cox <alan@redhat.com>, All Rights Reserved.
+ *				http://www.redhat.com
+ *
+ *	This program is free software; you can redistribute it and/or
+ *	modify it under the terms of the GNU General Public License
+ *	as published by the Free Software Foundation; either version
+ *	2 of the License, or (at your option) any later version.
+ *
+ *	Neither Alan Cox nor CymruNet Ltd. admit liability nor provide
+ *	warranty for any of this software. This material is provided
+ *	"AS-IS" and at no charge.
+ *
+ *	(c) Copyright 1995    Alan Cox <alan@redhat.com>
+ */
+
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#include <linux/fs.h>
+#include <linux/ioport.h>
+#include <linux/notifier.h>
+#include <linux/reboot.h>
+#include <linux/init.h>
+
+#include <asm/io.h>
+#include <asm/uaccess.h>
+#include <asm/system.h>
+
+#define WATCHDOG_NAME "w83697hg WDT"
+#define PFX WATCHDOG_NAME ": "
+#define WATCHDOG_TIMEOUT 60		/* 60 sec default timeout */
+
+static unsigned long wdt_is_open;
+static char expect_close;
+
+/* You must set this - there is no sane way to probe for this board. */
+static int wdt_io = 0x4e;
+module_param(wdt_io, int, 0);
+MODULE_PARM_DESC(wdt_io, "w83697hg WDT io port (default 0x4e, 0 = autodetect)");
+
+static int timeout = WATCHDOG_TIMEOUT;	/* in seconds */
+module_param(timeout, int, 0);
+MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds. 1<= timeout <=255, default=" __MODULE_STRING(WATCHDOG_TIMEOUT) ".");
+
+static int nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, int, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=CONFIG_WATCHDOG_NOWAYOUT)");
+
+/*
+ *	Kernel methods.
+ */
+
+#define W83697HG_EFER (wdt_io+0)   /* Extended function enable register */
+#define W83697HG_EFIR (wdt_io+0)   /* Extended function index register */
+#define W83697HG_EFDR (wdt_io+1)   /* Extended function data register */
+
+static inline void
+w83697hg_unlock(void)
+{
+	outb_p(0x87, W83697HG_EFER);
+	outb_p(0x87, W83697HG_EFER);
+}
+
+static inline void
+w83697hg_lock(void)
+{
+	outb_p(0xAA, W83697HG_EFER);
+}
+
+/*
+ *	The three functions w83697hg_get_reg(), w83697_set_reg() and
+ *	wdt_ctrl() must be called with the device unlocked.
+ */
+
+static unsigned char
+w83697hg_get_reg(unsigned char reg)
+{
+	outb_p(reg, W83697HG_EFIR);
+	return inb_p(W83697HG_EFDR);
+}
+
+static void
+w83697hg_set_reg(unsigned char reg, unsigned char data)
+{
+	outb_p(reg, W83697HG_EFIR);
+	outb_p(data, W83697HG_EFDR);
+}
+
+static void
+wdt_ctrl(int timeout)
+{
+	w83697hg_set_reg(0xF4, timeout);
+}
+
+static void
+w83697hg_select_wdt(void)
+{
+	w83697hg_unlock();
+	w83697hg_set_reg(0x07, 0x08);   /* Switch to logic device 8 */
+}
+
+static inline void
+w83697hg_deselect_wdt(void)
+{
+	w83697hg_lock();
+}
+
+static void
+wdt_ping(void)
+{
+	w83697hg_select_wdt();
+	wdt_ctrl(timeout);
+	w83697hg_deselect_wdt();
+}
+
+static void
+wdt_enable(void)
+{
+	unsigned char bbuf;
+
+	w83697hg_select_wdt();
+
+	wdt_ctrl(0);                    /* Disable while configuring */
+
+	bbuf = w83697hg_get_reg(0x29);
+	bbuf &= ~0x60;
+	bbuf |= 0x20;
+	w83697hg_set_reg(0x29, bbuf);   /* Set pin 119 to WDTO# mode */
+
+	bbuf = w83697hg_get_reg(0xF3);
+	bbuf &= ~0x04;
+	w83697hg_set_reg(0xF3, bbuf);   /* Count mode is seconds */
+
+	wdt_ctrl(timeout);              /* Start timer */
+	w83697hg_set_reg(0x30, 1);      /* Enable timer */
+
+	w83697hg_deselect_wdt();
+}
+
+static void
+wdt_disable(void)
+{
+	w83697hg_select_wdt();
+	w83697hg_set_reg(0x30, 0);       /* Disable timer */
+	wdt_ctrl(0);
+	w83697hg_deselect_wdt();
+}
+
+static int
+wdt_set_heartbeat(int t)
+{
+	if ((t < 1) || (t > 255))
+		return -EINVAL;
+
+	timeout = t;
+	return 0;
+}
+
+static ssize_t
+wdt_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
+{
+	if (count) {
+		if (!nowayout) {
+			size_t i;
+      
+			expect_close = 0;
+      
+			for (i = 0; i != count; i++) {
+				char c;
+				if (get_user(c, buf+i))
+					return -EFAULT;
+				if (c == 'V')
+					expect_close = 42;
+			}
+		}
+		wdt_ping();
+	}
+	return count;
+}
+
+static int
+wdt_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
+	  unsigned long arg)
+{
+	void __user *argp = (void __user *)arg;
+	int __user *p = argp;
+	int new_timeout;
+	static struct watchdog_info ident = {
+		.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE,
+		.firmware_version = 1,
+		.identity = "W83697HG WDT",
+	};
+
+	switch (cmd) {
+	case WDIOC_GETSUPPORT:
+		if (copy_to_user(argp, &ident, sizeof(ident)))
+			return -EFAULT;
+		break;
+
+	case WDIOC_GETSTATUS:
+	case WDIOC_GETBOOTSTATUS:
+		return put_user(0, p);
+
+	case WDIOC_KEEPALIVE:
+		wdt_ping();
+		break;
+
+	case WDIOC_SETTIMEOUT:
+		if (get_user(new_timeout, p))
+			return -EFAULT;
+		if (wdt_set_heartbeat(new_timeout))
+			return -EINVAL;
+		wdt_ping();
+		/* Fall */
+
+	case WDIOC_GETTIMEOUT:
+		return put_user(timeout, p);
+
+	case WDIOC_SETOPTIONS:
+	{
+		int options, retval = -EINVAL;
+      
+		if (get_user(options, p))
+			return -EFAULT;
+      
+		if (options & WDIOS_DISABLECARD) {
+			wdt_disable();
+			retval = 0;
+		}
+
+		if (options & WDIOS_ENABLECARD) {
+			wdt_ping();
+			retval = 0;
+		}
+
+		return retval;
+	}
+
+	default:
+		return -ENOIOCTLCMD;
+	}
+	return 0;
+}
+
+static int
+wdt_open(struct inode *inode, struct file *file)
+{
+	if (test_and_set_bit(0, &wdt_is_open))
+		return -EBUSY;
+	/*
+	 *	Activate
+	 */
+
+	wdt_enable();
+	return nonseekable_open(inode, file);
+}
+
+static int
+wdt_close(struct inode *inode, struct file *file)
+{
+	if (expect_close == 42) {
+		wdt_disable();
+	} else {
+		printk(KERN_CRIT PFX "Unexpected close, not stopping watchdog!\n");
+		wdt_ping();
+	}
+	expect_close = 0;
+	clear_bit(0, &wdt_is_open);
+	return 0;
+}
+
+/*
+ *	Notifier for system down
+ */
+
+static int
+wdt_notify_sys(struct notifier_block *this, unsigned long code,
+	       void *unused)
+{
+	if (code == SYS_DOWN || code == SYS_HALT) {
+		/* Turn the WDT off */
+		wdt_disable();
+	}
+	return NOTIFY_DONE;
+}
+
+/*
+ *	Kernel Interfaces
+ */
+
+static struct file_operations wdt_fops = {
+	.owner		= THIS_MODULE,
+	.llseek		= no_llseek,
+	.write		= wdt_write,
+	.ioctl		= wdt_ioctl,
+	.open		= wdt_open,
+	.release	= wdt_close,
+};
+
+static struct miscdevice wdt_miscdev = {
+	.minor = WATCHDOG_MINOR,
+	.name = "watchdog",
+	.fops = &wdt_fops,
+};
+
+/*
+ *	The WDT needs to learn about soft shutdowns in order to
+ *	turn the timebomb registers off.
+ */
+
+static struct notifier_block wdt_notifier = {
+	.notifier_call = wdt_notify_sys,
+};
+
+static int
+w83697hg_init(void)
+{
+	if (!request_region(wdt_io, 2, WATCHDOG_NAME)) {
+		printk(KERN_ERR PFX "I/O address 0x%x already in use\n", wdt_io);
+		return -EIO;
+	}
+  
+	printk(KERN_INFO PFX "Looking for W83697HG at address 0x%x\n", wdt_io);
+	w83697hg_unlock();
+	if (w83697hg_get_reg(0x20) == 0x60) {
+		printk(KERN_INFO PFX "W83697HG found at address 0x%x\n", wdt_io);
+		w83697hg_lock();
+		return 0;
+	}
+	w83697hg_lock();   /* Reprotect in case it was a compatible device */
+  
+	printk(KERN_INFO PFX "W83697HG not found at address 0x%x\n", wdt_io);
+	release_region(wdt_io, 2);
+	return -EIO;
+}
+
+static int __init
+wdt_init(void)
+{
+	int ret, autodetect;
+  
+	printk(KERN_INFO PFX "WDT driver for W83697HG initializing\n");
+  
+	autodetect = wdt_io == 0;
+	if (autodetect)
+		wdt_io = 0x2e;
+  
+	if (!w83697hg_init())
+		goto found;
+  
+	if (autodetect) {
+		wdt_io = 0x4e;
+		if (!w83697hg_init())
+			goto found;
+	}
+  
+	printk(KERN_ERR PFX "No W83697HG could be found\n");
+	ret = -EIO;
+	goto out;
+  
+ found:
+  
+	if (wdt_set_heartbeat(timeout)) {
+		wdt_set_heartbeat(WATCHDOG_TIMEOUT);
+		printk(KERN_INFO PFX "timeout value must be 1<=timeout<=255, using %d\n",
+		       WATCHDOG_TIMEOUT);
+	}
+  
+	ret = register_reboot_notifier(&wdt_notifier);
+	if (ret != 0) {
+		printk (KERN_ERR PFX "cannot register reboot notifier (err=%d)\n",
+			ret);
+		goto unreg_regions;
+	}
+  
+	ret = misc_register(&wdt_miscdev);
+	if (ret != 0) {
+		printk (KERN_ERR PFX "cannot register miscdev on minor=%d (err=%d)\n",
+			WATCHDOG_MINOR, ret);
+		goto unreg_reboot;
+	}
+  
+	printk (KERN_INFO PFX "initialized. timeout=%d sec (nowayout=%d)\n",
+		timeout, nowayout);
+  
+ out:
+	return ret;
+ unreg_reboot:
+	unregister_reboot_notifier(&wdt_notifier);
+ unreg_regions:
+	release_region(wdt_io, 2);
+	goto out;
+}
+
+static void __exit
+wdt_exit(void)
+{
+	misc_deregister(&wdt_miscdev);
+	unregister_reboot_notifier(&wdt_notifier);
+	release_region(wdt_io, 2);
+}
+
+module_init(wdt_init);
+module_exit(wdt_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Samuel Tardieu <sam@rfc1149.net>");
+MODULE_DESCRIPTION("w83697hg WDT driver");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/