[PATCH v3 0/6] watchdog: Add reboot API

[PATCH v3 0/6] watchdog: Add reboot API

[Guenter Roeck]

Some hardware implements reboot through its watchdog hardware, for example
by triggering a watchdog timeout or by writing into its watchdog register
set. Platform specific code starts to spread into watchdog drivers,
typically by setting pointers to a callback function which is then called
from the architecture's reset handler.

While global and exported callback function pointers (such as arm_pm_restart)
may be acceptable as long as they are used from platform and/or architecture
code, using such a mechanism across subsystems and drivers is less than
desirable. Ultimately, we'll need a better solution.

This patch series provides such a solution. It extends the watchdog
subsystem to support reboot functionality, provides an API function
call to trigger reboots, adds support for the new API to arm and arm64,
and converts the drivers providing reboot functionality to use the new
infrastructure.

The first patch in the series implements the new API. The second patch
documents it. The third and fourth patch modify the arm and arm64
architecture reset handlers to call the added API function. The final
two patches register the reboot handlers in the sunxi and moxart watchdog
drivers with the watchdog subsystem.

The sunxi patch depends on the most recent patch series sumitted by
Maxime Ripard.

Note that I did not address the comments asking for support of multiple
watchdogs with reset handlers, nor the request to add a flag to provide
'preferential' treatment for one of those watchdogs. This will require
additional discussion and needs to be addressed with a later patch if needed.
Key questions are how to add such support for non-DT systems, and if the
scope of restart handling is a function of the driver or of the system.
Also, if one of the watchdogs does not result in a complete system reset
but another one does, it is not clear to me why the less-than-perfect
watchdog would be configured in the first place (or how this situation
would be handled today).

Compile tested only for arm64. Tested on arm/moxart by Jonas Jensen.

[PATCH v3 1/6] watchdog: Add API to trigger reboots

[Guenter Roeck]

Some hardware implements reboot through its watchdog hardware,
for example by triggering a watchdog timeout. Platform specific
code starts to spread into watchdog drivers, typically by setting
pointers to a callback functions which is then called from the
platform reset handler.

To simplify code and provide a unified API to trigger reboots by
watchdog drivers, provide a single API to trigger such reboots
through the watchdog subsystem.

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Tested-by: Jonas Jensen <jonas.jensen@gmail.com>
Acked-by: Heiko Stuebner <heiko@sntech.de>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v3: Drop reboot mode and cmd string parameters from API
v2: Remove unnecessary check for ops in reboot function

 drivers/watchdog/watchdog_core.c | 15 +++++++++++++++
 include/linux/watchdog.h         |  8 ++++++++
 2 files changed, 23 insertions(+)

diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index cec9b55..24ce780 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -43,6 +43,15 @@
 static DEFINE_IDA(watchdog_ida);
 static struct class *watchdog_class;
 
+static struct watchdog_device *wdd_reboot_dev;
+
+void watchdog_do_reboot(void)
+{
+	if (wdd_reboot_dev)
+		wdd_reboot_dev->ops->reboot(wdd_reboot_dev);
+}
+EXPORT_SYMBOL(watchdog_do_reboot);
+
 static void watchdog_check_min_max_timeout(struct watchdog_device *wdd)
 {
 	/*
@@ -162,6 +171,9 @@ int watchdog_register_device(struct watchdog_device *wdd)
 		return ret;
 	}
 
+	if (wdd->ops->reboot)
+		wdd_reboot_dev = wdd;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(watchdog_register_device);
@@ -181,6 +193,9 @@ void watchdog_unregister_device(struct watchdog_device *wdd)
 	if (wdd == NULL)
 		return;
 
+	if (wdd == wdd_reboot_dev)
+		wdd_reboot_dev = NULL;
+
 	devno = wdd->cdev.dev;
 	ret = watchdog_dev_unregister(wdd);
 	if (ret)
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 2a3038e..1e9da0a 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -23,6 +23,7 @@ struct watchdog_device;
  * @start:	The routine for starting the watchdog device.
  * @stop:	The routine for stopping the watchdog device.
  * @ping:	The routine that sends a keepalive ping to the watchdog device.
+ * @reboot:	The routine for rebooting the system
  * @status:	The routine that shows the status of the watchdog device.
  * @set_timeout:The routine for setting the watchdog devices timeout value.
  * @get_timeleft:The routine that get's the time that's left before a reset.
@@ -42,6 +43,7 @@ struct watchdog_ops {
 	int (*stop)(struct watchdog_device *);
 	/* optional operations */
 	int (*ping)(struct watchdog_device *);
+	void (*reboot)(struct watchdog_device *);
 	unsigned int (*status)(struct watchdog_device *);
 	int (*set_timeout)(struct watchdog_device *, unsigned int);
 	unsigned int (*get_timeleft)(struct watchdog_device *);
@@ -142,4 +144,10 @@ extern int watchdog_init_timeout(struct watchdog_device *wdd,
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
 
+#ifdef CONFIG_WATCHDOG_CORE
+extern void watchdog_do_reboot(void);
+#else
+static inline void watchdog_do_reboot(void) { }
+#endif
+
 #endif  /* ifndef _LINUX_WATCHDOG_H */
-- 
1.9.1

[PATCH v3 1/6] watchdog: Add API to trigger reboots

[One Thousand Gnomes]

> +void watchdog_do_reboot(void)
> +{
> +	if (wdd_reboot_dev)
> +		wdd_reboot_dev->ops->reboot(wdd_reboot_dev);
> +}
> +EXPORT_SYMBOL(watchdog_do_reboot);

Crashes and burns if you are unloading a watchdog just as you try to
reboot. Yes its wildly unlikely but it's still conceptually wrong.

>  
> +	if (wdd->ops->reboot)
> +		wdd_reboot_dev = wdd;
> +

Two parallel registers from different bus types, parallel
register/unregister ?

This feels to me like a backward step. We've gone from various device
bits leaking into the core code (where they can work all the time) to
various core code leaking into the devices which is asking for init order
problems and other races.

Given we are talking about stuff of the order of 10-20 instructions I
think duplication is not only the lesser evil it's also smaller, more
reliable and easier to maintain.

IMHO this is a solution looking for a problem.

Alan

[PATCH v3 1/6] watchdog: Add API to trigger reboots

[Guenter Roeck]

On Thu, May 15, 2014 at 09:50:20PM +0100, One Thousand Gnomes wrote:
> > +void watchdog_do_reboot(void)
> > +{
> > +	if (wdd_reboot_dev)
> > +		wdd_reboot_dev->ops->reboot(wdd_reboot_dev);
> > +}
> > +EXPORT_SYMBOL(watchdog_do_reboot);
> 
> Crashes and burns if you are unloading a watchdog just as you try to
> reboot. Yes its wildly unlikely but it's still conceptually wrong.
> 
Possibly, but how is it different to the code it replaces ?

> >  
> > +	if (wdd->ops->reboot)
> > +		wdd_reboot_dev = wdd;
> > +
> 
> Two parallel registers from different bus types, parallel
> register/unregister ?
> 
Sorry, you lost me. What different bus types ?

> This feels to me like a backward step. We've gone from various device
> bits leaking into the core code (where they can work all the time) to
> various core code leaking into the devices which is asking for init order
> problems and other races.
> 
> Given we are talking about stuff of the order of 10-20 instructions I
> think duplication is not only the lesser evil it's also smaller, more
> reliable and easier to maintain.
> 
> IMHO this is a solution looking for a problem.
> 
Really ?  To me it seems to be much cleaner than setting the pointer to
arm_pm_restart directly from individual watchdog drivers. Also, and I was
told that other HW may benefit from it as well.

Do I understand it correctly that you prefer watchdog drivers to set
arm_pm_restart directly ? Maybe you can explain a bit why you believe
that to be a superior solution.

In addition to that, while I could obviously add some locking around access to
wdd_reboot_dev, existing code doesn't lock any changes to arm_pm_restart. I am
somewhat at loss why setting or clearing arm_pm_restart is less of a problem
that setting wdd_reboot_dev.

Thanks,
Guenter

[PATCH v4 1/6] watchdog: Add API to trigger reboots

[Guenter Roeck]

Some hardware implements reboot through its watchdog hardware,
for example by triggering a watchdog timeout. Platform specific
code starts to spread into watchdog drivers, typically by setting
pointers to a callback functions which is then called from the
platform reset handler.

To simplify code and provide a unified API to trigger reboots by
watchdog drivers, provide a single API to trigger such reboots
through the watchdog subsystem.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v4: Protect accesses to wdd_reboot_dev with spinlock to avoid potential
    race conditions.
    Drop Acked-by and Tested-by tags because changes are too significant
    and warrant re-evaluation and re-testing.
v3: Drop reboot mode and cmd string parameters from API
v2: Remove unnecessary check for ops in reboot function

 drivers/watchdog/watchdog_core.c | 27 +++++++++++++++++++++++++++
 include/linux/watchdog.h         |  8 ++++++++
 2 files changed, 35 insertions(+)

diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index cec9b55..e61f4ed 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -37,12 +37,27 @@
 #include <linux/idr.h>		/* For ida_* macros */
 #include <linux/err.h>		/* For IS_ERR macros */
 #include <linux/of.h>		/* For of_get_timeout_sec */
+#include <linux/spinlock.h>	/* For spinlock */
 
 #include "watchdog_core.h"	/* For watchdog_dev_register/... */
 
 static DEFINE_IDA(watchdog_ida);
 static struct class *watchdog_class;
 
+static DEFINE_SPINLOCK(wdd_reboot_lock);
+static struct watchdog_device *wdd_reboot_dev;
+
+void watchdog_do_reboot(void)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&wdd_reboot_lock, flags);
+	if (wdd_reboot_dev)
+		wdd_reboot_dev->ops->reboot(wdd_reboot_dev);
+	spin_unlock_irqrestore(&wdd_reboot_lock, flags);
+}
+EXPORT_SYMBOL(watchdog_do_reboot);
+
 static void watchdog_check_min_max_timeout(struct watchdog_device *wdd)
 {
 	/*
@@ -111,6 +126,7 @@ EXPORT_SYMBOL_GPL(watchdog_init_timeout);
 int watchdog_register_device(struct watchdog_device *wdd)
 {
 	int ret, id, devno;
+	unsigned long flags;
 
 	if (wdd == NULL || wdd->info == NULL || wdd->ops == NULL)
 		return -EINVAL;
@@ -162,6 +178,11 @@ int watchdog_register_device(struct watchdog_device *wdd)
 		return ret;
 	}
 
+	spin_lock_irqsave(&wdd_reboot_lock, flags);
+	if (wdd->ops->reboot && !wdd_reboot_dev)
+		wdd_reboot_dev = wdd;
+	spin_unlock_irqrestore(&wdd_reboot_lock, flags);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(watchdog_register_device);
@@ -177,10 +198,16 @@ void watchdog_unregister_device(struct watchdog_device *wdd)
 {
 	int ret;
 	int devno;
+	unsigned long flags;
 
 	if (wdd == NULL)
 		return;
 
+	spin_lock_irqsave(&wdd_reboot_lock, flags);
+	if (wdd == wdd_reboot_dev)
+		wdd_reboot_dev = NULL;
+	spin_unlock_irqrestore(&wdd_reboot_lock, flags);
+
 	devno = wdd->cdev.dev;
 	ret = watchdog_dev_unregister(wdd);
 	if (ret)
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index 2a3038e..1e9da0a 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -23,6 +23,7 @@ struct watchdog_device;
  * @start:	The routine for starting the watchdog device.
  * @stop:	The routine for stopping the watchdog device.
  * @ping:	The routine that sends a keepalive ping to the watchdog device.
+ * @reboot:	The routine for rebooting the system
  * @status:	The routine that shows the status of the watchdog device.
  * @set_timeout:The routine for setting the watchdog devices timeout value.
  * @get_timeleft:The routine that get's the time that's left before a reset.
@@ -42,6 +43,7 @@ struct watchdog_ops {
 	int (*stop)(struct watchdog_device *);
 	/* optional operations */
 	int (*ping)(struct watchdog_device *);
+	void (*reboot)(struct watchdog_device *);
 	unsigned int (*status)(struct watchdog_device *);
 	int (*set_timeout)(struct watchdog_device *, unsigned int);
 	unsigned int (*get_timeleft)(struct watchdog_device *);
@@ -142,4 +144,10 @@ extern int watchdog_init_timeout(struct watchdog_device *wdd,
 extern int watchdog_register_device(struct watchdog_device *);
 extern void watchdog_unregister_device(struct watchdog_device *);
 
+#ifdef CONFIG_WATCHDOG_CORE
+extern void watchdog_do_reboot(void);
+#else
+static inline void watchdog_do_reboot(void) { }
+#endif
+
 #endif  /* ifndef _LINUX_WATCHDOG_H */
-- 
1.9.1

[PATCH v3 2/6] watchdog: Document reboot API

[Guenter Roeck]

Document the new reboot API functionality.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v3: Added documentation patch

 Documentation/watchdog/watchdog-kernel-api.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index a0438f3..39f8e00 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -90,6 +90,7 @@ struct watchdog_ops {
 	int (*stop)(struct watchdog_device *);
 	/* optional operations */
 	int (*ping)(struct watchdog_device *);
+	void (*reboot)(struct watchdog_device *);
 	unsigned int (*status)(struct watchdog_device *);
 	int (*set_timeout)(struct watchdog_device *, unsigned int);
 	unsigned int (*get_timeleft)(struct watchdog_device *);
@@ -148,6 +149,8 @@ they are supported. These optional routines/operations are:
   info structure).
 * status: this routine checks the status of the watchdog timer device. The
   status of the device is reported with watchdog WDIOF_* status flags/bits.
+* reboot: if this routine is present, it may be called to reboot the system.
+  Parameter is the pointer to the watchdog device.
 * set_timeout: this routine checks and changes the timeout of the watchdog
   timer device. It returns 0 on success, -EINVAL for "parameter out of range"
   and -EIO for "could not write value to the watchdog". On success this
@@ -224,3 +227,11 @@ the device tree (if the module timeout parameter is invalid). Best practice is
 to set the default timeout value as timeout value in the watchdog_device and
 then use this function to set the user "preferred" timeout value.
 This routine returns zero on success and a negative errno code for failure.
+
+The watchdog_do_reboot function can be used to reboot the system. This is
+intended to be used in systems which do not have an explicit reboot capability,
+but implement reboot by programming the watchdog to expire immediately. If
+the function is called, and a watchdog driver with reboot functionality was
+previously registered, the reboot function of that driver will be called.
+Architecture code should call watchdog_do_reboot from its machine_reboot
+function after other means to reboot the system failed.
-- 
1.9.1

[PATCH v3 3/6] arm64: Support reboot through watchdog subsystem

[Guenter Roeck]

The watchdog subsystem provides an API to perform a system reboot.
Use it.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v3: Drop reboot mode and cmd string parameters from API
v2: No change

 arch/arm64/kernel/process.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 6391485..e30f146 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -42,6 +42,7 @@
 #include <linux/hw_breakpoint.h>
 #include <linux/personality.h>
 #include <linux/notifier.h>
+#include <linux/watchdog.h>
 
 #include <asm/compat.h>
 #include <asm/cacheflush.h>
@@ -144,6 +145,8 @@ void machine_restart(char *cmd)
 	if (arm_pm_restart)
 		arm_pm_restart(reboot_mode, cmd);
 
+	watchdog_do_reboot();
+
 	/*
 	 * Whoops - the architecture was unable to reboot.
 	 */
-- 
1.9.1

[PATCH v3 4/6] arm: Support reboot through watchdog subsystem

[Guenter Roeck]

The watchdog subsystem provides an API to perform a system reboot.
Use it.

With this change, the arm_pm_restart callback is now optional,
so check if it is set before calling it.

Tested-by: Jonas Jensen <jonas.jensen@gmail.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v3: Drop reboot mode and cmd string parameters from API
v2: No change

 arch/arm/kernel/process.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 81ef686..4bfbedc 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -32,6 +32,7 @@
 #include <linux/hw_breakpoint.h>
 #include <linux/leds.h>
 #include <linux/reboot.h>
+#include <linux/watchdog.h>
 
 #include <asm/cacheflush.h>
 #include <asm/idmap.h>
@@ -230,7 +231,10 @@ void machine_restart(char *cmd)
 	local_irq_disable();
 	smp_send_stop();
 
-	arm_pm_restart(reboot_mode, cmd);
+	if (arm_pm_restart)
+		arm_pm_restart(reboot_mode, cmd);
+
+	watchdog_do_reboot();
 
 	/* Give a grace period for failure to restart of 1s */
 	mdelay(1000);
-- 
1.9.1

[PATCH v3 5/6] watchdog: moxart: Register reboot handler with watchdog subsystem

[Guenter Roeck]

The watchdog subsystem now provides an API to trigger a system reboot.
Register with it.

Tested-by: Jonas Jensen <jonas.jensen@gmail.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v3: Drop reboot mode and cmd string parameters from API
v2: No change

 drivers/watchdog/moxart_wdt.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
index 4aa3a8a..9698c83 100644
--- a/drivers/watchdog/moxart_wdt.c
+++ b/drivers/watchdog/moxart_wdt.c
@@ -19,8 +19,6 @@
 #include <linux/watchdog.h>
 #include <linux/moduleparam.h>
 
-#include <asm/system_misc.h>
-
 #define REG_COUNT			0x4
 #define REG_MODE			0x8
 #define REG_ENABLE			0xC
@@ -31,15 +29,15 @@ struct moxart_wdt_dev {
 	unsigned int clock_frequency;
 };
 
-static struct moxart_wdt_dev *moxart_restart_ctx;
-
 static int heartbeat;
 
-static void moxart_wdt_restart(enum reboot_mode reboot_mode, const char *cmd)
+static void moxart_wdt_reboot(struct watchdog_device *wdt_dev)
 {
-	writel(1, moxart_restart_ctx->base + REG_COUNT);
-	writel(0x5ab9, moxart_restart_ctx->base + REG_MODE);
-	writel(0x03, moxart_restart_ctx->base + REG_ENABLE);
+	struct moxart_wdt_dev *moxart_wdt = watchdog_get_drvdata(wdt_dev);
+
+	writel(1, moxart_wdt->base + REG_COUNT);
+	writel(0x5ab9, moxart_wdt->base + REG_MODE);
+	writel(0x03, moxart_wdt->base + REG_ENABLE);
 }
 
 static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
@@ -81,6 +79,7 @@ static const struct watchdog_ops moxart_wdt_ops = {
 	.owner          = THIS_MODULE,
 	.start          = moxart_wdt_start,
 	.stop           = moxart_wdt_stop,
+	.reboot		= moxart_wdt_reboot,
 	.set_timeout    = moxart_wdt_set_timeout,
 };
 
@@ -136,9 +135,6 @@ static int moxart_wdt_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
-	moxart_restart_ctx = moxart_wdt;
-	arm_pm_restart = moxart_wdt_restart;
-
 	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
 		moxart_wdt->dev.timeout, nowayout);
 
@@ -149,7 +145,6 @@ static int moxart_wdt_remove(struct platform_device *pdev)
 {
 	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
 
-	arm_pm_restart = NULL;
 	moxart_wdt_stop(&moxart_wdt->dev);
 	watchdog_unregister_device(&moxart_wdt->dev);
 
-- 
1.9.1

[PATCH v3 6/6] watchdog: sunxi: Register reboot handler with watchdog subsystem

[Guenter Roeck]

The watchdog subsystem now provides an API to trigger a system reboot.
Register with it.

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v3: Drop reboot mode and cmd string parameters from API
v2: Rebased to v2 of Maxime's patch adding reboot support
    to the sunxi watchdog driver.

 drivers/watchdog/sunxi_wdt.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/watchdog/sunxi_wdt.c b/drivers/watchdog/sunxi_wdt.c
index 60deb9d..d8d4f8c 100644
--- a/drivers/watchdog/sunxi_wdt.c
+++ b/drivers/watchdog/sunxi_wdt.c
@@ -23,12 +23,9 @@
 #include <linux/moduleparam.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
-#include <linux/reboot.h>
 #include <linux/types.h>
 #include <linux/watchdog.h>
 
-#include <asm/system_misc.h>
-
 #define WDT_MAX_TIMEOUT         16
 #define WDT_MIN_TIMEOUT         1
 #define WDT_MODE_TIMEOUT(n)     ((n) << 3)
@@ -74,23 +71,24 @@ static const int wdt_timeout_map[] = {
 	[16] = 0xB, /* 16s */
 };
 
-static void __iomem *reboot_wdt_base;
-
-static void sun4i_wdt_restart(enum reboot_mode mode, const char *cmd)
+static void sunxi_wdt_reboot(struct watchdog_device *wdt_dev)
 {
+	struct sunxi_wdt_dev *sunxi_wdt = watchdog_get_drvdata(wdt_dev);
+
 	/* Enable timer and set reset bit in the watchdog */
-	writel(WDT_MODE_EN | WDT_MODE_RST_EN, reboot_wdt_base + WDT_MODE);
+	writel(WDT_MODE_EN | WDT_MODE_RST_EN,
+	       sunxi_wdt->wdt_base + WDT_MODE);
 
 	/*
 	 * Restart the watchdog. The default (and lowest) interval
 	 * value for the watchdog is 0.5s.
 	 */
-	writel(WDT_CTRL_RELOAD, reboot_wdt_base + WDT_CTRL);
+	writel(WDT_CTRL_RELOAD, sunxi_wdt->wdt_base + WDT_CTRL);
 
 	while (1) {
 		mdelay(5);
 		writel(WDT_MODE_EN | WDT_MODE_RST_EN,
-		       reboot_wdt_base + WDT_MODE);
+		       sunxi_wdt->wdt_base + WDT_MODE);
 	}
 }
 
@@ -167,6 +165,7 @@ static const struct watchdog_ops sunxi_wdt_ops = {
 	.start		= sunxi_wdt_start,
 	.stop		= sunxi_wdt_stop,
 	.ping		= sunxi_wdt_ping,
+	.reboot		= sunxi_wdt_reboot,
 	.set_timeout	= sunxi_wdt_set_timeout,
 };
 
@@ -205,9 +204,6 @@ static int sunxi_wdt_probe(struct platform_device *pdev)
 	if (unlikely(err))
 		return err;
 
-	reboot_wdt_base = sunxi_wdt->wdt_base;
-	arm_pm_restart = sun4i_wdt_restart;
-
 	dev_info(&pdev->dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)",
 			sunxi_wdt->wdt_dev.timeout, nowayout);
 
@@ -218,8 +214,6 @@ static int sunxi_wdt_remove(struct platform_device *pdev)
 {
 	struct sunxi_wdt_dev *sunxi_wdt = platform_get_drvdata(pdev);
 
-	arm_pm_restart = NULL;
-
 	watchdog_unregister_device(&sunxi_wdt->wdt_dev);
 	watchdog_set_drvdata(&sunxi_wdt->wdt_dev, NULL);
 
-- 
1.9.1