[WATCHDOG 14/57] ibmasr: coding style, locking verify

There is a new #if 0 section here which is a suggested fix for the horrible
PCI hack in the existing code. Would be good if someone with a box that uses
this device could test it.
Signed-off-by: Alan Cox <alan@redhat.com>
Signed-off-by: Wim Van Sebroeck <wim@iguana.be>

drivers/watchdog/ibmasr.c

@@ -19,9 +19,8 @@
 #include <linux/miscdevice.h>
 #include <linux/watchdog.h>
 #include <linux/dmi.h>
-
-#include <asm/io.h>
-#include <asm/uaccess.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
 
 
 enum {
@@ -70,10 +69,13 @@ static char asr_expect_close;
 static unsigned int asr_type, asr_base, asr_length;
 static unsigned int asr_read_addr, asr_write_addr;
 static unsigned char asr_toggle_mask, asr_disable_mask;
+static spinlock_t asr_lock;
 
-static void asr_toggle(void)
+static void __asr_toggle(void)
 {
-	unsigned char reg = inb(asr_read_addr);
+	unsigned char reg;
+
+	reg = inb(asr_read_addr);
 
 	outb(reg & ~asr_toggle_mask, asr_write_addr);
 	reg = inb(asr_read_addr);
@@ -83,12 +85,21 @@ static void asr_toggle(void)
 
 	outb(reg & ~asr_toggle_mask, asr_write_addr);
 	reg = inb(asr_read_addr);
+	spin_unlock(&asr_lock);
+}
+
+static void asr_toggle(void)
+{
+	spin_lock(&asr_lock);
+	__asr_toggle();
+	spin_unlock(&asr_lock);
 }
 
 static void asr_enable(void)
 {
 	unsigned char reg;
 
+	spin_lock(&asr_lock);
 	if (asr_type == ASMTYPE_TOPAZ) {
 		/* asr_write_addr == asr_read_addr */
 		reg = inb(asr_read_addr);
@@ -99,17 +110,21 @@ static void asr_enable(void)
 		 * First make sure the hardware timer is reset by toggling
 		 * ASR hardware timer line.
 		 */
-		asr_toggle();
+		__asr_toggle();
 
 		reg = inb(asr_read_addr);
 		outb(reg & ~asr_disable_mask, asr_write_addr);
 	}
 	reg = inb(asr_read_addr);
+	spin_unlock(&asr_lock);
 }
 
 static void asr_disable(void)
 {
-	unsigned char reg = inb(asr_read_addr);
+	unsigned char reg;
+
+	spin_lock(&asr_lock);
+	reg = inb(asr_read_addr);
 
 	if (asr_type == ASMTYPE_TOPAZ)
 		/* asr_write_addr == asr_read_addr */
@@ -122,6 +137,7 @@ static void asr_disable(void)
 		outb(reg | asr_disable_mask, asr_write_addr);
 	}
 	reg = inb(asr_read_addr);
+	spin_unlock(&asr_lock);
 }
 
 static int __init asr_get_base_address(void)
@@ -133,7 +149,8 @@ static int __init asr_get_base_address(void)
 
 	switch (asr_type) {
 	case ASMTYPE_TOPAZ:
-		/* SELECT SuperIO CHIP FOR QUERYING (WRITE 0x07 TO BOTH 0x2E and 0x2F) */
+		/* SELECT SuperIO CHIP FOR QUERYING
+		   (WRITE 0x07 TO BOTH 0x2E and 0x2F) */
 		outb(0x07, 0x2e);
 		outb(0x07, 0x2f);
 
@@ -154,14 +171,26 @@ static int __init asr_get_base_address(void)
 
 	case ASMTYPE_JASPER:
 		type = "Jaspers ";
-
-		/* FIXME: need to use pci_config_lock here, but it's not exported */
+#if 0
+		u32 r;
+		/* Suggested fix */
+		pdev = pci_get_bus_and_slot(0, DEVFN(0x1f, 0));
+		if (pdev == NULL)
+			return -ENODEV;
+		pci_read_config_dword(pdev, 0x58, &r);
+		asr_base = r & 0xFFFE;
+		pci_dev_put(pdev);
+#else
+		/* FIXME: need to use pci_config_lock here,
+		   but it's not exported */
 
 /*		spin_lock_irqsave(&pci_config_lock, flags);*/
 
 		/* Select the SuperIO chip in the PCI I/O port register */
 		outl(0x8000f858, 0xcf8);
 
+		/* BUS 0, Slot 1F, fnc 0, offset 58 */
+
 		/*
 		 * Read the base address for the SuperIO chip.
 		 * Only the lower 16 bits are valid, but the address is word
@@ -170,7 +199,7 @@ static int __init asr_get_base_address(void)
 		asr_base = inl(0xcfc) & 0xfffe;
 
 /*		spin_unlock_irqrestore(&pci_config_lock, flags);*/
-
+#endif
 		asr_read_addr = asr_write_addr =
 			asr_base + JASPER_ASR_REG_OFFSET;
 		asr_toggle_mask = JASPER_ASR_TOGGLE_MASK;
@@ -241,11 +270,10 @@ static ssize_t asr_write(struct file *file, const char __user *buf,
 	return count;
 }
 
-static int asr_ioctl(struct inode *inode, struct file *file,
-		     unsigned int cmd, unsigned long arg)
+static long asr_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	static const struct watchdog_info ident = {
-		.options =	WDIOF_KEEPALIVEPING | 
+		.options =	WDIOF_KEEPALIVEPING |
 				WDIOF_MAGICCLOSE,
 		.identity =	"IBM ASR"
 	};
@@ -254,53 +282,45 @@ static int asr_ioctl(struct inode *inode, struct file *file,
 	int heartbeat;
 
 	switch (cmd) {
-		case WDIOC_GETSUPPORT:
-			return copy_to_user(argp, &ident, sizeof(ident)) ?
-				-EFAULT : 0;
-
-		case WDIOC_GETSTATUS:
-		case WDIOC_GETBOOTSTATUS:
-			return put_user(0, p);
-
-		case WDIOC_KEEPALIVE:
+	case WDIOC_GETSUPPORT:
+		return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
+	case WDIOC_GETSTATUS:
+	case WDIOC_GETBOOTSTATUS:
+		return put_user(0, p);
+	case WDIOC_KEEPALIVE:
+		asr_toggle();
+		return 0;
+	/*
+	 * The hardware has a fixed timeout value, so no WDIOC_SETTIMEOUT
+	 * and WDIOC_GETTIMEOUT always returns 256.
+	 */
+	case WDIOC_GETTIMEOUT:
+		heartbeat = 256;
+		return put_user(heartbeat, p);
+	case WDIOC_SETOPTIONS:
+	{
+		int new_options, retval = -EINVAL;
+		if (get_user(new_options, p))
+			return -EFAULT;
+		if (new_options & WDIOS_DISABLECARD) {
+			asr_disable();
+			retval = 0;
+		}
+		if (new_options & WDIOS_ENABLECARD) {
+			asr_enable();
 			asr_toggle();
-			return 0;
-
-		/*
-		 * The hardware has a fixed timeout value, so no WDIOC_SETTIMEOUT
-		 * and WDIOC_GETTIMEOUT always returns 256.
-		 */
-		case WDIOC_GETTIMEOUT:
-			heartbeat = 256;
-			return put_user(heartbeat, p);
-
-		case WDIOC_SETOPTIONS: {
-			int new_options, retval = -EINVAL;
-
-			if (get_user(new_options, p))
-				return -EFAULT;
-
-			if (new_options & WDIOS_DISABLECARD) {
-				asr_disable();
-				retval = 0;
-			}
-
-			if (new_options & WDIOS_ENABLECARD) {
-				asr_enable();
-				asr_toggle();
-				retval = 0;
-			}
-
-			return retval;
+			retval = 0;
 		}
+		return retval;
+	}
+	default:
+		return -ENOTTY;
 	}
-
-	return -ENOTTY;
 }
 
 static int asr_open(struct inode *inode, struct file *file)
 {
-	if(test_and_set_bit(0, &asr_is_open))
+	if (test_and_set_bit(0, &asr_is_open))
 		return -EBUSY;
 
 	asr_toggle();
@@ -314,7 +334,8 @@ static int asr_release(struct inode *inode, struct file *file)
 	if (asr_expect_close == 42)
 		asr_disable();
 	else {
-		printk(KERN_CRIT PFX "unexpected close, not stopping watchdog!\n");
+		printk(KERN_CRIT PFX
+				"unexpected close, not stopping watchdog!\n");
 		asr_toggle();
 	}
 	clear_bit(0, &asr_is_open);
@@ -323,12 +344,12 @@ static int asr_release(struct inode *inode, struct file *file)
 }
 
 static const struct file_operations asr_fops = {
-	.owner =	THIS_MODULE,
-	.llseek	=	no_llseek,
-	.write =	asr_write,
-	.ioctl =	asr_ioctl,
-	.open =		asr_open,
-	.release =	asr_release,
+	.owner =		THIS_MODULE,
+	.llseek	=		no_llseek,
+	.write =		asr_write,
+	.unlocked_ioctl =	asr_ioctl,
+	.open =			asr_open,
+	.release =		asr_release,
 };
 
 static struct miscdevice asr_miscdev = {
@@ -367,6 +388,8 @@ static int __init ibmasr_init(void)
 	if (!asr_type)
 		return -ENODEV;
 
+	spin_lock_init(&asr_lock);
+
 	rc = asr_get_base_address();
 	if (rc)
 		return rc;
@@ -395,7 +418,9 @@ module_init(ibmasr_init);
 module_exit(ibmasr_exit);
 
 module_param(nowayout, int, 0);
-MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
+MODULE_PARM_DESC(nowayout,
+	"Watchdog cannot be stopped once started (default="
+				__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 
 MODULE_DESCRIPTION("IBM Automatic Server Restart driver");
 MODULE_AUTHOR("Andrey Panin");