[WATCHDOG] wafer5823wdt.c - patch2

fix possible wafwdt_is_open race
make wdt_stop and wdt_start module params
change wd_margin to timeout and make it a module_param
make expect_close the same system as in advantechwdt.c
clean-up ioctl handling
added extra printk's to report what problem occured
add MODULE_DESCRIPTION info

drivers/char/watchdog/wafer5823wdt.c

@@ -44,8 +44,8 @@
 #define WD_TIMO 60			/* 60 sec default timeout */
 
 static unsigned long wafwdt_is_open;
+static char expect_close;
 static spinlock_t wafwdt_lock;
-static int expect_close = 0;
 
 /*
  *	You must set these - there is no sane way to probe for this board.
@@ -56,10 +56,17 @@ static int expect_close = 0;
  *      to restart it again.
  */
 
-#define WDT_START 0x443
-#define WDT_STOP 0x843
+static int wdt_stop = 0x843;
+module_param(wdt_stop, int, 0);
+MODULE_PARM_DESC(wdt_stop, "Wafer 5823 WDT 'stop' io port (default 0x843)");
 
-static int wd_margin = WD_TIMO;
+static int wdt_start = 0x443;
+module_param(wdt_start, int, 0);
+MODULE_PARM_DESC(wdt_start, "Wafer 5823 WDT 'start' io port (default 0x443)");
+
+static int timeout = WD_TIMO;  /* in seconds */
+module_param(timeout, int, 0);
+MODULE_PARM_DESC(timeout, "Watchdog timeout in seconds. 1<= timeout <=255, default=" __MODULE_STRING(WATCHDOG_TIMEOUT) ".");
 
 #ifdef CONFIG_WATCHDOG_NOWAYOUT
 static int nowayout = 1;
@@ -73,24 +80,24 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=CON
 static void wafwdt_ping(void)
 {
 	/* pat watchdog */
-        spin_lock(&wafwdt_lock);
-	inb_p(WDT_STOP);
-	inb_p(WDT_START);
-        spin_unlock(&wafwdt_lock);
+	spin_lock(&wafwdt_lock);
+	inb_p(wdt_stop);
+	inb_p(wdt_start);
+	spin_unlock(&wafwdt_lock);
 }
 
 static void wafwdt_start(void)
 {
 	/* start up watchdog */
-	outb_p(wd_margin, WDT_START);
-	inb_p(WDT_START);
+	outb_p(timeout, wdt_start);
+	inb_p(wdt_start);
 }
 
 static void
 wafwdt_stop(void)
 {
 	/* stop watchdog */
-	inb_p(WDT_STOP);
+	inb_p(wdt_stop);
 }
 
 static ssize_t wafwdt_write(struct file *file, const char *buf, size_t count, loff_t * ppos)
@@ -99,6 +106,7 @@ static ssize_t wafwdt_write(struct file *file, const char *buf, size_t count, lo
 	if (ppos != &file->f_pos)
 		return -ESPIPE;
 
+	/* See if we got the magic character 'V' and reload the timer */
 	if (count) {
 		if (!nowayout) {
 			size_t i;
@@ -106,30 +114,30 @@ static ssize_t wafwdt_write(struct file *file, const char *buf, size_t count, lo
 			/* In case it was set long ago */
 			expect_close = 0;
 
+			/* scan to see wether or not we got the magic character */
 			for (i = 0; i != count; i++) {
 				char c;
 				if (get_user(c, buf + i))
 					return -EFAULT;
 				if (c == 'V')
-					expect_close = 1;
+					expect_close = 42;
 			}
 		}
+		/* Well, anyhow someone wrote to us, we should return that favour */
 		wafwdt_ping();
-		return 1;
 	}
-	return 0;
+	return count;
 }
 
 static int wafwdt_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
 	     unsigned long arg)
 {
-	int new_margin;
+	int new_timeout;
 	static struct watchdog_info ident = {
 		.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE,
 		.firmware_version = 1,
 		.identity = "Wafer 5823 WDT",
 	};
-	int one=1;
 
 	switch (cmd) {
 	case WDIOC_GETSUPPORT:
@@ -139,25 +147,24 @@ static int wafwdt_ioctl(struct inode *inode, struct file *file, unsigned int cmd
 		break;
 
 	case WDIOC_GETSTATUS:
-		if (copy_to_user((int *) arg, &one, sizeof (int)))
-			return -EFAULT;
-		break;
+	case WDIOC_GETBOOTSTATUS:
+		return put_user(0, (int *)arg);
 
 	case WDIOC_KEEPALIVE:
 		wafwdt_ping();
 		break;
 
 	case WDIOC_SETTIMEOUT:
-		if (get_user(new_margin, (int *)arg))
+		if (get_user(new_timeout, (int *)arg))
 			return -EFAULT;
-		if ((new_margin < 1) || (new_margin > 255))
+		if ((new_timeout < 1) || (new_timeout > 255))
 			return -EINVAL;
-		wd_margin = new_margin;
+		timeout = new_timeout;
 		wafwdt_stop();
 		wafwdt_start();
 		/* Fall */
 	case WDIOC_GETTIMEOUT:
-		return put_user(wd_margin, (int *)arg);
+		return put_user(timeout, (int *)arg);
 
 	default:
 		return -ENOTTY;
@@ -169,6 +176,10 @@ static int wafwdt_open(struct inode *inode, struct file *file)
 {
 	if (test_and_set_bit(0, &wafwdt_is_open))
 		return -EBUSY;
+
+	/*
+	 *      Activate
+	 */
 	wafwdt_start();
 	return 0;
 }
@@ -176,12 +187,14 @@ static int wafwdt_open(struct inode *inode, struct file *file)
 static int
 wafwdt_close(struct inode *inode, struct file *file)
 {
-	clear_bit(0, &wafwdt_is_open);
-	if (expect_close) {
+	if (expect_close == 42) {
 		wafwdt_stop();
 	} else {
 		printk(KERN_CRIT PFX "WDT device closed unexpectedly.  WDT will not stop!\n");
+		wafwdt_ping();
 	}
+	clear_bit(0, &wafwdt_is_open);
+	expect_close = 0;
 	return 0;
 }
 
@@ -230,37 +243,77 @@ static struct notifier_block wafwdt_notifier = {
 
 static int __init wafwdt_init(void)
 {
-	printk(KERN_INFO PFX "WDT driver for Wafer 5823 single board computer initialising.\n");
+	int ret;
+
+	printk(KERN_INFO "WDT driver for Wafer 5823 single board computer initialising.\n");
 
 	spin_lock_init(&wafwdt_lock);
-	if(!request_region(WDT_STOP, 1, "Wafer 5823 WDT"))
-		goto error;
-	if(!request_region(WDT_START, 1, "Wafer 5823 WDT"))
+
+	if (timeout < 1 || timeout > 63) {
+		timeout = WD_TIMO;
+		printk (KERN_INFO PFX "timeout value must be 1<=x<=255, using %d\n",
+			timeout);
+	}
+
+	if (wdt_stop != wdt_start) {
+		if(!request_region(wdt_stop, 1, "Wafer 5823 WDT")) {
+			printk (KERN_ERR PFX "I/O address 0x%04x already in use\n",
+			wdt_stop);
+			ret = -EIO;
+			goto error;
+		}
+	}
+
+	if(!request_region(wdt_start, 1, "Wafer 5823 WDT")) {
+		printk (KERN_ERR PFX "I/O address 0x%04x already in use\n",
+			wdt_start);
+		ret = -EIO;
 		goto error2;
-	if(misc_register(&wafwdt_miscdev)<0)
+	}
+
+	ret = register_reboot_notifier(&wafwdt_notifier);
+	if (ret != 0) {
+		printk (KERN_ERR PFX "cannot register reboot notifier (err=%d)\n",
+			ret);
 		goto error3;
-	register_reboot_notifier(&wafwdt_notifier);
-	return 0;
+	}
+
+	ret = misc_register(&wafwdt_miscdev);
+	if (ret != 0) {
+		printk (KERN_ERR PFX "cannot register miscdev on minor=%d (err=%d)\n",
+			WATCHDOG_MINOR, ret);
+		goto error4;
+	}
+
+	printk (KERN_INFO PFX "initialized. timeout=%d sec (nowayout=%d)\n",
+		timeout, nowayout);
+
+	return ret;
+error4:
+	unregister_reboot_notifier(&wafwdt_notifier);
 error3:
-	release_region(WDT_START, 1);
+	release_region(wdt_start, 1);
 error2:
-	release_region(WDT_STOP, 1);
+	if (wdt_stop != wdt_start)
+		release_region(wdt_stop, 1);
 error:
-	return -ENODEV;
+	return ret;
 }
 
 static void __exit wafwdt_exit(void)
 {
 	misc_deregister(&wafwdt_miscdev);
 	unregister_reboot_notifier(&wafwdt_notifier);
-	release_region(WDT_STOP, 1);
-	release_region(WDT_START, 1);
+	if(wdt_stop != wdt_start)
+		release_region(wdt_stop, 1);
+	release_region(wdt_start, 1);
 }
 
 module_init(wafwdt_init);
 module_exit(wafwdt_exit);
 
 MODULE_AUTHOR("Justin Cormack");
+MODULE_DESCRIPTION("ICP Wafer 5823 Single Board Computer WDT driver");
 MODULE_LICENSE("GPL");
 
 /* end of wafer5823wdt.c */