On Mon, Oct 16, 2017 at 07:25:26PM +0100, Radu Rendec wrote:
> Change the i6300esb driver to use the watchdog subsystem instead of the
> legacy watchdog API. This is mainly just getting rid of the "write" and
> "ioctl" methods and part of the watchdog control logic (which are all
> implemented by the watchdog subsystem).
>
> Even though the watchdog subsystem supports registering and handling
> multiple watchdog devices at the same time, the i6300esb driver still
> has a limitation of only one i6300esb device due to some global variable
> usage that comes from the original design. However, the driver can now
> coexist with other watchdog devices (supported by different drivers).
>
> Signed-off-by: Radu Rendec <rrendec@xxxxxxxxxx>
> ---
> drivers/watchdog/i6300esb.c | 191 +++++++++-----------------------------------
> 1 file changed, 39 insertions(+), 152 deletions(-)
>
> diff --git a/drivers/watchdog/i6300esb.c b/drivers/watchdog/i6300esb.c
> index d7befd58b391..1df41a09553c 100644
> --- a/drivers/watchdog/i6300esb.c
> +++ b/drivers/watchdog/i6300esb.c
> @@ -21,6 +21,8 @@
> * Version 0.02
> * 20050210 David Härdeman <david@xxxxxxxx>
> * Ported driver to kernel 2.6
> + * 20171016 Radu Rendec <rrendec@xxxxxxxxxx>
> + * Change driver to use the watchdog subsystem
> */
>
> /*
> @@ -77,10 +79,7 @@
> /* internal variables */
> static void __iomem *BASEADDR;
> static DEFINE_SPINLOCK(esb_lock); /* Guards the hardware */
> -static unsigned long timer_alive;
> static struct pci_dev *esb_pci;
> -static unsigned short triggered; /* The status of the watchdog upon boot */
> -static char esb_expect_close;
>
> /* We can only use 1 card due to the /dev/watchdog restriction */
> static int cards_found;
> @@ -116,21 +115,22 @@ static inline void esb_unlock_registers(void)
> writew(ESB_UNLOCK2, ESB_RELOAD_REG);
> }
>
> -static int esb_timer_start(void)
> +static int esb_timer_start(struct watchdog_device *wdd)
> {
> + int _wdd_nowayout = test_bit(WDOG_NO_WAY_OUT, &wdd->status);
> u8 val;
>
> spin_lock(&esb_lock);
> esb_unlock_registers();
> writew(ESB_WDT_RELOAD, ESB_RELOAD_REG);
> /* Enable or Enable + Lock? */
> - val = ESB_WDT_ENABLE | (nowayout ? ESB_WDT_LOCK : 0x00);
> + val = ESB_WDT_ENABLE | (_wdd_nowayout ? ESB_WDT_LOCK : 0x00);
> pci_write_config_byte(esb_pci, ESB_LOCK_REG, val);
> spin_unlock(&esb_lock);
> return 0;
> }
>
> -static int esb_timer_stop(void)
> +static int esb_timer_stop(struct watchdog_device *wdd)
> {
> u8 val;
>
> @@ -147,22 +147,21 @@ static int esb_timer_stop(void)
> return val & ESB_WDT_ENABLE;
> }
>
> -static void esb_timer_keepalive(void)
> +static int esb_timer_keepalive(struct watchdog_device *wdd)
> {
> spin_lock(&esb_lock);
> esb_unlock_registers();
> writew(ESB_WDT_RELOAD, ESB_RELOAD_REG);
> /* FIXME: Do we need to flush anything here? */
> spin_unlock(&esb_lock);
> + return 0;
> }
>
> -static int esb_timer_set_heartbeat(int time)
> +static int esb_timer_set_heartbeat(struct watchdog_device *wdd,
> + unsigned int time)
> {
> u32 val;
>
> - if (time < 0x1 || time > (2 * 0x03ff))
> - return -EINVAL;
> -
> spin_lock(&esb_lock);
>
> /* We shift by 9, so if we are passed a value of 1 sec,
> @@ -186,148 +185,33 @@ static int esb_timer_set_heartbeat(int time)
> /* FIXME: Do we need to flush everything out? */
>
> /* Done */
> - heartbeat = time;
> spin_unlock(&esb_lock);
Needs to set wdt->timeout.
> return 0;
> }
>
> /*
> - * /dev/watchdog handling
> + * Watchdog Subsystem Interfaces
> */
>
> -static int esb_open(struct inode *inode, struct file *file)
> -{
> - /* /dev/watchdog can only be opened once */
> - if (test_and_set_bit(0, &timer_alive))
> - return -EBUSY;
> -
> - /* Reload and activate timer */
> - esb_timer_start();
> -
> - return nonseekable_open(inode, file);
> -}
> -
> -static int esb_release(struct inode *inode, struct file *file)
> -{
> - /* Shut off the timer. */
> - if (esb_expect_close == 42)
> - esb_timer_stop();
> - else {
> - pr_crit("Unexpected close, not stopping watchdog!\n");
> - esb_timer_keepalive();
> - }
> - clear_bit(0, &timer_alive);
> - esb_expect_close = 0;
> - return 0;
> -}
> -
> -static ssize_t esb_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... */
> - esb_expect_close = 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')
> - esb_expect_close = 42;
> - }
> - }
> -
> - /* someone wrote to us, we should reload the timer */
> - esb_timer_keepalive();
> - }
> - return len;
> -}
> -
> -static long esb_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 = ESB_MODULE_NAME,
> - };
> -
> - switch (cmd) {
> - case WDIOC_GETSUPPORT:
> - return copy_to_user(argp, &ident,
> - sizeof(ident)) ? -EFAULT : 0;
> -
> - case WDIOC_GETSTATUS:
> - return put_user(0, p);
> -
> - case WDIOC_GETBOOTSTATUS:
> - return put_user(triggered, p);
> -
> - case WDIOC_SETOPTIONS:
> - {
> - if (get_user(new_options, p))
> - return -EFAULT;
> -
> - if (new_options & WDIOS_DISABLECARD) {
> - esb_timer_stop();
> - retval = 0;
> - }
> -
> - if (new_options & WDIOS_ENABLECARD) {
> - esb_timer_start();
> - retval = 0;
> - }
> - return retval;
> - }
> - case WDIOC_KEEPALIVE:
> - esb_timer_keepalive();
> - return 0;
> -
> - case WDIOC_SETTIMEOUT:
> - {
> - if (get_user(new_heartbeat, p))
> - return -EFAULT;
> - if (esb_timer_set_heartbeat(new_heartbeat))
> - return -EINVAL;
> - esb_timer_keepalive();
> - /* Fall */
> - }
> - case WDIOC_GETTIMEOUT:
> - return put_user(heartbeat, p);
> - default:
> - return -ENOTTY;
> - }
> -}
> -
> -/*
> - * Kernel Interfaces
> - */
> +static struct watchdog_info esb_info = {
> + .identity = ESB_MODULE_NAME,
> + .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
> +};
>
> -static const struct file_operations esb_fops = {
> +static const struct watchdog_ops esb_ops = {
> .owner = THIS_MODULE,
> - .llseek = no_llseek,
> - .write = esb_write,
> - .unlocked_ioctl = esb_ioctl,
> - .open = esb_open,
> - .release = esb_release,
> + .start = esb_timer_start,
> + .stop = esb_timer_stop,
> + .set_timeout = esb_timer_set_heartbeat,
> + .ping = esb_timer_keepalive,
> };
>
> -static struct miscdevice esb_miscdev = {
> - .minor = WATCHDOG_MINOR,
> - .name = "watchdog",
> - .fops = &esb_fops,
> +static struct watchdog_device esb_dev = {
> + .info = &esb_info,
> + .ops = &esb_ops,
> + .min_timeout = 1,
> + .max_timeout = 2 * 0x03ff,
> + .timeout = WATCHDOG_HEARTBEAT,
> };
>
> /*
> @@ -405,14 +289,14 @@ static void esb_initdevice(void)
> esb_unlock_registers();
> val2 = readw(ESB_RELOAD_REG);
> if (val2 & ESB_WDT_TIMEOUT)
> - triggered = WDIOF_CARDRESET;
> + esb_dev.bootstatus = WDIOF_CARDRESET;
>
> /* Reset WDT_TIMEOUT flag and timers */
> esb_unlock_registers();
> writew((ESB_WDT_TIMEOUT | ESB_WDT_RELOAD), ESB_RELOAD_REG);
>
> /* And set the correct timeout value */
> - esb_timer_set_heartbeat(heartbeat);
> + esb_timer_set_heartbeat(&esb_dev, esb_dev.timeout);
> }
>
> static int esb_probe(struct pci_dev *pdev,
> @@ -443,17 +327,18 @@ static int esb_probe(struct pci_dev *pdev,
> }
>
> /* Initialize the watchdog and make sure it does not run */
> + watchdog_init_timeout(&esb_dev, heartbeat, NULL);
> + watchdog_set_nowayout(&esb_dev, nowayout);
> esb_initdevice();
>
> /* Register the watchdog so that userspace has access to it */
> - ret = misc_register(&esb_miscdev);
> + ret = watchdog_register_device(&esb_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);
dev_err()
> goto err_unmap;
> }
> pr_info("initialized (0x%p). heartbeat=%d sec (nowayout=%d)\n",
dev_info()
> - BASEADDR, heartbeat, nowayout);
> + BASEADDR, esb_dev.timeout, nowayout);
> return 0;
>
> err_unmap:
> @@ -466,12 +351,14 @@ static int esb_probe(struct pci_dev *pdev,
>
> static void esb_remove(struct pci_dev *pdev)
> {
> + int _wdd_nowayout = test_bit(WDOG_NO_WAY_OUT, &esb_dev.status);
> +
> /* Stop the timer before we leave */
> - if (!nowayout)
> - esb_timer_stop();
> + if (!_wdd_nowayout)
> + esb_timer_stop(&esb_dev);
Can you call watchdog_stop_on_unregister() in probe instead ?
>
> /* Deregister */
> - misc_deregister(&esb_miscdev);
> + watchdog_unregister_device(&esb_dev);
> iounmap(BASEADDR);
> pci_release_region(esb_pci, 0);
> pci_disable_device(esb_pci);
> @@ -480,7 +367,7 @@ static void esb_remove(struct pci_dev *pdev)
>
> static void esb_shutdown(struct pci_dev *pdev)
> {
> - esb_timer_stop();
> + esb_timer_stop(&esb_dev);
Call watchdog_stop_on_reboot() in probe ?
> }
>
> static struct pci_driver esb_driver = {
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
