watchdog: Separate and maintain variables based on variable lifetime

All variables required by the watchdog core to manage a watchdog are
currently stored in struct watchdog_device. The lifetime of those
variables is determined by the watchdog driver. However, the lifetime
of variables used by the watchdog core differs from the lifetime of
struct watchdog_device. To remedy this situation, watchdog drivers
can implement ref and unref callbacks, to be used by the watchdog
core to lock struct watchdog_device in memory.

While this solves the immediate problem, it depends on watchdog drivers
to actually implement the ref/unref callbacks. This is error prone,
often not implemented in the first place, or not implemented correctly.

To solve the problem without requiring driver support, split the variables
in struct watchdog_device into two data structures - one for variables
associated with the watchdog driver, one for variables associated with
the watchdog core. With this approach, the watchdog core can keep track
of its variable lifetime and no longer depends on ref/unref callbacks
in the driver. As a side effect, some of the variables originally in
struct watchdog_driver are now private to the watchdog core and no longer
visible in watchdog drivers.

As a side effect of the changes made, an ioctl will now always fail
with -ENODEV after a watchdog device was unregistered with the character
device still open. Previously, it would only fail with -ENODEV in some
situations. Also, ioctl operations are now atomic from driver perspective.
With this change, it is now guaranteed that the driver will not unregister
a watchdog between a timeout change and the subsequent ping.

The 'ref' and 'unref' callbacks in struct watchdog_driver are no longer
used and marked as deprecated.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Wim Van Sebroeck <wim@iguana.be>

2 files changed, 203 insertions, 182 deletions

diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index f0293f7d2b80..ec1ab6c1a80b 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -210,8 +210,6 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
 	 * corrupted in a later stage then we expect a kernel panic!
 	 */
 
-	mutex_init(&wdd->lock);
-
 	/* Use alias for watchdog id if possible */
 	if (wdd->parent) {
 		ret = of_alias_get_id(wdd->parent->of_node, "watchdog");
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 7ba3fc6157c7..3cab6f6e7f1c 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -32,27 +32,51 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/module.h>	/* For module stuff/... */
-#include <linux/types.h>	/* For standard types (like size_t) */
+#include <linux/cdev.h>		/* For character device */
 #include <linux/errno.h>	/* For the -ENODEV/... values */
-#include <linux/kernel.h>	/* For printk/panic/... */
 #include <linux/fs.h>		/* For file operations */
-#include <linux/watchdog.h>	/* For watchdog specific items */
-#include <linux/miscdevice.h>	/* For handling misc devices */
 #include <linux/init.h>		/* For __init/__exit/... */
+#include <linux/kernel.h>	/* For printk/panic/... */
+#include <linux/kref.h>		/* For data references */
+#include <linux/miscdevice.h>	/* For handling misc devices */
+#include <linux/module.h>	/* For module stuff/... */
+#include <linux/mutex.h>	/* For mutexes */
+#include <linux/slab.h>		/* For memory functions */
+#include <linux/types.h>	/* For standard types (like size_t) */
+#include <linux/watchdog.h>	/* For watchdog specific items */
 #include <linux/uaccess.h>	/* For copy_to_user/put_user/... */
 
 #include "watchdog_core.h"
 
+/*
+ * struct watchdog_core_data - watchdog core internal data
+ * @kref:	Reference count.
+ * @cdev:	The watchdog's Character device.
+ * @wdd:	Pointer to watchdog device.
+ * @lock:	Lock for watchdog core.
+ * @status:	Watchdog core internal status bits.
+ */
+struct watchdog_core_data {
+	struct kref kref;
+	struct cdev cdev;
+	struct watchdog_device *wdd;
+	struct mutex lock;
+	unsigned long status;		/* Internal status bits */
+#define _WDOG_DEV_OPEN		0	/* Opened ? */
+#define _WDOG_ALLOW_RELEASE	1	/* Did we receive the magic char ? */
+};
+
 /* the dev_t structure to store the dynamically allocated watchdog devices */
 static dev_t watchdog_devt;
-/* the watchdog device behind /dev/watchdog */
-static struct watchdog_device *old_wdd;
+/* Reference to watchdog device behind /dev/watchdog */
+static struct watchdog_core_data *old_wd_data;
 
 /*
  *	watchdog_ping: ping the watchdog.
  *	@wdd: the watchdog device to ping
  *
+ *	The caller must hold wd_data->lock.
+ *
  *	If the watchdog has no own ping operation then it needs to be
  *	restarted via the start operation. This wrapper function does
  *	exactly that.
@@ -61,25 +85,16 @@ static struct watchdog_device *old_wdd;
 
 static int watchdog_ping(struct watchdog_device *wdd)
 {
-	int err = 0;
-
-	mutex_lock(&wdd->lock);
-
-	if (test_bit(WDOG_UNREGISTERED, &wdd->status)) {
-		err = -ENODEV;
-		goto out_ping;
-	}
+	int err;
 
 	if (!watchdog_active(wdd))
-		goto out_ping;
+		return 0;
 
 	if (wdd->ops->ping)
 		err = wdd->ops->ping(wdd);	/* ping the watchdog */
 	else
 		err = wdd->ops->start(wdd);	/* restart watchdog */
 
-out_ping:
-	mutex_unlock(&wdd->lock);
 	return err;
 }
 
@@ -87,6 +102,8 @@ out_ping:
  *	watchdog_start: wrapper to start the watchdog.
  *	@wdd: the watchdog device to start
  *
+ *	The caller must hold wd_data->lock.
+ *
  *	Start the watchdog if it is not active and mark it active.
  *	This function returns zero on success or a negative errno code for
  *	failure.
@@ -94,24 +111,15 @@ out_ping:
 
 static int watchdog_start(struct watchdog_device *wdd)
 {
-	int err = 0;
-
-	mutex_lock(&wdd->lock);
-
-	if (test_bit(WDOG_UNREGISTERED, &wdd->status)) {
-		err = -ENODEV;
-		goto out_start;
-	}
+	int err;
 
 	if (watchdog_active(wdd))
-		goto out_start;
+		return 0;
 
 	err = wdd->ops->start(wdd);
 	if (err == 0)
 		set_bit(WDOG_ACTIVE, &wdd->status);
 
-out_start:
-	mutex_unlock(&wdd->lock);
 	return err;
 }
 
@@ -119,6 +127,8 @@ out_start:
  *	watchdog_stop: wrapper to stop the watchdog.
  *	@wdd: the watchdog device to stop
  *
+ *	The caller must hold wd_data->lock.
+ *
  *	Stop the watchdog if it is still active and unmark it active.
  *	This function returns zero on success or a negative errno code for
  *	failure.
@@ -127,93 +137,58 @@ out_start:
 
 static int watchdog_stop(struct watchdog_device *wdd)
 {
-	int err = 0;
-
-	mutex_lock(&wdd->lock);
-
-	if (test_bit(WDOG_UNREGISTERED, &wdd->status)) {
-		err = -ENODEV;
-		goto out_stop;
-	}
+	int err;
 
 	if (!watchdog_active(wdd))
-		goto out_stop;
+		return 0;
 
 	if (test_bit(WDOG_NO_WAY_OUT, &wdd->status)) {
 		dev_info(wdd->dev, "nowayout prevents watchdog being stopped!\n");
-		err = -EBUSY;
-		goto out_stop;
+		return -EBUSY;
 	}
 
 	err = wdd->ops->stop(wdd);
 	if (err == 0)
 		clear_bit(WDOG_ACTIVE, &wdd->status);
 
-out_stop:
-	mutex_unlock(&wdd->lock);
 	return err;
 }
 
 /*
  *	watchdog_get_status: wrapper to get the watchdog status
  *	@wdd: the watchdog device to get the status from
- *	@status: the status of the watchdog device
+ *
+ *	The caller must hold wd_data->lock.
  *
  *	Get the watchdog's status flags.
  */
 
-static int watchdog_get_status(struct watchdog_device *wdd,
-							unsigned int *status)
+static unsigned int watchdog_get_status(struct watchdog_device *wdd)
 {
-	int err = 0;
-
-	*status = 0;
 	if (!wdd->ops->status)
-		return -EOPNOTSUPP;
-
-	mutex_lock(&wdd->lock);
-
-	if (test_bit(WDOG_UNREGISTERED, &wdd->status)) {
-		err = -ENODEV;
-		goto out_status;
-	}
-
-	*status = wdd->ops->status(wdd);
+		return 0;
 
-out_status:
-	mutex_unlock(&wdd->lock);
-	return err;
+	return wdd->ops->status(wdd);
 }
 
 /*
  *	watchdog_set_timeout: set the watchdog timer timeout
  *	@wdd: the watchdog device to set the timeout for
  *	@timeout: timeout to set in seconds
+ *
+ *	The caller must hold wd_data->lock.
  */
 
 static int watchdog_set_timeout(struct watchdog_device *wdd,
 							unsigned int timeout)
 {
-	int err;
-
 	if (!wdd->ops->set_timeout || !(wdd->info->options & WDIOF_SETTIMEOUT))
 		return -EOPNOTSUPP;
 
 	if (watchdog_timeout_invalid(wdd, timeout))
 		return -EINVAL;
 
-	mutex_lock(&wdd->lock);
-
-	if (test_bit(WDOG_UNREGISTERED, &wdd->status)) {
-		err = -ENODEV;
-		goto out_timeout;
-	}
-
-	err = wdd->ops->set_timeout(wdd, timeout);
-
-out_timeout:
-	mutex_unlock(&wdd->lock);
-	return err;
+	return wdd->ops->set_timeout(wdd, timeout);
 }
 
 /*
@@ -221,30 +196,22 @@ out_timeout:
  *	@wdd: the watchdog device to get the remaining time from
  *	@timeleft: the time that's left
  *
+ *	The caller must hold wd_data->lock.
+ *
  *	Get the time before a watchdog will reboot (if not pinged).
  */
 
 static int watchdog_get_timeleft(struct watchdog_device *wdd,
 							unsigned int *timeleft)
 {
-	int err = 0;
-
 	*timeleft = 0;
+
 	if (!wdd->ops->get_timeleft)
 		return -EOPNOTSUPP;
 
-	mutex_lock(&wdd->lock);
-
-	if (test_bit(WDOG_UNREGISTERED, &wdd->status)) {
-		err = -ENODEV;
-		goto out_timeleft;
-	}
-
 	*timeleft = wdd->ops->get_timeleft(wdd);
 
-out_timeleft:
-	mutex_unlock(&wdd->lock);
-	return err;
+	return 0;
 }
 
 #ifdef CONFIG_WATCHDOG_SYSFS
@@ -261,14 +228,14 @@ static ssize_t status_show(struct device *dev, struct device_attribute *attr,
 				char *buf)
 {
 	struct watchdog_device *wdd = dev_get_drvdata(dev);
-	ssize_t status;
-	unsigned int val;
+	struct watchdog_core_data *wd_data = wdd->wd_data;
+	unsigned int status;
 
-	status = watchdog_get_status(wdd, &val);
-	if (!status)
-		status = sprintf(buf, "%u\n", val);
+	mutex_lock(&wd_data->lock);
+	status = watchdog_get_status(wdd);
+	mutex_unlock(&wd_data->lock);
 
-	return status;
+	return sprintf(buf, "%u\n", status);
 }
 static DEVICE_ATTR_RO(status);
 
@@ -285,10 +252,13 @@ static ssize_t timeleft_show(struct device *dev, struct device_attribute *attr,
 				char *buf)
 {
 	struct watchdog_device *wdd = dev_get_drvdata(dev);
+	struct watchdog_core_data *wd_data = wdd->wd_data;
 	ssize_t status;
 	unsigned int val;
 
+	mutex_lock(&wd_data->lock);
 	status = watchdog_get_timeleft(wdd, &val);
+	mutex_unlock(&wd_data->lock);
 	if (!status)
 		status = sprintf(buf, "%u\n", val);
 
@@ -365,28 +335,17 @@ __ATTRIBUTE_GROUPS(wdt);
  *	@wdd: the watchdog device to do the ioctl on
  *	@cmd: watchdog command
  *	@arg: argument pointer
+ *
+ *	The caller must hold wd_data->lock.
  */
 
 static int watchdog_ioctl_op(struct watchdog_device *wdd, unsigned int cmd,
 							unsigned long arg)
 {
-	int err;
-
 	if (!wdd->ops->ioctl)
 		return -ENOIOCTLCMD;
 
-	mutex_lock(&wdd->lock);
-
-	if (test_bit(WDOG_UNREGISTERED, &wdd->status)) {
-		err = -ENODEV;
-		goto out_ioctl;
-	}
-
-	err = wdd->ops->ioctl(wdd, cmd, arg);
-
-out_ioctl:
-	mutex_unlock(&wdd->lock);
-	return err;
+	return wdd->ops->ioctl(wdd, cmd, arg);
 }
 
 /*
@@ -404,10 +363,11 @@ out_ioctl:
 static ssize_t watchdog_write(struct file *file, const char __user *data,
 						size_t len, loff_t *ppos)
 {
-	struct watchdog_device *wdd = file->private_data;
+	struct watchdog_core_data *wd_data = file->private_data;
+	struct watchdog_device *wdd;
+	int err;
 	size_t i;
 	char c;
-	int err;
 
 	if (len == 0)
 		return 0;
@@ -416,18 +376,25 @@ static ssize_t watchdog_write(struct file *file, const char __user *data,
 	 * Note: just in case someone wrote the magic character
 	 * five months ago...
 	 */
-	clear_bit(WDOG_ALLOW_RELEASE, &wdd->status);
+	clear_bit(_WDOG_ALLOW_RELEASE, &wd_data->status);
 
 	/* scan to see whether or not we got the magic character */
 	for (i = 0; i != len; i++) {
 		if (get_user(c, data + i))
 			return -EFAULT;
 		if (c == 'V')
-			set_bit(WDOG_ALLOW_RELEASE, &wdd->status);
+			set_bit(_WDOG_ALLOW_RELEASE, &wd_data->status);
 	}
 
 	/* someone wrote to us, so we send the watchdog a keepalive ping */
-	err = watchdog_ping(wdd);
+
+	err = -ENODEV;
+	mutex_lock(&wd_data->lock);
+	wdd = wd_data->wdd;
+	if (wdd)
+		err = watchdog_ping(wdd);
+	mutex_unlock(&wd_data->lock);
+
 	if (err < 0)
 		return err;
 
@@ -447,71 +414,94 @@ static ssize_t watchdog_write(struct file *file, const char __user *data,
 static long watchdog_ioctl(struct file *file, unsigned int cmd,
 							unsigned long arg)
 {
-	struct watchdog_device *wdd = file->private_data;
+	struct watchdog_core_data *wd_data = file->private_data;
 	void __user *argp = (void __user *)arg;
+	struct watchdog_device *wdd;
 	int __user *p = argp;
 	unsigned int val;
 	int err;
 
+	mutex_lock(&wd_data->lock);
+
+	wdd = wd_data->wdd;
+	if (!wdd) {
+		err = -ENODEV;
+		goto out_ioctl;
+	}
+
 	err = watchdog_ioctl_op(wdd, cmd, arg);
 	if (err != -ENOIOCTLCMD)
-		return err;
+		goto out_ioctl;
 
 	switch (cmd) {
 	case WDIOC_GETSUPPORT:
-		return copy_to_user(argp, wdd->info,
+		err = copy_to_user(argp, wdd->info,
 			sizeof(struct watchdog_info)) ? -EFAULT : 0;
+		break;
 	case WDIOC_GETSTATUS:
-		err = watchdog_get_status(wdd, &val);
-		if (err == -ENODEV)
-			return err;
-		return put_user(val, p);
+		val = watchdog_get_status(wdd);
+		err = put_user(val, p);
+		break;
 	case WDIOC_GETBOOTSTATUS:
-		return put_user(wdd->bootstatus, p);
+		err = put_user(wdd->bootstatus, p);
+		break;
 	case WDIOC_SETOPTIONS:
-		if (get_user(val, p))
-			return -EFAULT;
+		if (get_user(val, p)) {
+			err = -EFAULT;
+			break;
+		}
 		if (val & WDIOS_DISABLECARD) {
 			err = watchdog_stop(wdd);
 			if (err < 0)
-				return err;
+				break;
 		}
-		if (val & WDIOS_ENABLECARD) {
+		if (val & WDIOS_ENABLECARD)
 			err = watchdog_start(wdd);
-			if (err < 0)
-				return err;
-		}
-		return 0;
+		break;
 	case WDIOC_KEEPALIVE:
-		if (!(wdd->info->options & WDIOF_KEEPALIVEPING))
-			return -EOPNOTSUPP;
-		return watchdog_ping(wdd);
+		if (!(wdd->info->options & WDIOF_KEEPALIVEPING)) {
+			err = -EOPNOTSUPP;
+			break;
+		}
+		err = watchdog_ping(wdd);
+		break;
 	case WDIOC_SETTIMEOUT:
-		if (get_user(val, p))
-			return -EFAULT;
+		if (get_user(val, p)) {
+			err = -EFAULT;
+			break;
+		}
 		err = watchdog_set_timeout(wdd, val);
 		if (err < 0)
-			return err;
+			break;
 		/* If the watchdog is active then we send a keepalive ping
 		 * to make sure that the watchdog keep's running (and if
 		 * possible that it takes the new timeout) */
 		err = watchdog_ping(wdd);
 		if (err < 0)
-			return err;
+			break;
 		/* Fall */
 	case WDIOC_GETTIMEOUT:
 		/* timeout == 0 means that we don't know the timeout */
-		if (wdd->timeout == 0)
-			return -EOPNOTSUPP;
-		return put_user(wdd->timeout, p);
+		if (wdd->timeout == 0) {
+			err = -EOPNOTSUPP;
+			break;
+		}
+		err = put_user(wdd->timeout, p);
+		break;
 	case WDIOC_GETTIMELEFT:
 		err = watchdog_get_timeleft(wdd, &val);
-		if (err)
-			return err;
-		return put_user(val, p);
+		if (err < 0)
+			break;
+		err = put_user(val, p);
+		break;
 	default:
-		return -ENOTTY;
+		err = -ENOTTY;
+		break;
 	}
+
+out_ioctl:
+	mutex_unlock(&wd_data->lock);
+	return err;
 }
 
 /*
@@ -526,45 +516,59 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
 
 static int watchdog_open(struct inode *inode, struct file *file)
 {
-	int err = -EBUSY;
+	struct watchdog_core_data *wd_data;
 	struct watchdog_device *wdd;
+	int err;
 
 	/* Get the corresponding watchdog device */
 	if (imajor(inode) == MISC_MAJOR)
-		wdd = old_wdd;
+		wd_data = old_wd_data;
 	else
-		wdd = container_of(inode->i_cdev, struct watchdog_device, cdev);
+		wd_data = container_of(inode->i_cdev, struct watchdog_core_data,
+				       cdev);
 
 	/* the watchdog is single open! */
-	if (test_and_set_bit(WDOG_DEV_OPEN, &wdd->status))
+	if (test_and_set_bit(_WDOG_DEV_OPEN, &wd_data->status))
 		return -EBUSY;
 
+	wdd = wd_data->wdd;
+
 	/*
 	 * If the /dev/watchdog device is open, we don't want the module
 	 * to be unloaded.
 	 */
-	if (!try_module_get(wdd->ops->owner))
-		goto out;
+	if (!try_module_get(wdd->ops->owner)) {
+		err = -EBUSY;
+		goto out_clear;
+	}
 
 	err = watchdog_start(wdd);
 	if (err < 0)
 		goto out_mod;
 
-	file->private_data = wdd;
+	file->private_data = wd_data;
 
-	if (wdd->ops->ref)
-		wdd->ops->ref(wdd);
+	kref_get(&wd_data->kref);
 
 	/* dev/watchdog is a virtual (and thus non-seekable) filesystem */
 	return nonseekable_open(inode, file);
 
 out_mod:
-	module_put(wdd->ops->owner);
-out:
-	clear_bit(WDOG_DEV_OPEN, &wdd->status);
+	module_put(wd_data->wdd->ops->owner);
+out_clear:
+	clear_bit(_WDOG_DEV_OPEN, &wd_data->status);
 	return err;
 }
 
+static void watchdog_core_data_release(struct kref *kref)
+{
+	struct watchdog_core_data *wd_data;
+
+	wd_data = container_of(kref, struct watchdog_core_data, kref);
+
+	kfree(wd_data);
+}
+
 /*
  *	watchdog_release: release the watchdog device.
  *	@inode: inode of device
@@ -577,9 +581,16 @@ out:
 
 static int watchdog_release(struct inode *inode, struct file *file)
 {
-	struct watchdog_device *wdd = file->private_data;
+	struct watchdog_core_data *wd_data = file->private_data;
+	struct watchdog_device *wdd;
 	int err = -EBUSY;
 
+	mutex_lock(&wd_data->lock);
+
+	wdd = wd_data->wdd;
+	if (!wdd)
+		goto done;
+
 	/*
 	 * We only stop the watchdog if we received the magic character
 	 * or if WDIOF_MAGICCLOSE is not set. If nowayout was set then
@@ -587,29 +598,24 @@ static int watchdog_release(struct inode *inode, struct file *file)
 	 */
 	if (!test_bit(WDOG_ACTIVE, &wdd->status))
 		err = 0;
-	else if (test_and_clear_bit(WDOG_ALLOW_RELEASE, &wdd->status) ||
+	else if (test_and_clear_bit(_WDOG_ALLOW_RELEASE, &wd_data->status) ||
 		 !(wdd->info->options & WDIOF_MAGICCLOSE))
 		err = watchdog_stop(wdd);
 
 	/* If the watchdog was not stopped, send a keepalive ping */
 	if (err < 0) {
-		mutex_lock(&wdd->lock);
-		if (!test_bit(WDOG_UNREGISTERED, &wdd->status))
-			dev_crit(wdd->dev, "watchdog did not stop!\n");
-		mutex_unlock(&wdd->lock);
+		dev_crit(wdd->dev, "watchdog did not stop!\n");
 		watchdog_ping(wdd);
 	}
 
-	/* Allow the owner module to be unloaded again */
-	module_put(wdd->ops->owner);
-
 	/* make sure that /dev/watchdog can be re-opened */
-	clear_bit(WDOG_DEV_OPEN, &wdd->status);
-
-	/* Note wdd may be gone after this, do not use after this! */
-	if (wdd->ops->unref)
-		wdd->ops->unref(wdd);
+	clear_bit(_WDOG_DEV_OPEN, &wd_data->status);
 
+done:
+	mutex_unlock(&wd_data->lock);
+	/* Allow the owner module to be unloaded again */
+	module_put(wd_data->cdev.owner);
+	kref_put(&wd_data->kref, watchdog_core_data_release);
 	return 0;
 }
 
@@ -639,10 +645,20 @@ static struct miscdevice watchdog_miscdev = {
 
 static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno)
 {
+	struct watchdog_core_data *wd_data;
 	int err;
 
+	wd_data = kzalloc(sizeof(struct watchdog_core_data), GFP_KERNEL);
+	if (!wd_data)
+		return -ENOMEM;
+	kref_init(&wd_data->kref);
+	mutex_init(&wd_data->lock);
+
+	wd_data->wdd = wdd;
+	wdd->wd_data = wd_data;
+
 	if (wdd->id == 0) {
-		old_wdd = wdd;
+		old_wd_data = wd_data;
 		watchdog_miscdev.parent = wdd->parent;
 		err = misc_register(&watchdog_miscdev);
 		if (err != 0) {
@@ -651,23 +667,25 @@ static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno)
 			if (err == -EBUSY)
 				pr_err("%s: a legacy watchdog module is probably present.\n",
 					wdd->info->identity);
-			old_wdd = NULL;
+			old_wd_data = NULL;
+			kfree(wd_data);
 			return err;
 		}
 	}
 
 	/* Fill in the data structures */
-	cdev_init(&wdd->cdev, &watchdog_fops);
-	wdd->cdev.owner = wdd->ops->owner;
+	cdev_init(&wd_data->cdev, &watchdog_fops);
+	wd_data->cdev.owner = wdd->ops->owner;
 
 	/* Add the device */
-	err  = cdev_add(&wdd->cdev, devno, 1);
+	err = cdev_add(&wd_data->cdev, devno, 1);
 	if (err) {
 		pr_err("watchdog%d unable to add device %d:%d\n",
 			wdd->id,  MAJOR(watchdog_devt), wdd->id);
 		if (wdd->id == 0) {
 			misc_deregister(&watchdog_miscdev);
-			old_wdd = NULL;
+			old_wd_data = NULL;
+			kref_put(&wd_data->kref, watchdog_core_data_release);
 		}
 	}
 	return err;
@@ -683,15 +701,20 @@ static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno)
 
 static void watchdog_cdev_unregister(struct watchdog_device *wdd)
 {
-	mutex_lock(&wdd->lock);
-	set_bit(WDOG_UNREGISTERED, &wdd->status);
-	mutex_unlock(&wdd->lock);
+	struct watchdog_core_data *wd_data = wdd->wd_data;
 
-	cdev_del(&wdd->cdev);
+	cdev_del(&wd_data->cdev);
 	if (wdd->id == 0) {
 		misc_deregister(&watchdog_miscdev);
-		old_wdd = NULL;
+		old_wd_data = NULL;
 	}
+
+	mutex_lock(&wd_data->lock);
+	wd_data->wdd = NULL;
+	wdd->wd_data = NULL;
+	mutex_unlock(&wd_data->lock);
+
+	kref_put(&wd_data->kref, watchdog_core_data_release);
 }
 
 static struct class watchdog_class = {
@@ -742,9 +765,9 @@ int watchdog_dev_register(struct watchdog_device *wdd)
 
 void watchdog_dev_unregister(struct watchdog_device *wdd)
 {
-	watchdog_cdev_unregister(wdd);
 	device_destroy(&watchdog_class, wdd->dev->devt);
 	wdd->dev = NULL;
+	watchdog_cdev_unregister(wdd);
 }
 
 /*