2011-05-03 05:36:29

by Jimmy Chen (陳永達)

[permalink] [raw]
Subject: RE: [PATCH 1/2] watchdog: add support for MOXA V2100 watchdog driver

From: Jimmy Chen <[email protected]>

Add selection and help in menu to support watchdog driver

Signed-off-by: Jimmy Chen <[email protected]>
---
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 1b0f98b..39a6c65 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -384,6 +384,18 @@ config ADVANTECH_WDT
feature. More information can be found at
<http://www.advantech.com.tw/products/>

+config MOXA_WDT
+ tristate "MOXA V2100 WATCHDOG driver"
+ depends on X86
+ default n
+ help
+ If you say yes here you get support for the MOXA V2100 watchdog
+ driver.
+
+ This driver can also be built as a module. If so, the module
+ will be called moxa_swtd. More information can be found at
+ <http://www.moxa.com>
+
config ALIM1535_WDT
tristate "ALi M1535 PMU Watchdog Timer"
depends on X86 && PCI
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 3f8608b..da1200e 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -65,6 +65,7 @@ obj-$(CONFIG_BFIN_WDT) += bfin_wdt.o
# X86 (i386 + ia64 + x86_64) Architecture
obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
obj-$(CONFIG_ADVANTECH_WDT) += advantechwdt.o
+obj-$(CONFIG_MOXA_WDT) += moxa_wdt.o
obj-$(CONFIG_ALIM1535_WDT) += alim1535_wdt.o
obj-$(CONFIG_ALIM7101_WDT) += alim7101_wdt.o
obj-$(CONFIG_F71808E_WDT) += f71808e_wdt.o


2011-05-03 05:39:40

by Jimmy Chen (陳永達)

[permalink] [raw]
Subject: RE: [PATCH 2/2] watchdog: add support for MOXA V2100 watchdog driver

From: Jimmy Chen <[email protected]>

Add real function for watchdog driver

Signed-off-by: Jimmy Chen <[email protected]>
---
diff --git a/drivers/watchdog/moxa_wdt.c b/drivers/watchdog/moxa_wdt.c
new file mode 100644
index 0000000..1238e56
--- /dev/null
+++ b/drivers/watchdog/moxa_wdt.c
@@ -0,0 +1,387 @@
+/*
+ * serial driver for the MOXA V2100 platform.
+ *
+ * Copyright (c) MOXA Inc. All rights reserved.
+ * Jimmy Chen <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * History:
+ * Date Author Comment
+ * 04-29-2011 Jimmy Chen. copy wdt.c code
+ */
+
+#define __KERNEL_SYSCALLS__
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#include <linux/fs.h>
+#include <linux/ioport.h>
+#include <linux/notifier.h>
+#include <linux/reboot.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
+
+#include <asm/system.h>
+
+#include "moxa_wdt.h"
+
+#define MOXA_WDT_VERSION "v0.1.0"
+#define HW_VENDOR_ID_H 0x87
+#define HW_VENDOR_ID_L 0x83
+
+
+static unsigned long wdt_is_open;
+static char expect_close;
+
+static int timeout = DEFAULT_WATCHDOG_TIMEOUT;
+module_param(timeout, int, 0);
+MODULE_PARM_DESC(timeout,
+ "Watchdog timeout in seconds. 1<= timeout <=63, default="
+ __MODULE_STRING(WATCHDOG_TIMEOUT) ".");
+
+static int nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, int, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be "
+ "stopped once started (default=CONFIG_WATCHDOG_NOWAYOUT)");
+
+static int debug;
+module_param(debug, int, 0);
+MODULE_PARM_DESC(debug, "print the debug message in this driver");
+
+static spinlock_t wdt_lock = SPIN_LOCK_UNLOCKED;
+
+/**
+ * wdt_start:
+ *
+ * Start the watchdog driver.
+ */
+
+static int moxawdt_start(void)
+{
+ unsigned long flags;
+ unsigned char val;
+
+ if (debug)
+ printk(KERN_DEBUG "wdt_start: timeout=%d\n", timeout);
+
+ spin_lock_irqsave(&wdt_lock, flags);
+ superio_init();
+ superio_select_dev(7);
+ val = superio_get_reg(0x72) | 0x10;
+ superio_set_reg(val, 0x72);
+ superio_set_reg((timeout/1000), 0x73);
+ spin_unlock_irqrestore(&wdt_lock, flags);
+ return 0;
+}
+
+/**
+ * wdt_stop:
+ *
+ * Stop the watchdog driver.
+ */
+
+static int wdt_stop(void)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&wdt_lock, flags);
+
+ if (debug)
+ printk(KERN_DEBUG "wdt_disable\n");
+ superio_init();
+ superio_select_dev(7); /* logic device 8 */
+ superio_set_reg(0, 0x73); /* Reg:F6 counter register */
+ spin_unlock_irqrestore(&wdt_lock, flags);
+ return 0;
+}
+
+/**
+ * wdt_ping:
+ *
+ * Reload counter one with the watchdog heartbeat. We don't bother
+ * reloading the cascade counter.
+ */
+
+static void wdt_ping(void)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&wdt_lock, flags);
+
+ if (debug)
+ printk(KERN_DEBUG "wdt_ping: timeout=%d\n", timeout);
+ superio_init();
+ superio_select_dev(7); /* logic device 7 */
+ superio_set_reg((timeout/1000), 0x73); /* Reg:F6,30 sec */
+ spin_unlock_irqrestore(&wdt_lock, flags);
+}
+
+/**
+ * wdt_verify_vendor:
+ * return true if vendor ID match
+ */
+
+static int wdt_verify_vendor(void)
+{
+ unsigned char chip_id_h; /* chip id high byte */
+ unsigned char chip_id_l; /* chip id low byte */
+
+ superio_init();
+ superio_select_dev(7);
+ chip_id_h = superio_get_reg(0x20);
+ chip_id_l = superio_get_reg(0x21);
+ if ((chip_id_h == 0x87) && (chip_id_l == 0x83))
+ return 0;
+
+ return -1;
+}
+
+/**
+ * wdt_set_timeout:
+ * @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
+ * successful we return 0.
+ */
+
+static int wdt_set_timeout(int *t)
+{
+ if (*t < WATCHDOG_MIN_TIMEOUT || *t > WATCHDOG_MAX_TIMEOUT) {
+ *t = DEFAULT_WATCHDOG_TIMEOUT;
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int moxawdt_open(struct inode *inode, struct file *file)
+{
+
+ if (test_and_set_bit(0, &wdt_is_open))
+ return -EBUSY;
+ /*
+ * Activate
+ */
+ if (debug)
+ printk(KERN_DEBUG "moxawdt_open entry\n");
+ moxawdt_start();
+ return nonseekable_open(inode, file);
+
+ return 0;
+}
+
+static int moxawdt_release(struct inode *inode, struct file *file)
+{
+ if (debug)
+ printk(KERN_DEBUG "moxawdt_release entry\n");
+
+ if (expect_close == 42) {
+ wdt_stop();
+ clear_bit(0, &wdt_is_open);
+ } else {
+ printk(KERN_CRIT
+ "wdt: WDT device closed unexpectedly. WDT will not stop!\n");
+ wdt_ping();
+ }
+ expect_close = 0;
+
+ return 0;
+}
+
+static long moxawdt_ioctl(struct file *file,
+ unsigned int cmd, unsigned long arg)
+{
+ void __user *argp = (void __user *)arg;
+ int __user *p = argp;
+ int new_timeout;
+ int status;
+
+ static struct watchdog_info ident = {
+ .options = WDIOF_SETTIMEOUT|
+ WDIOF_MAGICCLOSE|
+ WDIOF_KEEPALIVEPING,
+ .firmware_version = 1,
+ .identity = "MOXA2100WDT ",
+ };
+
+ switch (cmd) {
+ case WDIOC_GETSUPPORT:
+ return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
+ case WDIOC_GETSTATUS:
+ status = 1;
+ 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_timeout, p))
+ return -EFAULT;
+ if (wdt_set_timeout(&new_timeout))
+ return -EINVAL;
+ wdt_ping();
+ /* Fall */
+ case WDIOC_GETTIMEOUT:
+ return put_user(timeout, p);
+ default:
+ return -ENOTTY;
+ }
+ return 0;
+}
+
+/*
+ * moxawdt_write:
+ * @file: file handle to the watchdog
+ * @buf: buffer to write (unused as data does not matter here
+ * @count: count of bytes
+ * @ppos: pointer to the position to write. No seeks allowed
+ *
+ * A write to a watchdog device is defined as a keepalive signal. Any
+ * write of data will do, as we we don't define content meaning.
+ */
+
+static ssize_t moxawdt_write(struct file *file, const char *buf, \
+ size_t count, loff_t *ppos)
+{
+ if (count) {
+ if (!nowayout) {
+ size_t i;
+
+ /* In case it was set long ago */
+ for (i = 0; i != count; i++) {
+ char c;
+ if (get_user(c, buf + i))
+ return -EFAULT;
+ if (c == 'V')
+ expect_close = 42;
+ }
+ }
+
+ }
+ return count;
+}
+
+/**
+ * notify_sys:
+ * @this: our notifier block
+ * @code: the event being reported
+ * @unused: unused
+ *
+ * Our notifier is called on system shutdowns. We want to turn the card
+ * off at reboot otherwise the machine will reboot again during memory
+ * test or worse yet during the following fsck. This would suck, in fact
+ * trust me - if it happens it does suck.
+ */
+
+static int wdt_notify_sys(struct notifier_block *this, unsigned long code,
+ void *unused)
+{
+ if (code == SYS_DOWN || code == SYS_HALT)
+ wdt_stop();
+ return NOTIFY_DONE;
+}
+
+/*
+ * The WDT card needs to learn about soft shutdowns in order to
+ * turn the timebomb registers off.
+ */
+
+static struct notifier_block wdt_notifier = {
+ .notifier_call = wdt_notify_sys,
+};
+
+static const struct file_operations moxawdt_fops = {
+ .owner = THIS_MODULE,
+ .llseek = no_llseek,
+ .open = moxawdt_open,
+ .write = moxawdt_write,
+ .unlocked_ioctl = moxawdt_ioctl,
+ .release = moxawdt_release,
+};
+
+static struct miscdevice wdt_miscdev = {
+ .minor = WATCHDOG_MINOR,
+ .name = "watchdog",
+ .fops = &moxawdt_fops,
+};
+
+static int __init moxawdt_init(void)
+{
+ int ret;
+
+ /* we suppose to check magic id in case wrong devices */
+ if (wdt_verify_vendor()) {
+ printk(KERN_ERR "hw device id not match!!\n");
+ ret = -ENODEV;
+ goto reqreg_err;
+ }
+
+ /* check timeout valid */
+ if (wdt_set_timeout(&timeout)) {
+ printk(KERN_ERR "timeout value must be "
+ "%lu < timeout < %lu, using %d\n",
+ WATCHDOG_MIN_TIMEOUT, WATCHDOG_MAX_TIMEOUT,
+ timeout);
+ }
+
+ if (!request_region(SUPERIO_PORT, 2, "moxawdt")) {
+ printk(KERN_ERR "moxawdt_init: can't get I/O "
+ "address 0x%x\n", SUPERIO_PORT);
+ ret = -EBUSY;
+ goto reqreg_err;
+ }
+
+ ret = register_reboot_notifier(&wdt_notifier);
+ if (ret) {
+ printk(KERN_ERR "can't register reboot notifier\n");
+ goto regreb_err;
+ }
+
+ ret = misc_register(&wdt_miscdev);
+ if (ret) {
+ printk(KERN_ERR "Moxa V2100-LX WatchDog: Register misc fail !\n");
+ goto regmisc_err;
+ }
+
+ printk(KERN_INFO "Moxa V2100 Watchdog Driver,
+ version %s\n", MOXA_WDT_VERSION);
+ printk(KERN_INFO "initialized. (nowayout=%d)\n", nowayout);
+ printk(KERN_INFO "initialized. (timeout=%d)\n", timeout);
+ printk(KERN_INFO "initialized. (debug=%d)\n", debug);
+
+ return 0;
+
+regmisc_err:
+ unregister_reboot_notifier(&wdt_notifier);
+regreb_err:
+ release_region(SUPERIO_PORT, 2);
+reqreg_err:
+ return ret;
+}
+
+static void __exit moxawdt_exit(void)
+{
+ misc_deregister(&wdt_miscdev);
+ unregister_reboot_notifier(&wdt_notifier);
+ release_region(SUPERIO_PORT, 2);
+ superio_release();
+}
+
+module_init(moxawdt_init);
+module_exit(moxawdt_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("[email protected]");
+MODULE_DESCRIPTION("Moxa V2100-LX WDT driver");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
+
diff --git a/drivers/watchdog/moxa_wdt.h b/drivers/watchdog/moxa_wdt.h
new file mode 100644
index 0000000..3f3ff68
--- /dev/null
+++ b/drivers/watchdog/moxa_wdt.h
@@ -0,0 +1,53 @@
+/*
+ * serial driver for the MOXA V2100 platform.
+ *
+ * Copyright (c) MOXA Inc. All rights reserved.
+ * Jimmy Chen <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __X86__MOXAWDT__
+#define __X86__MOXAWDT__
+
+#define SUPERIO_PORT ((u8)0x2e)
+
+#define DEFAULT_WATCHDOG_TIMEOUT (30UL*1000UL) /* 30 seconds */
+#define WATCHDOG_MIN_TIMEOUT (1UL*1000UL) /* 2 seconds */
+#define WATCHDOG_MAX_TIMEOUT (255UL*1000UL) /* 255 seconds */
+
+unsigned char superio_get_reg(u8 val)
+{
+ outb_p(val, SUPERIO_PORT);
+ val = inb_p(SUPERIO_PORT+1);
+ return val;
+}
+
+void superio_set_reg(u8 val, u8 index)
+{
+ outb_p(index, SUPERIO_PORT);
+ outb_p(val, (SUPERIO_PORT+1));
+}
+
+void superio_select_dev(u8 val)
+{
+ superio_set_reg(val, 0x07);
+}
+
+void superio_init(void)
+{
+ outb(0x87, SUPERIO_PORT);
+ outb(0x01, SUPERIO_PORT);
+ outb(0x55, SUPERIO_PORT);
+ outb(0x55, SUPERIO_PORT);
+}
+
+void superio_release(void)
+{
+ outb_p(0x02, SUPERIO_PORT);
+ outb_p(0x02, SUPERIO_PORT+1);
+}
+
+#endif /* __X86__MOXAWDT__ */

2011-05-03 09:36:04

by Alan

[permalink] [raw]
Subject: Re: [PATCH 2/2] watchdog: add support for MOXA V2100 watchdog driver

> Add real function for watchdog driver

Looks a lot better

> +#define __KERNEL_SYSCALLS__
> +

You shouldn't need that define now


> + /* we suppose to check magic id in case wrong devices */
> + if (wdt_verify_vendor()) {
> + printk(KERN_ERR "hw device id not match!!\n");
> + ret = -ENODEV;
> + goto reqreg_err;
> + }

This wants to happen after the request_region - if the region is busy
then you know your device isn't present but that the addresses in
question are currently in use so you shouldn't touch them

We tend to use pr_err()/pr_debug() etc now rather than printk(KERN_ERR
"Oops"); to be less long winded - but the code you have there isn't wrong
in using printk().

With the tests re-ordered this looks fairly sound to me

Alan

2011-05-03 10:25:55

by Jimmy Chen (陳永達)

[permalink] [raw]
Subject: RE: [PATCH 2/2] watchdog: add support for MOXA V2100 watchdog driver

From: Jimmy Chen <[email protected]>

-Add real function for watchdog driver
-Follow advices from Alan Cox

Signed-off-by: Jimmy Chen <[email protected]>
---
diff --git a/drivers/watchdog/moxa_wdt.c b/drivers/watchdog/moxa_wdt.c
new file mode 100644
index 0000000..bcd8164
--- /dev/null
+++ b/drivers/watchdog/moxa_wdt.c
@@ -0,0 +1,385 @@
+/*
+ * serial driver for the MOXA V2100 platform.
+ *
+ * Copyright (c) MOXA Inc. All rights reserved.
+ * Jimmy Chen <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * History:
+ * Date Author Comment
+ * 04-29-2011 Jimmy Chen. copy wdt.c code
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME":"fmt
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#include <linux/fs.h>
+#include <linux/ioport.h>
+#include <linux/notifier.h>
+#include <linux/reboot.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
+
+#include <asm/system.h>
+
+#include "moxa_wdt.h"
+
+#define MOXA_WDT_VERSION "v0.1.0"
+#define HW_VENDOR_ID_H 0x87
+#define HW_VENDOR_ID_L 0x83
+
+
+static unsigned long wdt_is_open;
+static char expect_close;
+
+static int timeout = DEFAULT_WATCHDOG_TIMEOUT;
+module_param(timeout, int, 0);
+MODULE_PARM_DESC(timeout,
+ "Watchdog timeout in seconds. 1<= timeout <=63, default="
+ __MODULE_STRING(WATCHDOG_TIMEOUT) ".");
+
+static int nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, int, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be "
+ "stopped once started (default=CONFIG_WATCHDOG_NOWAYOUT)");
+
+static int debug;
+module_param(debug, int, 0);
+MODULE_PARM_DESC(debug, "print the debug message in this driver");
+
+static spinlock_t wdt_lock = SPIN_LOCK_UNLOCKED;
+
+/**
+ * wdt_start:
+ *
+ * Start the watchdog driver.
+ */
+
+static int moxawdt_start(void)
+{
+ unsigned long flags;
+ unsigned char val;
+
+ if (debug)
+ pr_debug("wdt_start: timeout=%d\n", timeout);
+
+ spin_lock_irqsave(&wdt_lock, flags);
+ superio_init();
+ superio_select_dev(7);
+ val = superio_get_reg(0x72) | 0x10;
+ superio_set_reg(val, 0x72);
+ superio_set_reg((timeout/1000), 0x73);
+ spin_unlock_irqrestore(&wdt_lock, flags);
+ return 0;
+}
+
+/**
+ * wdt_stop:
+ *
+ * Stop the watchdog driver.
+ */
+
+static int wdt_stop(void)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&wdt_lock, flags);
+
+ if (debug)
+ pr_debug("wdt_disable\n");
+ superio_init();
+ superio_select_dev(7); /* logic device 8 */
+ superio_set_reg(0, 0x73); /* Reg:F6 counter register */
+ spin_unlock_irqrestore(&wdt_lock, flags);
+ return 0;
+}
+
+/**
+ * wdt_ping:
+ *
+ * Reload counter one with the watchdog heartbeat. We don't bother
+ * reloading the cascade counter.
+ */
+
+static void wdt_ping(void)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&wdt_lock, flags);
+
+ if (debug)
+ pr_debug("wdt_ping: timeout=%d\n", timeout);
+ superio_init();
+ superio_select_dev(7); /* logic device 7 */
+ superio_set_reg((timeout/1000), 0x73); /* Reg:F6,30 sec */
+ spin_unlock_irqrestore(&wdt_lock, flags);
+}
+
+/**
+ * wdt_verify_vendor:
+ * return true if vendor ID match
+ */
+
+static int wdt_verify_vendor(void)
+{
+ unsigned char chip_id_h; /* chip id high byte */
+ unsigned char chip_id_l; /* chip id low byte */
+
+ superio_init();
+ superio_select_dev(7);
+ chip_id_h = superio_get_reg(0x20);
+ chip_id_l = superio_get_reg(0x21);
+ if ((chip_id_h == HW_VENDOR_ID_H) && (chip_id_l == HW_VENDOR_ID_L))
+ return 0;
+
+ return -1;
+}
+
+/**
+ * wdt_set_timeout:
+ * @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
+ * successful we return 0.
+ */
+
+static int wdt_set_timeout(int *t)
+{
+ if (*t < WATCHDOG_MIN_TIMEOUT || *t > WATCHDOG_MAX_TIMEOUT) {
+ *t = DEFAULT_WATCHDOG_TIMEOUT;
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int moxawdt_open(struct inode *inode, struct file *file)
+{
+
+ if (test_and_set_bit(0, &wdt_is_open))
+ return -EBUSY;
+ /*
+ * Activate
+ */
+ if (debug)
+ pr_debug("moxawdt_open entry\n");
+ moxawdt_start();
+ return nonseekable_open(inode, file);
+
+ return 0;
+}
+
+static int moxawdt_release(struct inode *inode, struct file *file)
+{
+ if (debug)
+ pr_debug("moxawdt_release entry\n");
+
+ if (expect_close == 42) {
+ wdt_stop();
+ clear_bit(0, &wdt_is_open);
+ } else {
+ pr_crit("wdt: WDT device closed unexpectedly. WDT will not stop!\n");
+ wdt_ping();
+ }
+ expect_close = 0;
+
+ return 0;
+}
+
+static long moxawdt_ioctl(struct file *file,
+ unsigned int cmd, unsigned long arg)
+{
+ void __user *argp = (void __user *)arg;
+ int __user *p = argp;
+ int new_timeout;
+ int status;
+
+ static struct watchdog_info ident = {
+ .options = WDIOF_SETTIMEOUT|
+ WDIOF_MAGICCLOSE|
+ WDIOF_KEEPALIVEPING,
+ .firmware_version = 1,
+ .identity = "MOXA2100WDT ",
+ };
+
+ switch (cmd) {
+ case WDIOC_GETSUPPORT:
+ return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
+ case WDIOC_GETSTATUS:
+ status = 1;
+ 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_timeout, p))
+ return -EFAULT;
+ if (wdt_set_timeout(&new_timeout))
+ return -EINVAL;
+ wdt_ping();
+ /* Fall */
+ case WDIOC_GETTIMEOUT:
+ return put_user(timeout, p);
+ default:
+ return -ENOTTY;
+ }
+ return 0;
+}
+
+/*
+ * moxawdt_write:
+ * @file: file handle to the watchdog
+ * @buf: buffer to write (unused as data does not matter here
+ * @count: count of bytes
+ * @ppos: pointer to the position to write. No seeks allowed
+ *
+ * A write to a watchdog device is defined as a keepalive signal. Any
+ * write of data will do, as we we don't define content meaning.
+ */
+
+static ssize_t moxawdt_write(struct file *file, const char *buf, \
+ size_t count, loff_t *ppos)
+{
+ if (count) {
+ if (!nowayout) {
+ size_t i;
+
+ /* In case it was set long ago */
+ for (i = 0; i != count; i++) {
+ char c;
+ if (get_user(c, buf + i))
+ return -EFAULT;
+ if (c == 'V')
+ expect_close = 42;
+ }
+ }
+
+ }
+ return count;
+}
+
+/**
+ * notify_sys:
+ * @this: our notifier block
+ * @code: the event being reported
+ * @unused: unused
+ *
+ * Our notifier is called on system shutdowns. We want to turn the card
+ * off at reboot otherwise the machine will reboot again during memory
+ * test or worse yet during the following fsck. This would suck, in fact
+ * trust me - if it happens it does suck.
+ */
+
+static int wdt_notify_sys(struct notifier_block *this, unsigned long code,
+ void *unused)
+{
+ if (code == SYS_DOWN || code == SYS_HALT)
+ wdt_stop();
+ return NOTIFY_DONE;
+}
+
+/*
+ * The WDT card needs to learn about soft shutdowns in order to
+ * turn the timebomb registers off.
+ */
+
+static struct notifier_block wdt_notifier = {
+ .notifier_call = wdt_notify_sys,
+};
+
+static const struct file_operations moxawdt_fops = {
+ .owner = THIS_MODULE,
+ .llseek = no_llseek,
+ .open = moxawdt_open,
+ .write = moxawdt_write,
+ .unlocked_ioctl = moxawdt_ioctl,
+ .release = moxawdt_release,
+};
+
+static struct miscdevice wdt_miscdev = {
+ .minor = WATCHDOG_MINOR,
+ .name = "watchdog",
+ .fops = &moxawdt_fops,
+};
+
+static int __init moxawdt_init(void)
+{
+ int ret;
+
+ /* check timeout valid */
+ if (wdt_set_timeout(&timeout)) {
+ pr_err("timeout value must be "
+ "%lu < timeout < %lu, using %d\n",
+ WATCHDOG_MIN_TIMEOUT, WATCHDOG_MAX_TIMEOUT,
+ timeout);
+ }
+
+ if (!request_region(SUPERIO_PORT, 2, "moxawdt")) {
+ pr_err("moxawdt_init: can't get I/O "
+ "address 0x%x\n", SUPERIO_PORT);
+ ret = -EBUSY;
+ goto reqreg_err;
+ }
+
+ /* we suppose to check magic id in case wrong devices */
+ if (wdt_verify_vendor()) {
+ pr_err("hw device id not match!!\n");
+ ret = -ENODEV;
+ goto reqreg_err;
+ }
+
+ ret = register_reboot_notifier(&wdt_notifier);
+ if (ret) {
+ pr_err("can't register reboot notifier\n");
+ goto regreb_err;
+ }
+
+ ret = misc_register(&wdt_miscdev);
+ if (ret) {
+ pr_err("Moxa V2100-LX WatchDog: Register misc fail !\n");
+ goto regmisc_err;
+ }
+
+ pr_info("Moxa V2100 Watchdog Driver, version %s\n", MOXA_WDT_VERSION);
+ pr_info("initialized. (nowayout=%d)\n", nowayout);
+ pr_info("initialized. (timeout=%d)\n", timeout);
+ pr_info("initialized. (debug=%d)\n", debug);
+
+ return 0;
+
+regmisc_err:
+ unregister_reboot_notifier(&wdt_notifier);
+regreb_err:
+ release_region(SUPERIO_PORT, 2);
+reqreg_err:
+ return ret;
+}
+
+static void __exit moxawdt_exit(void)
+{
+ misc_deregister(&wdt_miscdev);
+ unregister_reboot_notifier(&wdt_notifier);
+ release_region(SUPERIO_PORT, 2);
+ superio_release();
+}
+
+module_init(moxawdt_init);
+module_exit(moxawdt_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("[email protected]");
+MODULE_DESCRIPTION("Moxa V2100-LX WDT driver");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
+
diff --git a/drivers/watchdog/moxa_wdt.h b/drivers/watchdog/moxa_wdt.h
new file mode 100644
index 0000000..3f3ff68
--- /dev/null
+++ b/drivers/watchdog/moxa_wdt.h
@@ -0,0 +1,53 @@
+/*
+ * serial driver for the MOXA V2100 platform.
+ *
+ * Copyright (c) MOXA Inc. All rights reserved.
+ * Jimmy Chen <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __X86__MOXAWDT__
+#define __X86__MOXAWDT__
+
+#define SUPERIO_PORT ((u8)0x2e)
+
+#define DEFAULT_WATCHDOG_TIMEOUT (30UL*1000UL) /* 30 seconds */
+#define WATCHDOG_MIN_TIMEOUT (1UL*1000UL) /* 2 seconds */
+#define WATCHDOG_MAX_TIMEOUT (255UL*1000UL) /* 255 seconds */
+
+unsigned char superio_get_reg(u8 val)
+{
+ outb_p(val, SUPERIO_PORT);
+ val = inb_p(SUPERIO_PORT+1);
+ return val;
+}
+
+void superio_set_reg(u8 val, u8 index)
+{
+ outb_p(index, SUPERIO_PORT);
+ outb_p(val, (SUPERIO_PORT+1));
+}
+
+void superio_select_dev(u8 val)
+{
+ superio_set_reg(val, 0x07);
+}
+
+void superio_init(void)
+{
+ outb(0x87, SUPERIO_PORT);
+ outb(0x01, SUPERIO_PORT);
+ outb(0x55, SUPERIO_PORT);
+ outb(0x55, SUPERIO_PORT);
+}
+
+void superio_release(void)
+{
+ outb_p(0x02, SUPERIO_PORT);
+ outb_p(0x02, SUPERIO_PORT+1);
+}
+
+#endif /* __X86__MOXAWDT__ */

2011-05-03 10:28:41

by Alan

[permalink] [raw]
Subject: Re: [PATCH 2/2] watchdog: add support for MOXA V2100 watchdog driver

On Tue, 3 May 2011 18:25:50 +0800
Jimmy Chen (???ùF) <[email protected]> wrote:

> From: Jimmy Chen <[email protected]>
>
> -Add real function for watchdog driver
> -Follow advices from Alan Cox
>
> Signed-off-by: Jimmy Chen <[email protected]>

Acked-by: Alan Cox <[email protected]>

2011-05-03 10:57:45

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH 2/2] watchdog: add support for MOXA V2100 watchdog driver

Hi,

On Tue, May 03, 2011 at 06:25:50PM +0800, Jimmy Chen (陳永達) wrote:
> From: Jimmy Chen <[email protected]>
>
> -Add real function for watchdog driver
> -Follow advices from Alan Cox
>
> Signed-off-by: Jimmy Chen <[email protected]>
> ---
> diff --git a/drivers/watchdog/moxa_wdt.c b/drivers/watchdog/moxa_wdt.c
> new file mode 100644
> index 0000000..bcd8164
> --- /dev/null
> +++ b/drivers/watchdog/moxa_wdt.c
> @@ -0,0 +1,385 @@
> +/*
> + * serial driver for the MOXA V2100 platform.
> + *
> + * Copyright (c) MOXA Inc. All rights reserved.
> + * Jimmy Chen <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * History:
> + * Date Author Comment
> + * 04-29-2011 Jimmy Chen. copy wdt.c code

Not needed, we have the git-repo-history.

> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME":"fmt
> +
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/moduleparam.h>
> +#include <linux/types.h>
> +#include <linux/miscdevice.h>
> +#include <linux/watchdog.h>
> +#include <linux/fs.h>
> +#include <linux/ioport.h>
> +#include <linux/notifier.h>
> +#include <linux/reboot.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/uaccess.h>
> +
> +#include <asm/system.h>
> +
> +#include "moxa_wdt.h"
> +
> +#define MOXA_WDT_VERSION "v0.1.0"

Not needed, we have the git-repo-history.

> +#define HW_VENDOR_ID_H 0x87
> +#define HW_VENDOR_ID_L 0x83
> +
> +
> +static unsigned long wdt_is_open;
> +static char expect_close;
> +
> +static int timeout = DEFAULT_WATCHDOG_TIMEOUT;
> +module_param(timeout, int, 0);
> +MODULE_PARM_DESC(timeout,
> + "Watchdog timeout in seconds. 1<= timeout <=63, default="
> + __MODULE_STRING(WATCHDOG_TIMEOUT) ".");
> +
> +static int nowayout = WATCHDOG_NOWAYOUT;
> +module_param(nowayout, int, 0);
> +MODULE_PARM_DESC(nowayout, "Watchdog cannot be "
> + "stopped once started (default=CONFIG_WATCHDOG_NOWAYOUT)");
> +
> +static int debug;
> +module_param(debug, int, 0);
> +MODULE_PARM_DESC(debug, "print the debug message in this driver");

Can go, there is a dynamic_debug Kconfig option.

> +
> +static spinlock_t wdt_lock = SPIN_LOCK_UNLOCKED;
> +
> +/**
> + * wdt_start:
> + *
> + * Start the watchdog driver.
> + */
> +
> +static int moxawdt_start(void)
> +{
> + unsigned long flags;
> + unsigned char val;
> +
> + if (debug)
> + pr_debug("wdt_start: timeout=%d\n", timeout);
> +
> + spin_lock_irqsave(&wdt_lock, flags);
> + superio_init();
> + superio_select_dev(7);
> + val = superio_get_reg(0x72) | 0x10;

How about register names instead of magic values?

> + superio_set_reg(val, 0x72);
> + superio_set_reg((timeout/1000), 0x73);

Spaces around operators. Please use checkpatch.pl for further hints.

> + spin_unlock_irqrestore(&wdt_lock, flags);
> + return 0;
> +}
> +
> +/**
> + * wdt_stop:
> + *
> + * Stop the watchdog driver.
> + */
> +
> +static int wdt_stop(void)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&wdt_lock, flags);
> +
> + if (debug)
> + pr_debug("wdt_disable\n");
> + superio_init();
> + superio_select_dev(7); /* logic device 8 */
> + superio_set_reg(0, 0x73); /* Reg:F6 counter register */
> + spin_unlock_irqrestore(&wdt_lock, flags);
> + return 0;
> +}
> +
> +/**
> + * wdt_ping:
> + *
> + * Reload counter one with the watchdog heartbeat. We don't bother
> + * reloading the cascade counter.
> + */
> +
> +static void wdt_ping(void)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&wdt_lock, flags);
> +
> + if (debug)
> + pr_debug("wdt_ping: timeout=%d\n", timeout);
> + superio_init();
> + superio_select_dev(7); /* logic device 7 */
> + superio_set_reg((timeout/1000), 0x73); /* Reg:F6,30 sec */
> + spin_unlock_irqrestore(&wdt_lock, flags);
> +}
> +
> +/**
> + * wdt_verify_vendor:
> + * return true if vendor ID match
> + */
> +
> +static int wdt_verify_vendor(void)
> +{
> + unsigned char chip_id_h; /* chip id high byte */
> + unsigned char chip_id_l; /* chip id low byte */

Comment can go IMHO (obvious).

> +
> + superio_init();
> + superio_select_dev(7);
> + chip_id_h = superio_get_reg(0x20);
> + chip_id_l = superio_get_reg(0x21);
> + if ((chip_id_h == HW_VENDOR_ID_H) && (chip_id_l == HW_VENDOR_ID_L))
> + return 0;
> +
> + return -1;

-EINVAL?

> +}
> +
> +/**
> + * wdt_set_timeout:
> + * @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
> + * successful we return 0.
> + */
> +
> +static int wdt_set_timeout(int *t)
> +{
> + if (*t < WATCHDOG_MIN_TIMEOUT || *t > WATCHDOG_MAX_TIMEOUT) {
> + *t = DEFAULT_WATCHDOG_TIMEOUT;
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int moxawdt_open(struct inode *inode, struct file *file)
> +{
> +
> + if (test_and_set_bit(0, &wdt_is_open))
> + return -EBUSY;
> + /*
> + * Activate
> + */

Comment can go (obvious).

> + if (debug)
> + pr_debug("moxawdt_open entry\n");
> + moxawdt_start();
> + return nonseekable_open(inode, file);
> +
> + return 0;
> +}
> +
> +static int moxawdt_release(struct inode *inode, struct file *file)
> +{
> + if (debug)
> + pr_debug("moxawdt_release entry\n");
> +
> + if (expect_close == 42) {
> + wdt_stop();
> + clear_bit(0, &wdt_is_open);
> + } else {
> + pr_crit("wdt: WDT device closed unexpectedly. WDT will not stop!\n");
> + wdt_ping();
> + }
> + expect_close = 0;
> +
> + return 0;
> +}
> +
> +static long moxawdt_ioctl(struct file *file,
> + unsigned int cmd, unsigned long arg)
> +{
> + void __user *argp = (void __user *)arg;
> + int __user *p = argp;
> + int new_timeout;
> + int status;
> +
> + static struct watchdog_info ident = {
> + .options = WDIOF_SETTIMEOUT|
> + WDIOF_MAGICCLOSE|
> + WDIOF_KEEPALIVEPING,
> + .firmware_version = 1,
> + .identity = "MOXA2100WDT ",
> + };
> +
> + switch (cmd) {
> + case WDIOC_GETSUPPORT:
> + return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
> + case WDIOC_GETSTATUS:
> + status = 1;
> + 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_timeout, p))
> + return -EFAULT;
> + if (wdt_set_timeout(&new_timeout))
> + return -EINVAL;
> + wdt_ping();
> + /* Fall */
> + case WDIOC_GETTIMEOUT:
> + return put_user(timeout, p);
> + default:
> + return -ENOTTY;
> + }
> + return 0;
> +}
> +
> +/*
> + * moxawdt_write:
> + * @file: file handle to the watchdog
> + * @buf: buffer to write (unused as data does not matter here
> + * @count: count of bytes
> + * @ppos: pointer to the position to write. No seeks allowed
> + *
> + * A write to a watchdog device is defined as a keepalive signal. Any
> + * write of data will do, as we we don't define content meaning.
> + */
> +
> +static ssize_t moxawdt_write(struct file *file, const char *buf, \
> + size_t count, loff_t *ppos)
> +{
> + if (count) {
> + if (!nowayout) {
> + size_t i;
> +
> + /* In case it was set long ago */
> + for (i = 0; i != count; i++) {
> + char c;
> + if (get_user(c, buf + i))
> + return -EFAULT;
> + if (c == 'V')
> + expect_close = 42;
> + }
> + }
> +
> + }
> + return count;
> +}
> +
> +/**
> + * notify_sys:
> + * @this: our notifier block
> + * @code: the event being reported
> + * @unused: unused
> + *
> + * Our notifier is called on system shutdowns. We want to turn the card
> + * off at reboot otherwise the machine will reboot again during memory
> + * test or worse yet during the following fsck. This would suck, in fact
> + * trust me - if it happens it does suck.
> + */
> +
> +static int wdt_notify_sys(struct notifier_block *this, unsigned long code,
> + void *unused)
> +{
> + if (code == SYS_DOWN || code == SYS_HALT)
> + wdt_stop();
> + return NOTIFY_DONE;
> +}
> +
> +/*
> + * The WDT card needs to learn about soft shutdowns in order to
> + * turn the timebomb registers off.
> + */
> +
> +static struct notifier_block wdt_notifier = {
> + .notifier_call = wdt_notify_sys,
> +};
> +
> +static const struct file_operations moxawdt_fops = {
> + .owner = THIS_MODULE,
> + .llseek = no_llseek,
> + .open = moxawdt_open,
> + .write = moxawdt_write,
> + .unlocked_ioctl = moxawdt_ioctl,
> + .release = moxawdt_release,
> +};
> +
> +static struct miscdevice wdt_miscdev = {
> + .minor = WATCHDOG_MINOR,
> + .name = "watchdog",
> + .fops = &moxawdt_fops,
> +};
> +
> +static int __init moxawdt_init(void)
> +{
> + int ret;
> +
> + /* check timeout valid */

Comment can go (obvious).

> + if (wdt_set_timeout(&timeout)) {
> + pr_err("timeout value must be "
> + "%lu < timeout < %lu, using %d\n",

String should fit into one line.

> + WATCHDOG_MIN_TIMEOUT, WATCHDOG_MAX_TIMEOUT,
> + timeout);
> + }
> +
> + if (!request_region(SUPERIO_PORT, 2, "moxawdt")) {
> + pr_err("moxawdt_init: can't get I/O "
> + "address 0x%x\n", SUPERIO_PORT);
> + ret = -EBUSY;
> + goto reqreg_err;
> + }
> +
> + /* we suppose to check magic id in case wrong devices */

Comment can go (obvious).

> + if (wdt_verify_vendor()) {
> + pr_err("hw device id not match!!\n");
> + ret = -ENODEV;
> + goto reqreg_err;
> + }
> +
> + ret = register_reboot_notifier(&wdt_notifier);
> + if (ret) {
> + pr_err("can't register reboot notifier\n");
> + goto regreb_err;
> + }
> +
> + ret = misc_register(&wdt_miscdev);
> + if (ret) {
> + pr_err("Moxa V2100-LX WatchDog: Register misc fail !\n");
> + goto regmisc_err;
> + }
> +
> + pr_info("Moxa V2100 Watchdog Driver, version %s\n", MOXA_WDT_VERSION);
> + pr_info("initialized. (nowayout=%d)\n", nowayout);
> + pr_info("initialized. (timeout=%d)\n", timeout);
> + pr_info("initialized. (debug=%d)\n", debug);

Too excessive IMHO. Imagine every driver in your system printing that
much.

> +
> + return 0;
> +
> +regmisc_err:
> + unregister_reboot_notifier(&wdt_notifier);
> +regreb_err:
> + release_region(SUPERIO_PORT, 2);
> +reqreg_err:
> + return ret;
> +}
> +
> +static void __exit moxawdt_exit(void)
> +{
> + misc_deregister(&wdt_miscdev);
> + unregister_reboot_notifier(&wdt_notifier);
> + release_region(SUPERIO_PORT, 2);
> + superio_release();
> +}
> +
> +module_init(moxawdt_init);

Most people put this right below the referenced function.

> +module_exit(moxawdt_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("[email protected]");
> +MODULE_DESCRIPTION("Moxa V2100-LX WDT driver");
> +MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);
> +
> diff --git a/drivers/watchdog/moxa_wdt.h b/drivers/watchdog/moxa_wdt.h
> new file mode 100644
> index 0000000..3f3ff68
> --- /dev/null
> +++ b/drivers/watchdog/moxa_wdt.h
> @@ -0,0 +1,53 @@
> +/*
> + * serial driver for the MOXA V2100 platform.
> + *
> + * Copyright (c) MOXA Inc. All rights reserved.
> + * Jimmy Chen <[email protected]>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __X86__MOXAWDT__
> +#define __X86__MOXAWDT__
> +
> +#define SUPERIO_PORT ((u8)0x2e)
> +
> +#define DEFAULT_WATCHDOG_TIMEOUT (30UL*1000UL) /* 30 seconds */
> +#define WATCHDOG_MIN_TIMEOUT (1UL*1000UL) /* 2 seconds */
> +#define WATCHDOG_MAX_TIMEOUT (255UL*1000UL) /* 255 seconds */
> +
> +unsigned char superio_get_reg(u8 val)
> +{
> + outb_p(val, SUPERIO_PORT);
> + val = inb_p(SUPERIO_PORT+1);
> + return val;
> +}

Hmm, code in a header? Can't this go into the main source? Hmm, at least
it87_wdt.c has something very similar. Maybe it can even be shared? What
about locking BTW?

> +
> +void superio_set_reg(u8 val, u8 index)
> +{
> + outb_p(index, SUPERIO_PORT);
> + outb_p(val, (SUPERIO_PORT+1));
> +}
> +
> +void superio_select_dev(u8 val)
> +{
> + superio_set_reg(val, 0x07);
> +}
> +
> +void superio_init(void)
> +{
> + outb(0x87, SUPERIO_PORT);
> + outb(0x01, SUPERIO_PORT);
> + outb(0x55, SUPERIO_PORT);
> + outb(0x55, SUPERIO_PORT);
> +}
> +
> +void superio_release(void)
> +{
> + outb_p(0x02, SUPERIO_PORT);
> + outb_p(0x02, SUPERIO_PORT+1);
> +}
> +
> +#endif /* __X86__MOXAWDT__ */

Regards,

Wolfram

--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |


Attachments:
(No filename) (12.24 kB)
signature.asc (198.00 B)
Digital signature
Download all attachments

2011-05-03 11:14:05

by Alan

[permalink] [raw]
Subject: Re: [PATCH 2/2] watchdog: add support for MOXA V2100 watchdog driver

> Hmm, code in a header? Can't this go into the main source? Hmm, at least
> it87_wdt.c has something very similar. Maybe it can even be shared? What
> about locking BTW?

The spinlock handles it if you look at the caller points, because you
have to own the super I/O and select it each time.


Ultimately it wants to be using request_muxed_* but that isn't in any of
the mainstream code yet so can be done in the future when the mux patches
go in.

Alan

2011-05-03 12:52:16

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/2] watchdog: add support for MOXA V2100 watchdog driver

On Tuesday 03 May 2011, Alan Cox wrote:
> > Hmm, code in a header? Can't this go into the main source? Hmm, at least
> > it87_wdt.c has something very similar. Maybe it can even be shared? What
> > about locking BTW?
>
> The spinlock handles it if you look at the caller points, because you
> have to own the super I/O and select it each time.

I suppose the functions should at least be 'static inline' so they avoid
polluting the global namespace.

> Ultimately it wants to be using request_muxed_* but that isn't in any of
> the mainstream code yet so can be done in the future when the mux patches
> go in.

Another thing that might be nice here is to have one module that probes
the superio-port and then registers a platform device that other drivers
can can bind to. Right now, you have to load the module manually and
then check if it's there, something we no longer do for most devices.

Arnd

2011-05-03 13:34:12

by Alan

[permalink] [raw]
Subject: Re: [PATCH 2/2] watchdog: add support for MOXA V2100 watchdog driver

> Another thing that might be nice here is to have one module that probes
> the superio-port and then registers a platform device that other drivers

That doesn't help. See the it87 request_muxed patches pending. Because
the port can be muxed we need to actually implement port muxing across
the super I/O devices. That's a separate and larger problem altogether
and this device will get fixed as we do the other so not a problem.

> can can bind to. Right now, you have to load the module manually and
> then check if it's there, something we no longer do for most devices.

A lot of these super I/O devices have no entirely safe autodetect, and
the same is true for it87 and many others. There isn't a nice elegant
solution really.

Alan

2011-05-04 09:02:26

by Jimmy Chen (陳永達)

[permalink] [raw]
Subject: RE: [PATCH 2/2] watchdog: add support for MOXA V2100 watchdog driver

From: Jimmy Chen <[email protected]>

-Add real function for watchdog driver
-Follow advices from Alan Cox
-Follow advices from Wolfram

Signed-off-by: Jimmy Chen <[email protected]>
---
diff --git a/drivers/watchdog/moxa_wdt.c b/drivers/watchdog/moxa_wdt.c
new file mode 100644
index 0000000..fdecc9e
--- /dev/null
+++ b/drivers/watchdog/moxa_wdt.c
@@ -0,0 +1,409 @@
+/*
+ * serial driver for the MOXA V2100 platform.
+ *
+ * Copyright (c) MOXA Inc. All rights reserved.
+ * Jimmy Chen <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME":"fmt
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#include <linux/fs.h>
+#include <linux/ioport.h>
+#include <linux/notifier.h>
+#include <linux/reboot.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
+
+#include <asm/system.h>
+
+#define HW_VENDOR_ID_H ((u8)0x87)
+#define HW_VENDOR_ID_L ((u8)0x83)
+
+#define SUPERIO_PORT ((u8)0x2e)
+#define CHIP_ID_BYTE1 ((u8)0x20)
+#define CHIP_ID_BYTE2 ((u8)0x21)
+#define LOGIC_DEVICE_NUMBER ((u8)0x07)
+#define GPIO_CONFIG_REG ((u8)0x07)
+#define WATCHDOG_TIMER1_CTRL ((u8)0x71)
+#define WATCHDOG_TIMER1_CONFIG_REG ((u8)0x72)
+#define WATCHDOG_TIMER1_TIMEOUT_VAL ((u8)0x73)
+#define WATCHDOG_TIMER2_CTRL ((u8)0x81)
+#define WATCHDOG_TIMER2_CONFIG_REG ((u8)0x82)
+#define WATCHDOG_TIMER2_TIMEOUT_VAL ((u8)0x83)
+
+#define DEFAULT_WATCHDOG_TIMEOUT (30UL*1000UL) /* 30 seconds */
+#define WATCHDOG_MIN_TIMEOUT (1UL*1000UL) /* 2 seconds */
+#define WATCHDOG_MAX_TIMEOUT (255UL*1000UL) /* 255 seconds */
+
+
+static unsigned long wdt_is_open;
+static char expect_close;
+
+static int timeout = DEFAULT_WATCHDOG_TIMEOUT;
+module_param(timeout, int, 0);
+MODULE_PARM_DESC(timeout,
+ "Watchdog timeout in seconds. 1<= timeout <=63, default="
+ __MODULE_STRING(WATCHDOG_TIMEOUT) ".");
+
+static int nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, int, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be "
+ "stopped once started (default=CONFIG_WATCHDOG_NOWAYOUT)");
+
+static spinlock_t wdt_lock = SPIN_LOCK_UNLOCKED;
+
+static inline unsigned char superio_get_reg(u8 val)
+{
+ outb_p(val, SUPERIO_PORT);
+ val = inb_p(SUPERIO_PORT+1);
+ return val;
+}
+
+static inline void superio_set_reg(u8 val, u8 index)
+{
+ outb_p(index, SUPERIO_PORT);
+ outb_p(val, (SUPERIO_PORT+1));
+}
+
+static inline void superio_select_dev(u8 val)
+{
+ superio_set_reg(val, LOGIC_DEVICE_NUMBER);
+}
+
+static inline void superio_init(void)
+{
+ outb(0x87, SUPERIO_PORT);
+ outb(0x01, SUPERIO_PORT);
+ outb(0x55, SUPERIO_PORT);
+ outb(0x55, SUPERIO_PORT);
+}
+
+static inline void superio_release(void)
+{
+ outb_p(0x02, SUPERIO_PORT);
+ outb_p(0x02, SUPERIO_PORT+1);
+}
+
+/**
+ * wdt_start:
+ *
+ * Start the watchdog driver.
+ */
+
+static int moxawdt_start(void)
+{
+ unsigned long flags;
+ unsigned char val;
+
+ pr_debug("wdt_start: timeout=%d\n", timeout);
+
+ spin_lock_irqsave(&wdt_lock, flags);
+ superio_init();
+ superio_select_dev(GPIO_CONFIG_REG);
+ val = superio_get_reg(WATCHDOG_TIMER1_CONFIG_REG) | 0x10;
+ superio_set_reg(val, WATCHDOG_TIMER1_CONFIG_REG);
+ superio_set_reg((timeout / 1000), WATCHDOG_TIMER1_TIMEOUT_VAL);
+ spin_unlock_irqrestore(&wdt_lock, flags);
+ return 0;
+}
+
+/**
+ * wdt_stop:
+ *
+ * Stop the watchdog driver.
+ */
+
+static int wdt_stop(void)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&wdt_lock, flags);
+
+ pr_debug("wdt_disable\n");
+ superio_init();
+ superio_select_dev(GPIO_CONFIG_REG);
+ superio_set_reg(0, WATCHDOG_TIMER1_TIMEOUT_VAL);
+ spin_unlock_irqrestore(&wdt_lock, flags);
+ return 0;
+}
+
+/**
+ * wdt_ping:
+ *
+ * Reload counter one with the watchdog heartbeat. We don't bother
+ * reloading the cascade counter.
+ */
+
+static void wdt_ping(void)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&wdt_lock, flags);
+
+ pr_debug("wdt_ping: timeout=%d\n", timeout);
+ superio_init();
+ superio_select_dev(GPIO_CONFIG_REG);
+ superio_set_reg((timeout / 1000), WATCHDOG_TIMER1_TIMEOUT_VAL);
+ spin_unlock_irqrestore(&wdt_lock, flags);
+}
+
+/**
+ * wdt_verify_vendor:
+ * return true if vendor ID match
+ */
+
+static int wdt_verify_vendor(void)
+{
+ unsigned char chip_id_h;
+ unsigned char chip_id_l;
+
+ superio_init();
+ superio_select_dev(GPIO_CONFIG_REG);
+ chip_id_h = superio_get_reg(CHIP_ID_BYTE1);
+ chip_id_l = superio_get_reg(CHIP_ID_BYTE2);
+ if ((chip_id_h == HW_VENDOR_ID_H) && (chip_id_l == HW_VENDOR_ID_L))
+ return 0;
+
+ return 1;
+}
+
+/**
+ * wdt_set_timeout:
+ * @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
+ * successful we return 0.
+ */
+
+static int wdt_set_timeout(int *t)
+{
+ if (*t < WATCHDOG_MIN_TIMEOUT || *t > WATCHDOG_MAX_TIMEOUT) {
+ *t = DEFAULT_WATCHDOG_TIMEOUT;
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int moxawdt_open(struct inode *inode, struct file *file)
+{
+
+ if (test_and_set_bit(0, &wdt_is_open))
+ return -EBUSY;
+
+ pr_debug("moxawdt_open entry\n");
+ moxawdt_start();
+ return nonseekable_open(inode, file);
+
+ return 0;
+}
+
+static int moxawdt_release(struct inode *inode, struct file *file)
+{
+ pr_debug("moxawdt_release entry\n");
+
+ if (expect_close == 42) {
+ wdt_stop();
+ clear_bit(0, &wdt_is_open);
+ } else {
+ pr_crit("wdt: WDT device closed unexpectedly. WDT will not stop!\n");
+ wdt_ping();
+ }
+ expect_close = 0;
+
+ return 0;
+}
+
+static long moxawdt_ioctl(struct file *file,
+ unsigned int cmd, unsigned long arg)
+{
+ void __user *argp = (void __user *)arg;
+ int __user *p = argp;
+ int new_timeout;
+ int status;
+
+ static struct watchdog_info ident = {
+ .options = WDIOF_SETTIMEOUT|
+ WDIOF_MAGICCLOSE|
+ WDIOF_KEEPALIVEPING,
+ .firmware_version = 1,
+ .identity = "MOXA2100WDT ",
+ };
+
+ switch (cmd) {
+ case WDIOC_GETSUPPORT:
+ return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
+ case WDIOC_GETSTATUS:
+ status = 1;
+ 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_timeout, p))
+ return -EFAULT;
+ if (wdt_set_timeout(&new_timeout))
+ return -EINVAL;
+ wdt_ping();
+ /* Fall */
+ case WDIOC_GETTIMEOUT:
+ return put_user(timeout, p);
+ default:
+ return -ENOTTY;
+ }
+ return 0;
+}
+
+/*
+ * moxawdt_write:
+ * @file: file handle to the watchdog
+ * @buf: buffer to write (unused as data does not matter here
+ * @count: count of bytes
+ * @ppos: pointer to the position to write. No seeks allowed
+ *
+ * A write to a watchdog device is defined as a keepalive signal. Any
+ * write of data will do, as we we don't define content meaning.
+ */
+
+static ssize_t moxawdt_write(struct file *file, const char *buf, \
+ size_t count, loff_t *ppos)
+{
+ if (count) {
+ if (!nowayout) {
+ size_t i;
+
+ /* In case it was set long ago */
+ for (i = 0; i != count; i++) {
+ char c;
+ if (get_user(c, buf + i))
+ return -EFAULT;
+ if (c == 'V')
+ expect_close = 42;
+ }
+ }
+
+ }
+ return count;
+}
+
+/**
+ * notify_sys:
+ * @this: our notifier block
+ * @code: the event being reported
+ * @unused: unused
+ *
+ * Our notifier is called on system shutdowns. We want to turn the card
+ * off at reboot otherwise the machine will reboot again during memory
+ * test or worse yet during the following fsck. This would suck, in fact
+ * trust me - if it happens it does suck.
+ */
+
+static int wdt_notify_sys(struct notifier_block *this, unsigned long code,
+ void *unused)
+{
+ if (code == SYS_DOWN || code == SYS_HALT)
+ wdt_stop();
+ return NOTIFY_DONE;
+}
+
+/*
+ * The WDT card needs to learn about soft shutdowns in order to
+ * turn the timebomb registers off.
+ */
+
+static struct notifier_block wdt_notifier = {
+ .notifier_call = wdt_notify_sys,
+};
+
+static const struct file_operations moxawdt_fops = {
+ .owner = THIS_MODULE,
+ .llseek = no_llseek,
+ .open = moxawdt_open,
+ .write = moxawdt_write,
+ .unlocked_ioctl = moxawdt_ioctl,
+ .release = moxawdt_release,
+};
+
+static struct miscdevice wdt_miscdev = {
+ .minor = WATCHDOG_MINOR,
+ .name = "watchdog",
+ .fops = &moxawdt_fops,
+};
+
+static void __exit moxawdt_exit(void)
+{
+ misc_deregister(&wdt_miscdev);
+ unregister_reboot_notifier(&wdt_notifier);
+ release_region(SUPERIO_PORT, 2);
+ superio_release();
+}
+
+static int __init moxawdt_init(void)
+{
+ int ret;
+
+ if (wdt_set_timeout(&timeout)) {
+ pr_err("timeout value must be %lu < timeout < %lu, using %d\n",
+ WATCHDOG_MIN_TIMEOUT, WATCHDOG_MAX_TIMEOUT,
+ timeout);
+ }
+
+ if (!request_region(SUPERIO_PORT, 2, "moxawdt")) {
+ pr_err("moxawdt_init: can't get I/O address 0x%x\n", SUPERIO_PORT);
+ ret = -EBUSY;
+ goto reqreg_err;
+ }
+
+ if (wdt_verify_vendor()) {
+ pr_err("hw device id not match!!\n");
+ ret = -ENODEV;
+ goto reqreg_err;
+ }
+
+ ret = register_reboot_notifier(&wdt_notifier);
+ if (ret) {
+ pr_err("can't register reboot notifier\n");
+ goto regreb_err;
+ }
+
+ ret = misc_register(&wdt_miscdev);
+ if (ret) {
+ pr_err("Moxa V2100-LX WatchDog: Register misc fail !\n");
+ goto regmisc_err;
+ }
+
+ pr_info("Moxa V2100 Watchdog Driver. nowayout=%d, timeout=%d\n", nowayout, timeout);
+
+ return 0;
+
+regmisc_err:
+ unregister_reboot_notifier(&wdt_notifier);
+regreb_err:
+ release_region(SUPERIO_PORT, 2);
+reqreg_err:
+ return ret;
+}
+
+module_init(moxawdt_init);
+module_exit(moxawdt_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("[email protected]");
+MODULE_DESCRIPTION("Moxa V2100-LX WDT driver");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);

2011-05-06 05:52:57

by Jimmy Chen (陳永達)

[permalink] [raw]
Subject: RE: [PATCH 2/2] watchdog: add support for MOXA V2100 watchdog driver

From: Jimmy Chen <[email protected]>

Does anyone have comment on this driver?

Signed-off-by: Jimmy Chen <[email protected]>
---
diff --git a/drivers/watchdog/moxa_wdt.c b/drivers/watchdog/moxa_wdt.c
new file mode 100644
index 0000000..fdecc9e
--- /dev/null
+++ b/drivers/watchdog/moxa_wdt.c
@@ -0,0 +1,409 @@
+/*
+ * serial driver for the MOXA V2100 platform.
+ *
+ * Copyright (c) MOXA Inc. All rights reserved.
+ * Jimmy Chen <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME":"fmt
+
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+#include <linux/miscdevice.h>
+#include <linux/watchdog.h>
+#include <linux/fs.h>
+#include <linux/ioport.h>
+#include <linux/notifier.h>
+#include <linux/reboot.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/uaccess.h>
+
+#include <asm/system.h>
+
+#define HW_VENDOR_ID_H ((u8)0x87)
+#define HW_VENDOR_ID_L ((u8)0x83)
+
+#define SUPERIO_PORT ((u8)0x2e)
+#define CHIP_ID_BYTE1 ((u8)0x20)
+#define CHIP_ID_BYTE2 ((u8)0x21)
+#define LOGIC_DEVICE_NUMBER ((u8)0x07)
+#define GPIO_CONFIG_REG ((u8)0x07)
+#define WATCHDOG_TIMER1_CTRL ((u8)0x71)
+#define WATCHDOG_TIMER1_CONFIG_REG ((u8)0x72)
+#define WATCHDOG_TIMER1_TIMEOUT_VAL ((u8)0x73)
+#define WATCHDOG_TIMER2_CTRL ((u8)0x81)
+#define WATCHDOG_TIMER2_CONFIG_REG ((u8)0x82)
+#define WATCHDOG_TIMER2_TIMEOUT_VAL ((u8)0x83)
+
+#define DEFAULT_WATCHDOG_TIMEOUT (30UL*1000UL) /* 30 seconds */
+#define WATCHDOG_MIN_TIMEOUT (1UL*1000UL) /* 2 seconds */
+#define WATCHDOG_MAX_TIMEOUT (255UL*1000UL) /* 255 seconds */
+
+
+static unsigned long wdt_is_open;
+static char expect_close;
+
+static int timeout = DEFAULT_WATCHDOG_TIMEOUT;
+module_param(timeout, int, 0);
+MODULE_PARM_DESC(timeout,
+ "Watchdog timeout in seconds. 1<= timeout <=63, default="
+ __MODULE_STRING(WATCHDOG_TIMEOUT) ".");
+
+static int nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, int, 0);
+MODULE_PARM_DESC(nowayout, "Watchdog cannot be "
+ "stopped once started (default=CONFIG_WATCHDOG_NOWAYOUT)");
+
+static spinlock_t wdt_lock = SPIN_LOCK_UNLOCKED;
+
+static inline unsigned char superio_get_reg(u8 val)
+{
+ outb_p(val, SUPERIO_PORT);
+ val = inb_p(SUPERIO_PORT+1);
+ return val;
+}
+
+static inline void superio_set_reg(u8 val, u8 index)
+{
+ outb_p(index, SUPERIO_PORT);
+ outb_p(val, (SUPERIO_PORT+1));
+}
+
+static inline void superio_select_dev(u8 val)
+{
+ superio_set_reg(val, LOGIC_DEVICE_NUMBER);
+}
+
+static inline void superio_init(void)
+{
+ outb(0x87, SUPERIO_PORT);
+ outb(0x01, SUPERIO_PORT);
+ outb(0x55, SUPERIO_PORT);
+ outb(0x55, SUPERIO_PORT);
+}
+
+static inline void superio_release(void)
+{
+ outb_p(0x02, SUPERIO_PORT);
+ outb_p(0x02, SUPERIO_PORT+1);
+}
+
+/**
+ * wdt_start:
+ *
+ * Start the watchdog driver.
+ */
+
+static int moxawdt_start(void)
+{
+ unsigned long flags;
+ unsigned char val;
+
+ pr_debug("wdt_start: timeout=%d\n", timeout);
+
+ spin_lock_irqsave(&wdt_lock, flags);
+ superio_init();
+ superio_select_dev(GPIO_CONFIG_REG);
+ val = superio_get_reg(WATCHDOG_TIMER1_CONFIG_REG) | 0x10;
+ superio_set_reg(val, WATCHDOG_TIMER1_CONFIG_REG);
+ superio_set_reg((timeout / 1000), WATCHDOG_TIMER1_TIMEOUT_VAL);
+ spin_unlock_irqrestore(&wdt_lock, flags);
+ return 0;
+}
+
+/**
+ * wdt_stop:
+ *
+ * Stop the watchdog driver.
+ */
+
+static int wdt_stop(void)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&wdt_lock, flags);
+
+ pr_debug("wdt_disable\n");
+ superio_init();
+ superio_select_dev(GPIO_CONFIG_REG);
+ superio_set_reg(0, WATCHDOG_TIMER1_TIMEOUT_VAL);
+ spin_unlock_irqrestore(&wdt_lock, flags);
+ return 0;
+}
+
+/**
+ * wdt_ping:
+ *
+ * Reload counter one with the watchdog heartbeat. We don't bother
+ * reloading the cascade counter.
+ */
+
+static void wdt_ping(void)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&wdt_lock, flags);
+
+ pr_debug("wdt_ping: timeout=%d\n", timeout);
+ superio_init();
+ superio_select_dev(GPIO_CONFIG_REG);
+ superio_set_reg((timeout / 1000), WATCHDOG_TIMER1_TIMEOUT_VAL);
+ spin_unlock_irqrestore(&wdt_lock, flags);
+}
+
+/**
+ * wdt_verify_vendor:
+ * return true if vendor ID match
+ */
+
+static int wdt_verify_vendor(void)
+{
+ unsigned char chip_id_h;
+ unsigned char chip_id_l;
+
+ superio_init();
+ superio_select_dev(GPIO_CONFIG_REG);
+ chip_id_h = superio_get_reg(CHIP_ID_BYTE1);
+ chip_id_l = superio_get_reg(CHIP_ID_BYTE2);
+ if ((chip_id_h == HW_VENDOR_ID_H) && (chip_id_l == HW_VENDOR_ID_L))
+ return 0;
+
+ return 1;
+}
+
+/**
+ * wdt_set_timeout:
+ * @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
+ * successful we return 0.
+ */
+
+static int wdt_set_timeout(int *t)
+{
+ if (*t < WATCHDOG_MIN_TIMEOUT || *t > WATCHDOG_MAX_TIMEOUT) {
+ *t = DEFAULT_WATCHDOG_TIMEOUT;
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int moxawdt_open(struct inode *inode, struct file *file)
+{
+
+ if (test_and_set_bit(0, &wdt_is_open))
+ return -EBUSY;
+
+ pr_debug("moxawdt_open entry\n");
+ moxawdt_start();
+ return nonseekable_open(inode, file);
+
+ return 0;
+}
+
+static int moxawdt_release(struct inode *inode, struct file *file)
+{
+ pr_debug("moxawdt_release entry\n");
+
+ if (expect_close == 42) {
+ wdt_stop();
+ clear_bit(0, &wdt_is_open);
+ } else {
+ pr_crit("wdt: WDT device closed unexpectedly. WDT will not stop!\n");
+ wdt_ping();
+ }
+ expect_close = 0;
+
+ return 0;
+}
+
+static long moxawdt_ioctl(struct file *file,
+ unsigned int cmd, unsigned long arg)
+{
+ void __user *argp = (void __user *)arg;
+ int __user *p = argp;
+ int new_timeout;
+ int status;
+
+ static struct watchdog_info ident = {
+ .options = WDIOF_SETTIMEOUT|
+ WDIOF_MAGICCLOSE|
+ WDIOF_KEEPALIVEPING,
+ .firmware_version = 1,
+ .identity = "MOXA2100WDT ",
+ };
+
+ switch (cmd) {
+ case WDIOC_GETSUPPORT:
+ return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
+ case WDIOC_GETSTATUS:
+ status = 1;
+ 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_timeout, p))
+ return -EFAULT;
+ if (wdt_set_timeout(&new_timeout))
+ return -EINVAL;
+ wdt_ping();
+ /* Fall */
+ case WDIOC_GETTIMEOUT:
+ return put_user(timeout, p);
+ default:
+ return -ENOTTY;
+ }
+ return 0;
+}
+
+/*
+ * moxawdt_write:
+ * @file: file handle to the watchdog
+ * @buf: buffer to write (unused as data does not matter here
+ * @count: count of bytes
+ * @ppos: pointer to the position to write. No seeks allowed
+ *
+ * A write to a watchdog device is defined as a keepalive signal. Any
+ * write of data will do, as we we don't define content meaning.
+ */
+
+static ssize_t moxawdt_write(struct file *file, const char *buf, \
+ size_t count, loff_t *ppos)
+{
+ if (count) {
+ if (!nowayout) {
+ size_t i;
+
+ /* In case it was set long ago */
+ for (i = 0; i != count; i++) {
+ char c;
+ if (get_user(c, buf + i))
+ return -EFAULT;
+ if (c == 'V')
+ expect_close = 42;
+ }
+ }
+
+ }
+ return count;
+}
+
+/**
+ * notify_sys:
+ * @this: our notifier block
+ * @code: the event being reported
+ * @unused: unused
+ *
+ * Our notifier is called on system shutdowns. We want to turn the card
+ * off at reboot otherwise the machine will reboot again during memory
+ * test or worse yet during the following fsck. This would suck, in fact
+ * trust me - if it happens it does suck.
+ */
+
+static int wdt_notify_sys(struct notifier_block *this, unsigned long code,
+ void *unused)
+{
+ if (code == SYS_DOWN || code == SYS_HALT)
+ wdt_stop();
+ return NOTIFY_DONE;
+}
+
+/*
+ * The WDT card needs to learn about soft shutdowns in order to
+ * turn the timebomb registers off.
+ */
+
+static struct notifier_block wdt_notifier = {
+ .notifier_call = wdt_notify_sys,
+};
+
+static const struct file_operations moxawdt_fops = {
+ .owner = THIS_MODULE,
+ .llseek = no_llseek,
+ .open = moxawdt_open,
+ .write = moxawdt_write,
+ .unlocked_ioctl = moxawdt_ioctl,
+ .release = moxawdt_release,
+};
+
+static struct miscdevice wdt_miscdev = {
+ .minor = WATCHDOG_MINOR,
+ .name = "watchdog",
+ .fops = &moxawdt_fops,
+};
+
+static void __exit moxawdt_exit(void)
+{
+ misc_deregister(&wdt_miscdev);
+ unregister_reboot_notifier(&wdt_notifier);
+ release_region(SUPERIO_PORT, 2);
+ superio_release();
+}
+
+static int __init moxawdt_init(void)
+{
+ int ret;
+
+ if (wdt_set_timeout(&timeout)) {
+ pr_err("timeout value must be %lu < timeout < %lu, using %d\n",
+ WATCHDOG_MIN_TIMEOUT, WATCHDOG_MAX_TIMEOUT,
+ timeout);
+ }
+
+ if (!request_region(SUPERIO_PORT, 2, "moxawdt")) {
+ pr_err("moxawdt_init: can't get I/O address 0x%x\n", SUPERIO_PORT);
+ ret = -EBUSY;
+ goto reqreg_err;
+ }
+
+ if (wdt_verify_vendor()) {
+ pr_err("hw device id not match!!\n");
+ ret = -ENODEV;
+ goto reqreg_err;
+ }
+
+ ret = register_reboot_notifier(&wdt_notifier);
+ if (ret) {
+ pr_err("can't register reboot notifier\n");
+ goto regreb_err;
+ }
+
+ ret = misc_register(&wdt_miscdev);
+ if (ret) {
+ pr_err("Moxa V2100-LX WatchDog: Register misc fail !\n");
+ goto regmisc_err;
+ }
+
+ pr_info("Moxa V2100 Watchdog Driver. nowayout=%d, timeout=%d\n", nowayout, timeout);
+
+ return 0;
+
+regmisc_err:
+ unregister_reboot_notifier(&wdt_notifier);
+regreb_err:
+ release_region(SUPERIO_PORT, 2);
+reqreg_err:
+ return ret;
+}
+
+module_init(moxawdt_init);
+module_exit(moxawdt_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("[email protected]");
+MODULE_DESCRIPTION("Moxa V2100-LX WDT driver");
+MODULE_ALIAS_MISCDEV(WATCHDOG_MINOR);

2011-05-06 07:00:25

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 2/2] watchdog: add support for MOXA V2100 watchdog driver

On Friday 06 May 2011, Jimmy Chen (???ùF) wrote:
> From: Jimmy Chen <[email protected]>
>
> Does anyone have comment on this driver?
>
> Signed-off-by: Jimmy Chen <[email protected]>

Looks good to me.

Acked-by: Arnd Bergmann <[email protected]>

2011-05-19 16:01:27

by Wim Van Sebroeck

[permalink] [raw]
Subject: Re: [PATCH 2/2] watchdog: add support for MOXA V2100 watchdog driver

Hi Jimmy,

> From: Jimmy Chen <[email protected]>
>
> Does anyone have comment on this driver?
>
> Signed-off-by: Jimmy Chen <[email protected]>

I'm not going to add this driver. Reason is that the underlying hardware is an ITE IT8783 EC LPC superI/O chipset.
Support for this should actually go in it87_wdt.c . I think the following patch replaces your driver (see below).
Could you test it?

(and we can probably add the IT8781F and the IT8782F also, and the hwmon guys can probably add all 3 also in it87.c).

Kind regards,
Wim.


diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 1b0f98b..0afa0bf 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -590,7 +590,8 @@ config IT87_WDT
depends on X86 && EXPERIMENTAL
---help---
This is the driver for the hardware watchdog on the ITE IT8702,
- IT8712, IT8716, IT8718, IT8720, IT8721, IT8726 Super I/O chips.
+ IT8712, IT8716, IT8718, IT8720, IT8721, IT8726, IT8783 Super I/O
+ chips.
This watchdog simply watches your kernel to make sure it doesn't
freeze, and if it does, it reboots your computer after a certain
amount of time.
diff --git a/drivers/watchdog/it87_wdt.c b/drivers/watchdog/it87_wdt.c
index b1bc72f..3529e8c 100644
--- a/drivers/watchdog/it87_wdt.c
+++ b/drivers/watchdog/it87_wdt.c
@@ -12,7 +12,7 @@
* http://www.ite.com.tw/
*
* Support of the watchdog timers, which are available on
- * IT8702, IT8712, IT8716, IT8718, IT8720, IT8721 and IT8726.
+ * IT8702, IT8712, IT8716, IT8718, IT8720, IT8721, IT8726 and IT8783.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU General Public License
@@ -84,6 +84,7 @@
#define IT8720_ID 0x8720
#define IT8721_ID 0x8721
#define IT8726_ID 0x8726 /* the data sheet suggest wrongly 0x8716 */
+#define IT8783_ID 0x8783

/* GPIO Configuration Registers LDN=0x07 */
#define WDTCTRL 0x71
@@ -585,6 +586,7 @@ static int __init it87_wdt_init(void)
case IT8718_ID:
case IT8720_ID:
case IT8721_ID:
+ case IT8783_ID:
max_units = 65535;
try_gameport = 0;
break;