From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from relay.parallels.com ([195.214.232.42]:48672 "EHLO relay.parallels.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751207Ab2FSKUs convert rfc822-to-8bit (ORCPT ); Tue, 19 Jun 2012 06:20:48 -0400 Message-ID: <4FE05279.7080301@parallels.com> Date: Tue, 19 Jun 2012 14:20:41 +0400 From: Tony Zelenoff MIME-Version: 1.0 To: Anton Zelenov CC: "linux-watchdog@vger.kernel.org" , "wim@iguana.be" Subject: Re: [PATCH] iTCO_wdt: move watchdog to proper kernel interface References: <1339150154-25247-1-git-send-email-antonz@parallels.com> In-Reply-To: <1339150154-25247-1-git-send-email-antonz@parallels.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Sender: linux-watchdog-owner@vger.kernel.org List-Id: linux-watchdog@vger.kernel.org Content-Transfer-Encoding: quoted-printable Ping. Is this patch wrong? Or just missed? 8/06/12 2:09 PM, Anton Zelenov =D0=BF=D0=B8=D1=88=D0=B5=D1=82: > Remove obsoleted fs interface and move to proper usage of > watchdog_device interface. > > Signed-off-by: Tony Zelenoff > --- > 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 iT= CO_wdt device */ > > /* module parameters */ > #define WATCHDOG_HEARTBEAT 30 /* 30 sec default heartbeat */ > -static int heartbeat =3D WATCHDOG_HEARTBEAT; /* in seconds */ > -module_param(heartbeat, int, 0); > -MODULE_PARM_DESC(heartbeat, "Watchdog timeout in seconds. " > +static unsigned int heartbeat_param =3D WATCHDOG_HEARTBEAT; /* in sec= onds */ > +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=3D" > __MODULE_STRING(WATCHDOG_HEARTBEAT) ")"); > > @@ -178,13 +180,13 @@ static int iTCO_wdt_unset_NO_REBOOT_bit(void) > return ret; /* returns: 0 =3D OK, -EIO =3D 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 =3D=3D 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 =3D 0; > + > + if (iTCO_wdt_private.iTCO_version =3D=3D 2) > + timeout =3D iTCO_V2_MAX_TIMEOUT; > + > + if (iTCO_wdt_private.iTCO_version =3D=3D 1) > + timeout =3D 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 =3D=3D 2) && (tmrval > 0x3ff)) || > - ((iTCO_wdt_private.iTCO_version =3D=3D 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 =3D t; > + w->timeout =3D 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 &=3D 0x3ff; > spin_unlock(&iTCO_wdt_private.io_lock); > > - *time_left =3D (val16 * 6) / 10; > - } else if (iTCO_wdt_private.iTCO_version =3D=3D 1) { > + return (val16 * 6) / 10; > + } > + > + if (iTCO_wdt_private.iTCO_version =3D=3D 1) { > spin_lock(&iTCO_wdt_private.io_lock); > val8 =3D inb(TCO_RLD); > val8 &=3D 0x3f; > @@ -329,156 +350,39 @@ static int iTCO_wdt_get_timeleft(int *time_left) > val8 +=3D (inb(TCOv1_TMR) & 0x3f); > spin_unlock(&iTCO_wdt_private.io_lock); > > - *time_left =3D (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 =3D=3D 42) { > - iTCO_wdt_stop(); > - } else { > - pr_crit("Unexpected close, not stopping watchdog!\n"); > - iTCO_wdt_keepalive(); > - } > - clear_bit(0, &is_active); > - expect_release =3D 0; > - return 0; > -} > - > -static ssize_t iTCO_wdt_write(struct file *file, const char __user *da= ta, > - 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 =3D 0; > - > - /* scan to see whether or not we got the > - magic character */ > - for (i =3D 0; i !=3D len; i++) { > - char c; > - if (get_user(c, data + i)) > - return -EFAULT; > - if (c =3D=3D 'V') > - expect_release =3D 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 =3D -EINVAL; > - int new_heartbeat; > - void __user *argp =3D (void __user *)arg; > - int __user *p =3D argp; > - static const struct watchdog_info ident =3D { > - .options =3D WDIOF_SETTIMEOUT | > - WDIOF_KEEPALIVEPING | > - WDIOF_MAGICCLOSE, > - .firmware_version =3D 0, > - .identity =3D 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 =3D 0; > - } > - if (new_options & WDIOS_ENABLECARD) { > - iTCO_wdt_keepalive(); > - iTCO_wdt_start(); > - retval =3D 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 =3D { > - .owner =3D THIS_MODULE, > - .llseek =3D no_llseek, > - .write =3D iTCO_wdt_write, > - .unlocked_ioctl =3D iTCO_wdt_ioctl, > - .open =3D iTCO_wdt_open, > - .release =3D iTCO_wdt_release, > +static const struct watchdog_info iTCO_wdt_info =3D { > + .options =3D WDIOF_SETTIMEOUT | > + WDIOF_KEEPALIVEPING | > + WDIOF_MAGICCLOSE, > + .firmware_version =3D 0, > + .identity =3D DRV_NAME, > +}; > + > +static struct watchdog_ops iTCO_wdt_ops =3D { > + .owner =3D THIS_MODULE, > + .start =3D iTCO_wdt_start, > + .stop =3D iTCO_wdt_stop, > + .ping =3D iTCO_wdt_ping, > + .set_timeout =3D iTCO_wdt_set_timeout, > + .get_timeleft =3D iTCO_wdt_get_timeleft, > }; > > -static struct miscdevice iTCO_wdt_miscdev =3D { > - .minor =3D WATCHDOG_MINOR, > - .name =3D "watchdog", > - .fops =3D &iTCO_wdt_fops, > +static struct watchdog_device iTCO_wdt_dev =3D { > + .info =3D &iTCO_wdt_info, > + .ops =3D &iTCO_wdt_ops, > + .min_timeout =3D 1, > + /* It will be set up at _probe */ > + .max_timeout =3D 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 platfor= m_device *dev) > ret =3D -EIO; > goto unreg_gcs; > } > - } > + > + iTCO_wdt_dev.max_timeout =3D iTCO_V2_MAX_TIMEOUT; > + } else > + iTCO_wdt_dev.max_timeout =3D 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 platfo= rm_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 =3D misc_register(&iTCO_wdt_miscdev); > + watchdog_set_nowayout(&iTCO_wdt_dev, nowayout); > + > + ret =3D watchdog_register_device(&iTCO_wdt_dev); > if (ret !=3D 0) { > - pr_err("cannot register miscdev on minor=3D%d (err=3D%d)\n", > - WATCHDOG_MINOR, ret); > + pr_err("cannot register watchdog device (err=3D%d)\n", ret); > goto unreg_tco; > } > > pr_info("initialized. heartbeat=3D%d sec (nowayout=3D%d)\n", > - heartbeat, nowayout); > + heartbeat_param, nowayout); > > return 0; > > @@ -659,7 +567,7 @@ static int __devexit iTCO_wdt_remove(struct platfor= m_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 =3D { > -- 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