mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/6] Subset of watchdog fixes
@ 2008-08-04 16:52 Alan Cox
  2008-08-04 16:53 ` [PATCH 1/6] Fix locking Alan Cox
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Alan Cox @ 2008-08-04 16:52 UTC (permalink / raw)
  To: torvalds, linux-kernel, wim

These have been pending since May with Wim occasionally surfacing to promise
something will happen then vanishing again with nothing moved on.

The patch set is a tiny subset of the complete set of patches to clean up the
locking bugs, switch to unlocked_ioctl and the like for watchdog drivers

These specific ones are the ones that
- Remove use of inb_p() on PCI bus which is unsafe with some newer boxes
- Switch from using a sempahore held across the return to user space for
  watchdog locking - and thus help remove more users of down_trylock.


 drivers/watchdog/ar7_wdt.c     |   29 ++--
 drivers/watchdog/it8712f_wdt.c |   77 ++++------
 drivers/watchdog/s3c2410_wdt.c |  148 +++++++++++---------
 drivers/watchdog/sc1200wdt.c   |  205 +++++++++++++++------------
 drivers/watchdog/wdt.c         |  176 ++++++++++++-----------
 drivers/watchdog/wdt_pci.c     |  301 +++++++++++++++++++++++-----------------
 6 files changed, 503 insertions(+), 433 deletions(-)

-- 
"The IETF already has more than enough RFCs that codify the obvious, make
stupidity illegal, support truth, justice, and the IETF way, and generally
demonstrate the author is a brilliant and valuable Contributor to The
Standards Process." -- Vernon Schryver

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/6] Fix locking
  2008-08-04 16:52 [PATCH 0/6] Subset of watchdog fixes Alan Cox
@ 2008-08-04 16:53 ` Alan Cox
  2008-08-04 16:54 ` [PATCH 2/6] it8712f_wdt: Locking and coding style Alan Cox
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Alan Cox @ 2008-08-04 16:53 UTC (permalink / raw)
  To: torvalds, linux-kernel, wim

From: Alan Cox <alan@redhat.com>

Use unlocked_ioctl
Remove semaphores

Signed-off-by: Alan Cox <alan@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/watchdog/ar7_wdt.c |   29 +++++++++++++++++------------
 1 files changed, 17 insertions(+), 12 deletions(-)


diff --git a/drivers/watchdog/ar7_wdt.c b/drivers/watchdog/ar7_wdt.c
index 2eb48c0..ef7b0d6 100644
--- a/drivers/watchdog/ar7_wdt.c
+++ b/drivers/watchdog/ar7_wdt.c
@@ -69,7 +69,8 @@ struct ar7_wdt {
 	u32 prescale;
 };
 
-static struct semaphore open_semaphore;
+static unsigned long wdt_is_open;
+static spinlock_t wdt_lock;
 static unsigned expect_close;
 
 /* XXX currently fixed, allows max margin ~68.72 secs */
@@ -154,8 +155,10 @@ static void ar7_wdt_update_margin(int new_margin)
 	u32 change;
 
 	change = new_margin * (ar7_vbus_freq() / prescale_value);
-	if (change < 1) change = 1;
-	if (change > 0xffff) change = 0xffff;
+	if (change < 1)
+		change = 1;
+	if (change > 0xffff)
+		change = 0xffff;
 	ar7_wdt_change(change);
 	margin = change * prescale_value / ar7_vbus_freq();
 	printk(KERN_INFO DRVNAME
@@ -179,7 +182,7 @@ static void ar7_wdt_disable_wdt(void)
 static int ar7_wdt_open(struct inode *inode, struct file *file)
 {
 	/* only allow one at a time */
-	if (down_trylock(&open_semaphore))
+	if (test_and_set_bit(0, &wdt_is_open))
 		return -EBUSY;
 	ar7_wdt_enable_wdt();
 	expect_close = 0;
@@ -195,9 +198,7 @@ static int ar7_wdt_release(struct inode *inode, struct file *file)
 		"will not disable the watchdog timer\n");
 	else if (!nowayout)
 		ar7_wdt_disable_wdt();
-
-	up(&open_semaphore);
-
+	clear_bit(0, &wdt_is_open);
 	return 0;
 }
 
@@ -222,7 +223,9 @@ static ssize_t ar7_wdt_write(struct file *file, const char *data,
 	if (len) {
 		size_t i;
 
+		spin_lock(&wdt_lock);
 		ar7_wdt_kick(1);
+		spin_unlock(&wdt_lock);
 
 		expect_close = 0;
 		for (i = 0; i < len; ++i) {
@@ -237,8 +240,8 @@ static ssize_t ar7_wdt_write(struct file *file, const char *data,
 	return len;
 }
 
-static int ar7_wdt_ioctl(struct inode *inode, struct file *file,
-			 unsigned int cmd, unsigned long arg)
+static long ar7_wdt_ioctl(struct file *file,
+					unsigned int cmd, unsigned long arg)
 {
 	static struct watchdog_info ident = {
 		.identity = LONGNAME,
@@ -269,8 +272,10 @@ static int ar7_wdt_ioctl(struct inode *inode, struct file *file,
 		if (new_margin < 1)
 			return -EINVAL;
 
+		spin_lock(&wdt_lock);
 		ar7_wdt_update_margin(new_margin);
 		ar7_wdt_kick(1);
+		spin_unlock(&wdt_lock);
 
 	case WDIOC_GETTIMEOUT:
 		if (put_user(margin, (int *)arg))
@@ -282,7 +287,7 @@ static int ar7_wdt_ioctl(struct inode *inode, struct file *file,
 static const struct file_operations ar7_wdt_fops = {
 	.owner		= THIS_MODULE,
 	.write		= ar7_wdt_write,
-	.ioctl		= ar7_wdt_ioctl,
+	.unlocked_ioctl	= ar7_wdt_ioctl,
 	.open		= ar7_wdt_open,
 	.release	= ar7_wdt_release,
 };
@@ -297,6 +302,8 @@ static int __init ar7_wdt_init(void)
 {
 	int rc;
 
+	spin_lock_init(&wdt_lock);
+
 	ar7_wdt_get_regs();
 
 	if (!request_mem_region(ar7_regs_wdt, sizeof(struct ar7_wdt),
@@ -312,8 +319,6 @@ static int __init ar7_wdt_init(void)
 	ar7_wdt_prescale(prescale_value);
 	ar7_wdt_update_margin(margin);
 
-	sema_init(&open_semaphore, 1);
-
 	rc = register_reboot_notifier(&ar7_wdt_notifier);
 	if (rc) {
 		printk(KERN_ERR DRVNAME


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/6] it8712f_wdt: Locking and coding style
  2008-08-04 16:52 [PATCH 0/6] Subset of watchdog fixes Alan Cox
  2008-08-04 16:53 ` [PATCH 1/6] Fix locking Alan Cox
@ 2008-08-04 16:54 ` Alan Cox
  2008-08-04 16:54 ` [PATCH 3/6] " Alan Cox
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Alan Cox @ 2008-08-04 16:54 UTC (permalink / raw)
  To: torvalds, linux-kernel, wim

From: Alan Cox <alan@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Alan Cox <alan@redhat.com>
---

 drivers/watchdog/it8712f_wdt.c |   77 ++++++++++++++--------------------------
 1 files changed, 27 insertions(+), 50 deletions(-)


diff --git a/drivers/watchdog/it8712f_wdt.c b/drivers/watchdog/it8712f_wdt.c
index 445b7e8..51bfd57 100644
--- a/drivers/watchdog/it8712f_wdt.c
+++ b/drivers/watchdog/it8712f_wdt.c
@@ -30,9 +30,8 @@
 #include <linux/fs.h>
 #include <linux/pci.h>
 #include <linux/spinlock.h>
-
-#include <asm/uaccess.h>
-#include <asm/io.h>
+#include <linux/uaccess.h>
+#include <linux/io.h>
 
 #define NAME "it8712f_wdt"
 
@@ -50,7 +49,7 @@ static int nowayout = WATCHDOG_NOWAYOUT;
 module_param(nowayout, int, 0);
 MODULE_PARM_DESC(nowayout, "Disable watchdog shutdown on close");
 
-static struct semaphore it8712f_wdt_sem;
+static unsigned long wdt_open;
 static unsigned expect_close;
 static spinlock_t io_lock;
 static unsigned char revision;
@@ -86,22 +85,19 @@ static unsigned short address;
 #define WDT_OUT_PWROK	0x10
 #define WDT_OUT_KRST	0x40
 
-static int
-superio_inb(int reg)
+static int superio_inb(int reg)
 {
 	outb(reg, REG);
 	return inb(VAL);
 }
 
-static void
-superio_outb(int val, int reg)
+static void superio_outb(int val, int reg)
 {
 	outb(reg, REG);
 	outb(val, VAL);
 }
 
-static int
-superio_inw(int reg)
+static int superio_inw(int reg)
 {
 	int val;
 	outb(reg++, REG);
@@ -111,15 +107,13 @@ superio_inw(int reg)
 	return val;
 }
 
-static inline void
-superio_select(int ldn)
+static inline void superio_select(int ldn)
 {
 	outb(LDN, REG);
 	outb(ldn, VAL);
 }
 
-static inline void
-superio_enter(void)
+static inline void superio_enter(void)
 {
 	spin_lock(&io_lock);
 	outb(0x87, REG);
@@ -128,22 +122,19 @@ superio_enter(void)
 	outb(0x55, REG);
 }
 
-static inline void
-superio_exit(void)
+static inline void superio_exit(void)
 {
 	outb(0x02, REG);
 	outb(0x02, VAL);
 	spin_unlock(&io_lock);
 }
 
-static inline void
-it8712f_wdt_ping(void)
+static inline void it8712f_wdt_ping(void)
 {
 	inb(address);
 }
 
-static void
-it8712f_wdt_update_margin(void)
+static void it8712f_wdt_update_margin(void)
 {
 	int config = WDT_OUT_KRST | WDT_OUT_PWROK;
 	int units = margin;
@@ -165,8 +156,7 @@ it8712f_wdt_update_margin(void)
 	superio_outb(units, WDT_TIMEOUT);
 }
 
-static int
-it8712f_wdt_get_status(void)
+static int it8712f_wdt_get_status(void)
 {
 	if (superio_inb(WDT_CONTROL) & 0x01)
 		return WDIOF_CARDRESET;
@@ -174,8 +164,7 @@ it8712f_wdt_get_status(void)
 		return 0;
 }
 
-static void
-it8712f_wdt_enable(void)
+static void it8712f_wdt_enable(void)
 {
 	printk(KERN_DEBUG NAME ": enabling watchdog timer\n");
 	superio_enter();
@@ -190,8 +179,7 @@ it8712f_wdt_enable(void)
 	it8712f_wdt_ping();
 }
 
-static void
-it8712f_wdt_disable(void)
+static void it8712f_wdt_disable(void)
 {
 	printk(KERN_DEBUG NAME ": disabling watchdog timer\n");
 
@@ -207,8 +195,7 @@ it8712f_wdt_disable(void)
 	superio_exit();
 }
 
-static int
-it8712f_wdt_notify(struct notifier_block *this,
+static int it8712f_wdt_notify(struct notifier_block *this,
 		    unsigned long code, void *unused)
 {
 	if (code == SYS_HALT || code == SYS_POWER_OFF)
@@ -222,9 +209,8 @@ static struct notifier_block it8712f_wdt_notifier = {
 	.notifier_call = it8712f_wdt_notify,
 };
 
-static ssize_t
-it8712f_wdt_write(struct file *file, const char __user *data,
-	size_t len, loff_t *ppos)
+static ssize_t it8712f_wdt_write(struct file *file, const char __user *data,
+					size_t len, loff_t *ppos)
 {
 	/* check for a magic close character */
 	if (len) {
@@ -245,9 +231,8 @@ it8712f_wdt_write(struct file *file, const char __user *data,
 	return len;
 }
 
-static int
-it8712f_wdt_ioctl(struct inode *inode, struct file *file,
-	unsigned int cmd, unsigned long arg)
+static long it8712f_wdt_ioctl(struct file *file, unsigned int cmd,
+							unsigned long arg)
 {
 	void __user *argp = (void __user *)arg;
 	int __user *p = argp;
@@ -302,19 +287,16 @@ it8712f_wdt_ioctl(struct inode *inode, struct file *file,
 	}
 }
 
-static int
-it8712f_wdt_open(struct inode *inode, struct file *file)
+static int it8712f_wdt_open(struct inode *inode, struct file *file)
 {
 	/* only allow one at a time */
-	if (down_trylock(&it8712f_wdt_sem))
+	if (test_and_set_bit(0, &wdt_open))
 		return -EBUSY;
 	it8712f_wdt_enable();
-
 	return nonseekable_open(inode, file);
 }
 
-static int
-it8712f_wdt_release(struct inode *inode, struct file *file)
+static int it8712f_wdt_release(struct inode *inode, struct file *file)
 {
 	if (expect_close != 42) {
 		printk(KERN_WARNING NAME
@@ -324,7 +306,7 @@ it8712f_wdt_release(struct inode *inode, struct file *file)
 		it8712f_wdt_disable();
 	}
 	expect_close = 0;
-	up(&it8712f_wdt_sem);
+	clear_bit(0, &wdt_open);
 
 	return 0;
 }
@@ -333,7 +315,7 @@ static const struct file_operations it8712f_wdt_fops = {
 	.owner = THIS_MODULE,
 	.llseek = no_llseek,
 	.write = it8712f_wdt_write,
-	.ioctl = it8712f_wdt_ioctl,
+	.unlocked_ioctl = it8712f_wdt_ioctl,
 	.open = it8712f_wdt_open,
 	.release = it8712f_wdt_release,
 };
@@ -344,8 +326,7 @@ static struct miscdevice it8712f_wdt_miscdev = {
 	.fops = &it8712f_wdt_fops,
 };
 
-static int __init
-it8712f_wdt_find(unsigned short *address)
+static int __init it8712f_wdt_find(unsigned short *address)
 {
 	int err = -ENODEV;
 	int chip_type;
@@ -387,8 +368,7 @@ exit:
 	return err;
 }
 
-static int __init
-it8712f_wdt_init(void)
+static int __init it8712f_wdt_init(void)
 {
 	int err = 0;
 
@@ -404,8 +384,6 @@ it8712f_wdt_init(void)
 
 	it8712f_wdt_disable();
 
-	sema_init(&it8712f_wdt_sem, 1);
-
 	err = register_reboot_notifier(&it8712f_wdt_notifier);
 	if (err) {
 		printk(KERN_ERR NAME ": unable to register reboot notifier\n");
@@ -430,8 +408,7 @@ out:
 	return err;
 }
 
-static void __exit
-it8712f_wdt_exit(void)
+static void __exit it8712f_wdt_exit(void)
 {
 	misc_deregister(&it8712f_wdt_miscdev);
 	unregister_reboot_notifier(&it8712f_wdt_notifier);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/6] Locking and coding style
  2008-08-04 16:52 [PATCH 0/6] Subset of watchdog fixes Alan Cox
  2008-08-04 16:53 ` [PATCH 1/6] Fix locking Alan Cox
  2008-08-04 16:54 ` [PATCH 2/6] it8712f_wdt: Locking and coding style Alan Cox
@ 2008-08-04 16:54 ` Alan Cox
  2008-08-04 16:55 ` [PATCH 4/6] wdt: Cleanup and sort out locking and inb_p Alan Cox
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Alan Cox @ 2008-08-04 16:54 UTC (permalink / raw)
  To: torvalds, linux-kernel, wim

Kill off use of semaphores.
Fix ioctl races and locking holes.

From: Alan Cox <alan@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Alan Cox <alan@redhat.com>
---

 drivers/watchdog/s3c2410_wdt.c |  148 ++++++++++++++++++++++------------------
 1 files changed, 80 insertions(+), 68 deletions(-)


diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 98532c0..97b4a2e 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -46,9 +46,8 @@
 #include <linux/platform_device.h>
 #include <linux/interrupt.h>
 #include <linux/clk.h>
-
-#include <asm/uaccess.h>
-#include <asm/io.h>
+#include <linux/uaccess.h>
+#include <linux/io.h>
 
 #include <asm/arch/map.h>
 
@@ -65,8 +64,8 @@
 static int nowayout	= WATCHDOG_NOWAYOUT;
 static int tmr_margin	= CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME;
 static int tmr_atboot	= CONFIG_S3C2410_WATCHDOG_ATBOOT;
-static int soft_noboot	= 0;
-static int debug	= 0;
+static int soft_noboot;
+static int debug;
 
 module_param(tmr_margin,  int, 0);
 module_param(tmr_atboot,  int, 0);
@@ -74,24 +73,23 @@ module_param(nowayout,    int, 0);
 module_param(soft_noboot, int, 0);
 module_param(debug,	  int, 0);
 
-MODULE_PARM_DESC(tmr_margin, "Watchdog tmr_margin in seconds. default=" __MODULE_STRING(CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME) ")");
-
-MODULE_PARM_DESC(tmr_atboot, "Watchdog is started at boot time if set to 1, default=" __MODULE_STRING(CONFIG_S3C2410_WATCHDOG_ATBOOT));
-
-MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default=" __MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
-
+MODULE_PARM_DESC(tmr_margin, "Watchdog tmr_margin in seconds. default="
+		__MODULE_STRING(CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME) ")");
+MODULE_PARM_DESC(tmr_atboot,
+		"Watchdog is started at boot time if set to 1, default="
+			__MODULE_STRING(CONFIG_S3C2410_WATCHDOG_ATBOOT));
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
+			__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
 MODULE_PARM_DESC(soft_noboot, "Watchdog action, set to 1 to ignore reboots, 0 to reboot (default depends on ONLY_TESTING)");
-
 MODULE_PARM_DESC(debug, "Watchdog debug, set to >1 for debug, (default 0)");
 
 
 typedef enum close_state {
 	CLOSE_STATE_NOT,
-	CLOSE_STATE_ALLOW=0x4021
+	CLOSE_STATE_ALLOW = 0x4021
 } close_state_t;
 
-static DECLARE_MUTEX(open_lock);
-
+static unsigned long open_lock;
 static struct device    *wdt_dev;	/* platform device attached to */
 static struct resource	*wdt_mem;
 static struct resource	*wdt_irq;
@@ -99,38 +97,58 @@ static struct clk	*wdt_clock;
 static void __iomem	*wdt_base;
 static unsigned int	 wdt_count;
 static close_state_t	 allow_close;
+static DEFINE_SPINLOCK(wdt_lock);
 
 /* watchdog control routines */
 
 #define DBG(msg...) do { \
 	if (debug) \
 		printk(KERN_INFO msg); \
-	} while(0)
+	} while (0)
 
 /* functions */
 
-static int s3c2410wdt_keepalive(void)
+static void s3c2410wdt_keepalive(void)
 {
+	spin_lock(&wdt_lock);
 	writel(wdt_count, wdt_base + S3C2410_WTCNT);
-	return 0;
+	spin_unlock(&wdt_lock);
 }
 
-static int s3c2410wdt_stop(void)
+static void __s3c2410wdt_stop(void)
 {
 	unsigned long wtcon;
 
+	spin_lock(&wdt_lock);
 	wtcon = readl(wdt_base + S3C2410_WTCON);
 	wtcon &= ~(S3C2410_WTCON_ENABLE | S3C2410_WTCON_RSTEN);
 	writel(wtcon, wdt_base + S3C2410_WTCON);
+	spin_unlock(&wdt_lock);
+}
 
-	return 0;
+static void __s3c2410wdt_stop(void)
+{
+	unsigned long wtcon;
+
+	wtcon = readl(wdt_base + S3C2410_WTCON);
+	wtcon &= ~(S3C2410_WTCON_ENABLE | S3C2410_WTCON_RSTEN);
+	writel(wtcon, wdt_base + S3C2410_WTCON);
+}
+
+static void s3c2410wdt_stop(void)
+{
+	spin_lock(&wdt_lock);
+	__s3c2410wdt_stop();
+	spin_unlock(&wdt_lock);
 }
 
-static int s3c2410wdt_start(void)
+static void s3c2410wdt_start(void)
 {
 	unsigned long wtcon;
 
-	s3c2410wdt_stop();
+	spin_lock(&wdt_lock);
+
+	__s3c2410wdt_stop();
 
 	wtcon = readl(wdt_base + S3C2410_WTCON);
 	wtcon |= S3C2410_WTCON_ENABLE | S3C2410_WTCON_DIV128;
@@ -149,6 +167,7 @@ static int s3c2410wdt_start(void)
 	writel(wdt_count, wdt_base + S3C2410_WTDAT);
 	writel(wdt_count, wdt_base + S3C2410_WTCNT);
 	writel(wtcon, wdt_base + S3C2410_WTCON);
+	spin_unlock(&wdt_lock);
 
 	return 0;
 }
@@ -211,7 +230,7 @@ static int s3c2410wdt_set_heartbeat(int timeout)
 
 static int s3c2410wdt_open(struct inode *inode, struct file *file)
 {
-	if(down_trylock(&open_lock))
+	if (test_and_set_bit(0, &open_lock))
 		return -EBUSY;
 
 	if (nowayout)
@@ -231,15 +250,14 @@ static int s3c2410wdt_release(struct inode *inode, struct file *file)
 	 * 	Lock it in if it's a module and we set nowayout
 	 */
 
-	if (allow_close == CLOSE_STATE_ALLOW) {
+	if (allow_close == CLOSE_STATE_ALLOW)
 		s3c2410wdt_stop();
-	} else {
+	else {
 		dev_err(wdt_dev, "Unexpected close, not stopping watchdog\n");
 		s3c2410wdt_keepalive();
 	}
-
 	allow_close = CLOSE_STATE_NOT;
-	up(&open_lock);
+	clear_bit(0, &open_lock);
 	return 0;
 }
 
@@ -249,7 +267,7 @@ static ssize_t s3c2410wdt_write(struct file *file, const char __user *data,
 	/*
 	 *	Refresh the timer.
 	 */
-	if(len) {
+	if (len) {
 		if (!nowayout) {
 			size_t i;
 
@@ -265,7 +283,6 @@ static ssize_t s3c2410wdt_write(struct file *file, const char __user *data,
 					allow_close = CLOSE_STATE_ALLOW;
 			}
 		}
-
 		s3c2410wdt_keepalive();
 	}
 	return len;
@@ -273,48 +290,41 @@ static ssize_t s3c2410wdt_write(struct file *file, const char __user *data,
 
 #define OPTIONS WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE
 
-static struct watchdog_info s3c2410_wdt_ident = {
+static const struct watchdog_info s3c2410_wdt_ident = {
 	.options          =     OPTIONS,
 	.firmware_version =	0,
 	.identity         =	"S3C2410 Watchdog",
 };
 
 
-static int s3c2410wdt_ioctl(struct inode *inode, struct file *file,
-	unsigned int cmd, unsigned long arg)
+static long s3c2410wdt_ioctl(struct file *file,	unsigned int cmd,
+							unsigned long arg)
 {
 	void __user *argp = (void __user *)arg;
 	int __user *p = argp;
 	int new_margin;
 
 	switch (cmd) {
-		default:
-			return -ENOTTY;
-
-		case WDIOC_GETSUPPORT:
-			return copy_to_user(argp, &s3c2410_wdt_ident,
-				sizeof(s3c2410_wdt_ident)) ? -EFAULT : 0;
-
-		case WDIOC_GETSTATUS:
-		case WDIOC_GETBOOTSTATUS:
-			return put_user(0, p);
-
-		case WDIOC_KEEPALIVE:
-			s3c2410wdt_keepalive();
-			return 0;
-
-		case WDIOC_SETTIMEOUT:
-			if (get_user(new_margin, p))
-				return -EFAULT;
-
-			if (s3c2410wdt_set_heartbeat(new_margin))
-				return -EINVAL;
-
-			s3c2410wdt_keepalive();
-			return put_user(tmr_margin, p);
-
-		case WDIOC_GETTIMEOUT:
-			return put_user(tmr_margin, p);
+	default:
+		return -ENOTTY;
+	case WDIOC_GETSUPPORT:
+		return copy_to_user(argp, &s3c2410_wdt_ident,
+			sizeof(s3c2410_wdt_ident)) ? -EFAULT : 0;
+	case WDIOC_GETSTATUS:
+	case WDIOC_GETBOOTSTATUS:
+		return put_user(0, p);
+	case WDIOC_KEEPALIVE:
+		s3c2410wdt_keepalive();
+		return 0;
+	case WDIOC_SETTIMEOUT:
+		if (get_user(new_margin, p))
+			return -EFAULT;
+		if (s3c2410wdt_set_heartbeat(new_margin))
+			return -EINVAL;
+		s3c2410wdt_keepalive();
+		return put_user(tmr_margin, p);
+	case WDIOC_GETTIMEOUT:
+		return put_user(tmr_margin, p);
 	}
 }
 
@@ -324,7 +334,7 @@ static const struct file_operations s3c2410wdt_fops = {
 	.owner		= THIS_MODULE,
 	.llseek		= no_llseek,
 	.write		= s3c2410wdt_write,
-	.ioctl		= s3c2410wdt_ioctl,
+	.unlocked_ioctl	= s3c2410wdt_ioctl,
 	.open		= s3c2410wdt_open,
 	.release	= s3c2410wdt_release,
 };
@@ -411,14 +421,15 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
 	 * not, try the default value */
 
 	if (s3c2410wdt_set_heartbeat(tmr_margin)) {
-		started = s3c2410wdt_set_heartbeat(CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME);
+		started = s3c2410wdt_set_heartbeat(
+					CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME);
 
-		if (started == 0) {
-			dev_info(dev,"tmr_margin value out of range, default %d used\n",
+		if (started == 0)
+			dev_info(dev,
+			   "tmr_margin value out of range, default %d used\n",
 			       CONFIG_S3C2410_WATCHDOG_DEFAULT_TIME);
-		} else {
+		else
 			dev_info(dev, "default timer value is out of range, cannot start\n");
-		}
 	}
 
 	ret = misc_register(&s3c2410wdt_miscdev);
@@ -447,7 +458,7 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
 		 (wtcon & S3C2410_WTCON_ENABLE) ?  "" : "in",
 		 (wtcon & S3C2410_WTCON_RSTEN) ? "" : "dis",
 		 (wtcon & S3C2410_WTCON_INTEN) ? "" : "en");
-	
+
 	return 0;
 
  err_clk:
@@ -487,7 +498,7 @@ static int s3c2410wdt_remove(struct platform_device *dev)
 
 static void s3c2410wdt_shutdown(struct platform_device *dev)
 {
-	s3c2410wdt_stop();	
+	s3c2410wdt_stop();
 }
 
 #ifdef CONFIG_PM
@@ -540,7 +551,8 @@ static struct platform_driver s3c2410wdt_driver = {
 };
 
 
-static char banner[] __initdata = KERN_INFO "S3C2410 Watchdog Timer, (c) 2004 Simtec Electronics\n";
+static char banner[] __initdata =
+	KERN_INFO "S3C2410 Watchdog Timer, (c) 2004 Simtec Electronics\n";
 
 static int __init watchdog_init(void)
 {


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 4/6] wdt: Cleanup and sort out locking and inb_p
  2008-08-04 16:52 [PATCH 0/6] Subset of watchdog fixes Alan Cox
                   ` (2 preceding siblings ...)
  2008-08-04 16:54 ` [PATCH 3/6] " Alan Cox
@ 2008-08-04 16:55 ` Alan Cox
  2008-08-04 16:56 ` [PATCH 5/6] alpha: Fix breakage in wdt_pci Alan Cox
  2008-08-04 16:56 ` [PATCH 6/6] sc1200: Fix locking, sems and coding style Alan Cox
  5 siblings, 0 replies; 7+ messages in thread
From: Alan Cox @ 2008-08-04 16:55 UTC (permalink / raw)
  To: torvalds, linux-kernel, wim

From: Alan Cox <alan@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Alan Cox <alan@redhat.com>
---

 drivers/watchdog/wdt.c     |  176 ++++++++++++++------------
 drivers/watchdog/wdt_pci.c |  300 +++++++++++++++++++++++++-------------------
 2 files changed, 265 insertions(+), 211 deletions(-)


diff --git a/drivers/watchdog/wdt.c b/drivers/watchdog/wdt.c
index 756fb15..53a6b18 100644
--- a/drivers/watchdog/wdt.c
+++ b/drivers/watchdog/wdt.c
@@ -24,9 +24,10 @@
  *					Matt Crocker).
  *		Alan Cox	:	Added wdt= boot option
  *		Alan Cox	:	Cleaned up copy/user stuff
- *		Tim Hockin	:	Added insmod parameters, comment cleanup
- *					Parameterized timeout
- *		Tigran Aivazian	:	Restructured wdt_init() to handle failures
+ *		Tim Hockin	:	Added insmod parameters, comment
+ *					cleanup, parameterized timeout
+ *		Tigran Aivazian	:	Restructured wdt_init() to handle
+ *					failures
  *		Joel Becker	:	Added WDIOC_GET/SETTIMEOUT
  *		Matt Domsch	:	Added nowayout module option
  */
@@ -42,9 +43,9 @@
 #include <linux/notifier.h>
 #include <linux/reboot.h>
 #include <linux/init.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
 
-#include <asm/io.h>
-#include <asm/uaccess.h>
 #include <asm/system.h>
 #include "wd501p.h"
 
@@ -60,15 +61,19 @@ static char expect_close;
 static int heartbeat = WD_TIMO;
 static int wd_heartbeat;
 module_param(heartbeat, int, 0);
-MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds. (0<heartbeat<65536, default=" __MODULE_STRING(WD_TIMO) ")");
+MODULE_PARM_DESC(heartbeat,
+	"Watchdog heartbeat in seconds. (0 < heartbeat < 65536, default="
+				__MODULE_STRING(WD_TIMO) ")");
 
 static int nowayout = WATCHDOG_NOWAYOUT;
 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) ")");
 
 /* You must set these - there is no sane way to probe for this board. */
-static int io=0x240;
-static int irq=11;
+static int io = 0x240;
+static int irq = 11;
 
 static DEFINE_SPINLOCK(wdt_lock);
 
@@ -82,7 +87,8 @@ MODULE_PARM_DESC(irq, "WDT irq (default=11)");
 static int tachometer;
 
 module_param(tachometer, int, 0);
-MODULE_PARM_DESC(tachometer, "WDT501-P Fan Tachometer support (0=disable, default=0)");
+MODULE_PARM_DESC(tachometer,
+		"WDT501-P Fan Tachometer support (0=disable, default=0)");
 #endif /* CONFIG_WDT_501 */
 
 /*
@@ -91,9 +97,9 @@ MODULE_PARM_DESC(tachometer, "WDT501-P Fan Tachometer support (0=disable, defaul
 
 static void wdt_ctr_mode(int ctr, int mode)
 {
-	ctr<<=6;
-	ctr|=0x30;
-	ctr|=(mode<<1);
+	ctr <<= 6;
+	ctr |= 0x30;
+	ctr |= (mode << 1);
 	outb_p(ctr, WDT_CR);
 }
 
@@ -114,12 +120,15 @@ static int wdt_start(void)
 	unsigned long flags;
 	spin_lock_irqsave(&wdt_lock, flags);
 	inb_p(WDT_DC);			/* Disable watchdog */
-	wdt_ctr_mode(0,3);		/* Program CTR0 for Mode 3: Square Wave Generator */
-	wdt_ctr_mode(1,2);		/* Program CTR1 for Mode 2: Rate Generator */
-	wdt_ctr_mode(2,0);		/* Program CTR2 for Mode 0: Pulse on Terminal Count */
+	wdt_ctr_mode(0, 3);		/* Program CTR0 for Mode 3:
+						Square Wave Generator */
+	wdt_ctr_mode(1, 2);		/* Program CTR1 for Mode 2:
+						Rate Generator */
+	wdt_ctr_mode(2, 0);		/* Program CTR2 for Mode 0:
+						Pulse on Terminal Count */
 	wdt_ctr_load(0, 8948);		/* Count at 100Hz */
-	wdt_ctr_load(1,wd_heartbeat);	/* Heartbeat */
-	wdt_ctr_load(2,65535);		/* Length of reset pulse */
+	wdt_ctr_load(1, wd_heartbeat);	/* Heartbeat */
+	wdt_ctr_load(2, 65535);		/* Length of reset pulse */
 	outb_p(0, WDT_DC);		/* Enable watchdog */
 	spin_unlock_irqrestore(&wdt_lock, flags);
 	return 0;
@@ -131,13 +140,13 @@ static int wdt_start(void)
  *	Stop the watchdog driver.
  */
 
-static int wdt_stop (void)
+static int wdt_stop(void)
 {
 	unsigned long flags;
 	spin_lock_irqsave(&wdt_lock, flags);
 	/* Turn the card off */
 	inb_p(WDT_DC);			/* Disable watchdog */
-	wdt_ctr_load(2,0);		/* 0 length reset pulses now */
+	wdt_ctr_load(2, 0);		/* 0 length reset pulses now */
 	spin_unlock_irqrestore(&wdt_lock, flags);
 	return 0;
 }
@@ -145,8 +154,8 @@ static int wdt_stop (void)
 /**
  *	wdt_ping:
  *
- *	Reload counter one with the watchdog heartbeat. We don't bother reloading
- *	the cascade counter.
+ *	Reload counter one with the watchdog heartbeat. We don't bother
+ *	reloading the cascade counter.
  */
 
 static int wdt_ping(void)
@@ -155,8 +164,9 @@ static int wdt_ping(void)
 	spin_lock_irqsave(&wdt_lock, flags);
 	/* Write a watchdog value */
 	inb_p(WDT_DC);			/* Disable watchdog */
-	wdt_ctr_mode(1,2);		/* Re-Program CTR1 for Mode 2: Rate Generator */
-	wdt_ctr_load(1,wd_heartbeat);	/* Heartbeat */
+	wdt_ctr_mode(1, 2);		/* Re-Program CTR1 for Mode 2:
+							Rate Generator */
+	wdt_ctr_load(1, wd_heartbeat);	/* Heartbeat */
 	outb_p(0, WDT_DC);		/* Enable watchdog */
 	spin_unlock_irqrestore(&wdt_lock, flags);
 	return 0;
@@ -166,13 +176,14 @@ static int wdt_ping(void)
  *	wdt_set_heartbeat:
  *	@t:		the new heartbeat value that needs to be set.
  *
- *	Set a new heartbeat value for the watchdog device. If the heartbeat value is
- *	incorrect we keep the old value and return -EINVAL. If successfull we
- *	return 0.
+ *	Set a new heartbeat value for the watchdog device. If the heartbeat
+ *	value is incorrect we keep the old value and return -EINVAL. If
+ *	successful we return 0.
  */
+
 static int wdt_set_heartbeat(int t)
 {
-	if ((t < 1) || (t > 65535))
+	if (t < 1 || t > 65535)
 		return -EINVAL;
 
 	heartbeat = t;
@@ -200,7 +211,7 @@ static int wdt_get_status(int *status)
 	new_status = inb_p(WDT_SR);
 	spin_unlock_irqrestore(&wdt_lock, flags);
 
-	*status=0;
+	*status = 0;
 	if (new_status & WDC_SR_ISOI0)
 		*status |= WDIOF_EXTERN1;
 	if (new_status & WDC_SR_ISII1)
@@ -266,7 +277,7 @@ static irqreturn_t wdt_interrupt(int irq, void *dev_id)
 
 #ifdef CONFIG_WDT_501
 	if (!(status & WDC_SR_TGOOD))
-		printk(KERN_CRIT "Overheat alarm.(%d)\n",inb_p(WDT_RT));
+		printk(KERN_CRIT "Overheat alarm.(%d)\n", inb_p(WDT_RT));
 	if (!(status & WDC_SR_PSUOVER))
 		printk(KERN_CRIT "PSU over voltage.\n");
 	if (!(status & WDC_SR_PSUUNDR))
@@ -304,9 +315,10 @@ static irqreturn_t wdt_interrupt(int irq, void *dev_id)
  *	write of data will do, as we we don't define content meaning.
  */
 
-static ssize_t wdt_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
+static ssize_t wdt_write(struct file *file, const char __user *buf,
+						size_t count, loff_t *ppos)
 {
-	if(count) {
+	if (count) {
 		if (!nowayout) {
 			size_t i;
 
@@ -328,7 +340,6 @@ static ssize_t wdt_write(struct file *file, const char __user *buf, size_t count
 
 /**
  *	wdt_ioctl:
- *	@inode: inode of the device
  *	@file: file handle to the device
  *	@cmd: watchdog command
  *	@arg: argument pointer
@@ -338,8 +349,7 @@ static ssize_t wdt_write(struct file *file, const char __user *buf, size_t count
  *	querying capabilities and current status.
  */
 
-static int wdt_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
-	unsigned long arg)
+static long wdt_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	void __user *argp = (void __user *)arg;
 	int __user *p = argp;
@@ -362,32 +372,28 @@ static int wdt_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
 		ident.options |= WDIOF_FANFAULT;
 #endif /* CONFIG_WDT_501 */
 
-	switch(cmd)
-	{
-		default:
-			return -ENOTTY;
-		case WDIOC_GETSUPPORT:
-			return copy_to_user(argp, &ident, sizeof(ident))?-EFAULT:0;
-
-		case WDIOC_GETSTATUS:
-			wdt_get_status(&status);
-			return put_user(status, p);
-		case WDIOC_GETBOOTSTATUS:
-			return put_user(0, p);
-		case WDIOC_KEEPALIVE:
-			wdt_ping();
-			return 0;
-		case WDIOC_SETTIMEOUT:
-			if (get_user(new_heartbeat, p))
-				return -EFAULT;
-
-			if (wdt_set_heartbeat(new_heartbeat))
-				return -EINVAL;
-
-			wdt_ping();
-			/* Fall */
-		case WDIOC_GETTIMEOUT:
-			return put_user(heartbeat, p);
+	switch (cmd) {
+	default:
+		return -ENOTTY;
+	case WDIOC_GETSUPPORT:
+		return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
+	case WDIOC_GETSTATUS:
+		wdt_get_status(&status);
+		return put_user(status, p);
+	case WDIOC_GETBOOTSTATUS:
+		return put_user(0, p);
+	case WDIOC_KEEPALIVE:
+		wdt_ping();
+		return 0;
+	case WDIOC_SETTIMEOUT:
+		if (get_user(new_heartbeat, p))
+			return -EFAULT;
+		if (wdt_set_heartbeat(new_heartbeat))
+			return -EINVAL;
+		wdt_ping();
+		/* Fall */
+	case WDIOC_GETTIMEOUT:
+		return put_user(heartbeat, p);
 	}
 }
 
@@ -405,7 +411,7 @@ static int wdt_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
 
 static int wdt_open(struct inode *inode, struct file *file)
 {
-	if(test_and_set_bit(0, &wdt_is_open))
+	if (test_and_set_bit(0, &wdt_is_open))
 		return -EBUSY;
 	/*
 	 *	Activate
@@ -432,7 +438,8 @@ static int wdt_release(struct inode *inode, struct file *file)
 		wdt_stop();
 		clear_bit(0, &wdt_is_open);
 	} else {
-		printk(KERN_CRIT "wdt: WDT device closed unexpectedly.  WDT will not stop!\n");
+		printk(KERN_CRIT
+		 "wdt: WDT device closed unexpectedly.  WDT will not stop!\n");
 		wdt_ping();
 	}
 	expect_close = 0;
@@ -451,14 +458,15 @@ static int wdt_release(struct inode *inode, struct file *file)
  *	farenheit. It was designed by an imperial measurement luddite.
  */
 
-static ssize_t wdt_temp_read(struct file *file, char __user *buf, size_t count, loff_t *ptr)
+static ssize_t wdt_temp_read(struct file *file, char __user *buf,
+						size_t count, loff_t *ptr)
 {
 	int temperature;
 
 	if (wdt_get_temperature(&temperature))
 		return -EFAULT;
 
-	if (copy_to_user (buf, &temperature, 1))
+	if (copy_to_user(buf, &temperature, 1))
 		return -EFAULT;
 
 	return 1;
@@ -506,10 +514,8 @@ static int wdt_temp_release(struct inode *inode, struct file *file)
 static int wdt_notify_sys(struct notifier_block *this, unsigned long code,
 	void *unused)
 {
-	if(code==SYS_DOWN || code==SYS_HALT) {
-		/* Turn the card off */
+	if (code == SYS_DOWN || code == SYS_HALT)
 		wdt_stop();
-	}
 	return NOTIFY_DONE;
 }
 
@@ -522,7 +528,7 @@ static const struct file_operations wdt_fops = {
 	.owner		= THIS_MODULE,
 	.llseek		= no_llseek,
 	.write		= wdt_write,
-	.ioctl		= wdt_ioctl,
+	.unlocked_ioctl	= wdt_ioctl,
 	.open		= wdt_open,
 	.release	= wdt_release,
 };
@@ -576,7 +582,7 @@ static void __exit wdt_exit(void)
 #endif /* CONFIG_WDT_501 */
 	unregister_reboot_notifier(&wdt_notifier);
 	free_irq(irq, NULL);
-	release_region(io,8);
+	release_region(io, 8);
 }
 
 /**
@@ -591,44 +597,49 @@ static int __init wdt_init(void)
 {
 	int ret;
 
-	/* Check that the heartbeat value is within it's range ; if not reset to the default */
+	/* Check that the heartbeat value is within it's range;
+	   if not reset to the default */
 	if (wdt_set_heartbeat(heartbeat)) {
 		wdt_set_heartbeat(WD_TIMO);
-		printk(KERN_INFO "wdt: heartbeat value must be 0<heartbeat<65536, using %d\n",
+		printk(KERN_INFO "wdt: heartbeat value must be 0 < heartbeat < 65536, using %d\n",
 			WD_TIMO);
 	}
 
 	if (!request_region(io, 8, "wdt501p")) {
-		printk(KERN_ERR "wdt: I/O address 0x%04x already in use\n", io);
+		printk(KERN_ERR
+			"wdt: I/O address 0x%04x already in use\n", io);
 		ret = -EBUSY;
 		goto out;
 	}
 
 	ret = request_irq(irq, wdt_interrupt, IRQF_DISABLED, "wdt501p", NULL);
-	if(ret) {
+	if (ret) {
 		printk(KERN_ERR "wdt: IRQ %d is not free.\n", irq);
 		goto outreg;
 	}
 
 	ret = register_reboot_notifier(&wdt_notifier);
-	if(ret) {
-		printk(KERN_ERR "wdt: cannot register reboot notifier (err=%d)\n", ret);
+	if (ret) {
+		printk(KERN_ERR
+		      "wdt: cannot register reboot notifier (err=%d)\n", ret);
 		goto outirq;
 	}
 
 #ifdef CONFIG_WDT_501
 	ret = misc_register(&temp_miscdev);
 	if (ret) {
-		printk(KERN_ERR "wdt: cannot register miscdev on minor=%d (err=%d)\n",
-			TEMP_MINOR, ret);
+		printk(KERN_ERR
+			"wdt: cannot register miscdev on minor=%d (err=%d)\n",
+							TEMP_MINOR, ret);
 		goto outrbt;
 	}
 #endif /* CONFIG_WDT_501 */
 
 	ret = misc_register(&wdt_miscdev);
 	if (ret) {
-		printk(KERN_ERR "wdt: cannot register miscdev on minor=%d (err=%d)\n",
-			WATCHDOG_MINOR, ret);
+		printk(KERN_ERR
+			"wdt: cannot register miscdev on minor=%d (err=%d)\n",
+							WATCHDOG_MINOR, ret);
 		goto outmisc;
 	}
 
@@ -636,7 +647,8 @@ static int __init wdt_init(void)
 	printk(KERN_INFO "WDT500/501-P driver 0.10 at 0x%04x (Interrupt %d). heartbeat=%d sec (nowayout=%d)\n",
 		io, irq, heartbeat, nowayout);
 #ifdef CONFIG_WDT_501
-	printk(KERN_INFO "wdt: Fan Tachometer is %s\n", (tachometer ? "Enabled" : "Disabled"));
+	printk(KERN_INFO "wdt: Fan Tachometer is %s\n",
+				(tachometer ? "Enabled" : "Disabled"));
 #endif /* CONFIG_WDT_501 */
 
 out:
@@ -651,7 +663,7 @@ outrbt:
 outirq:
 	free_irq(irq, NULL);
 outreg:
-	release_region(io,8);
+	release_region(io, 8);
 	goto out;
 }
 
diff --git a/drivers/watchdog/wdt_pci.c b/drivers/watchdog/wdt_pci.c
index 1355608..078f37f 100644
--- a/drivers/watchdog/wdt_pci.c
+++ b/drivers/watchdog/wdt_pci.c
@@ -29,9 +29,11 @@
  *		JP Nollmann	:	Added support for PCI wdt501p
  *		Alan Cox	:	Split ISA and PCI cards into two drivers
  *		Jeff Garzik	:	PCI cleanups
- *		Tigran Aivazian	:	Restructured wdtpci_init_one() to handle failures
+ *		Tigran Aivazian	:	Restructured wdtpci_init_one() to handle
+ *					failures
  *		Joel Becker 	:	Added WDIOC_GET/SETTIMEOUT
- *		Zwane Mwaikambo	:	Magic char closing, locking changes, cleanups
+ *		Zwane Mwaikambo	:	Magic char closing, locking changes,
+ *					cleanups
  *		Matt Domsch	:	nowayout module option
  */
 
@@ -47,9 +49,9 @@
 #include <linux/init.h>
 #include <linux/fs.h>
 #include <linux/pci.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
 
-#include <asm/io.h>
-#include <asm/uaccess.h>
 #include <asm/system.h>
 
 #define WDT_IS_PCI
@@ -73,7 +75,7 @@
 /* We can only use 1 card due to the /dev/watchdog restriction */
 static int dev_count;
 
-static struct semaphore open_sem;
+static unsigned long open_lock;
 static DEFINE_SPINLOCK(wdtpci_lock);
 static char expect_close;
 
@@ -86,18 +88,23 @@ static int irq;
 static int heartbeat = WD_TIMO;
 static int wd_heartbeat;
 module_param(heartbeat, int, 0);
-MODULE_PARM_DESC(heartbeat, "Watchdog heartbeat in seconds. (0<heartbeat<65536, default=" __MODULE_STRING(WD_TIMO) ")");
+MODULE_PARM_DESC(heartbeat,
+		"Watchdog heartbeat in seconds. (0<heartbeat<65536, default="
+				__MODULE_STRING(WD_TIMO) ")");
 
 static int nowayout = WATCHDOG_NOWAYOUT;
 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) ")");
 
 #ifdef CONFIG_WDT_501_PCI
 /* Support for the Fan Tachometer on the PCI-WDT501 */
 static int tachometer;
 
 module_param(tachometer, int, 0);
-MODULE_PARM_DESC(tachometer, "PCI-WDT501 Fan Tachometer support (0=disable, default=0)");
+MODULE_PARM_DESC(tachometer,
+	"PCI-WDT501 Fan Tachometer support (0=disable, default=0)");
 #endif /* CONFIG_WDT_501_PCI */
 
 /*
@@ -106,16 +113,19 @@ MODULE_PARM_DESC(tachometer, "PCI-WDT501 Fan Tachometer support (0=disable, defa
 
 static void wdtpci_ctr_mode(int ctr, int mode)
 {
-	ctr<<=6;
-	ctr|=0x30;
-	ctr|=(mode<<1);
-	outb_p(ctr, WDT_CR);
+	ctr <<= 6;
+	ctr |= 0x30;
+	ctr |= (mode << 1);
+	outb(ctr, WDT_CR);
+	udelay(8);
 }
 
 static void wdtpci_ctr_load(int ctr, int val)
 {
-	outb_p(val&0xFF, WDT_COUNT0+ctr);
-	outb_p(val>>8, WDT_COUNT0+ctr);
+	outb(val & 0xFF, WDT_COUNT0 + ctr);
+	udelay(8);
+	outb(val >> 8, WDT_COUNT0 + ctr);
+	udelay(8);
 }
 
 /**
@@ -134,23 +144,35 @@ static int wdtpci_start(void)
 	 * "pet" the watchdog, as Access says.
 	 * This resets the clock outputs.
 	 */
-	inb_p(WDT_DC);			/* Disable watchdog */
-	wdtpci_ctr_mode(2,0);		/* Program CTR2 for Mode 0: Pulse on Terminal Count */
-	outb_p(0, WDT_DC);		/* Enable watchdog */
-
-	inb_p(WDT_DC);			/* Disable watchdog */
-	outb_p(0, WDT_CLOCK);		/* 2.0833MHz clock */
-	inb_p(WDT_BUZZER);		/* disable */
-	inb_p(WDT_OPTONOTRST);		/* disable */
-	inb_p(WDT_OPTORST);		/* disable */
-	inb_p(WDT_PROGOUT);		/* disable */
-	wdtpci_ctr_mode(0,3);		/* Program CTR0 for Mode 3: Square Wave Generator */
-	wdtpci_ctr_mode(1,2);		/* Program CTR1 for Mode 2: Rate Generator */
-	wdtpci_ctr_mode(2,1);		/* Program CTR2 for Mode 1: Retriggerable One-Shot */
-	wdtpci_ctr_load(0,20833);	/* count at 100Hz */
-	wdtpci_ctr_load(1,wd_heartbeat);/* Heartbeat */
+	inb(WDT_DC);			/* Disable watchdog */
+	udelay(8);
+	wdtpci_ctr_mode(2, 0);		/* Program CTR2 for Mode 0:
+						Pulse on Terminal Count */
+	outb(0, WDT_DC);		/* Enable watchdog */
+	udelay(8);
+	inb(WDT_DC);			/* Disable watchdog */
+	udelay(8);
+	outb(0, WDT_CLOCK);		/* 2.0833MHz clock */
+	udelay(8);
+	inb(WDT_BUZZER);		/* disable */
+	udelay(8);
+	inb(WDT_OPTONOTRST);		/* disable */
+	udelay(8);
+	inb(WDT_OPTORST);		/* disable */
+	udelay(8);
+	inb(WDT_PROGOUT);		/* disable */
+	udelay(8);
+	wdtpci_ctr_mode(0, 3);		/* Program CTR0 for Mode 3:
+						Square Wave Generator */
+	wdtpci_ctr_mode(1, 2);		/* Program CTR1 for Mode 2:
+						Rate Generator */
+	wdtpci_ctr_mode(2, 1);		/* Program CTR2 for Mode 1:
+						Retriggerable One-Shot */
+	wdtpci_ctr_load(0, 20833);	/* count at 100Hz */
+	wdtpci_ctr_load(1, wd_heartbeat);/* Heartbeat */
 	/* DO NOT LOAD CTR2 on PCI card! -- JPN */
-	outb_p(0, WDT_DC);		/* Enable watchdog */
+	outb(0, WDT_DC);		/* Enable watchdog */
+	udelay(8);
 
 	spin_unlock_irqrestore(&wdtpci_lock, flags);
 	return 0;
@@ -162,14 +184,15 @@ static int wdtpci_start(void)
  *	Stop the watchdog driver.
  */
 
-static int wdtpci_stop (void)
+static int wdtpci_stop(void)
 {
 	unsigned long flags;
 
 	/* Turn the card off */
 	spin_lock_irqsave(&wdtpci_lock, flags);
-	inb_p(WDT_DC);			/* Disable watchdog */
-	wdtpci_ctr_load(2,0);		/* 0 length reset pulses now */
+	inb(WDT_DC);			/* Disable watchdog */
+	udelay(8);
+	wdtpci_ctr_load(2, 0);		/* 0 length reset pulses now */
 	spin_unlock_irqrestore(&wdtpci_lock, flags);
 	return 0;
 }
@@ -177,20 +200,23 @@ static int wdtpci_stop (void)
 /**
  *	wdtpci_ping:
  *
- *	Reload counter one with the watchdog heartbeat. We don't bother reloading
- *	the cascade counter.
+ *	Reload counter one with the watchdog heartbeat. We don't bother
+ *	reloading the cascade counter.
  */
 
 static int wdtpci_ping(void)
 {
 	unsigned long flags;
 
-	/* Write a watchdog value */
 	spin_lock_irqsave(&wdtpci_lock, flags);
-	inb_p(WDT_DC);			/* Disable watchdog */
-	wdtpci_ctr_mode(1,2);		/* Re-Program CTR1 for Mode 2: Rate Generator */
-	wdtpci_ctr_load(1,wd_heartbeat);/* Heartbeat */
-	outb_p(0, WDT_DC);		/* Enable watchdog */
+	/* Write a watchdog value */
+	inb(WDT_DC);			/* Disable watchdog */
+	udelay(8);
+	wdtpci_ctr_mode(1, 2);		/* Re-Program CTR1 for Mode 2:
+							Rate Generator */
+	wdtpci_ctr_load(1, wd_heartbeat);/* Heartbeat */
+	outb(0, WDT_DC);		/* Enable watchdog */
+	udelay(8);
 	spin_unlock_irqrestore(&wdtpci_lock, flags);
 	return 0;
 }
@@ -199,14 +225,14 @@ static int wdtpci_ping(void)
  *	wdtpci_set_heartbeat:
  *	@t:		the new heartbeat value that needs to be set.
  *
- *	Set a new heartbeat value for the watchdog device. If the heartbeat value is
- *	incorrect we keep the old value and return -EINVAL. If successfull we
- *	return 0.
+ *	Set a new heartbeat value for the watchdog device. If the heartbeat
+ *	value is incorrect we keep the old value and return -EINVAL.
+ *	If successful we return 0.
  */
 static int wdtpci_set_heartbeat(int t)
 {
 	/* Arbitrary, can't find the card's limits */
-	if ((t < 1) || (t > 65535))
+	if (t < 1 || t > 65535)
 		return -EINVAL;
 
 	heartbeat = t;
@@ -227,9 +253,14 @@ static int wdtpci_set_heartbeat(int t)
 
 static int wdtpci_get_status(int *status)
 {
-	unsigned char new_status=inb_p(WDT_SR);
+	unsigned char new_status;
+	unsigned long flags;
+
+	spin_lock_irqsave(&wdtpci_lock, flags);
+	new_status = inb(WDT_SR);
+	spin_unlock_irqrestore(&wdtpci_lock, flags);
 
-	*status=0;
+	*status = 0;
 	if (new_status & WDC_SR_ISOI0)
 		*status |= WDIOF_EXTERN1;
 	if (new_status & WDC_SR_ISII1)
@@ -259,8 +290,12 @@ static int wdtpci_get_status(int *status)
 
 static int wdtpci_get_temperature(int *temperature)
 {
-	unsigned short c=inb_p(WDT_RT);
-
+	unsigned short c;
+	unsigned long flags;
+	spin_lock_irqsave(&wdtpci_lock, flags);
+	c = inb(WDT_RT);
+	udelay(8);
+	spin_unlock_irqrestore(&wdtpci_lock, flags);
 	*temperature = (c * 11 / 15) + 7;
 	return 0;
 }
@@ -282,17 +317,25 @@ static irqreturn_t wdtpci_interrupt(int irq, void *dev_id)
 	 *	Read the status register see what is up and
 	 *	then printk it.
 	 */
-	unsigned char status=inb_p(WDT_SR);
+	unsigned char status;
+
+	spin_lock(&wdtpci_lock);
+
+	status = inb(WDT_SR);
+	udelay(8);
 
 	printk(KERN_CRIT PFX "status %d\n", status);
 
 #ifdef CONFIG_WDT_501_PCI
-	if (!(status & WDC_SR_TGOOD))
- 		printk(KERN_CRIT PFX "Overheat alarm.(%d)\n",inb_p(WDT_RT));
+	if (!(status & WDC_SR_TGOOD)) {
+		u8 alarm = inb(WDT_RT);
+		printk(KERN_CRIT PFX "Overheat alarm.(%d)\n", alarm);
+		udelay(8);
+	}
 	if (!(status & WDC_SR_PSUOVER))
- 		printk(KERN_CRIT PFX "PSU over voltage.\n");
+		printk(KERN_CRIT PFX "PSU over voltage.\n");
 	if (!(status & WDC_SR_PSUUNDR))
- 		printk(KERN_CRIT PFX "PSU under voltage.\n");
+		printk(KERN_CRIT PFX "PSU under voltage.\n");
 	if (tachometer) {
 		if (!(status & WDC_SR_FANGOOD))
 			printk(KERN_CRIT PFX "Possible fan fault.\n");
@@ -310,6 +353,7 @@ static irqreturn_t wdtpci_interrupt(int irq, void *dev_id)
 		printk(KERN_CRIT PFX "Reset in 5ms.\n");
 #endif
 	}
+	spin_unlock(&wdtpci_lock);
 	return IRQ_HANDLED;
 }
 
@@ -325,7 +369,8 @@ static irqreturn_t wdtpci_interrupt(int irq, void *dev_id)
  *	write of data will do, as we we don't define content meaning.
  */
 
-static ssize_t wdtpci_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
+static ssize_t wdtpci_write(struct file *file, const char __user *buf,
+					size_t count, loff_t *ppos)
 {
 	if (count) {
 		if (!nowayout) {
@@ -335,7 +380,7 @@ static ssize_t wdtpci_write(struct file *file, const char __user *buf, size_t co
 
 			for (i = 0; i != count; i++) {
 				char c;
-				if(get_user(c, buf+i))
+				if (get_user(c, buf+i))
 					return -EFAULT;
 				if (c == 'V')
 					expect_close = 42;
@@ -343,13 +388,11 @@ static ssize_t wdtpci_write(struct file *file, const char __user *buf, size_t co
 		}
 		wdtpci_ping();
 	}
-
 	return count;
 }
 
 /**
  *	wdtpci_ioctl:
- *	@inode: inode of the device
  *	@file: file handle to the device
  *	@cmd: watchdog command
  *	@arg: argument pointer
@@ -359,8 +402,8 @@ static ssize_t wdtpci_write(struct file *file, const char __user *buf, size_t co
  *	querying capabilities and current status.
  */
 
-static int wdtpci_ioctl(struct inode *inode, struct file *file, unsigned int cmd,
-	unsigned long arg)
+static long wdtpci_ioctl(struct file *file, unsigned int cmd,
+							unsigned long arg)
 {
 	int new_heartbeat;
 	int status;
@@ -383,33 +426,29 @@ static int wdtpci_ioctl(struct inode *inode, struct file *file, unsigned int cmd
 		ident.options |= WDIOF_FANFAULT;
 #endif /* CONFIG_WDT_501_PCI */
 
-	switch(cmd)
-	{
-		default:
-			return -ENOTTY;
-		case WDIOC_GETSUPPORT:
-			return copy_to_user(argp, &ident, sizeof(ident))?-EFAULT:0;
-
-		case WDIOC_GETSTATUS:
-			wdtpci_get_status(&status);
-			return put_user(status, p);
-		case WDIOC_GETBOOTSTATUS:
-			return put_user(0, p);
-		case WDIOC_KEEPALIVE:
-			wdtpci_ping();
-			return 0;
-		case WDIOC_SETTIMEOUT:
-			if (get_user(new_heartbeat, p))
-				return -EFAULT;
-
-			if (wdtpci_set_heartbeat(new_heartbeat))
-				return -EINVAL;
-
-			wdtpci_ping();
-			/* Fall */
-		case WDIOC_GETTIMEOUT:
-			return put_user(heartbeat, p);
-	}
+	switch (cmd) {
+	default:
+		return -ENOTTY;
+	case WDIOC_GETSUPPORT:
+		return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
+	case WDIOC_GETSTATUS:
+		wdtpci_get_status(&status);
+		return put_user(status, p);
+	case WDIOC_GETBOOTSTATUS:
+		return put_user(0, p);
+	case WDIOC_KEEPALIVE:
+		wdtpci_ping();
+		return 0;
+	case WDIOC_SETTIMEOUT:
+		if (get_user(new_heartbeat, p))
+			return -EFAULT;
+		if (wdtpci_set_heartbeat(new_heartbeat))
+			return -EINVAL;
+		wdtpci_ping();
+		/* Fall */
+	case WDIOC_GETTIMEOUT:
+		return put_user(heartbeat, p);
+		}
 }
 
 /**
@@ -426,12 +465,11 @@ static int wdtpci_ioctl(struct inode *inode, struct file *file, unsigned int cmd
 
 static int wdtpci_open(struct inode *inode, struct file *file)
 {
-	if (down_trylock(&open_sem))
+	if (test_and_set_bit(0, &open_lock))
 		return -EBUSY;
 
-	if (nowayout) {
+	if (nowayout)
 		__module_get(THIS_MODULE);
-	}
 	/*
 	 *	Activate
 	 */
@@ -460,7 +498,7 @@ static int wdtpci_release(struct inode *inode, struct file *file)
 		wdtpci_ping();
 	}
 	expect_close = 0;
-	up(&open_sem);
+	clear_bit(0, &open_lock);
 	return 0;
 }
 
@@ -476,14 +514,15 @@ static int wdtpci_release(struct inode *inode, struct file *file)
  *	fahrenheit. It was designed by an imperial measurement luddite.
  */
 
-static ssize_t wdtpci_temp_read(struct file *file, char __user *buf, size_t count, loff_t *ptr)
+static ssize_t wdtpci_temp_read(struct file *file, char __user *buf,
+						size_t count, loff_t *ptr)
 {
 	int temperature;
 
 	if (wdtpci_get_temperature(&temperature))
 		return -EFAULT;
 
-	if (copy_to_user (buf, &temperature, 1))
+	if (copy_to_user(buf, &temperature, 1))
 		return -EFAULT;
 
 	return 1;
@@ -529,12 +568,10 @@ static int wdtpci_temp_release(struct inode *inode, struct file *file)
  */
 
 static int wdtpci_notify_sys(struct notifier_block *this, unsigned long code,
-	void *unused)
+							void *unused)
 {
-	if (code==SYS_DOWN || code==SYS_HALT) {
-		/* Turn the card off */
+	if (code == SYS_DOWN || code == SYS_HALT)
 		wdtpci_stop();
-	}
 	return NOTIFY_DONE;
 }
 
@@ -547,7 +584,7 @@ static const struct file_operations wdtpci_fops = {
 	.owner		= THIS_MODULE,
 	.llseek		= no_llseek,
 	.write		= wdtpci_write,
-	.ioctl		= wdtpci_ioctl,
+	.unlocked_ioctl	= wdtpci_ioctl,
 	.open		= wdtpci_open,
 	.release	= wdtpci_release,
 };
@@ -584,80 +621,85 @@ static struct notifier_block wdtpci_notifier = {
 };
 
 
-static int __devinit wdtpci_init_one (struct pci_dev *dev,
-				   const struct pci_device_id *ent)
+static int __devinit wdtpci_init_one(struct pci_dev *dev,
+					const struct pci_device_id *ent)
 {
 	int ret = -EIO;
 
 	dev_count++;
 	if (dev_count > 1) {
-		printk (KERN_ERR PFX "this driver only supports 1 device\n");
+		printk(KERN_ERR PFX "This driver only supports one device\n");
 		return -ENODEV;
 	}
 
-	if (pci_enable_device (dev)) {
-		printk (KERN_ERR PFX "Not possible to enable PCI Device\n");
+	if (pci_enable_device(dev)) {
+		printk(KERN_ERR PFX "Not possible to enable PCI Device\n");
 		return -ENODEV;
 	}
 
-	if (pci_resource_start (dev, 2) == 0x0000) {
-		printk (KERN_ERR PFX "No I/O-Address for card detected\n");
+	if (pci_resource_start(dev, 2) == 0x0000) {
+		printk(KERN_ERR PFX "No I/O-Address for card detected\n");
 		ret = -ENODEV;
 		goto out_pci;
 	}
 
-	sema_init(&open_sem, 1);
-
 	irq = dev->irq;
-	io = pci_resource_start (dev, 2);
+	io = pci_resource_start(dev, 2);
 
-	if (request_region (io, 16, "wdt_pci") == NULL) {
-		printk (KERN_ERR PFX "I/O address 0x%04x already in use\n", io);
+	if (request_region(io, 16, "wdt_pci") == NULL) {
+		printk(KERN_ERR PFX "I/O address 0x%04x already in use\n", io);
 		goto out_pci;
 	}
 
-	if (request_irq (irq, wdtpci_interrupt, IRQF_DISABLED | IRQF_SHARED,
+	if (request_irq(irq, wdtpci_interrupt, IRQF_DISABLED | IRQF_SHARED,
 			 "wdt_pci", &wdtpci_miscdev)) {
-		printk (KERN_ERR PFX "IRQ %d is not free\n", irq);
+		printk(KERN_ERR PFX "IRQ %d is not free\n", irq);
 		goto out_reg;
 	}
 
-	printk ("PCI-WDT500/501 (PCI-WDG-CSM) driver 0.10 at 0x%04x (Interrupt %d)\n",
-		io, irq);
+	printk(KERN_INFO
+	 "PCI-WDT500/501 (PCI-WDG-CSM) driver 0.10 at 0x%04x (Interrupt %d)\n",
+								io, irq);
 
-	/* Check that the heartbeat value is within it's range ; if not reset to the default */
+	/* Check that the heartbeat value is within its range;
+	   if not reset to the default */
 	if (wdtpci_set_heartbeat(heartbeat)) {
 		wdtpci_set_heartbeat(WD_TIMO);
-		printk(KERN_INFO PFX "heartbeat value must be 0<heartbeat<65536, using %d\n",
-			WD_TIMO);
+		printk(KERN_INFO PFX
+		  "heartbeat value must be 0 < heartbeat < 65536, using %d\n",
+								WD_TIMO);
 	}
 
-	ret = register_reboot_notifier (&wdtpci_notifier);
+	ret = register_reboot_notifier(&wdtpci_notifier);
 	if (ret) {
-		printk (KERN_ERR PFX "cannot register reboot notifier (err=%d)\n", ret);
+		printk(KERN_ERR PFX
+			"cannot register reboot notifier (err=%d)\n", ret);
 		goto out_irq;
 	}
 
 #ifdef CONFIG_WDT_501_PCI
-	ret = misc_register (&temp_miscdev);
+	ret = misc_register(&temp_miscdev);
 	if (ret) {
-		printk (KERN_ERR PFX "cannot register miscdev on minor=%d (err=%d)\n",
-			TEMP_MINOR, ret);
+		printk(KERN_ERR PFX
+			"cannot register miscdev on minor=%d (err=%d)\n",
+					TEMP_MINOR, ret);
 		goto out_rbt;
 	}
 #endif /* CONFIG_WDT_501_PCI */
 
-	ret = misc_register (&wdtpci_miscdev);
+	ret = misc_register(&wdtpci_miscdev);
 	if (ret) {
-		printk (KERN_ERR PFX "cannot register miscdev on minor=%d (err=%d)\n",
-			WATCHDOG_MINOR, ret);
+		printk(KERN_ERR PFX
+			"cannot register miscdev on minor=%d (err=%d)\n",
+						WATCHDOG_MINOR, ret);
 		goto out_misc;
 	}
 
 	printk(KERN_INFO PFX "initialized. heartbeat=%d sec (nowayout=%d)\n",
 		heartbeat, nowayout);
 #ifdef CONFIG_WDT_501_PCI
-	printk(KERN_INFO "wdt: Fan Tachometer is %s\n", (tachometer ? "Enabled" : "Disabled"));
+	printk(KERN_INFO "wdt: Fan Tachometer is %s\n",
+				(tachometer ? "Enabled" : "Disabled"));
 #endif /* CONFIG_WDT_501_PCI */
 
 	ret = 0;
@@ -673,14 +715,14 @@ out_rbt:
 out_irq:
 	free_irq(irq, &wdtpci_miscdev);
 out_reg:
-	release_region (io, 16);
+	release_region(io, 16);
 out_pci:
 	pci_disable_device(dev);
 	goto out;
 }
 
 
-static void __devexit wdtpci_remove_one (struct pci_dev *pdev)
+static void __devexit wdtpci_remove_one(struct pci_dev *pdev)
 {
 	/* here we assume only one device will ever have
 	 * been picked up and registered by probe function */
@@ -728,7 +770,7 @@ static struct pci_driver wdtpci_driver = {
 
 static void __exit wdtpci_cleanup(void)
 {
-	pci_unregister_driver (&wdtpci_driver);
+	pci_unregister_driver(&wdtpci_driver);
 }
 
 
@@ -742,7 +784,7 @@ static void __exit wdtpci_cleanup(void)
 
 static int __init wdtpci_init(void)
 {
-	return pci_register_driver (&wdtpci_driver);
+	return pci_register_driver(&wdtpci_driver);
 }
 
 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 5/6] alpha: Fix breakage in wdt_pci
  2008-08-04 16:52 [PATCH 0/6] Subset of watchdog fixes Alan Cox
                   ` (3 preceding siblings ...)
  2008-08-04 16:55 ` [PATCH 4/6] wdt: Cleanup and sort out locking and inb_p Alan Cox
@ 2008-08-04 16:56 ` Alan Cox
  2008-08-04 16:56 ` [PATCH 6/6] sc1200: Fix locking, sems and coding style Alan Cox
  5 siblings, 0 replies; 7+ messages in thread
From: Alan Cox @ 2008-08-04 16:56 UTC (permalink / raw)
  To: torvalds, linux-kernel, wim

From: Andrew Morton <akpm@linux-foundation.org>

drivers/watchdog/wdt_pci.c: In function 'wdtpci_ctr_mode':
drivers/watchdog/wdt_pci.c:120: error: implicit declaration of function 'udelay'
{standard input}: Assembler messages:

Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Alan Cox <alan@redhat.com>
---

 drivers/watchdog/wdt_pci.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)


diff --git a/drivers/watchdog/wdt_pci.c b/drivers/watchdog/wdt_pci.c
index 078f37f..5d922fd 100644
--- a/drivers/watchdog/wdt_pci.c
+++ b/drivers/watchdog/wdt_pci.c
@@ -44,6 +44,7 @@
 #include <linux/miscdevice.h>
 #include <linux/watchdog.h>
 #include <linux/ioport.h>
+#include <linux/delay.h>
 #include <linux/notifier.h>
 #include <linux/reboot.h>
 #include <linux/init.h>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 6/6] sc1200: Fix locking, sems and coding style
  2008-08-04 16:52 [PATCH 0/6] Subset of watchdog fixes Alan Cox
                   ` (4 preceding siblings ...)
  2008-08-04 16:56 ` [PATCH 5/6] alpha: Fix breakage in wdt_pci Alan Cox
@ 2008-08-04 16:56 ` Alan Cox
  5 siblings, 0 replies; 7+ messages in thread
From: Alan Cox @ 2008-08-04 16:56 UTC (permalink / raw)
  To: torvalds, linux-kernel, wim

From: Alan Cox <alan@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Alan Cox <alan@redhat.com>
---

 drivers/watchdog/sc1200wdt.c |  205 +++++++++++++++++++++++-------------------
 1 files changed, 113 insertions(+), 92 deletions(-)


diff --git a/drivers/watchdog/sc1200wdt.c b/drivers/watchdog/sc1200wdt.c
index 35cddff..621ebad 100644
--- a/drivers/watchdog/sc1200wdt.c
+++ b/drivers/watchdog/sc1200wdt.c
@@ -15,14 +15,18 @@
  *
  *	Changelog:
  *	20020220 Zwane Mwaikambo	Code based on datasheet, no hardware.
- *	20020221 Zwane Mwaikambo	Cleanups as suggested by Jeff Garzik and Alan Cox.
+ *	20020221 Zwane Mwaikambo	Cleanups as suggested by Jeff Garzik
+ *					and Alan Cox.
  *	20020222 Zwane Mwaikambo	Added probing.
  *	20020225 Zwane Mwaikambo	Added ISAPNP support.
  *	20020412 Rob Radez		Broke out start/stop functions
- *		 <rob@osinvestor.com>	Return proper status instead of temperature warning
- *					Add WDIOC_GETBOOTSTATUS and WDIOC_SETOPTIONS ioctls
+ *		 <rob@osinvestor.com>	Return proper status instead of
+ *					temperature warning
+ *					Add WDIOC_GETBOOTSTATUS and
+ *					WDIOC_SETOPTIONS ioctls
  *					Fix CONFIG_WATCHDOG_NOWAYOUT
- *	20020530 Joel Becker		Add Matt Domsch's nowayout module option
+ *	20020530 Joel Becker		Add Matt Domsch's nowayout module
+ *					option
  *	20030116 Adam Belay		Updated to the latest pnp code
  *
  */
@@ -39,9 +43,8 @@
 #include <linux/pnp.h>
 #include <linux/fs.h>
 #include <linux/semaphore.h>
-
-#include <asm/io.h>
-#include <asm/uaccess.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
 
 #define SC1200_MODULE_VER	"build 20020303"
 #define SC1200_MODULE_NAME	"sc1200wdt"
@@ -72,7 +75,7 @@ static char banner[] __initdata = KERN_INFO PFX SC1200_MODULE_VER;
 static int timeout = 1;
 static int io = -1;
 static int io_len = 2;		/* for non plug and play */
-static struct semaphore open_sem;
+static unsigned long open_flag;
 static char expect_close;
 static DEFINE_SPINLOCK(sc1200wdt_lock);	/* io port access serialisation */
 
@@ -81,7 +84,8 @@ static int isapnp = 1;
 static struct pnp_dev *wdt_dev;
 
 module_param(isapnp, int, 0);
-MODULE_PARM_DESC(isapnp, "When set to 0 driver ISA PnP support will be disabled");
+MODULE_PARM_DESC(isapnp,
+	"When set to 0 driver ISA PnP support will be disabled");
 #endif
 
 module_param(io, int, 0);
@@ -91,26 +95,40 @@ MODULE_PARM_DESC(timeout, "range is 0-255 minutes, default is 1");
 
 static int nowayout = WATCHDOG_NOWAYOUT;
 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) ")");
 
 
 
 /* Read from Data Register */
-static inline void sc1200wdt_read_data(unsigned char index, unsigned char *data)
+static inline void __sc1200wdt_read_data(unsigned char index,
+						unsigned char *data)
 {
-	spin_lock(&sc1200wdt_lock);
 	outb_p(index, PMIR);
 	*data = inb(PMDR);
-	spin_unlock(&sc1200wdt_lock);
 }
 
+static void sc1200wdt_read_data(unsigned char index, unsigned char *data)
+{
+	spin_lock(&sc1200wdt_lock);
+	__sc1200wdt_read_data(index, data);
+	spin_unlock(&sc1200wdt_lock);
+}
 
 /* Write to Data Register */
-static inline void sc1200wdt_write_data(unsigned char index, unsigned char data)
+static inline void __sc1200wdt_write_data(unsigned char index,
+						unsigned char data)
 {
-	spin_lock(&sc1200wdt_lock);
 	outb_p(index, PMIR);
 	outb(data, PMDR);
+}
+
+static inline void sc1200wdt_write_data(unsigned char index,
+						unsigned char data)
+{
+	spin_lock(&sc1200wdt_lock);
+	__sc1200wdt_write_data(index, data);
 	spin_unlock(&sc1200wdt_lock);
 }
 
@@ -118,22 +136,23 @@ static inline void sc1200wdt_write_data(unsigned char index, unsigned char data)
 static void sc1200wdt_start(void)
 {
 	unsigned char reg;
+	spin_lock(&sc1200wdt_lock);
 
-	sc1200wdt_read_data(WDCF, &reg);
+	__sc1200wdt_read_data(WDCF, &reg);
 	/* assert WDO when any of the following interrupts are triggered too */
 	reg |= (KBC_IRQ | MSE_IRQ | UART1_IRQ | UART2_IRQ);
-	sc1200wdt_write_data(WDCF, reg);
+	__sc1200wdt_write_data(WDCF, reg);
 	/* set the timeout and get the ball rolling */
-	sc1200wdt_write_data(WDTO, timeout);
-}
+	__sc1200wdt_write_data(WDTO, timeout);
 
+	spin_unlock(&sc1200wdt_lock);
+}
 
 static void sc1200wdt_stop(void)
 {
 	sc1200wdt_write_data(WDTO, 0);
 }
 
-
 /* This returns the status of the WDO signal, inactive high. */
 static inline int sc1200wdt_status(void)
 {
@@ -144,14 +163,13 @@ static inline int sc1200wdt_status(void)
 	 * KEEPALIVEPING which is a bit of a kludge because there's nothing
 	 * else for enabled/disabled status
 	 */
-	return (ret & 0x01) ? 0 : WDIOF_KEEPALIVEPING;	/* bits 1 - 7 are undefined */
+	return (ret & 0x01) ? 0 : WDIOF_KEEPALIVEPING;
 }
 
-
 static int sc1200wdt_open(struct inode *inode, struct file *file)
 {
 	/* allow one at a time */
-	if (down_trylock(&open_sem))
+	if (test_and_set_bit(0, &open_flag))
 		return -EBUSY;
 
 	if (timeout > MAX_TIMEOUT)
@@ -164,71 +182,71 @@ static int sc1200wdt_open(struct inode *inode, struct file *file)
 }
 
 
-static int sc1200wdt_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
+static long sc1200wdt_ioctl(struct file *file, unsigned int cmd,
+						unsigned long arg)
 {
 	int new_timeout;
 	void __user *argp = (void __user *)arg;
 	int __user *p = argp;
-	static struct watchdog_info ident = {
-		.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE,
+	static const struct watchdog_info ident = {
+		.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT |
+							WDIOF_MAGICCLOSE,
 		.firmware_version = 0,
 		.identity = "PC87307/PC97307",
 	};
 
 	switch (cmd) {
-		default:
-			return -ENOTTY;
-
-		case WDIOC_GETSUPPORT:
-			if (copy_to_user(argp, &ident, sizeof ident))
-				return -EFAULT;
-			return 0;
-
-		case WDIOC_GETSTATUS:
-			return put_user(sc1200wdt_status(), p);
-
-		case WDIOC_GETBOOTSTATUS:
-			return put_user(0, p);
-
-		case WDIOC_KEEPALIVE:
-			sc1200wdt_write_data(WDTO, timeout);
-			return 0;
 
-		case WDIOC_SETTIMEOUT:
-			if (get_user(new_timeout, p))
-				return -EFAULT;
+	case WDIOC_GETSUPPORT:
+		if (copy_to_user(argp, &ident, sizeof ident))
+			return -EFAULT;
+		return 0;
 
-			/* the API states this is given in secs */
-			new_timeout /= 60;
-			if (new_timeout < 0 || new_timeout > MAX_TIMEOUT)
-				return -EINVAL;
+	case WDIOC_GETSTATUS:
+		return put_user(sc1200wdt_status(), p);
 
-			timeout = new_timeout;
-			sc1200wdt_write_data(WDTO, timeout);
-			/* fall through and return the new timeout */
+	case WDIOC_GETBOOTSTATUS:
+		return put_user(0, p);
 
-		case WDIOC_GETTIMEOUT:
-			return put_user(timeout * 60, p);
+	case WDIOC_KEEPALIVE:
+		sc1200wdt_write_data(WDTO, timeout);
+		return 0;
+
+	case WDIOC_SETTIMEOUT:
+		if (get_user(new_timeout, p))
+			return -EFAULT;
+		/* the API states this is given in secs */
+		new_timeout /= 60;
+		if (new_timeout < 0 || new_timeout > MAX_TIMEOUT)
+			return -EINVAL;
+		timeout = new_timeout;
+		sc1200wdt_write_data(WDTO, timeout);
+		/* fall through and return the new timeout */
 
-		case WDIOC_SETOPTIONS:
-		{
-			int options, retval = -EINVAL;
+	case WDIOC_GETTIMEOUT:
+		return put_user(timeout * 60, p);
 
-			if (get_user(options, p))
-				return -EFAULT;
+	case WDIOC_SETOPTIONS:
+	{
+		int options, retval = -EINVAL;
 
-			if (options & WDIOS_DISABLECARD) {
-				sc1200wdt_stop();
-				retval = 0;
-			}
+		if (get_user(options, p))
+			return -EFAULT;
 
-			if (options & WDIOS_ENABLECARD) {
-				sc1200wdt_start();
-				retval = 0;
-			}
+		if (options & WDIOS_DISABLECARD) {
+			sc1200wdt_stop();
+			retval = 0;
+		}
 
-			return retval;
+		if (options & WDIOS_ENABLECARD) {
+			sc1200wdt_start();
+			retval = 0;
 		}
+
+		return retval;
+	}
+	default:
+		return -ENOTTY;
 	}
 }
 
@@ -240,16 +258,18 @@ static int sc1200wdt_release(struct inode *inode, struct file *file)
 		printk(KERN_INFO PFX "Watchdog disabled\n");
 	} else {
 		sc1200wdt_write_data(WDTO, timeout);
-		printk(KERN_CRIT PFX "Unexpected close!, timeout = %d min(s)\n", timeout);
+		printk(KERN_CRIT PFX
+			"Unexpected close!, timeout = %d min(s)\n", timeout);
 	}
-	up(&open_sem);
+	clear_bit(0, &open_flag);
 	expect_close = 0;
 
 	return 0;
 }
 
 
-static ssize_t sc1200wdt_write(struct file *file, const char __user *data, size_t len, loff_t *ppos)
+static ssize_t sc1200wdt_write(struct file *file, const char __user *data,
+						size_t len, loff_t *ppos)
 {
 	if (len) {
 		if (!nowayout) {
@@ -275,7 +295,8 @@ static ssize_t sc1200wdt_write(struct file *file, const char __user *data, size_
 }
 
 
-static int sc1200wdt_notify_sys(struct notifier_block *this, unsigned long code, void *unused)
+static int sc1200wdt_notify_sys(struct notifier_block *this,
+					unsigned long code, void *unused)
 {
 	if (code == SYS_DOWN || code == SYS_HALT)
 		sc1200wdt_stop();
@@ -284,23 +305,20 @@ static int sc1200wdt_notify_sys(struct notifier_block *this, unsigned long code,
 }
 
 
-static struct notifier_block sc1200wdt_notifier =
-{
+static struct notifier_block sc1200wdt_notifier = {
 	.notifier_call =	sc1200wdt_notify_sys,
 };
 
-static const struct file_operations sc1200wdt_fops =
-{
+static const struct file_operations sc1200wdt_fops = {
 	.owner		= THIS_MODULE,
 	.llseek		= no_llseek,
 	.write		= sc1200wdt_write,
-	.ioctl		= sc1200wdt_ioctl,
+	.unlocked_ioctl = sc1200wdt_ioctl,
 	.open		= sc1200wdt_open,
 	.release	= sc1200wdt_release,
 };
 
-static struct miscdevice sc1200wdt_miscdev =
-{
+static struct miscdevice sc1200wdt_miscdev = {
 	.minor		= WATCHDOG_MINOR,
 	.name		= "watchdog",
 	.fops		= &sc1200wdt_fops,
@@ -312,14 +330,14 @@ static int __init sc1200wdt_probe(void)
 	/* The probe works by reading the PMC3 register's default value of 0x0e
 	 * there is one caveat, if the device disables the parallel port or any
 	 * of the UARTs we won't be able to detect it.
-	 * Nb. This could be done with accuracy by reading the SID registers, but
-	 * we don't have access to those io regions.
+	 * NB. This could be done with accuracy by reading the SID registers,
+	 * but we don't have access to those io regions.
 	 */
 
 	unsigned char reg;
 
 	sc1200wdt_read_data(PMC3, &reg);
-	reg &= 0x0f;				/* we don't want the UART busy bits */
+	reg &= 0x0f;		/* we don't want the UART busy bits */
 	return (reg == 0x0e) ? 0 : -ENODEV;
 }
 
@@ -332,7 +350,8 @@ static struct pnp_device_id scl200wdt_pnp_devices[] = {
 	{.id = ""},
 };
 
-static int scl200wdt_pnp_probe(struct pnp_dev * dev, const struct pnp_device_id *dev_id)
+static int scl200wdt_pnp_probe(struct pnp_dev *dev,
+					const struct pnp_device_id *dev_id)
 {
 	/* this driver only supports one card at a time */
 	if (wdt_dev || !isapnp)
@@ -347,13 +366,14 @@ static int scl200wdt_pnp_probe(struct pnp_dev * dev, const struct pnp_device_id
 		return -EBUSY;
 	}
 
-	printk(KERN_INFO "scl200wdt: PnP device found at io port %#x/%d\n", io, io_len);
+	printk(KERN_INFO "scl200wdt: PnP device found at io port %#x/%d\n",
+								io, io_len);
 	return 0;
 }
 
-static void scl200wdt_pnp_remove(struct pnp_dev * dev)
+static void scl200wdt_pnp_remove(struct pnp_dev *dev)
 {
-	if (wdt_dev){
+	if (wdt_dev) {
 		release_region(io, io_len);
 		wdt_dev = NULL;
 	}
@@ -375,8 +395,6 @@ static int __init sc1200wdt_init(void)
 
 	printk("%s\n", banner);
 
-	sema_init(&open_sem, 1);
-
 #if defined CONFIG_PNP
 	if (isapnp) {
 		ret = pnp_register_driver(&scl200wdt_pnp_driver);
@@ -410,13 +428,16 @@ static int __init sc1200wdt_init(void)
 
 	ret = register_reboot_notifier(&sc1200wdt_notifier);
 	if (ret) {
-		printk(KERN_ERR PFX "Unable to register reboot notifier err = %d\n", ret);
+		printk(KERN_ERR PFX
+			"Unable to register reboot notifier err = %d\n", ret);
 		goto out_io;
 	}
 
 	ret = misc_register(&sc1200wdt_miscdev);
 	if (ret) {
-		printk(KERN_ERR PFX "Unable to register miscdev on minor %d\n", WATCHDOG_MINOR);
+		printk(KERN_ERR PFX
+			"Unable to register miscdev on minor %d\n",
+							WATCHDOG_MINOR);
 		goto out_rbt;
 	}
 
@@ -446,7 +467,7 @@ static void __exit sc1200wdt_exit(void)
 	unregister_reboot_notifier(&sc1200wdt_notifier);
 
 #if defined CONFIG_PNP
-	if(isapnp)
+	if (isapnp)
 		pnp_unregister_driver(&scl200wdt_pnp_driver);
 	else
 #endif


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2008-08-04 17:14 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-08-04 16:52 [PATCH 0/6] Subset of watchdog fixes Alan Cox
2008-08-04 16:53 ` [PATCH 1/6] Fix locking Alan Cox
2008-08-04 16:54 ` [PATCH 2/6] it8712f_wdt: Locking and coding style Alan Cox
2008-08-04 16:54 ` [PATCH 3/6] " Alan Cox
2008-08-04 16:55 ` [PATCH 4/6] wdt: Cleanup and sort out locking and inb_p Alan Cox
2008-08-04 16:56 ` [PATCH 5/6] alpha: Fix breakage in wdt_pci Alan Cox
2008-08-04 16:56 ` [PATCH 6/6] sc1200: Fix locking, sems and coding style Alan Cox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome