* [PATCH] iTCO_wdt: move watchdog to proper kernel interface
@ 2012-06-08 10:09 Tony Zelenoff
2012-06-08 10:09 ` [PATCH] iTCO_wdt: remove unnecessary includes Tony Zelenoff
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Tony Zelenoff @ 2012-06-08 10:09 UTC (permalink / raw)
To: linux-watchdog; +Cc: wim, antonz
Remove obsoleted fs interface and move to proper usage of
watchdog_device interface.
Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
drivers/watchdog/iTCO_wdt.c | 246 ++++++++++++++-----------------------------
1 files changed, 77 insertions(+), 169 deletions(-)
diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index bc47e90..14e48d9 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -87,9 +87,11 @@
#define TCO2_CNT (TCOBASE + 0x0a) /* TCO2 Control Register */
#define TCOv2_TMR (TCOBASE + 0x12) /* TCOv2 Timer Initial Value */
+/* Maximum allowed timeouts for iTCO */
+#define iTCO_V1_MAX_TIMEOUT 0x03f
+#define iTCO_V2_MAX_TIMEOUT 0x3ff
+
/* internal variables */
-static unsigned long is_active;
-static char expect_release;
static struct { /* this is private data for the iTCO_wdt device */
/* TCO version/generation */
unsigned int iTCO_version;
@@ -107,9 +109,9 @@ static struct { /* this is private data for the iTCO_wdt device */
/* module parameters */
#define WATCHDOG_HEARTBEAT 30 /* 30 sec default heartbeat */
-static int heartbeat = WATCHDOG_HEARTBEAT; /* in seconds */
-module_param(heartbeat, int, 0);
-MODULE_PARM_DESC(heartbeat, "Watchdog timeout in seconds. "
+static unsigned int heartbeat_param = WATCHDOG_HEARTBEAT; /* in seconds */
+module_param(heartbeat_param, uint, 0);
+MODULE_PARM_DESC(heartbeat_param, "Watchdog timeout in seconds. "
"5..76 (TCO v1) or 3..614 (TCO v2), default="
__MODULE_STRING(WATCHDOG_HEARTBEAT) ")");
@@ -178,13 +180,13 @@ static int iTCO_wdt_unset_NO_REBOOT_bit(void)
return ret; /* returns: 0 = OK, -EIO = Error */
}
-static int iTCO_wdt_start(void)
+static int iTCO_wdt_start(struct watchdog_device *w)
{
unsigned int val;
spin_lock(&iTCO_wdt_private.io_lock);
- iTCO_vendor_pre_start(iTCO_wdt_private.smi_res, heartbeat);
+ iTCO_vendor_pre_start(iTCO_wdt_private.smi_res, w->timeout);
/* disable chipset's NO_REBOOT bit */
if (iTCO_wdt_unset_NO_REBOOT_bit()) {
@@ -212,7 +214,7 @@ static int iTCO_wdt_start(void)
return 0;
}
-static int iTCO_wdt_stop(void)
+static int iTCO_wdt_stop(struct watchdog_device *w)
{
unsigned int val;
@@ -236,11 +238,11 @@ static int iTCO_wdt_stop(void)
return 0;
}
-static int iTCO_wdt_keepalive(void)
+static int iTCO_wdt_ping(struct watchdog_device *w)
{
spin_lock(&iTCO_wdt_private.io_lock);
- iTCO_vendor_pre_keepalive(iTCO_wdt_private.smi_res, heartbeat);
+ iTCO_vendor_pre_keepalive(iTCO_wdt_private.smi_res, w->timeout);
/* Reload the timer by writing to the TCO Timer Counter register */
if (iTCO_wdt_private.iTCO_version == 2)
@@ -257,7 +259,24 @@ static int iTCO_wdt_keepalive(void)
return 0;
}
-static int iTCO_wdt_set_heartbeat(int t)
+static inline int iTCO_wdt_timeout_valid(unsigned int t)
+{
+ unsigned int timeout = 0;
+
+ if (iTCO_wdt_private.iTCO_version == 2)
+ timeout = iTCO_V2_MAX_TIMEOUT;
+
+ if (iTCO_wdt_private.iTCO_version == 1)
+ timeout = iTCO_V1_MAX_TIMEOUT;
+
+ if (t > timeout)
+ return 0;
+
+ return 1;
+}
+
+static int iTCO_wdt_set_timeout(struct watchdog_device *w,
+ unsigned int t)
{
unsigned int val16;
unsigned char val8;
@@ -273,8 +292,8 @@ static int iTCO_wdt_set_heartbeat(int t)
/* "Values of 0h-3h are ignored and should not be attempted" */
if (tmrval < 0x04)
return -EINVAL;
- if (((iTCO_wdt_private.iTCO_version == 2) && (tmrval > 0x3ff)) ||
- ((iTCO_wdt_private.iTCO_version == 1) && (tmrval > 0x03f)))
+
+ if (!iTCO_wdt_timeout_valid(t))
return -EINVAL;
iTCO_vendor_pre_set_heartbeat(tmrval);
@@ -304,11 +323,11 @@ static int iTCO_wdt_set_heartbeat(int t)
return -EINVAL;
}
- heartbeat = t;
+ w->timeout = t;
return 0;
}
-static int iTCO_wdt_get_timeleft(int *time_left)
+static unsigned int iTCO_wdt_get_timeleft(struct watchdog_device *w)
{
unsigned int val16;
unsigned char val8;
@@ -320,8 +339,10 @@ static int iTCO_wdt_get_timeleft(int *time_left)
val16 &= 0x3ff;
spin_unlock(&iTCO_wdt_private.io_lock);
- *time_left = (val16 * 6) / 10;
- } else if (iTCO_wdt_private.iTCO_version == 1) {
+ return (val16 * 6) / 10;
+ }
+
+ if (iTCO_wdt_private.iTCO_version == 1) {
spin_lock(&iTCO_wdt_private.io_lock);
val8 = inb(TCO_RLD);
val8 &= 0x3f;
@@ -329,156 +350,39 @@ static int iTCO_wdt_get_timeleft(int *time_left)
val8 += (inb(TCOv1_TMR) & 0x3f);
spin_unlock(&iTCO_wdt_private.io_lock);
- *time_left = (val8 * 6) / 10;
- } else
- return -EINVAL;
- return 0;
-}
-
-/*
- * /dev/watchdog handling
- */
-
-static int iTCO_wdt_open(struct inode *inode, struct file *file)
-{
- /* /dev/watchdog can only be opened once */
- if (test_and_set_bit(0, &is_active))
- return -EBUSY;
-
- /*
- * Reload and activate timer
- */
- iTCO_wdt_start();
- return nonseekable_open(inode, file);
-}
-
-static int iTCO_wdt_release(struct inode *inode, struct file *file)
-{
- /*
- * Shut off the timer.
- */
- if (expect_release == 42) {
- iTCO_wdt_stop();
- } else {
- pr_crit("Unexpected close, not stopping watchdog!\n");
- iTCO_wdt_keepalive();
- }
- clear_bit(0, &is_active);
- expect_release = 0;
- return 0;
-}
-
-static ssize_t iTCO_wdt_write(struct file *file, const char __user *data,
- size_t len, loff_t *ppos)
-{
- /* See if we got the magic character 'V' and reload the timer */
- if (len) {
- if (!nowayout) {
- size_t i;
-
- /* note: just in case someone wrote the magic
- character five months ago... */
- expect_release = 0;
-
- /* scan to see whether or not we got the
- magic character */
- for (i = 0; i != len; i++) {
- char c;
- if (get_user(c, data + i))
- return -EFAULT;
- if (c == 'V')
- expect_release = 42;
- }
- }
-
- /* someone wrote to us, we should reload the timer */
- iTCO_wdt_keepalive();
- }
- return len;
-}
-
-static long iTCO_wdt_ioctl(struct file *file, unsigned int cmd,
- unsigned long arg)
-{
- int new_options, retval = -EINVAL;
- int new_heartbeat;
- void __user *argp = (void __user *)arg;
- int __user *p = argp;
- static const struct watchdog_info ident = {
- .options = WDIOF_SETTIMEOUT |
- WDIOF_KEEPALIVEPING |
- WDIOF_MAGICCLOSE,
- .firmware_version = 0,
- .identity = DRV_NAME,
- };
-
- switch (cmd) {
- case WDIOC_GETSUPPORT:
- return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
- case WDIOC_GETSTATUS:
- case WDIOC_GETBOOTSTATUS:
- return put_user(0, p);
-
- case WDIOC_SETOPTIONS:
- {
- if (get_user(new_options, p))
- return -EFAULT;
-
- if (new_options & WDIOS_DISABLECARD) {
- iTCO_wdt_stop();
- retval = 0;
- }
- if (new_options & WDIOS_ENABLECARD) {
- iTCO_wdt_keepalive();
- iTCO_wdt_start();
- retval = 0;
- }
- return retval;
+ return (val8 * 6) / 10;
}
- case WDIOC_KEEPALIVE:
- iTCO_wdt_keepalive();
- return 0;
- case WDIOC_SETTIMEOUT:
- {
- if (get_user(new_heartbeat, p))
- return -EFAULT;
- if (iTCO_wdt_set_heartbeat(new_heartbeat))
- return -EINVAL;
- iTCO_wdt_keepalive();
- /* Fall */
- }
- case WDIOC_GETTIMEOUT:
- return put_user(heartbeat, p);
- case WDIOC_GETTIMELEFT:
- {
- int time_left;
- if (iTCO_wdt_get_timeleft(&time_left))
- return -EINVAL;
- return put_user(time_left, p);
- }
- default:
- return -ENOTTY;
- }
+ return -EINVAL;
}
/*
* Kernel Interfaces
*/
-static const struct file_operations iTCO_wdt_fops = {
- .owner = THIS_MODULE,
- .llseek = no_llseek,
- .write = iTCO_wdt_write,
- .unlocked_ioctl = iTCO_wdt_ioctl,
- .open = iTCO_wdt_open,
- .release = iTCO_wdt_release,
+static const struct watchdog_info iTCO_wdt_info = {
+ .options = WDIOF_SETTIMEOUT |
+ WDIOF_KEEPALIVEPING |
+ WDIOF_MAGICCLOSE,
+ .firmware_version = 0,
+ .identity = DRV_NAME,
+};
+
+static struct watchdog_ops iTCO_wdt_ops = {
+ .owner = THIS_MODULE,
+ .start = iTCO_wdt_start,
+ .stop = iTCO_wdt_stop,
+ .ping = iTCO_wdt_ping,
+ .set_timeout = iTCO_wdt_set_timeout,
+ .get_timeleft = iTCO_wdt_get_timeleft,
};
-static struct miscdevice iTCO_wdt_miscdev = {
- .minor = WATCHDOG_MINOR,
- .name = "watchdog",
- .fops = &iTCO_wdt_fops,
+static struct watchdog_device iTCO_wdt_dev = {
+ .info = &iTCO_wdt_info,
+ .ops = &iTCO_wdt_ops,
+ .min_timeout = 1,
+ /* It will be set up at _probe */
+ .max_timeout = 0xFFFF
};
/*
@@ -489,10 +393,10 @@ static void __devexit iTCO_wdt_cleanup(void)
{
/* Stop the timer before we leave */
if (!nowayout)
- iTCO_wdt_stop();
+ iTCO_wdt_stop(&iTCO_wdt_dev);
/* Deregister */
- misc_deregister(&iTCO_wdt_miscdev);
+ watchdog_unregister_device(&iTCO_wdt_dev);
/* release resources */
release_region(iTCO_wdt_private.tco_res->start,
@@ -559,7 +463,10 @@ static int __devinit iTCO_wdt_probe(struct platform_device *dev)
ret = -EIO;
goto unreg_gcs;
}
- }
+
+ iTCO_wdt_dev.max_timeout = iTCO_V2_MAX_TIMEOUT;
+ } else
+ iTCO_wdt_dev.max_timeout = iTCO_V1_MAX_TIMEOUT;
/* Check chipset's NO_REBOOT bit */
if (iTCO_wdt_unset_NO_REBOOT_bit() && iTCO_vendor_check_noreboot_on()) {
@@ -606,24 +513,25 @@ static int __devinit iTCO_wdt_probe(struct platform_device *dev)
outw(0x0004, TCO2_STS); /* Clear BOOT_STS bit */
/* Make sure the watchdog is not running */
- iTCO_wdt_stop();
+ iTCO_wdt_stop(&iTCO_wdt_dev);
/* Check that the heartbeat value is within it's range;
if not reset to the default */
- if (iTCO_wdt_set_heartbeat(heartbeat)) {
- iTCO_wdt_set_heartbeat(WATCHDOG_HEARTBEAT);
- pr_info("timeout value out of range, using %d\n", heartbeat);
+ if (iTCO_wdt_set_timeout(&iTCO_wdt_dev, heartbeat_param)) {
+ iTCO_wdt_set_timeout(&iTCO_wdt_dev, WATCHDOG_HEARTBEAT);
+ pr_info("timeout value out of range, using %d\n", heartbeat_param);
}
- ret = misc_register(&iTCO_wdt_miscdev);
+ watchdog_set_nowayout(&iTCO_wdt_dev, nowayout);
+
+ ret = watchdog_register_device(&iTCO_wdt_dev);
if (ret != 0) {
- pr_err("cannot register miscdev on minor=%d (err=%d)\n",
- WATCHDOG_MINOR, ret);
+ pr_err("cannot register watchdog device (err=%d)\n", ret);
goto unreg_tco;
}
pr_info("initialized. heartbeat=%d sec (nowayout=%d)\n",
- heartbeat, nowayout);
+ heartbeat_param, nowayout);
return 0;
@@ -659,7 +567,7 @@ static int __devexit iTCO_wdt_remove(struct platform_device *dev)
static void iTCO_wdt_shutdown(struct platform_device *dev)
{
- iTCO_wdt_stop();
+ iTCO_wdt_stop(&iTCO_wdt_dev);
}
static struct platform_driver iTCO_wdt_driver = {
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] iTCO_wdt: remove unnecessary includes
2012-06-08 10:09 [PATCH] iTCO_wdt: move watchdog to proper kernel interface Tony Zelenoff
@ 2012-06-08 10:09 ` Tony Zelenoff
2012-06-19 10:20 ` [PATCH] iTCO_wdt: move watchdog to proper kernel interface Tony Zelenoff
2012-06-26 18:45 ` Wim Van Sebroeck
2 siblings, 0 replies; 6+ messages in thread
From: Tony Zelenoff @ 2012-06-08 10:09 UTC (permalink / raw)
To: linux-watchdog; +Cc: wim, antonz
After switching to new kernel interface, those
includes are not needed anymore.
Signed-off-by: Tony Zelenoff <antonz@parallels.com>
---
drivers/watchdog/iTCO_wdt.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
index 14e48d9..8cfa574 100644
--- a/drivers/watchdog/iTCO_wdt.c
+++ b/drivers/watchdog/iTCO_wdt.c
@@ -59,12 +59,10 @@
(WATCHDOG_MINOR) */
#include <linux/watchdog.h> /* For the watchdog specific items */
#include <linux/init.h> /* For __init/__exit/... */
-#include <linux/fs.h> /* For file operations */
#include <linux/platform_device.h> /* For platform_driver framework */
#include <linux/pci.h> /* For pci functions */
#include <linux/ioport.h> /* For io-port access */
#include <linux/spinlock.h> /* For spin_lock/spin_unlock/... */
-#include <linux/uaccess.h> /* For copy_to_user/put_user/... */
#include <linux/io.h> /* For inb/outb/... */
#include <linux/mfd/core.h>
#include <linux/mfd/lpc_ich.h>
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] iTCO_wdt: move watchdog to proper kernel interface
2012-06-08 10:09 [PATCH] iTCO_wdt: move watchdog to proper kernel interface Tony Zelenoff
2012-06-08 10:09 ` [PATCH] iTCO_wdt: remove unnecessary includes Tony Zelenoff
@ 2012-06-19 10:20 ` Tony Zelenoff
2012-06-19 12:40 ` Hans de Goede
2012-06-26 18:45 ` Wim Van Sebroeck
2 siblings, 1 reply; 6+ messages in thread
From: Tony Zelenoff @ 2012-06-19 10:20 UTC (permalink / raw)
To: Anton Zelenov; +Cc: linux-watchdog@vger.kernel.org, wim@iguana.be
Ping.
Is this patch wrong? Or just missed?
8/06/12 2:09 PM, Anton Zelenov пишет:
> Remove obsoleted fs interface and move to proper usage of
> watchdog_device interface.
>
> Signed-off-by: Tony Zelenoff <antonz@parallels.com>
> ---
> drivers/watchdog/iTCO_wdt.c | 246 ++++++++++++++-----------------------------
> 1 files changed, 77 insertions(+), 169 deletions(-)
>
> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
> index bc47e90..14e48d9 100644
> --- a/drivers/watchdog/iTCO_wdt.c
> +++ b/drivers/watchdog/iTCO_wdt.c
> @@ -87,9 +87,11 @@
> #define TCO2_CNT (TCOBASE + 0x0a) /* TCO2 Control Register */
> #define TCOv2_TMR (TCOBASE + 0x12) /* TCOv2 Timer Initial Value */
>
> +/* Maximum allowed timeouts for iTCO */
> +#define iTCO_V1_MAX_TIMEOUT 0x03f
> +#define iTCO_V2_MAX_TIMEOUT 0x3ff
> +
> /* internal variables */
> -static unsigned long is_active;
> -static char expect_release;
> static struct { /* this is private data for the iTCO_wdt device */
> /* TCO version/generation */
> unsigned int iTCO_version;
> @@ -107,9 +109,9 @@ static struct { /* this is private data for the iTCO_wdt device */
>
> /* module parameters */
> #define WATCHDOG_HEARTBEAT 30 /* 30 sec default heartbeat */
> -static int heartbeat = WATCHDOG_HEARTBEAT; /* in seconds */
> -module_param(heartbeat, int, 0);
> -MODULE_PARM_DESC(heartbeat, "Watchdog timeout in seconds. "
> +static unsigned int heartbeat_param = WATCHDOG_HEARTBEAT; /* in seconds */
> +module_param(heartbeat_param, uint, 0);
> +MODULE_PARM_DESC(heartbeat_param, "Watchdog timeout in seconds. "
> "5..76 (TCO v1) or 3..614 (TCO v2), default="
> __MODULE_STRING(WATCHDOG_HEARTBEAT) ")");
>
> @@ -178,13 +180,13 @@ static int iTCO_wdt_unset_NO_REBOOT_bit(void)
> return ret; /* returns: 0 = OK, -EIO = Error */
> }
>
> -static int iTCO_wdt_start(void)
> +static int iTCO_wdt_start(struct watchdog_device *w)
> {
> unsigned int val;
>
> spin_lock(&iTCO_wdt_private.io_lock);
>
> - iTCO_vendor_pre_start(iTCO_wdt_private.smi_res, heartbeat);
> + iTCO_vendor_pre_start(iTCO_wdt_private.smi_res, w->timeout);
>
> /* disable chipset's NO_REBOOT bit */
> if (iTCO_wdt_unset_NO_REBOOT_bit()) {
> @@ -212,7 +214,7 @@ static int iTCO_wdt_start(void)
> return 0;
> }
>
> -static int iTCO_wdt_stop(void)
> +static int iTCO_wdt_stop(struct watchdog_device *w)
> {
> unsigned int val;
>
> @@ -236,11 +238,11 @@ static int iTCO_wdt_stop(void)
> return 0;
> }
>
> -static int iTCO_wdt_keepalive(void)
> +static int iTCO_wdt_ping(struct watchdog_device *w)
> {
> spin_lock(&iTCO_wdt_private.io_lock);
>
> - iTCO_vendor_pre_keepalive(iTCO_wdt_private.smi_res, heartbeat);
> + iTCO_vendor_pre_keepalive(iTCO_wdt_private.smi_res, w->timeout);
>
> /* Reload the timer by writing to the TCO Timer Counter register */
> if (iTCO_wdt_private.iTCO_version == 2)
> @@ -257,7 +259,24 @@ static int iTCO_wdt_keepalive(void)
> return 0;
> }
>
> -static int iTCO_wdt_set_heartbeat(int t)
> +static inline int iTCO_wdt_timeout_valid(unsigned int t)
> +{
> + unsigned int timeout = 0;
> +
> + if (iTCO_wdt_private.iTCO_version == 2)
> + timeout = iTCO_V2_MAX_TIMEOUT;
> +
> + if (iTCO_wdt_private.iTCO_version == 1)
> + timeout = iTCO_V1_MAX_TIMEOUT;
> +
> + if (t > timeout)
> + return 0;
> +
> + return 1;
> +}
> +
> +static int iTCO_wdt_set_timeout(struct watchdog_device *w,
> + unsigned int t)
> {
> unsigned int val16;
> unsigned char val8;
> @@ -273,8 +292,8 @@ static int iTCO_wdt_set_heartbeat(int t)
> /* "Values of 0h-3h are ignored and should not be attempted" */
> if (tmrval < 0x04)
> return -EINVAL;
> - if (((iTCO_wdt_private.iTCO_version == 2) && (tmrval > 0x3ff)) ||
> - ((iTCO_wdt_private.iTCO_version == 1) && (tmrval > 0x03f)))
> +
> + if (!iTCO_wdt_timeout_valid(t))
> return -EINVAL;
>
> iTCO_vendor_pre_set_heartbeat(tmrval);
> @@ -304,11 +323,11 @@ static int iTCO_wdt_set_heartbeat(int t)
> return -EINVAL;
> }
>
> - heartbeat = t;
> + w->timeout = t;
> return 0;
> }
>
> -static int iTCO_wdt_get_timeleft(int *time_left)
> +static unsigned int iTCO_wdt_get_timeleft(struct watchdog_device *w)
> {
> unsigned int val16;
> unsigned char val8;
> @@ -320,8 +339,10 @@ static int iTCO_wdt_get_timeleft(int *time_left)
> val16 &= 0x3ff;
> spin_unlock(&iTCO_wdt_private.io_lock);
>
> - *time_left = (val16 * 6) / 10;
> - } else if (iTCO_wdt_private.iTCO_version == 1) {
> + return (val16 * 6) / 10;
> + }
> +
> + if (iTCO_wdt_private.iTCO_version == 1) {
> spin_lock(&iTCO_wdt_private.io_lock);
> val8 = inb(TCO_RLD);
> val8 &= 0x3f;
> @@ -329,156 +350,39 @@ static int iTCO_wdt_get_timeleft(int *time_left)
> val8 += (inb(TCOv1_TMR) & 0x3f);
> spin_unlock(&iTCO_wdt_private.io_lock);
>
> - *time_left = (val8 * 6) / 10;
> - } else
> - return -EINVAL;
> - return 0;
> -}
> -
> -/*
> - * /dev/watchdog handling
> - */
> -
> -static int iTCO_wdt_open(struct inode *inode, struct file *file)
> -{
> - /* /dev/watchdog can only be opened once */
> - if (test_and_set_bit(0, &is_active))
> - return -EBUSY;
> -
> - /*
> - * Reload and activate timer
> - */
> - iTCO_wdt_start();
> - return nonseekable_open(inode, file);
> -}
> -
> -static int iTCO_wdt_release(struct inode *inode, struct file *file)
> -{
> - /*
> - * Shut off the timer.
> - */
> - if (expect_release == 42) {
> - iTCO_wdt_stop();
> - } else {
> - pr_crit("Unexpected close, not stopping watchdog!\n");
> - iTCO_wdt_keepalive();
> - }
> - clear_bit(0, &is_active);
> - expect_release = 0;
> - return 0;
> -}
> -
> -static ssize_t iTCO_wdt_write(struct file *file, const char __user *data,
> - size_t len, loff_t *ppos)
> -{
> - /* See if we got the magic character 'V' and reload the timer */
> - if (len) {
> - if (!nowayout) {
> - size_t i;
> -
> - /* note: just in case someone wrote the magic
> - character five months ago... */
> - expect_release = 0;
> -
> - /* scan to see whether or not we got the
> - magic character */
> - for (i = 0; i != len; i++) {
> - char c;
> - if (get_user(c, data + i))
> - return -EFAULT;
> - if (c == 'V')
> - expect_release = 42;
> - }
> - }
> -
> - /* someone wrote to us, we should reload the timer */
> - iTCO_wdt_keepalive();
> - }
> - return len;
> -}
> -
> -static long iTCO_wdt_ioctl(struct file *file, unsigned int cmd,
> - unsigned long arg)
> -{
> - int new_options, retval = -EINVAL;
> - int new_heartbeat;
> - void __user *argp = (void __user *)arg;
> - int __user *p = argp;
> - static const struct watchdog_info ident = {
> - .options = WDIOF_SETTIMEOUT |
> - WDIOF_KEEPALIVEPING |
> - WDIOF_MAGICCLOSE,
> - .firmware_version = 0,
> - .identity = DRV_NAME,
> - };
> -
> - switch (cmd) {
> - case WDIOC_GETSUPPORT:
> - return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
> - case WDIOC_GETSTATUS:
> - case WDIOC_GETBOOTSTATUS:
> - return put_user(0, p);
> -
> - case WDIOC_SETOPTIONS:
> - {
> - if (get_user(new_options, p))
> - return -EFAULT;
> -
> - if (new_options & WDIOS_DISABLECARD) {
> - iTCO_wdt_stop();
> - retval = 0;
> - }
> - if (new_options & WDIOS_ENABLECARD) {
> - iTCO_wdt_keepalive();
> - iTCO_wdt_start();
> - retval = 0;
> - }
> - return retval;
> + return (val8 * 6) / 10;
> }
> - case WDIOC_KEEPALIVE:
> - iTCO_wdt_keepalive();
> - return 0;
>
> - case WDIOC_SETTIMEOUT:
> - {
> - if (get_user(new_heartbeat, p))
> - return -EFAULT;
> - if (iTCO_wdt_set_heartbeat(new_heartbeat))
> - return -EINVAL;
> - iTCO_wdt_keepalive();
> - /* Fall */
> - }
> - case WDIOC_GETTIMEOUT:
> - return put_user(heartbeat, p);
> - case WDIOC_GETTIMELEFT:
> - {
> - int time_left;
> - if (iTCO_wdt_get_timeleft(&time_left))
> - return -EINVAL;
> - return put_user(time_left, p);
> - }
> - default:
> - return -ENOTTY;
> - }
> + return -EINVAL;
> }
>
> /*
> * Kernel Interfaces
> */
>
> -static const struct file_operations iTCO_wdt_fops = {
> - .owner = THIS_MODULE,
> - .llseek = no_llseek,
> - .write = iTCO_wdt_write,
> - .unlocked_ioctl = iTCO_wdt_ioctl,
> - .open = iTCO_wdt_open,
> - .release = iTCO_wdt_release,
> +static const struct watchdog_info iTCO_wdt_info = {
> + .options = WDIOF_SETTIMEOUT |
> + WDIOF_KEEPALIVEPING |
> + WDIOF_MAGICCLOSE,
> + .firmware_version = 0,
> + .identity = DRV_NAME,
> +};
> +
> +static struct watchdog_ops iTCO_wdt_ops = {
> + .owner = THIS_MODULE,
> + .start = iTCO_wdt_start,
> + .stop = iTCO_wdt_stop,
> + .ping = iTCO_wdt_ping,
> + .set_timeout = iTCO_wdt_set_timeout,
> + .get_timeleft = iTCO_wdt_get_timeleft,
> };
>
> -static struct miscdevice iTCO_wdt_miscdev = {
> - .minor = WATCHDOG_MINOR,
> - .name = "watchdog",
> - .fops = &iTCO_wdt_fops,
> +static struct watchdog_device iTCO_wdt_dev = {
> + .info = &iTCO_wdt_info,
> + .ops = &iTCO_wdt_ops,
> + .min_timeout = 1,
> + /* It will be set up at _probe */
> + .max_timeout = 0xFFFF
> };
>
> /*
> @@ -489,10 +393,10 @@ static void __devexit iTCO_wdt_cleanup(void)
> {
> /* Stop the timer before we leave */
> if (!nowayout)
> - iTCO_wdt_stop();
> + iTCO_wdt_stop(&iTCO_wdt_dev);
>
> /* Deregister */
> - misc_deregister(&iTCO_wdt_miscdev);
> + watchdog_unregister_device(&iTCO_wdt_dev);
>
> /* release resources */
> release_region(iTCO_wdt_private.tco_res->start,
> @@ -559,7 +463,10 @@ static int __devinit iTCO_wdt_probe(struct platform_device *dev)
> ret = -EIO;
> goto unreg_gcs;
> }
> - }
> +
> + iTCO_wdt_dev.max_timeout = iTCO_V2_MAX_TIMEOUT;
> + } else
> + iTCO_wdt_dev.max_timeout = iTCO_V1_MAX_TIMEOUT;
>
> /* Check chipset's NO_REBOOT bit */
> if (iTCO_wdt_unset_NO_REBOOT_bit() && iTCO_vendor_check_noreboot_on()) {
> @@ -606,24 +513,25 @@ static int __devinit iTCO_wdt_probe(struct platform_device *dev)
> outw(0x0004, TCO2_STS); /* Clear BOOT_STS bit */
>
> /* Make sure the watchdog is not running */
> - iTCO_wdt_stop();
> + iTCO_wdt_stop(&iTCO_wdt_dev);
>
> /* Check that the heartbeat value is within it's range;
> if not reset to the default */
> - if (iTCO_wdt_set_heartbeat(heartbeat)) {
> - iTCO_wdt_set_heartbeat(WATCHDOG_HEARTBEAT);
> - pr_info("timeout value out of range, using %d\n", heartbeat);
> + if (iTCO_wdt_set_timeout(&iTCO_wdt_dev, heartbeat_param)) {
> + iTCO_wdt_set_timeout(&iTCO_wdt_dev, WATCHDOG_HEARTBEAT);
> + pr_info("timeout value out of range, using %d\n", heartbeat_param);
> }
>
> - ret = misc_register(&iTCO_wdt_miscdev);
> + watchdog_set_nowayout(&iTCO_wdt_dev, nowayout);
> +
> + ret = watchdog_register_device(&iTCO_wdt_dev);
> if (ret != 0) {
> - pr_err("cannot register miscdev on minor=%d (err=%d)\n",
> - WATCHDOG_MINOR, ret);
> + pr_err("cannot register watchdog device (err=%d)\n", ret);
> goto unreg_tco;
> }
>
> pr_info("initialized. heartbeat=%d sec (nowayout=%d)\n",
> - heartbeat, nowayout);
> + heartbeat_param, nowayout);
>
> return 0;
>
> @@ -659,7 +567,7 @@ static int __devexit iTCO_wdt_remove(struct platform_device *dev)
>
> static void iTCO_wdt_shutdown(struct platform_device *dev)
> {
> - iTCO_wdt_stop();
> + iTCO_wdt_stop(&iTCO_wdt_dev);
> }
>
> static struct platform_driver iTCO_wdt_driver = {
>
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iTCO_wdt: move watchdog to proper kernel interface
2012-06-19 10:20 ` [PATCH] iTCO_wdt: move watchdog to proper kernel interface Tony Zelenoff
@ 2012-06-19 12:40 ` Hans de Goede
2012-06-19 13:11 ` Tony Zelenoff
0 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2012-06-19 12:40 UTC (permalink / raw)
To: Tony Zelenoff; +Cc: linux-watchdog@vger.kernel.org, wim@iguana.be
Hi,
On 06/19/2012 12:20 PM, Tony Zelenoff wrote:
> Ping.
>
> Is this patch wrong? Or just missed?
Definitely not wrong (on the conceptual level I've not actually
checked the code). We do want the iTCO_wdt driver to be converted
to the (new) watchdog device framework.
I think you just need to give Wim some more time to take a good
look at this. Wim does the watchdog subsystem maintainer ship in
his spare time, so sometimes you need to be a bit patient :)
Regards,
Hans
>
> 8/06/12 2:09 PM, Anton Zelenov пишет:
>> Remove obsoleted fs interface and move to proper usage of
>> watchdog_device interface.
>>
>> Signed-off-by: Tony Zelenoff <antonz@parallels.com>
>> ---
>> drivers/watchdog/iTCO_wdt.c | 246 ++++++++++++++-----------------------------
>> 1 files changed, 77 insertions(+), 169 deletions(-)
>>
>> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
>> index bc47e90..14e48d9 100644
>> --- a/drivers/watchdog/iTCO_wdt.c
>> +++ b/drivers/watchdog/iTCO_wdt.c
>> @@ -87,9 +87,11 @@
>> #define TCO2_CNT (TCOBASE + 0x0a) /* TCO2 Control Register */
>> #define TCOv2_TMR (TCOBASE + 0x12) /* TCOv2 Timer Initial Value */
>>
>> +/* Maximum allowed timeouts for iTCO */
>> +#define iTCO_V1_MAX_TIMEOUT 0x03f
>> +#define iTCO_V2_MAX_TIMEOUT 0x3ff
>> +
>> /* internal variables */
>> -static unsigned long is_active;
>> -static char expect_release;
>> static struct { /* this is private data for the iTCO_wdt device */
>> /* TCO version/generation */
>> unsigned int iTCO_version;
>> @@ -107,9 +109,9 @@ static struct { /* this is private data for the iTCO_wdt device */
>>
>> /* module parameters */
>> #define WATCHDOG_HEARTBEAT 30 /* 30 sec default heartbeat */
>> -static int heartbeat = WATCHDOG_HEARTBEAT; /* in seconds */
>> -module_param(heartbeat, int, 0);
>> -MODULE_PARM_DESC(heartbeat, "Watchdog timeout in seconds. "
>> +static unsigned int heartbeat_param = WATCHDOG_HEARTBEAT; /* in seconds */
>> +module_param(heartbeat_param, uint, 0);
>> +MODULE_PARM_DESC(heartbeat_param, "Watchdog timeout in seconds. "
>> "5..76 (TCO v1) or 3..614 (TCO v2), default="
>> __MODULE_STRING(WATCHDOG_HEARTBEAT) ")");
>>
>> @@ -178,13 +180,13 @@ static int iTCO_wdt_unset_NO_REBOOT_bit(void)
>> return ret; /* returns: 0 = OK, -EIO = Error */
>> }
>>
>> -static int iTCO_wdt_start(void)
>> +static int iTCO_wdt_start(struct watchdog_device *w)
>> {
>> unsigned int val;
>>
>> spin_lock(&iTCO_wdt_private.io_lock);
>>
>> - iTCO_vendor_pre_start(iTCO_wdt_private.smi_res, heartbeat);
>> + iTCO_vendor_pre_start(iTCO_wdt_private.smi_res, w->timeout);
>>
>> /* disable chipset's NO_REBOOT bit */
>> if (iTCO_wdt_unset_NO_REBOOT_bit()) {
>> @@ -212,7 +214,7 @@ static int iTCO_wdt_start(void)
>> return 0;
>> }
>>
>> -static int iTCO_wdt_stop(void)
>> +static int iTCO_wdt_stop(struct watchdog_device *w)
>> {
>> unsigned int val;
>>
>> @@ -236,11 +238,11 @@ static int iTCO_wdt_stop(void)
>> return 0;
>> }
>>
>> -static int iTCO_wdt_keepalive(void)
>> +static int iTCO_wdt_ping(struct watchdog_device *w)
>> {
>> spin_lock(&iTCO_wdt_private.io_lock);
>>
>> - iTCO_vendor_pre_keepalive(iTCO_wdt_private.smi_res, heartbeat);
>> + iTCO_vendor_pre_keepalive(iTCO_wdt_private.smi_res, w->timeout);
>>
>> /* Reload the timer by writing to the TCO Timer Counter register */
>> if (iTCO_wdt_private.iTCO_version == 2)
>> @@ -257,7 +259,24 @@ static int iTCO_wdt_keepalive(void)
>> return 0;
>> }
>>
>> -static int iTCO_wdt_set_heartbeat(int t)
>> +static inline int iTCO_wdt_timeout_valid(unsigned int t)
>> +{
>> + unsigned int timeout = 0;
>> +
>> + if (iTCO_wdt_private.iTCO_version == 2)
>> + timeout = iTCO_V2_MAX_TIMEOUT;
>> +
>> + if (iTCO_wdt_private.iTCO_version == 1)
>> + timeout = iTCO_V1_MAX_TIMEOUT;
>> +
>> + if (t > timeout)
>> + return 0;
>> +
>> + return 1;
>> +}
>> +
>> +static int iTCO_wdt_set_timeout(struct watchdog_device *w,
>> + unsigned int t)
>> {
>> unsigned int val16;
>> unsigned char val8;
>> @@ -273,8 +292,8 @@ static int iTCO_wdt_set_heartbeat(int t)
>> /* "Values of 0h-3h are ignored and should not be attempted" */
>> if (tmrval < 0x04)
>> return -EINVAL;
>> - if (((iTCO_wdt_private.iTCO_version == 2) && (tmrval > 0x3ff)) ||
>> - ((iTCO_wdt_private.iTCO_version == 1) && (tmrval > 0x03f)))
>> +
>> + if (!iTCO_wdt_timeout_valid(t))
>> return -EINVAL;
>>
>> iTCO_vendor_pre_set_heartbeat(tmrval);
>> @@ -304,11 +323,11 @@ static int iTCO_wdt_set_heartbeat(int t)
>> return -EINVAL;
>> }
>>
>> - heartbeat = t;
>> + w->timeout = t;
>> return 0;
>> }
>>
>> -static int iTCO_wdt_get_timeleft(int *time_left)
>> +static unsigned int iTCO_wdt_get_timeleft(struct watchdog_device *w)
>> {
>> unsigned int val16;
>> unsigned char val8;
>> @@ -320,8 +339,10 @@ static int iTCO_wdt_get_timeleft(int *time_left)
>> val16 &= 0x3ff;
>> spin_unlock(&iTCO_wdt_private.io_lock);
>>
>> - *time_left = (val16 * 6) / 10;
>> - } else if (iTCO_wdt_private.iTCO_version == 1) {
>> + return (val16 * 6) / 10;
>> + }
>> +
>> + if (iTCO_wdt_private.iTCO_version == 1) {
>> spin_lock(&iTCO_wdt_private.io_lock);
>> val8 = inb(TCO_RLD);
>> val8 &= 0x3f;
>> @@ -329,156 +350,39 @@ static int iTCO_wdt_get_timeleft(int *time_left)
>> val8 += (inb(TCOv1_TMR) & 0x3f);
>> spin_unlock(&iTCO_wdt_private.io_lock);
>>
>> - *time_left = (val8 * 6) / 10;
>> - } else
>> - return -EINVAL;
>> - return 0;
>> -}
>> -
>> -/*
>> - * /dev/watchdog handling
>> - */
>> -
>> -static int iTCO_wdt_open(struct inode *inode, struct file *file)
>> -{
>> - /* /dev/watchdog can only be opened once */
>> - if (test_and_set_bit(0, &is_active))
>> - return -EBUSY;
>> -
>> - /*
>> - * Reload and activate timer
>> - */
>> - iTCO_wdt_start();
>> - return nonseekable_open(inode, file);
>> -}
>> -
>> -static int iTCO_wdt_release(struct inode *inode, struct file *file)
>> -{
>> - /*
>> - * Shut off the timer.
>> - */
>> - if (expect_release == 42) {
>> - iTCO_wdt_stop();
>> - } else {
>> - pr_crit("Unexpected close, not stopping watchdog!\n");
>> - iTCO_wdt_keepalive();
>> - }
>> - clear_bit(0, &is_active);
>> - expect_release = 0;
>> - return 0;
>> -}
>> -
>> -static ssize_t iTCO_wdt_write(struct file *file, const char __user *data,
>> - size_t len, loff_t *ppos)
>> -{
>> - /* See if we got the magic character 'V' and reload the timer */
>> - if (len) {
>> - if (!nowayout) {
>> - size_t i;
>> -
>> - /* note: just in case someone wrote the magic
>> - character five months ago... */
>> - expect_release = 0;
>> -
>> - /* scan to see whether or not we got the
>> - magic character */
>> - for (i = 0; i != len; i++) {
>> - char c;
>> - if (get_user(c, data + i))
>> - return -EFAULT;
>> - if (c == 'V')
>> - expect_release = 42;
>> - }
>> - }
>> -
>> - /* someone wrote to us, we should reload the timer */
>> - iTCO_wdt_keepalive();
>> - }
>> - return len;
>> -}
>> -
>> -static long iTCO_wdt_ioctl(struct file *file, unsigned int cmd,
>> - unsigned long arg)
>> -{
>> - int new_options, retval = -EINVAL;
>> - int new_heartbeat;
>> - void __user *argp = (void __user *)arg;
>> - int __user *p = argp;
>> - static const struct watchdog_info ident = {
>> - .options = WDIOF_SETTIMEOUT |
>> - WDIOF_KEEPALIVEPING |
>> - WDIOF_MAGICCLOSE,
>> - .firmware_version = 0,
>> - .identity = DRV_NAME,
>> - };
>> -
>> - switch (cmd) {
>> - case WDIOC_GETSUPPORT:
>> - return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
>> - case WDIOC_GETSTATUS:
>> - case WDIOC_GETBOOTSTATUS:
>> - return put_user(0, p);
>> -
>> - case WDIOC_SETOPTIONS:
>> - {
>> - if (get_user(new_options, p))
>> - return -EFAULT;
>> -
>> - if (new_options & WDIOS_DISABLECARD) {
>> - iTCO_wdt_stop();
>> - retval = 0;
>> - }
>> - if (new_options & WDIOS_ENABLECARD) {
>> - iTCO_wdt_keepalive();
>> - iTCO_wdt_start();
>> - retval = 0;
>> - }
>> - return retval;
>> + return (val8 * 6) / 10;
>> }
>> - case WDIOC_KEEPALIVE:
>> - iTCO_wdt_keepalive();
>> - return 0;
>>
>> - case WDIOC_SETTIMEOUT:
>> - {
>> - if (get_user(new_heartbeat, p))
>> - return -EFAULT;
>> - if (iTCO_wdt_set_heartbeat(new_heartbeat))
>> - return -EINVAL;
>> - iTCO_wdt_keepalive();
>> - /* Fall */
>> - }
>> - case WDIOC_GETTIMEOUT:
>> - return put_user(heartbeat, p);
>> - case WDIOC_GETTIMELEFT:
>> - {
>> - int time_left;
>> - if (iTCO_wdt_get_timeleft(&time_left))
>> - return -EINVAL;
>> - return put_user(time_left, p);
>> - }
>> - default:
>> - return -ENOTTY;
>> - }
>> + return -EINVAL;
>> }
>>
>> /*
>> * Kernel Interfaces
>> */
>>
>> -static const struct file_operations iTCO_wdt_fops = {
>> - .owner = THIS_MODULE,
>> - .llseek = no_llseek,
>> - .write = iTCO_wdt_write,
>> - .unlocked_ioctl = iTCO_wdt_ioctl,
>> - .open = iTCO_wdt_open,
>> - .release = iTCO_wdt_release,
>> +static const struct watchdog_info iTCO_wdt_info = {
>> + .options = WDIOF_SETTIMEOUT |
>> + WDIOF_KEEPALIVEPING |
>> + WDIOF_MAGICCLOSE,
>> + .firmware_version = 0,
>> + .identity = DRV_NAME,
>> +};
>> +
>> +static struct watchdog_ops iTCO_wdt_ops = {
>> + .owner = THIS_MODULE,
>> + .start = iTCO_wdt_start,
>> + .stop = iTCO_wdt_stop,
>> + .ping = iTCO_wdt_ping,
>> + .set_timeout = iTCO_wdt_set_timeout,
>> + .get_timeleft = iTCO_wdt_get_timeleft,
>> };
>>
>> -static struct miscdevice iTCO_wdt_miscdev = {
>> - .minor = WATCHDOG_MINOR,
>> - .name = "watchdog",
>> - .fops = &iTCO_wdt_fops,
>> +static struct watchdog_device iTCO_wdt_dev = {
>> + .info = &iTCO_wdt_info,
>> + .ops = &iTCO_wdt_ops,
>> + .min_timeout = 1,
>> + /* It will be set up at _probe */
>> + .max_timeout = 0xFFFF
>> };
>>
>> /*
>> @@ -489,10 +393,10 @@ static void __devexit iTCO_wdt_cleanup(void)
>> {
>> /* Stop the timer before we leave */
>> if (!nowayout)
>> - iTCO_wdt_stop();
>> + iTCO_wdt_stop(&iTCO_wdt_dev);
>>
>> /* Deregister */
>> - misc_deregister(&iTCO_wdt_miscdev);
>> + watchdog_unregister_device(&iTCO_wdt_dev);
>>
>> /* release resources */
>> release_region(iTCO_wdt_private.tco_res->start,
>> @@ -559,7 +463,10 @@ static int __devinit iTCO_wdt_probe(struct platform_device *dev)
>> ret = -EIO;
>> goto unreg_gcs;
>> }
>> - }
>> +
>> + iTCO_wdt_dev.max_timeout = iTCO_V2_MAX_TIMEOUT;
>> + } else
>> + iTCO_wdt_dev.max_timeout = iTCO_V1_MAX_TIMEOUT;
>>
>> /* Check chipset's NO_REBOOT bit */
>> if (iTCO_wdt_unset_NO_REBOOT_bit() && iTCO_vendor_check_noreboot_on()) {
>> @@ -606,24 +513,25 @@ static int __devinit iTCO_wdt_probe(struct platform_device *dev)
>> outw(0x0004, TCO2_STS); /* Clear BOOT_STS bit */
>>
>> /* Make sure the watchdog is not running */
>> - iTCO_wdt_stop();
>> + iTCO_wdt_stop(&iTCO_wdt_dev);
>>
>> /* Check that the heartbeat value is within it's range;
>> if not reset to the default */
>> - if (iTCO_wdt_set_heartbeat(heartbeat)) {
>> - iTCO_wdt_set_heartbeat(WATCHDOG_HEARTBEAT);
>> - pr_info("timeout value out of range, using %d\n", heartbeat);
>> + if (iTCO_wdt_set_timeout(&iTCO_wdt_dev, heartbeat_param)) {
>> + iTCO_wdt_set_timeout(&iTCO_wdt_dev, WATCHDOG_HEARTBEAT);
>> + pr_info("timeout value out of range, using %d\n", heartbeat_param);
>> }
>>
>> - ret = misc_register(&iTCO_wdt_miscdev);
>> + watchdog_set_nowayout(&iTCO_wdt_dev, nowayout);
>> +
>> + ret = watchdog_register_device(&iTCO_wdt_dev);
>> if (ret != 0) {
>> - pr_err("cannot register miscdev on minor=%d (err=%d)\n",
>> - WATCHDOG_MINOR, ret);
>> + pr_err("cannot register watchdog device (err=%d)\n", ret);
>> goto unreg_tco;
>> }
>>
>> pr_info("initialized. heartbeat=%d sec (nowayout=%d)\n",
>> - heartbeat, nowayout);
>> + heartbeat_param, nowayout);
>>
>> return 0;
>>
>> @@ -659,7 +567,7 @@ static int __devexit iTCO_wdt_remove(struct platform_device *dev)
>>
>> static void iTCO_wdt_shutdown(struct platform_device *dev)
>> {
>> - iTCO_wdt_stop();
>> + iTCO_wdt_stop(&iTCO_wdt_dev);
>> }
>>
>> static struct platform_driver iTCO_wdt_driver = {
>>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iTCO_wdt: move watchdog to proper kernel interface
2012-06-19 12:40 ` Hans de Goede
@ 2012-06-19 13:11 ` Tony Zelenoff
0 siblings, 0 replies; 6+ messages in thread
From: Tony Zelenoff @ 2012-06-19 13:11 UTC (permalink / raw)
To: Hans de Goede; +Cc: linux-watchdog@vger.kernel.org, wim@iguana.be
19/06/12 4:40 PM, Hans de Goede пишет:
> Hi,
>
> On 06/19/2012 12:20 PM, Tony Zelenoff wrote:
>> Ping.
>>
>> Is this patch wrong? Or just missed?
>
> Definitely not wrong (on the conceptual level I've not actually
> checked the code). We do want the iTCO_wdt driver to be converted
> to the (new) watchdog device framework.
>
> I think you just need to give Wim some more time to take a good
> look at this. Wim does the watchdog subsystem maintainer ship in
> his spare time, so sometimes you need to be a bit patient :)
:)
Of course i be patient. Just want to be sure, that patch is not missed.
Thanks for your reply.
> Regards,
>
> Hans
>
>
>
>>
>> 8/06/12 2:09 PM, Anton Zelenov пишет:
>>> Remove obsoleted fs interface and move to proper usage of
>>> watchdog_device interface.
>>>
>>> Signed-off-by: Tony Zelenoff <antonz@parallels.com>
>>> ---
>>> drivers/watchdog/iTCO_wdt.c | 246 ++++++++++++++-----------------------------
>>> 1 files changed, 77 insertions(+), 169 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/iTCO_wdt.c b/drivers/watchdog/iTCO_wdt.c
>>> index bc47e90..14e48d9 100644
>>> --- a/drivers/watchdog/iTCO_wdt.c
>>> +++ b/drivers/watchdog/iTCO_wdt.c
>>> @@ -87,9 +87,11 @@
>>> #define TCO2_CNT (TCOBASE + 0x0a) /* TCO2 Control Register */
>>> #define TCOv2_TMR (TCOBASE + 0x12) /* TCOv2 Timer Initial Value */
>>>
>>> +/* Maximum allowed timeouts for iTCO */
>>> +#define iTCO_V1_MAX_TIMEOUT 0x03f
>>> +#define iTCO_V2_MAX_TIMEOUT 0x3ff
>>> +
>>> /* internal variables */
>>> -static unsigned long is_active;
>>> -static char expect_release;
>>> static struct { /* this is private data for the iTCO_wdt device */
>>> /* TCO version/generation */
>>> unsigned int iTCO_version;
>>> @@ -107,9 +109,9 @@ static struct { /* this is private data for the iTCO_wdt device */
>>>
>>> /* module parameters */
>>> #define WATCHDOG_HEARTBEAT 30 /* 30 sec default heartbeat */
>>> -static int heartbeat = WATCHDOG_HEARTBEAT; /* in seconds */
>>> -module_param(heartbeat, int, 0);
>>> -MODULE_PARM_DESC(heartbeat, "Watchdog timeout in seconds. "
>>> +static unsigned int heartbeat_param = WATCHDOG_HEARTBEAT; /* in seconds */
>>> +module_param(heartbeat_param, uint, 0);
>>> +MODULE_PARM_DESC(heartbeat_param, "Watchdog timeout in seconds. "
>>> "5..76 (TCO v1) or 3..614 (TCO v2), default="
>>> __MODULE_STRING(WATCHDOG_HEARTBEAT) ")");
>>>
>>> @@ -178,13 +180,13 @@ static int iTCO_wdt_unset_NO_REBOOT_bit(void)
>>> return ret; /* returns: 0 = OK, -EIO = Error */
>>> }
>>>
>>> -static int iTCO_wdt_start(void)
>>> +static int iTCO_wdt_start(struct watchdog_device *w)
>>> {
>>> unsigned int val;
>>>
>>> spin_lock(&iTCO_wdt_private.io_lock);
>>>
>>> - iTCO_vendor_pre_start(iTCO_wdt_private.smi_res, heartbeat);
>>> + iTCO_vendor_pre_start(iTCO_wdt_private.smi_res, w->timeout);
>>>
>>> /* disable chipset's NO_REBOOT bit */
>>> if (iTCO_wdt_unset_NO_REBOOT_bit()) {
>>> @@ -212,7 +214,7 @@ static int iTCO_wdt_start(void)
>>> return 0;
>>> }
>>>
>>> -static int iTCO_wdt_stop(void)
>>> +static int iTCO_wdt_stop(struct watchdog_device *w)
>>> {
>>> unsigned int val;
>>>
>>> @@ -236,11 +238,11 @@ static int iTCO_wdt_stop(void)
>>> return 0;
>>> }
>>>
>>> -static int iTCO_wdt_keepalive(void)
>>> +static int iTCO_wdt_ping(struct watchdog_device *w)
>>> {
>>> spin_lock(&iTCO_wdt_private.io_lock);
>>>
>>> - iTCO_vendor_pre_keepalive(iTCO_wdt_private.smi_res, heartbeat);
>>> + iTCO_vendor_pre_keepalive(iTCO_wdt_private.smi_res, w->timeout);
>>>
>>> /* Reload the timer by writing to the TCO Timer Counter register */
>>> if (iTCO_wdt_private.iTCO_version == 2)
>>> @@ -257,7 +259,24 @@ static int iTCO_wdt_keepalive(void)
>>> return 0;
>>> }
>>>
>>> -static int iTCO_wdt_set_heartbeat(int t)
>>> +static inline int iTCO_wdt_timeout_valid(unsigned int t)
>>> +{
>>> + unsigned int timeout = 0;
>>> +
>>> + if (iTCO_wdt_private.iTCO_version == 2)
>>> + timeout = iTCO_V2_MAX_TIMEOUT;
>>> +
>>> + if (iTCO_wdt_private.iTCO_version == 1)
>>> + timeout = iTCO_V1_MAX_TIMEOUT;
>>> +
>>> + if (t > timeout)
>>> + return 0;
>>> +
>>> + return 1;
>>> +}
>>> +
>>> +static int iTCO_wdt_set_timeout(struct watchdog_device *w,
>>> + unsigned int t)
>>> {
>>> unsigned int val16;
>>> unsigned char val8;
>>> @@ -273,8 +292,8 @@ static int iTCO_wdt_set_heartbeat(int t)
>>> /* "Values of 0h-3h are ignored and should not be attempted" */
>>> if (tmrval < 0x04)
>>> return -EINVAL;
>>> - if (((iTCO_wdt_private.iTCO_version == 2) && (tmrval > 0x3ff)) ||
>>> - ((iTCO_wdt_private.iTCO_version == 1) && (tmrval > 0x03f)))
>>> +
>>> + if (!iTCO_wdt_timeout_valid(t))
>>> return -EINVAL;
>>>
>>> iTCO_vendor_pre_set_heartbeat(tmrval);
>>> @@ -304,11 +323,11 @@ static int iTCO_wdt_set_heartbeat(int t)
>>> return -EINVAL;
>>> }
>>>
>>> - heartbeat = t;
>>> + w->timeout = t;
>>> return 0;
>>> }
>>>
>>> -static int iTCO_wdt_get_timeleft(int *time_left)
>>> +static unsigned int iTCO_wdt_get_timeleft(struct watchdog_device *w)
>>> {
>>> unsigned int val16;
>>> unsigned char val8;
>>> @@ -320,8 +339,10 @@ static int iTCO_wdt_get_timeleft(int *time_left)
>>> val16 &= 0x3ff;
>>> spin_unlock(&iTCO_wdt_private.io_lock);
>>>
>>> - *time_left = (val16 * 6) / 10;
>>> - } else if (iTCO_wdt_private.iTCO_version == 1) {
>>> + return (val16 * 6) / 10;
>>> + }
>>> +
>>> + if (iTCO_wdt_private.iTCO_version == 1) {
>>> spin_lock(&iTCO_wdt_private.io_lock);
>>> val8 = inb(TCO_RLD);
>>> val8 &= 0x3f;
>>> @@ -329,156 +350,39 @@ static int iTCO_wdt_get_timeleft(int *time_left)
>>> val8 += (inb(TCOv1_TMR) & 0x3f);
>>> spin_unlock(&iTCO_wdt_private.io_lock);
>>>
>>> - *time_left = (val8 * 6) / 10;
>>> - } else
>>> - return -EINVAL;
>>> - return 0;
>>> -}
>>> -
>>> -/*
>>> - * /dev/watchdog handling
>>> - */
>>> -
>>> -static int iTCO_wdt_open(struct inode *inode, struct file *file)
>>> -{
>>> - /* /dev/watchdog can only be opened once */
>>> - if (test_and_set_bit(0, &is_active))
>>> - return -EBUSY;
>>> -
>>> - /*
>>> - * Reload and activate timer
>>> - */
>>> - iTCO_wdt_start();
>>> - return nonseekable_open(inode, file);
>>> -}
>>> -
>>> -static int iTCO_wdt_release(struct inode *inode, struct file *file)
>>> -{
>>> - /*
>>> - * Shut off the timer.
>>> - */
>>> - if (expect_release == 42) {
>>> - iTCO_wdt_stop();
>>> - } else {
>>> - pr_crit("Unexpected close, not stopping watchdog!\n");
>>> - iTCO_wdt_keepalive();
>>> - }
>>> - clear_bit(0, &is_active);
>>> - expect_release = 0;
>>> - return 0;
>>> -}
>>> -
>>> -static ssize_t iTCO_wdt_write(struct file *file, const char __user *data,
>>> - size_t len, loff_t *ppos)
>>> -{
>>> - /* See if we got the magic character 'V' and reload the timer */
>>> - if (len) {
>>> - if (!nowayout) {
>>> - size_t i;
>>> -
>>> - /* note: just in case someone wrote the magic
>>> - character five months ago... */
>>> - expect_release = 0;
>>> -
>>> - /* scan to see whether or not we got the
>>> - magic character */
>>> - for (i = 0; i != len; i++) {
>>> - char c;
>>> - if (get_user(c, data + i))
>>> - return -EFAULT;
>>> - if (c == 'V')
>>> - expect_release = 42;
>>> - }
>>> - }
>>> -
>>> - /* someone wrote to us, we should reload the timer */
>>> - iTCO_wdt_keepalive();
>>> - }
>>> - return len;
>>> -}
>>> -
>>> -static long iTCO_wdt_ioctl(struct file *file, unsigned int cmd,
>>> - unsigned long arg)
>>> -{
>>> - int new_options, retval = -EINVAL;
>>> - int new_heartbeat;
>>> - void __user *argp = (void __user *)arg;
>>> - int __user *p = argp;
>>> - static const struct watchdog_info ident = {
>>> - .options = WDIOF_SETTIMEOUT |
>>> - WDIOF_KEEPALIVEPING |
>>> - WDIOF_MAGICCLOSE,
>>> - .firmware_version = 0,
>>> - .identity = DRV_NAME,
>>> - };
>>> -
>>> - switch (cmd) {
>>> - case WDIOC_GETSUPPORT:
>>> - return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
>>> - case WDIOC_GETSTATUS:
>>> - case WDIOC_GETBOOTSTATUS:
>>> - return put_user(0, p);
>>> -
>>> - case WDIOC_SETOPTIONS:
>>> - {
>>> - if (get_user(new_options, p))
>>> - return -EFAULT;
>>> -
>>> - if (new_options & WDIOS_DISABLECARD) {
>>> - iTCO_wdt_stop();
>>> - retval = 0;
>>> - }
>>> - if (new_options & WDIOS_ENABLECARD) {
>>> - iTCO_wdt_keepalive();
>>> - iTCO_wdt_start();
>>> - retval = 0;
>>> - }
>>> - return retval;
>>> + return (val8 * 6) / 10;
>>> }
>>> - case WDIOC_KEEPALIVE:
>>> - iTCO_wdt_keepalive();
>>> - return 0;
>>>
>>> - case WDIOC_SETTIMEOUT:
>>> - {
>>> - if (get_user(new_heartbeat, p))
>>> - return -EFAULT;
>>> - if (iTCO_wdt_set_heartbeat(new_heartbeat))
>>> - return -EINVAL;
>>> - iTCO_wdt_keepalive();
>>> - /* Fall */
>>> - }
>>> - case WDIOC_GETTIMEOUT:
>>> - return put_user(heartbeat, p);
>>> - case WDIOC_GETTIMELEFT:
>>> - {
>>> - int time_left;
>>> - if (iTCO_wdt_get_timeleft(&time_left))
>>> - return -EINVAL;
>>> - return put_user(time_left, p);
>>> - }
>>> - default:
>>> - return -ENOTTY;
>>> - }
>>> + return -EINVAL;
>>> }
>>>
>>> /*
>>> * Kernel Interfaces
>>> */
>>>
>>> -static const struct file_operations iTCO_wdt_fops = {
>>> - .owner = THIS_MODULE,
>>> - .llseek = no_llseek,
>>> - .write = iTCO_wdt_write,
>>> - .unlocked_ioctl = iTCO_wdt_ioctl,
>>> - .open = iTCO_wdt_open,
>>> - .release = iTCO_wdt_release,
>>> +static const struct watchdog_info iTCO_wdt_info = {
>>> + .options = WDIOF_SETTIMEOUT |
>>> + WDIOF_KEEPALIVEPING |
>>> + WDIOF_MAGICCLOSE,
>>> + .firmware_version = 0,
>>> + .identity = DRV_NAME,
>>> +};
>>> +
>>> +static struct watchdog_ops iTCO_wdt_ops = {
>>> + .owner = THIS_MODULE,
>>> + .start = iTCO_wdt_start,
>>> + .stop = iTCO_wdt_stop,
>>> + .ping = iTCO_wdt_ping,
>>> + .set_timeout = iTCO_wdt_set_timeout,
>>> + .get_timeleft = iTCO_wdt_get_timeleft,
>>> };
>>>
>>> -static struct miscdevice iTCO_wdt_miscdev = {
>>> - .minor = WATCHDOG_MINOR,
>>> - .name = "watchdog",
>>> - .fops = &iTCO_wdt_fops,
>>> +static struct watchdog_device iTCO_wdt_dev = {
>>> + .info = &iTCO_wdt_info,
>>> + .ops = &iTCO_wdt_ops,
>>> + .min_timeout = 1,
>>> + /* It will be set up at _probe */
>>> + .max_timeout = 0xFFFF
>>> };
>>>
>>> /*
>>> @@ -489,10 +393,10 @@ static void __devexit iTCO_wdt_cleanup(void)
>>> {
>>> /* Stop the timer before we leave */
>>> if (!nowayout)
>>> - iTCO_wdt_stop();
>>> + iTCO_wdt_stop(&iTCO_wdt_dev);
>>>
>>> /* Deregister */
>>> - misc_deregister(&iTCO_wdt_miscdev);
>>> + watchdog_unregister_device(&iTCO_wdt_dev);
>>>
>>> /* release resources */
>>> release_region(iTCO_wdt_private.tco_res->start,
>>> @@ -559,7 +463,10 @@ static int __devinit iTCO_wdt_probe(struct platform_device *dev)
>>> ret = -EIO;
>>> goto unreg_gcs;
>>> }
>>> - }
>>> +
>>> + iTCO_wdt_dev.max_timeout = iTCO_V2_MAX_TIMEOUT;
>>> + } else
>>> + iTCO_wdt_dev.max_timeout = iTCO_V1_MAX_TIMEOUT;
>>>
>>> /* Check chipset's NO_REBOOT bit */
>>> if (iTCO_wdt_unset_NO_REBOOT_bit() && iTCO_vendor_check_noreboot_on()) {
>>> @@ -606,24 +513,25 @@ static int __devinit iTCO_wdt_probe(struct platform_device *dev)
>>> outw(0x0004, TCO2_STS); /* Clear BOOT_STS bit */
>>>
>>> /* Make sure the watchdog is not running */
>>> - iTCO_wdt_stop();
>>> + iTCO_wdt_stop(&iTCO_wdt_dev);
>>>
>>> /* Check that the heartbeat value is within it's range;
>>> if not reset to the default */
>>> - if (iTCO_wdt_set_heartbeat(heartbeat)) {
>>> - iTCO_wdt_set_heartbeat(WATCHDOG_HEARTBEAT);
>>> - pr_info("timeout value out of range, using %d\n", heartbeat);
>>> + if (iTCO_wdt_set_timeout(&iTCO_wdt_dev, heartbeat_param)) {
>>> + iTCO_wdt_set_timeout(&iTCO_wdt_dev, WATCHDOG_HEARTBEAT);
>>> + pr_info("timeout value out of range, using %d\n", heartbeat_param);
>>> }
>>>
>>> - ret = misc_register(&iTCO_wdt_miscdev);
>>> + watchdog_set_nowayout(&iTCO_wdt_dev, nowayout);
>>> +
>>> + ret = watchdog_register_device(&iTCO_wdt_dev);
>>> if (ret != 0) {
>>> - pr_err("cannot register miscdev on minor=%d (err=%d)\n",
>>> - WATCHDOG_MINOR, ret);
>>> + pr_err("cannot register watchdog device (err=%d)\n", ret);
>>> goto unreg_tco;
>>> }
>>>
>>> pr_info("initialized. heartbeat=%d sec (nowayout=%d)\n",
>>> - heartbeat, nowayout);
>>> + heartbeat_param, nowayout);
>>>
>>> return 0;
>>>
>>> @@ -659,7 +567,7 @@ static int __devexit iTCO_wdt_remove(struct platform_device *dev)
>>>
>>> static void iTCO_wdt_shutdown(struct platform_device *dev)
>>> {
>>> - iTCO_wdt_stop();
>>> + iTCO_wdt_stop(&iTCO_wdt_dev);
>>> }
>>>
>>> static struct platform_driver iTCO_wdt_driver = {
>>>
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iTCO_wdt: move watchdog to proper kernel interface
2012-06-08 10:09 [PATCH] iTCO_wdt: move watchdog to proper kernel interface Tony Zelenoff
2012-06-08 10:09 ` [PATCH] iTCO_wdt: remove unnecessary includes Tony Zelenoff
2012-06-19 10:20 ` [PATCH] iTCO_wdt: move watchdog to proper kernel interface Tony Zelenoff
@ 2012-06-26 18:45 ` Wim Van Sebroeck
2 siblings, 0 replies; 6+ messages in thread
From: Wim Van Sebroeck @ 2012-06-26 18:45 UTC (permalink / raw)
To: Tony Zelenoff; +Cc: linux-watchdog
Hi Tony,
> Remove obsoleted fs interface and move to proper usage of
> watchdog_device interface.
>
> Signed-off-by: Tony Zelenoff <antonz@parallels.com>
Code seems OK, but as I allready told other people: I have allready the code
for converting the iTCO_wdt driver (I tested the generic framework with it).
But I had to stall it because of the lpc_ich conversion.
Anyway: my code is in linux-next now. I will be adding some extra patches in
the coming days also.
Kind regards,
Wim.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-06-26 18:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-08 10:09 [PATCH] iTCO_wdt: move watchdog to proper kernel interface Tony Zelenoff
2012-06-08 10:09 ` [PATCH] iTCO_wdt: remove unnecessary includes Tony Zelenoff
2012-06-19 10:20 ` [PATCH] iTCO_wdt: move watchdog to proper kernel interface Tony Zelenoff
2012-06-19 12:40 ` Hans de Goede
2012-06-19 13:11 ` Tony Zelenoff
2012-06-26 18:45 ` Wim Van Sebroeck
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.