All of lore.kernel.org
 help / color / mirror / Atom feed
From: Radu Rendec <rrendec@arista.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Wim Van Sebroeck <wim@iguana.be>, linux-watchdog@vger.kernel.org
Subject: [PATCH v2 1/4] watchdog: i6300esb: use the watchdog subsystem
Date: Wed, 25 Oct 2017 16:39:11 +0100	[thread overview]
Message-ID: <20171025153914.3467-1-rrendec@arista.com> (raw)
In-Reply-To: <20171022170947.GA6997@roeck-us.net>

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@arista.com>
---
 drivers/watchdog/i6300esb.c | 222 ++++++++++----------------------------------
 1 file changed, 47 insertions(+), 175 deletions(-)

diff --git a/drivers/watchdog/i6300esb.c b/drivers/watchdog/i6300esb.c
index d7befd58b391..6e8578dd5dab 100644
--- a/drivers/watchdog/i6300esb.c
+++ b/drivers/watchdog/i6300esb.c
@@ -21,6 +21,8 @@
  *	Version 0.02
  *  20050210 David Härdeman <david@2gen.com>
  *	Ported driver to kernel 2.6
+ *  20171016 Radu Rendec <rrendec@arista.com>
+ *	Change driver to use the watchdog subsystem
  */
 
 /*
@@ -44,7 +46,6 @@
 /* Module and version information */
 #define ESB_VERSION "0.05"
 #define ESB_MODULE_NAME "i6300ESB timer"
-#define ESB_DRIVER_NAME ESB_MODULE_NAME ", v" ESB_VERSION
 
 /* PCI configuration registers */
 #define ESB_CONFIG_REG  0x60            /* Config register                   */
@@ -77,10 +78,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;
@@ -88,7 +86,7 @@ static int cards_found;
 /* module parameters */
 /* 30 sec default heartbeat (1 < heartbeat < 2*1023) */
 #define WATCHDOG_HEARTBEAT 30
-static int heartbeat = WATCHDOG_HEARTBEAT;  /* in seconds */
+static int heartbeat; /* in seconds */
 module_param(heartbeat, int, 0);
 MODULE_PARM_DESC(heartbeat,
 		"Watchdog heartbeat in seconds. (1<heartbeat<2046, default="
@@ -116,21 +114,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 +146,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 +184,34 @@ static int esb_timer_set_heartbeat(int time)
 	/* FIXME: Do we need to flush everything out? */
 
 	/* Done */
-	heartbeat = time;
+	wdd->timeout = time;
 	spin_unlock(&esb_lock);
 	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,
 };
 
 /*
@@ -346,19 +230,19 @@ MODULE_DEVICE_TABLE(pci, esb_pci_tbl);
 static unsigned char esb_getdevice(struct pci_dev *pdev)
 {
 	if (pci_enable_device(pdev)) {
-		pr_err("failed to enable device\n");
+		dev_err(&pdev->dev, "failed to enable device\n");
 		goto err_devput;
 	}
 
 	if (pci_request_region(pdev, 0, ESB_MODULE_NAME)) {
-		pr_err("failed to request region\n");
+		dev_err(&pdev->dev, "failed to request region\n");
 		goto err_disable;
 	}
 
 	BASEADDR = pci_ioremap_bar(pdev, 0);
 	if (BASEADDR == NULL) {
 		/* Something's wrong here, BASEADDR has to be set */
-		pr_err("failed to get BASEADDR\n");
+		dev_err(&pdev->dev, "failed to get BASEADDR\n");
 		goto err_release;
 	}
 
@@ -396,7 +280,7 @@ static void esb_initdevice(void)
 	/* Check that the WDT isn't already locked */
 	pci_read_config_byte(esb_pci, ESB_LOCK_REG, &val1);
 	if (val1 & ESB_WDT_LOCK)
-		pr_warn("nowayout already set\n");
+		dev_warn(&esb_pci->dev, "nowayout already set\n");
 
 	/* Set the timer to watchdog mode and disable it for now */
 	pci_write_config_byte(esb_pci, ESB_LOCK_REG, 0x00);
@@ -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,
@@ -434,26 +318,25 @@ static int esb_probe(struct pci_dev *pdev,
 	if (!esb_getdevice(pdev) || esb_pci == NULL)
 		return -ENODEV;
 
-	/* Check that the heartbeat value is within it's range;
-	   if not reset to the default */
-	if (heartbeat < 0x1 || heartbeat > 2 * 0x03ff) {
-		heartbeat = WATCHDOG_HEARTBEAT;
-		pr_info("heartbeat value must be 1<heartbeat<2046, using %d\n",
-			heartbeat);
-	}
-
 	/* Initialize the watchdog and make sure it does not run */
+	if (watchdog_init_timeout(&esb_dev, heartbeat, NULL))
+		pr_info("heartbeat value must be 1<heartbeat<2046, using %u\n",
+			esb_dev.timeout);
+	watchdog_set_nowayout(&esb_dev, nowayout);
+	watchdog_stop_on_reboot(&esb_dev);
+	watchdog_stop_on_unregister(&esb_dev);
 	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);
+		dev_err(&pdev->dev,
+			"cannot register watchdog device (err=%d)\n", ret);
 		goto err_unmap;
 	}
-	pr_info("initialized (0x%p). heartbeat=%d sec (nowayout=%d)\n",
-		BASEADDR, heartbeat, nowayout);
+	dev_info(&pdev->dev,
+		"initialized (0x%p). heartbeat=%d sec (nowayout=%d)\n",
+		BASEADDR, esb_dev.timeout, nowayout);
 	return 0;
 
 err_unmap:
@@ -466,29 +349,18 @@ static int esb_probe(struct pci_dev *pdev,
 
 static void esb_remove(struct pci_dev *pdev)
 {
-	/* Stop the timer before we leave */
-	if (!nowayout)
-		esb_timer_stop();
-
-	/* Deregister */
-	misc_deregister(&esb_miscdev);
+	watchdog_unregister_device(&esb_dev);
 	iounmap(BASEADDR);
 	pci_release_region(esb_pci, 0);
 	pci_disable_device(esb_pci);
 	esb_pci = NULL;
 }
 
-static void esb_shutdown(struct pci_dev *pdev)
-{
-	esb_timer_stop();
-}
-
 static struct pci_driver esb_driver = {
 	.name		= ESB_MODULE_NAME,
 	.id_table	= esb_pci_tbl,
 	.probe          = esb_probe,
 	.remove         = esb_remove,
-	.shutdown       = esb_shutdown,
 };
 
 module_pci_driver(esb_driver);
-- 
2.13.6

  reply	other threads:[~2017-10-25 15:39 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-16 18:25 [PATCH 1/3] watchdog: i6300esb: use the watchdog subsystem Radu Rendec
2017-10-16 18:25 ` [PATCH 2/3] watchdog: i6300esb: support multiple devices Radu Rendec
2017-10-22 17:05   ` [2/3] " Guenter Roeck
2017-10-16 18:25 ` [PATCH 3/3] watchdog: i6300esb: do not hardcode heartbeat limits Radu Rendec
2017-10-22 17:09   ` [3/3] " Guenter Roeck
2017-10-25 15:39     ` Radu Rendec [this message]
2017-10-25 20:51       ` [PATCH v2 1/4] watchdog: i6300esb: use the watchdog subsystem Guenter Roeck
2017-10-25 15:39     ` [PATCH v2 2/4] watchdog: i6300esb: support multiple devices Radu Rendec
2017-10-25 20:55       ` Guenter Roeck
2017-10-26 11:35         ` Radu Rendec
2017-10-26 13:51           ` Guenter Roeck
2017-10-26 16:10             ` [PATCH v3 1/4] watchdog: i6300esb: use the watchdog subsystem Radu Rendec
2017-10-27  1:03               ` Guenter Roeck
2017-10-26 16:10             ` [PATCH v3 2/4] watchdog: i6300esb: support multiple devices Radu Rendec
2017-10-27  1:02               ` Guenter Roeck
2017-10-27  1:04               ` Guenter Roeck
2017-10-26 16:10             ` [PATCH v3 3/4] watchdog: i6300esb: do not hardcode heartbeat limits Radu Rendec
2017-10-27  1:04               ` Guenter Roeck
2017-10-26 16:10             ` [PATCH v3 4/4] watchdog: i6300esb: remove info message and version number Radu Rendec
2017-10-27  1:05               ` Guenter Roeck
2017-10-27 10:58                 ` Radu Rendec
2017-10-25 15:39     ` [PATCH v2 3/4] watchdog: i6300esb: do not hardcode heartbeat limits Radu Rendec
2017-10-25 20:56       ` Guenter Roeck
2017-10-25 15:39     ` [PATCH v2 4/4] watchdog: i6300esb: bump version to 0.6 Radu Rendec
2017-10-25 20:57       ` Guenter Roeck
2017-10-22 17:02 ` [1/3] watchdog: i6300esb: use the watchdog subsystem Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171025153914.3467-1-rrendec@arista.com \
    --to=rrendec@arista.com \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=wim@iguana.be \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.