[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>

diff --git a/drivers/watchdog/ibmasr.c b/drivers/watchdog/ibmasr.c
index 94155f6136c2369a10135cc62b9aef1896755d66..6824bf80b37e783b530d02ed189c5dec96ff66ee 100644 (file)
--- a/drivers/watchdog/ibmasr.c
+++ b/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");