IPMI Watchdog patch

[Corey Minyard <minyard@acm.org>]

[-- Attachment #1: Type: text/plain, Size: 452 bytes --]

Andrew,

The following patch is ready for inclusion; it makes the IPMI
watchdog more consistent with the other watchdog drivers.

I have tested this and it seems to work correctly.  I also added docs
for the interface change.

- support disabling watchdog by writing 'V' to device.
- unify printk()
- use atomic bit operations on ipmi_wdog_open

Signed-off-by: Arkadiusz Miskiewicz <arekm@pld-linux.org>
Signed-off-by: Corey Minyard <minyard@acm.org>


[-- Attachment #2: ipmi_wdog_consistify.diff --]
[-- Type: text/plain, Size: 7341 bytes --]


Index: linux-ipmi/Documentation/IPMI.txt
===================================================================
--- linux-ipmi.orig/Documentation/IPMI.txt	2004-08-02 12:47:34.000000000 -0500
+++ linux-ipmi/Documentation/IPMI.txt	2004-08-02 13:25:53.000000000 -0500
@@ -496,3 +496,8 @@
 Note that if you use the NMI preaction for the watchdog, you MUST
 NOT use nmi watchdog mode 1.  If you use the NMI watchdog, you
 must use mode 2.
+
+Once you open the watchdog timer, you must write a 'V' character to the
+device to close it, or the timer will not stop.  This is a new semantic
+for the driver, but makes it consistent with the rest of the watchdog
+drivers in Linux.
Index: linux-ipmi/drivers/char/ipmi/ipmi_watchdog.c
===================================================================
--- linux-ipmi.orig/drivers/char/ipmi/ipmi_watchdog.c	2004-08-02 13:22:46.000000000 -0500
+++ linux-ipmi/drivers/char/ipmi/ipmi_watchdog.c	2004-08-02 13:22:51.000000000 -0500
@@ -51,6 +51,8 @@
 #include <asm/apic.h>
 #endif
 
+#define	PFX "IPMI Watchdog: "
+
 #define IPMI_WATCHDOG_VERSION "v32"
 
 /*
@@ -160,6 +162,7 @@
 static DECLARE_WAIT_QUEUE_HEAD(read_q);
 static struct fasync_struct *fasync_q = NULL;
 static char pretimeout_since_last_heartbeat = 0;
+static char expect_close;
 
 /* If true, the driver will start running as soon as it is configured
    and ready. */
@@ -191,7 +194,7 @@
 static int ipmi_ignore_heartbeat = 0;
 
 /* Is someone using the watchdog?  Only one user is allowed. */
-static int ipmi_wdog_open = 0;
+static unsigned long ipmi_wdog_open = 0;
 
 /* 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
@@ -287,7 +290,7 @@
 				      recv_msg,
 				      1);
 	if (rv) {
-		printk(KERN_WARNING "IPMI Watchdog, set timeout error: %d\n",
+		printk(KERN_WARNING PFX "set timeout error: %d\n",
 		       rv);
 	}
 
@@ -464,7 +467,7 @@
 				      1);
 	if (rv) {
 		up(&heartbeat_lock);
-		printk(KERN_WARNING "IPMI Watchdog, heartbeat failure: %d\n",
+		printk(KERN_WARNING PFX "heartbeat failure: %d\n",
 		       rv);
 		return rv;
 	}
@@ -606,6 +609,21 @@
 		return -ESPIPE;
 
 	if (len) {
+	    	if (!nowayout) {
+		    	size_t i;
+
+			/* In case it was set long ago */
+			expect_close = 0;
+			
+    			for (i = 0; i != len; i++) {
+				char c;
+
+				if (get_user(c, buf + i))
+					return -EFAULT;
+				if (c == 'V')
+					expect_close = 42;
+			}
+		}
 		rv = ipmi_heartbeat();
 		if (rv)
 			return rv;
@@ -673,11 +691,9 @@
         switch (iminor(ino))
         {
                 case WATCHDOG_MINOR:
-                    if (ipmi_wdog_open)
+		    if(test_and_set_bit(0, &ipmi_wdog_open))
                         return -EBUSY;
 
-                    ipmi_wdog_open = 1;
-
 		    /* Don't start the timer now, let it start on the
 		       first heartbeat. */
 		    ipmi_start_timer_on_heartbeat = 1;
@@ -715,14 +731,18 @@
 {
 	if (iminor(ino)==WATCHDOG_MINOR)
 	{
-		if (!nowayout) {
+		if (expect_close == 42) {
 			ipmi_watchdog_state = WDOG_TIMEOUT_NONE;
 			ipmi_set_timeout(IPMI_SET_TIMEOUT_NO_HB);
+			clear_bit(0, &ipmi_wdog_open);
+		} else {
+			printk(KERN_CRIT PFX "Unexpected close, not stopping watchdog!\n");
+			ipmi_heartbeat();
 		}
-	        ipmi_wdog_open = 0;
 	}
 
 	ipmi_fasync (-1, filep, 0);
+	expect_close = 0;
 
 	return 0;
 }
@@ -750,7 +770,7 @@
 				  void                 *handler_data)
 {
 	if (msg->msg.data[0] != 0) {
-		printk(KERN_ERR "IPMI Watchdog response: Error %x on cmd %x\n",
+		printk(KERN_ERR PFX "response: Error %x on cmd %x\n",
 		       msg->msg.data[0],
 		       msg->msg.cmd);
 	}
@@ -795,7 +815,7 @@
 
 	rv = ipmi_create_user(ipmi_intf, &ipmi_hndlrs, NULL, &watchdog_user);
 	if (rv < 0) {
-		printk("IPMI watchdog: Unable to register with ipmi\n");
+		printk(KERN_CRIT PFX "Unable to register with ipmi\n");
 		goto out;
 	}
 
@@ -807,7 +827,7 @@
 	if (rv < 0) {
 		ipmi_destroy_user(watchdog_user);
 		watchdog_user = NULL;
-		printk("IPMI watchdog: Unable to register misc device\n");
+		printk(KERN_CRIT PFX "Unable to register misc device\n");
 	}
 
  out:
@@ -818,7 +838,7 @@
 		start_now = 0; /* Disable this function after first startup. */
 		ipmi_watchdog_state = action_val;
 		ipmi_set_timeout(IPMI_SET_TIMEOUT_FORCE_HB);
-		printk("Starting IPMI Watchdog now!\n");
+		printk(KERN_INFO PFX "Starting now!\n");
 	}
 }
 
@@ -829,7 +849,7 @@
 	/* If no one else handled the NMI, we assume it was the IPMI
            watchdog. */
 	if ((!handled) && (preop_val == WDOG_PREOP_PANIC))
-		panic("IPMI watchdog pre-timeout");
+		panic(PFX "pre-timeout");
 
 	/* On some machines, the heartbeat will give
 	   an error and not work unless we re-enable
@@ -935,7 +955,7 @@
 {
 	int rv;
 
-	printk(KERN_INFO "IPMI watchdog driver version "
+	printk(KERN_INFO PFX "driver version "
 	       IPMI_WATCHDOG_VERSION "\n");
 
 	if (strcmp(action, "reset") == 0) {
@@ -948,7 +968,7 @@
 		action_val = WDOG_TIMEOUT_POWER_DOWN;
 	} else {
 		action_val = WDOG_TIMEOUT_RESET;
-		printk("ipmi_watchdog: Unknown action '%s', defaulting to"
+		printk(KERN_INFO PFX "Unknown action '%s', defaulting to"
 		       " reset\n", action);
 	}
 
@@ -964,7 +984,7 @@
 		preaction_val = WDOG_PRETIMEOUT_MSG_INT;
 	} else {
 		preaction_val = WDOG_PRETIMEOUT_NONE;
-		printk("ipmi_watchdog: Unknown preaction '%s', defaulting to"
+		printk(KERN_INFO PFX "Unknown preaction '%s', defaulting to"
 		       " none\n", preaction);
 	}
 
@@ -976,23 +996,21 @@
 		preop_val = WDOG_PREOP_GIVE_DATA;
 	} else {
 		preop_val = WDOG_PREOP_NONE;
-		printk("ipmi_watchdog: Unknown preop '%s', defaulting to"
+		printk(KERN_INFO PFX "Unknown preop '%s', defaulting to"
 		       " none\n", preop);
 	}
 
 #ifdef HAVE_NMI_HANDLER
 	if (preaction_val == WDOG_PRETIMEOUT_NMI) {
 		if (preop_val == WDOG_PREOP_GIVE_DATA) {
-			printk(KERN_WARNING
-			       "ipmi_watchdog: Pretimeout op is to give data"
+			printk(KERN_WARNING PFX "Pretimeout op is to give data"
 			       " but NMI pretimeout is enabled, setting"
 			       " pretimeout op to none\n");
 			preop_val = WDOG_PREOP_NONE;
 		}
 #ifdef CONFIG_X86_LOCAL_APIC
 		if (nmi_watchdog == NMI_IO_APIC) {
-			printk(KERN_WARNING
-			       "ipmi_watchdog: nmi_watchdog is set to IO APIC"
+			printk(KERN_WARNING PFX "nmi_watchdog is set to IO APIC"
 			       " mode (value is %d), that is incompatible"
 			       " with using NMI in the IPMI watchdog."
 			       " Disabling IPMI nmi pretimeout.\n",
@@ -1002,8 +1020,7 @@
 #endif
 		rv = request_nmi(&ipmi_nmi_handler);
 		if (rv) {
-			printk(KERN_WARNING
-			       "ipmi_watchdog: Can't register nmi handler\n");
+			printk(KERN_WARNING PFX "Can't register nmi handler\n");
 			return rv;
 		}
 #ifdef CONFIG_X86_LOCAL_APIC
@@ -1018,8 +1035,7 @@
 		if (preaction_val == WDOG_PRETIMEOUT_NMI)
 			release_nmi(&ipmi_nmi_handler);
 #endif
-		printk(KERN_WARNING
-		       "ipmi_watchdog: can't register smi watcher\n");
+		printk(KERN_WARNING PFX "can't register smi watcher\n");
 		return rv;
 	}
 
@@ -1064,8 +1080,7 @@
 	/* Disconnect from IPMI. */
 	rv = ipmi_destroy_user(watchdog_user);
 	if (rv) {
-		printk(KERN_WARNING
-		       "IPMI Watchdog, error unlinking from IPMI: %d\n",
+		printk(KERN_WARNING PFX "error unlinking from IPMI: %d\n",
 		       rv);
 	}
 	watchdog_user = NULL;