[RFC PATCH 00/13] watchdog: factorize restart handler registration

[RFC PATCH 00/13] watchdog: factorize restart handler registration

[Damien Riegel]

Many drivers implements the exact same piece of code to register a
restart handler. It can be nice to factorize this in the watchdog core.

The first patch adds an optional restart watchdog operation. If a driver
defines this operation, a restart handler is registered. By default, the
restart handler priority is set to 0, but a helper function
watchdog_set_restart_priority is provided to change it.

The following patches bring this change to the current watchdog drivers
that use watchdog_core.

This change has been compile-tested on da9063, imx2, lpc18xx, imgpdc.
It has been tested with (not mainlined yet) ts-4800's watchdog driver.

Damien Riegel (13):
  watchdog: core: add restart handler support
  watchdog: bcm47xx_wdt: use core restart handler
  watchdog: da9063_wdt: use core restart handler
  watchdog: digicolor_wdt: use core restart handler
  watchdog: imgpdc_wdt: use core restart handler
  watchdog: imx2_wdt: use core restart handler
  watchdog: lpc18xx_wdt: use core restart handler
  watchdog: meson_wdt: use core restart handler
  watchdog: moxart_wdt: use core restart handler
  watchdog: mtk_wdt: use core restart handler
  watchdog: qcom-wdt: use core restart handler
  watchdog: s3c2410_wdt: use core restart handler
  watchdog: sunxi_wdt: use core restart handler

 Documentation/watchdog/watchdog-kernel-api.txt |  2 +
 drivers/watchdog/bcm47xx_wdt.c                 | 21 +++-----
 drivers/watchdog/da9063_wdt.c                  | 23 +++------
 drivers/watchdog/digicolor_wdt.c               | 18 ++-----
 drivers/watchdog/imgpdc_wdt.c                  | 34 +++++--------
 drivers/watchdog/imx2_wdt.c                    | 22 +++------
 drivers/watchdog/lpc18xx_wdt.c                 | 68 +++++++++++---------------
 drivers/watchdog/meson_wdt.c                   | 22 ++-------
 drivers/watchdog/moxart_wdt.c                  | 22 +++------
 drivers/watchdog/mtk_wdt.c                     | 19 ++-----
 drivers/watchdog/qcom-wdt.c                    | 63 ++++++++++--------------
 drivers/watchdog/s3c2410_wdt.c                 | 60 ++++++++++-------------
 drivers/watchdog/sunxi_wdt.c                   | 22 ++-------
 drivers/watchdog/watchdog_core.c               | 35 +++++++++++++
 include/linux/bcm47xx_wdt.h                    |  1 -
 include/linux/watchdog.h                       |  5 ++
 16 files changed, 178 insertions(+), 259 deletions(-)

-- 
2.5.0

[RFC PATCH 01/13] watchdog: core: add restart handler support

[Damien Riegel]

Many watchdog drivers implement the same code to register a restart
handler. This patch provides a generic way to set such a function.

The patch adds a new restart watchdog operation. If a restart priority
greater than 0 is needed, the driver can call
watchdog_set_restart_priority to set it.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 Documentation/watchdog/watchdog-kernel-api.txt |  2 ++
 drivers/watchdog/watchdog_core.c               | 35 ++++++++++++++++++++++++++
 include/linux/watchdog.h                       |  5 ++++
 3 files changed, 42 insertions(+)

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index d8b0d33..10e6132 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -100,6 +100,7 @@ struct watchdog_ops {
 	unsigned int (*status)(struct watchdog_device *);
 	int (*set_timeout)(struct watchdog_device *, unsigned int);
 	unsigned int (*get_timeleft)(struct watchdog_device *);
+	int (*restart)(struct watchdog_device *);
 	void (*ref)(struct watchdog_device *);
 	void (*unref)(struct watchdog_device *);
 	long (*ioctl)(struct watchdog_device *, unsigned int, unsigned long);
@@ -164,6 +165,7 @@ they are supported. These optional routines/operations are:
   (Note: the WDIOF_SETTIMEOUT needs to be set in the options field of the
   watchdog's info structure).
 * get_timeleft: this routines returns the time that's left before a reset.
+* restart: this routine restarts the machine.
 * ref: the operation that calls kref_get on the kref of a dynamically
   allocated watchdog_device struct.
 * unref: the operation that calls kref_put on the kref of a dynamically
diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index 873f139..9c8a8e8 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -32,6 +32,7 @@
 #include <linux/types.h>	/* For standard types */
 #include <linux/errno.h>	/* For the -ENODEV/... values */
 #include <linux/kernel.h>	/* For printk/panic/... */
+#include <linux/reboot.h>	/* For restart handler */
 #include <linux/watchdog.h>	/* For watchdog specific items */
 #include <linux/init.h>		/* For __init/__exit/... */
 #include <linux/idr.h>		/* For ida_* macros */
@@ -137,6 +138,28 @@ int watchdog_init_timeout(struct watchdog_device *wdd,
 }
 EXPORT_SYMBOL_GPL(watchdog_init_timeout);
 
+static int watchdog_restart_notifier(struct notifier_block *nb,
+				     unsigned long action, void *data)
+{
+	struct watchdog_device *wdd = container_of(nb, struct watchdog_device,
+						   restart_nb);
+
+	int ret;
+
+	ret = wdd->ops->restart(wdd);
+	if (ret)
+		return NOTIFY_BAD;
+
+	return NOTIFY_DONE;
+}
+
+void watchdog_set_restart_priority(struct watchdog_device *wdd,
+				   int priority)
+{
+	wdd->restart_nb.priority = priority;
+}
+EXPORT_SYMBOL_GPL(watchdog_set_restart_priority);
+
 static int __watchdog_register_device(struct watchdog_device *wdd)
 {
 	int ret, id = -1, devno;
@@ -202,6 +225,15 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
 		return ret;
 	}
 
+	if (wdd->ops->restart) {
+		wdd->restart_nb.notifier_call = watchdog_restart_notifier;
+
+		ret = register_restart_handler(&wdd->restart_nb);
+		if (ret)
+			dev_err(wdd->dev, "Cannot register restart handler (%d)\n",
+				ret);
+	}
+
 	return 0;
 }
 
@@ -238,6 +270,9 @@ static void __watchdog_unregister_device(struct watchdog_device *wdd)
 	if (wdd == NULL)
 		return;
 
+	if (wdd->ops->restart)
+		unregister_restart_handler(&wdd->restart_nb);
+
 	devno = wdd->cdev.dev;
 	ret = watchdog_dev_unregister(wdd);
 	if (ret)
diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
index e90e3ea..e1a4dc4 100644
--- a/include/linux/watchdog.h
+++ b/include/linux/watchdog.h
@@ -26,6 +26,7 @@ struct watchdog_device;
  * @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.
+ * @restart:	The routine for restarting the machine.
  * @ref:	The ref operation for dyn. allocated watchdog_device structs
  * @unref:	The unref operation for dyn. allocated watchdog_device structs
  * @ioctl:	The routines that handles extra ioctl calls.
@@ -45,6 +46,7 @@ struct watchdog_ops {
 	unsigned int (*status)(struct watchdog_device *);
 	int (*set_timeout)(struct watchdog_device *, unsigned int);
 	unsigned int (*get_timeleft)(struct watchdog_device *);
+	int (*restart)(struct watchdog_device *);
 	void (*ref)(struct watchdog_device *);
 	void (*unref)(struct watchdog_device *);
 	long (*ioctl)(struct watchdog_device *, unsigned int, unsigned long);
@@ -88,6 +90,7 @@ struct watchdog_device {
 	unsigned int timeout;
 	unsigned int min_timeout;
 	unsigned int max_timeout;
+	struct notifier_block restart_nb;
 	void *driver_data;
 	struct mutex lock;
 	unsigned long status;
@@ -142,6 +145,8 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
 }
 
 /* drivers/watchdog/watchdog_core.c */
+extern void watchdog_set_restart_priority(struct watchdog_device *wdd,
+					  int priority);
 extern int watchdog_init_timeout(struct watchdog_device *wdd,
 				  unsigned int timeout_parm, struct device *dev);
 extern int watchdog_register_device(struct watchdog_device *);
-- 
2.5.0

Re: [RFC PATCH 01/13] watchdog: core: add restart handler support

[Guenter Roeck]

On 11/02/2015 05:36 PM, Damien Riegel wrote:
> Many watchdog drivers implement the same code to register a restart
> handler. This patch provides a generic way to set such a function.
>
> The patch adds a new restart watchdog operation. If a restart priority
> greater than 0 is needed, the driver can call
> watchdog_set_restart_priority to set it.
>
> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Makes sense, and good idea. Unless the patch was written by Vivien,
the second tag should probably be a Reviewed-by: or Acked-by:, though.

Couple of additional comments below.

> ---
>   Documentation/watchdog/watchdog-kernel-api.txt |  2 ++
>   drivers/watchdog/watchdog_core.c               | 35 ++++++++++++++++++++++++++
>   include/linux/watchdog.h                       |  5 ++++
>   3 files changed, 42 insertions(+)
>
> diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
> index d8b0d33..10e6132 100644
> --- a/Documentation/watchdog/watchdog-kernel-api.txt
> +++ b/Documentation/watchdog/watchdog-kernel-api.txt
> @@ -100,6 +100,7 @@ struct watchdog_ops {
>   	unsigned int (*status)(struct watchdog_device *);
>   	int (*set_timeout)(struct watchdog_device *, unsigned int);
>   	unsigned int (*get_timeleft)(struct watchdog_device *);
> +	int (*restart)(struct watchdog_device *);
>   	void (*ref)(struct watchdog_device *);
>   	void (*unref)(struct watchdog_device *);
>   	long (*ioctl)(struct watchdog_device *, unsigned int, unsigned long);
> @@ -164,6 +165,7 @@ they are supported. These optional routines/operations are:
>     (Note: the WDIOF_SETTIMEOUT needs to be set in the options field of the
>     watchdog's info structure).
>   * get_timeleft: this routines returns the time that's left before a reset.
> +* restart: this routine restarts the machine.

Please describe the expected return values.
0 - ok, or a negative error value, presumably.

>   * ref: the operation that calls kref_get on the kref of a dynamically
>     allocated watchdog_device struct.
>   * unref: the operation that calls kref_put on the kref of a dynamically
> diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
> index 873f139..9c8a8e8 100644
> --- a/drivers/watchdog/watchdog_core.c
> +++ b/drivers/watchdog/watchdog_core.c
> @@ -32,6 +32,7 @@
>   #include <linux/types.h>	/* For standard types */
>   #include <linux/errno.h>	/* For the -ENODEV/... values */
>   #include <linux/kernel.h>	/* For printk/panic/... */
> +#include <linux/reboot.h>	/* For restart handler */
>   #include <linux/watchdog.h>	/* For watchdog specific items */
>   #include <linux/init.h>		/* For __init/__exit/... */
>   #include <linux/idr.h>		/* For ida_* macros */
> @@ -137,6 +138,28 @@ int watchdog_init_timeout(struct watchdog_device *wdd,
>   }
>   EXPORT_SYMBOL_GPL(watchdog_init_timeout);
>
> +static int watchdog_restart_notifier(struct notifier_block *nb,
> +				     unsigned long action, void *data)
> +{
> +	struct watchdog_device *wdd = container_of(nb, struct watchdog_device,
> +						   restart_nb);
> +
> +	int ret;
> +
> +	ret = wdd->ops->restart(wdd);
> +	if (ret)
> +		return NOTIFY_BAD;
> +
> +	return NOTIFY_DONE;
> +}
> +
> +void watchdog_set_restart_priority(struct watchdog_device *wdd,
> +				   int priority)

Fits one line.

> +{
> +	wdd->restart_nb.priority = priority;
> +}
> +EXPORT_SYMBOL_GPL(watchdog_set_restart_priority);
> +
>   static int __watchdog_register_device(struct watchdog_device *wdd)
>   {
>   	int ret, id = -1, devno;
> @@ -202,6 +225,15 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
>   		return ret;
>   	}
>
> +	if (wdd->ops->restart) {
> +		wdd->restart_nb.notifier_call = watchdog_restart_notifier;
> +
> +		ret = register_restart_handler(&wdd->restart_nb);
> +		if (ret)
> +			dev_err(wdd->dev, "Cannot register restart handler (%d)\n",
> +				ret);

dev_warn, please.

> +	}
> +
>   	return 0;
>   }
>
> @@ -238,6 +270,9 @@ static void __watchdog_unregister_device(struct watchdog_device *wdd)
>   	if (wdd == NULL)
>   		return;
>
> +	if (wdd->ops->restart)
> +		unregister_restart_handler(&wdd->restart_nb);
> +
>   	devno = wdd->cdev.dev;
>   	ret = watchdog_dev_unregister(wdd);
>   	if (ret)
> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> index e90e3ea..e1a4dc4 100644
> --- a/include/linux/watchdog.h
> +++ b/include/linux/watchdog.h
> @@ -26,6 +26,7 @@ struct watchdog_device;
>    * @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.
> + * @restart:	The routine for restarting the machine.
>    * @ref:	The ref operation for dyn. allocated watchdog_device structs
>    * @unref:	The unref operation for dyn. allocated watchdog_device structs
>    * @ioctl:	The routines that handles extra ioctl calls.
> @@ -45,6 +46,7 @@ struct watchdog_ops {
>   	unsigned int (*status)(struct watchdog_device *);
>   	int (*set_timeout)(struct watchdog_device *, unsigned int);
>   	unsigned int (*get_timeleft)(struct watchdog_device *);
> +	int (*restart)(struct watchdog_device *);
>   	void (*ref)(struct watchdog_device *);
>   	void (*unref)(struct watchdog_device *);
>   	long (*ioctl)(struct watchdog_device *, unsigned int, unsigned long);
> @@ -88,6 +90,7 @@ struct watchdog_device {
>   	unsigned int timeout;
>   	unsigned int min_timeout;
>   	unsigned int max_timeout;
> +	struct notifier_block restart_nb;
>   	void *driver_data;
>   	struct mutex lock;
>   	unsigned long status;
> @@ -142,6 +145,8 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd)
>   }
>
>   /* drivers/watchdog/watchdog_core.c */
> +extern void watchdog_set_restart_priority(struct watchdog_device *wdd,
> +					  int priority);

extern is unnecessary (yes, we'll need to fix that for the other exported
functions, but no need to introduce new ones), and then it fits one line.

>   extern int watchdog_init_timeout(struct watchdog_device *wdd,
>   				  unsigned int timeout_parm, struct device *dev);
>   extern int watchdog_register_device(struct watchdog_device *);
>

Re: [RFC PATCH 01/13] watchdog: core: add restart handler support

[Vivien Didelot]

Hi Guenter,

On Nov. Monday 02 (45) 06:25 PM, Guenter Roeck wrote:
> On 11/02/2015 05:36 PM, Damien Riegel wrote:
> >Many watchdog drivers implement the same code to register a restart
> >handler. This patch provides a generic way to set such a function.
> >
> >The patch adds a new restart watchdog operation. If a restart priority
> >greater than 0 is needed, the driver can call
> >watchdog_set_restart_priority to set it.
> >
> >Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> >Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> 
> Makes sense, and good idea. Unless the patch was written by Vivien,
> the second tag should probably be a Reviewed-by: or Acked-by:, though.

We wrote that together, but Damien did most of the work. So I think
Reviewed-by: for me will indeed be most appropriate here.

Also is it OK to include your Reviewed-by: tag in the v1 (given we add
your comments) or not?

Thanks!
-v

Re: [RFC PATCH 01/13] watchdog: core: add restart handler support

[Guenter Roeck]

On 11/02/2015 06:51 PM, Vivien Didelot wrote:
> Hi Guenter,
>
> On Nov. Monday 02 (45) 06:25 PM, Guenter Roeck wrote:
>> On 11/02/2015 05:36 PM, Damien Riegel wrote:
>>> Many watchdog drivers implement the same code to register a restart
>>> handler. This patch provides a generic way to set such a function.
>>>
>>> The patch adds a new restart watchdog operation. If a restart priority
>>> greater than 0 is needed, the driver can call
>>> watchdog_set_restart_priority to set it.
>>>
>>> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
>>> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>>
>> Makes sense, and good idea. Unless the patch was written by Vivien,
>> the second tag should probably be a Reviewed-by: or Acked-by:, though.
>
> We wrote that together, but Damien did most of the work. So I think
> Reviewed-by: for me will indeed be most appropriate here.
>
> Also is it OK to include your Reviewed-by: tag in the v1 (given we add
> your comments) or not?
>

Hi Vivien,

For the entire series, if you address my comments, it is ok to add my
Reviewed-by:.

Couple of additional comments:
- Please describe the notifier field added to struct watchdog_device
   in Documentation/watchdog/watchdog-kernel-api.txt.
- Please include linux/notifier.h from include/linux/watchdog.h.

Thanks,
Guenter

[RFC PATCH 02/13] watchdog: bcm47xx_wdt: use core restart handler

[Damien Riegel]

Get rid of the custom restart handler by using the one provided by the
watchdog core.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/watchdog/bcm47xx_wdt.c | 21 +++++++--------------
 include/linux/bcm47xx_wdt.h    |  1 -
 2 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/watchdog/bcm47xx_wdt.c b/drivers/watchdog/bcm47xx_wdt.c
index 4064a43..534ffb4 100644
--- a/drivers/watchdog/bcm47xx_wdt.c
+++ b/drivers/watchdog/bcm47xx_wdt.c
@@ -94,6 +94,7 @@ static struct watchdog_ops bcm47xx_wdt_hard_ops = {
 	.stop		= bcm47xx_wdt_hard_stop,
 	.ping		= bcm47xx_wdt_hard_keepalive,
 	.set_timeout	= bcm47xx_wdt_hard_set_timeout,
+	.restart        = bcm47xx_wdt_restart,
 };
 
 static void bcm47xx_wdt_soft_timer_tick(unsigned long data)
@@ -169,15 +170,13 @@ static int bcm47xx_wdt_notify_sys(struct notifier_block *this,
 	return NOTIFY_DONE;
 }
 
-static int bcm47xx_wdt_restart(struct notifier_block *this, unsigned long mode,
-			       void *cmd)
+static int bcm47xx_wdt_restart(struct watchdog_device *wdd)
 {
-	struct bcm47xx_wdt *wdt;
+	struct bcm47xx_wdt *wdt = bcm47xx_wdt_get(wdd);
 
-	wdt = container_of(this, struct bcm47xx_wdt, restart_handler);
 	wdt->timer_set(wdt, 1);
 
-	return NOTIFY_DONE;
+	return 0;
 }
 
 static struct watchdog_ops bcm47xx_wdt_soft_ops = {
@@ -186,6 +185,7 @@ static struct watchdog_ops bcm47xx_wdt_soft_ops = {
 	.stop		= bcm47xx_wdt_soft_stop,
 	.ping		= bcm47xx_wdt_soft_keepalive,
 	.set_timeout	= bcm47xx_wdt_soft_set_timeout,
+	.restart        = bcm47xx_wdt_restart,
 };
 
 static int bcm47xx_wdt_probe(struct platform_device *pdev)
@@ -214,6 +214,7 @@ static int bcm47xx_wdt_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_timer;
 	watchdog_set_nowayout(&wdt->wdd, nowayout);
+	watchdog_set_restart_priority(&wdt->wdd, 64);
 
 	wdt->notifier.notifier_call = &bcm47xx_wdt_notify_sys;
 
@@ -221,23 +222,15 @@ static int bcm47xx_wdt_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_timer;
 
-	wdt->restart_handler.notifier_call = &bcm47xx_wdt_restart;
-	wdt->restart_handler.priority = 64;
-	ret = register_restart_handler(&wdt->restart_handler);
-	if (ret)
-		goto err_notifier;
-
 	ret = watchdog_register_device(&wdt->wdd);
 	if (ret)
-		goto err_handler;
+		goto err_notifier;
 
 	dev_info(&pdev->dev, "BCM47xx Watchdog Timer enabled (%d seconds%s%s)\n",
 		timeout, nowayout ? ", nowayout" : "",
 		soft ? ", Software Timer" : "");
 	return 0;
 
-err_handler:
-	unregister_restart_handler(&wdt->restart_handler);
 err_notifier:
 	unregister_reboot_notifier(&wdt->notifier);
 err_timer:
diff --git a/include/linux/bcm47xx_wdt.h b/include/linux/bcm47xx_wdt.h
index 5582c21..b708786 100644
--- a/include/linux/bcm47xx_wdt.h
+++ b/include/linux/bcm47xx_wdt.h
@@ -16,7 +16,6 @@ struct bcm47xx_wdt {
 
 	struct watchdog_device wdd;
 	struct notifier_block notifier;
-	struct notifier_block restart_handler;
 
 	struct timer_list soft_timer;
 	atomic_t soft_ticks;
-- 
2.5.0

Re: [RFC PATCH 02/13] watchdog: bcm47xx_wdt: use core restart handler

[Guenter Roeck]

On 11/02/2015 05:36 PM, Damien Riegel wrote:
> Get rid of the custom restart handler by using the one provided by the
> watchdog core.
>
> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>   drivers/watchdog/bcm47xx_wdt.c | 21 +++++++--------------
>   include/linux/bcm47xx_wdt.h    |  1 -
>   2 files changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/watchdog/bcm47xx_wdt.c b/drivers/watchdog/bcm47xx_wdt.c
> index 4064a43..534ffb4 100644
> --- a/drivers/watchdog/bcm47xx_wdt.c
> +++ b/drivers/watchdog/bcm47xx_wdt.c
> @@ -94,6 +94,7 @@ static struct watchdog_ops bcm47xx_wdt_hard_ops = {
>   	.stop		= bcm47xx_wdt_hard_stop,
>   	.ping		= bcm47xx_wdt_hard_keepalive,
>   	.set_timeout	= bcm47xx_wdt_hard_set_timeout,
> +	.restart        = bcm47xx_wdt_restart,
>   };
>
>   static void bcm47xx_wdt_soft_timer_tick(unsigned long data)
> @@ -169,15 +170,13 @@ static int bcm47xx_wdt_notify_sys(struct notifier_block *this,
>   	return NOTIFY_DONE;
>   }
>
> -static int bcm47xx_wdt_restart(struct notifier_block *this, unsigned long mode,
> -			       void *cmd)
> +static int bcm47xx_wdt_restart(struct watchdog_device *wdd)
>   {
> -	struct bcm47xx_wdt *wdt;
> +	struct bcm47xx_wdt *wdt = bcm47xx_wdt_get(wdd);
>
> -	wdt = container_of(this, struct bcm47xx_wdt, restart_handler);
>   	wdt->timer_set(wdt, 1);
>
> -	return NOTIFY_DONE;
> +	return 0;
>   }
>
>   static struct watchdog_ops bcm47xx_wdt_soft_ops = {
> @@ -186,6 +185,7 @@ static struct watchdog_ops bcm47xx_wdt_soft_ops = {
>   	.stop		= bcm47xx_wdt_soft_stop,
>   	.ping		= bcm47xx_wdt_soft_keepalive,
>   	.set_timeout	= bcm47xx_wdt_soft_set_timeout,
> +	.restart        = bcm47xx_wdt_restart,
>   };
>
>   static int bcm47xx_wdt_probe(struct platform_device *pdev)
> @@ -214,6 +214,7 @@ static int bcm47xx_wdt_probe(struct platform_device *pdev)
>   	if (ret)
>   		goto err_timer;
>   	watchdog_set_nowayout(&wdt->wdd, nowayout);
> +	watchdog_set_restart_priority(&wdt->wdd, 64);
>
>   	wdt->notifier.notifier_call = &bcm47xx_wdt_notify_sys;
>
> @@ -221,23 +222,15 @@ static int bcm47xx_wdt_probe(struct platform_device *pdev)
>   	if (ret)
>   		goto err_timer;
>
> -	wdt->restart_handler.notifier_call = &bcm47xx_wdt_restart;
> -	wdt->restart_handler.priority = 64;
> -	ret = register_restart_handler(&wdt->restart_handler);
> -	if (ret)
> -		goto err_notifier;
> -
>   	ret = watchdog_register_device(&wdt->wdd);
>   	if (ret)
> -		goto err_handler;
> +		goto err_notifier;

		goto err_handler;

>
>   	dev_info(&pdev->dev, "BCM47xx Watchdog Timer enabled (%d seconds%s%s)\n",
>   		timeout, nowayout ? ", nowayout" : "",
>   		soft ? ", Software Timer" : "");
>   	return 0;
>
> -err_handler:
> -	unregister_restart_handler(&wdt->restart_handler);
>   err_notifier:
>   	unregister_reboot_notifier(&wdt->notifier);
>   err_timer:
> diff --git a/include/linux/bcm47xx_wdt.h b/include/linux/bcm47xx_wdt.h
> index 5582c21..b708786 100644
> --- a/include/linux/bcm47xx_wdt.h
> +++ b/include/linux/bcm47xx_wdt.h
> @@ -16,7 +16,6 @@ struct bcm47xx_wdt {
>
>   	struct watchdog_device wdd;
>   	struct notifier_block notifier;
> -	struct notifier_block restart_handler;
>
>   	struct timer_list soft_timer;
>   	atomic_t soft_ticks;
>

Re: [RFC PATCH 02/13] watchdog: bcm47xx_wdt: use core restart handler

[Vivien Didelot]

Hi Guenter,

On Nov. Monday 02 (45) 06:26 PM, Guenter Roeck wrote:
> On 11/02/2015 05:36 PM, Damien Riegel wrote:
> >Get rid of the custom restart handler by using the one provided by the
> >watchdog core.
> >
> >Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> >Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

<snip>

> >@@ -221,23 +222,15 @@ static int bcm47xx_wdt_probe(struct platform_device *pdev)
> >  	if (ret)
> >  		goto err_timer;
> >
> >-	wdt->restart_handler.notifier_call = &bcm47xx_wdt_restart;
> >-	wdt->restart_handler.priority = 64;
> >-	ret = register_restart_handler(&wdt->restart_handler);
> >-	if (ret)
> >-		goto err_notifier;
> >-
> >  	ret = watchdog_register_device(&wdt->wdd);
> >  	if (ret)
> >-		goto err_handler;
> >+		goto err_notifier;
> 
> 		goto err_handler;

I think we want err_notifier here, since it is the label which
unregisters the reboot notifier, and err_handler gets removed.

> 
> >
> >  	dev_info(&pdev->dev, "BCM47xx Watchdog Timer enabled (%d seconds%s%s)\n",
> >  		timeout, nowayout ? ", nowayout" : "",
> >  		soft ? ", Software Timer" : "");
> >  	return 0;
> >
> >-err_handler:
> >-	unregister_restart_handler(&wdt->restart_handler);
> >  err_notifier:
> >  	unregister_reboot_notifier(&wdt->notifier);
> >  err_timer:

Thanks,
-v

Re: [RFC PATCH 02/13] watchdog: bcm47xx_wdt: use core restart handler

[Guenter Roeck]

On 11/03/2015 06:21 AM, Vivien Didelot wrote:
> Hi Guenter,
>
> On Nov. Monday 02 (45) 06:26 PM, Guenter Roeck wrote:
>> On 11/02/2015 05:36 PM, Damien Riegel wrote:
>>> Get rid of the custom restart handler by using the one provided by the
>>> watchdog core.
>>>
>>> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
>>> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
>
> <snip>
>
>>> @@ -221,23 +222,15 @@ static int bcm47xx_wdt_probe(struct platform_device *pdev)
>>>   	if (ret)
>>>   		goto err_timer;
>>>
>>> -	wdt->restart_handler.notifier_call = &bcm47xx_wdt_restart;
>>> -	wdt->restart_handler.priority = 64;
>>> -	ret = register_restart_handler(&wdt->restart_handler);
>>> -	if (ret)
>>> -		goto err_notifier;
>>> -
>>>   	ret = watchdog_register_device(&wdt->wdd);
>>>   	if (ret)
>>> -		goto err_handler;
>>> +		goto err_notifier;
>>
>> 		goto err_handler;
>
> I think we want err_notifier here, since it is the label which
> unregisters the reboot notifier, and err_handler gets removed.
>
Yes, you are right. Sorry for the noise.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

>>
>>>
>>>   	dev_info(&pdev->dev, "BCM47xx Watchdog Timer enabled (%d seconds%s%s)\n",
>>>   		timeout, nowayout ? ", nowayout" : "",
>>>   		soft ? ", Software Timer" : "");
>>>   	return 0;
>>>
>>> -err_handler:
>>> -	unregister_restart_handler(&wdt->restart_handler);
>>>   err_notifier:
>>>   	unregister_reboot_notifier(&wdt->notifier);
>>>   err_timer:
>
> Thanks,
> -v
> --
> 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
>

[RFC PATCH 03/13] watchdog: da9063_wdt: use core restart handler

[Damien Riegel]

Get rid of the custom restart handler by using the one provided by the
watchdog core.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/watchdog/da9063_wdt.c | 23 ++++++-----------------
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/drivers/watchdog/da9063_wdt.c b/drivers/watchdog/da9063_wdt.c
index 6bf130b..11e8875 100644
--- a/drivers/watchdog/da9063_wdt.c
+++ b/drivers/watchdog/da9063_wdt.c
@@ -20,7 +20,6 @@
 #include <linux/delay.h>
 #include <linux/mfd/da9063/registers.h>
 #include <linux/mfd/da9063/core.h>
-#include <linux/reboot.h>
 #include <linux/regmap.h>
 
 /*
@@ -39,7 +38,6 @@ static const unsigned int wdt_timeout[] = { 0, 2, 4, 8, 16, 32, 65, 131 };
 struct da9063_watchdog {
 	struct da9063 *da9063;
 	struct watchdog_device wdtdev;
-	struct notifier_block restart_handler;
 };
 
 static unsigned int da9063_wdt_timeout_to_sel(unsigned int secs)
@@ -121,12 +119,9 @@ static int da9063_wdt_set_timeout(struct watchdog_device *wdd,
 	return ret;
 }
 
-static int da9063_wdt_restart_handler(struct notifier_block *this,
-				      unsigned long mode, void *cmd)
+static int da9063_wdt_restart(struct watchdog_device *wdd)
 {
-	struct da9063_watchdog *wdt = container_of(this,
-						   struct da9063_watchdog,
-						   restart_handler);
+	struct da9063_watchdog *wdt = watchdog_get_drvdata(wdd);
 	int ret;
 
 	ret = regmap_write(wdt->da9063->regmap, DA9063_REG_CONTROL_F,
@@ -135,7 +130,7 @@ static int da9063_wdt_restart_handler(struct notifier_block *this,
 		dev_alert(wdt->da9063->dev, "Failed to shutdown (err = %d)\n",
 			  ret);
 
-	return NOTIFY_DONE;
+	return ret;
 }
 
 static const struct watchdog_info da9063_watchdog_info = {
@@ -149,6 +144,7 @@ static const struct watchdog_ops da9063_watchdog_ops = {
 	.stop = da9063_wdt_stop,
 	.ping = da9063_wdt_ping,
 	.set_timeout = da9063_wdt_set_timeout,
+	.restart = da9063_wdt_restart,
 };
 
 static int da9063_wdt_probe(struct platform_device *pdev)
@@ -179,6 +175,8 @@ static int da9063_wdt_probe(struct platform_device *pdev)
 
 	wdt->wdtdev.status = WATCHDOG_NOWAYOUT_INIT_STATUS;
 
+	watchdog_set_restart_priority(&wdt->wdtdev, 128);
+
 	watchdog_set_drvdata(&wdt->wdtdev, wdt);
 	dev_set_drvdata(&pdev->dev, wdt);
 
@@ -186,13 +184,6 @@ static int da9063_wdt_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	wdt->restart_handler.notifier_call = da9063_wdt_restart_handler;
-	wdt->restart_handler.priority = 128;
-	ret = register_restart_handler(&wdt->restart_handler);
-	if (ret)
-		dev_err(wdt->da9063->dev,
-			"Failed to register restart handler (err = %d)\n", ret);
-
 	return 0;
 }
 
@@ -200,8 +191,6 @@ static int da9063_wdt_remove(struct platform_device *pdev)
 {
 	struct da9063_watchdog *wdt = dev_get_drvdata(&pdev->dev);
 
-	unregister_restart_handler(&wdt->restart_handler);
-
 	watchdog_unregister_device(&wdt->wdtdev);
 
 	return 0;
-- 
2.5.0

Re: [RFC PATCH 03/13] watchdog: da9063_wdt: use core restart handler

[Guenter Roeck]

On 11/02/2015 05:36 PM, Damien Riegel wrote:
> Get rid of the custom restart handler by using the one provided by the
> watchdog core.
>
> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/watchdog/da9063_wdt.c | 23 ++++++-----------------
>   1 file changed, 6 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/watchdog/da9063_wdt.c b/drivers/watchdog/da9063_wdt.c
> index 6bf130b..11e8875 100644
> --- a/drivers/watchdog/da9063_wdt.c
> +++ b/drivers/watchdog/da9063_wdt.c
> @@ -20,7 +20,6 @@
>   #include <linux/delay.h>
>   #include <linux/mfd/da9063/registers.h>
>   #include <linux/mfd/da9063/core.h>
> -#include <linux/reboot.h>
>   #include <linux/regmap.h>
>
>   /*
> @@ -39,7 +38,6 @@ static const unsigned int wdt_timeout[] = { 0, 2, 4, 8, 16, 32, 65, 131 };
>   struct da9063_watchdog {
>   	struct da9063 *da9063;
>   	struct watchdog_device wdtdev;
> -	struct notifier_block restart_handler;
>   };
>
>   static unsigned int da9063_wdt_timeout_to_sel(unsigned int secs)
> @@ -121,12 +119,9 @@ static int da9063_wdt_set_timeout(struct watchdog_device *wdd,
>   	return ret;
>   }
>
> -static int da9063_wdt_restart_handler(struct notifier_block *this,
> -				      unsigned long mode, void *cmd)
> +static int da9063_wdt_restart(struct watchdog_device *wdd)
>   {
> -	struct da9063_watchdog *wdt = container_of(this,
> -						   struct da9063_watchdog,
> -						   restart_handler);
> +	struct da9063_watchdog *wdt = watchdog_get_drvdata(wdd);
>   	int ret;
>
>   	ret = regmap_write(wdt->da9063->regmap, DA9063_REG_CONTROL_F,
> @@ -135,7 +130,7 @@ static int da9063_wdt_restart_handler(struct notifier_block *this,
>   		dev_alert(wdt->da9063->dev, "Failed to shutdown (err = %d)\n",
>   			  ret);
>
> -	return NOTIFY_DONE;
> +	return ret;
>   }
>
>   static const struct watchdog_info da9063_watchdog_info = {
> @@ -149,6 +144,7 @@ static const struct watchdog_ops da9063_watchdog_ops = {
>   	.stop = da9063_wdt_stop,
>   	.ping = da9063_wdt_ping,
>   	.set_timeout = da9063_wdt_set_timeout,
> +	.restart = da9063_wdt_restart,
>   };
>
>   static int da9063_wdt_probe(struct platform_device *pdev)
> @@ -179,6 +175,8 @@ static int da9063_wdt_probe(struct platform_device *pdev)
>
>   	wdt->wdtdev.status = WATCHDOG_NOWAYOUT_INIT_STATUS;
>
> +	watchdog_set_restart_priority(&wdt->wdtdev, 128);
> +
>   	watchdog_set_drvdata(&wdt->wdtdev, wdt);
>   	dev_set_drvdata(&pdev->dev, wdt);
>
> @@ -186,13 +184,6 @@ static int da9063_wdt_probe(struct platform_device *pdev)
>   	if (ret)
>   		return ret;
>
> -	wdt->restart_handler.notifier_call = da9063_wdt_restart_handler;
> -	wdt->restart_handler.priority = 128;
> -	ret = register_restart_handler(&wdt->restart_handler);
> -	if (ret)
> -		dev_err(wdt->da9063->dev,
> -			"Failed to register restart handler (err = %d)\n", ret);
> -
>   	return 0;
>   }
>
> @@ -200,8 +191,6 @@ static int da9063_wdt_remove(struct platform_device *pdev)
>   {
>   	struct da9063_watchdog *wdt = dev_get_drvdata(&pdev->dev);
>
> -	unregister_restart_handler(&wdt->restart_handler);
> -
>   	watchdog_unregister_device(&wdt->wdtdev);
>
>   	return 0;
>

[RFC PATCH 04/13] watchdog: digicolor_wdt: use core restart handler

[Damien Riegel]

Get rid of the custom restart handler by using the one provided by the
watchdog core.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/watchdog/digicolor_wdt.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/watchdog/digicolor_wdt.c b/drivers/watchdog/digicolor_wdt.c
index 50abe1b..1ccb0b2 100644
--- a/drivers/watchdog/digicolor_wdt.c
+++ b/drivers/watchdog/digicolor_wdt.c
@@ -15,7 +15,6 @@
 #include <linux/delay.h>
 #include <linux/clk.h>
 #include <linux/watchdog.h>
-#include <linux/reboot.h>
 #include <linux/platform_device.h>
 #include <linux/of_address.h>
 
@@ -28,7 +27,6 @@
 struct dc_wdt {
 	void __iomem		*base;
 	struct clk		*clk;
-	struct notifier_block	restart_handler;
 	spinlock_t		lock;
 };
 
@@ -50,16 +48,15 @@ static void dc_wdt_set(struct dc_wdt *wdt, u32 ticks)
 	spin_unlock_irqrestore(&wdt->lock, flags);
 }
 
-static int dc_restart_handler(struct notifier_block *this, unsigned long mode,
-			      void *cmd)
+static int dc_wdt_restart(struct watchdog_device *wdog)
 {
-	struct dc_wdt *wdt = container_of(this, struct dc_wdt, restart_handler);
+	struct dc_wdt *wdt = watchdog_get_drvdata(wdog);
 
 	dc_wdt_set(wdt, 1);
 	/* wait for reset to assert... */
 	mdelay(500);
 
-	return NOTIFY_DONE;
+	return 0;
 }
 
 static int dc_wdt_start(struct watchdog_device *wdog)
@@ -104,6 +101,7 @@ static struct watchdog_ops dc_wdt_ops = {
 	.stop		= dc_wdt_stop,
 	.set_timeout	= dc_wdt_set_timeout,
 	.get_timeleft	= dc_wdt_get_timeleft,
+	.restart        = dc_wdt_restart,
 };
 
 static struct watchdog_info dc_wdt_info = {
@@ -148,6 +146,7 @@ static int dc_wdt_probe(struct platform_device *pdev)
 	spin_lock_init(&wdt->lock);
 
 	watchdog_set_drvdata(&dc_wdt_wdd, wdt);
+	watchdog_set_restart_priority(&dc_wdt_wdd, 128);
 	watchdog_init_timeout(&dc_wdt_wdd, timeout, dev);
 	ret = watchdog_register_device(&dc_wdt_wdd);
 	if (ret) {
@@ -155,12 +154,6 @@ static int dc_wdt_probe(struct platform_device *pdev)
 		goto err_iounmap;
 	}
 
-	wdt->restart_handler.notifier_call = dc_restart_handler;
-	wdt->restart_handler.priority = 128;
-	ret = register_restart_handler(&wdt->restart_handler);
-	if (ret)
-		dev_warn(&pdev->dev, "cannot register restart handler\n");
-
 	return 0;
 
 err_iounmap:
@@ -172,7 +165,6 @@ static int dc_wdt_remove(struct platform_device *pdev)
 {
 	struct dc_wdt *wdt = platform_get_drvdata(pdev);
 
-	unregister_restart_handler(&wdt->restart_handler);
 	watchdog_unregister_device(&dc_wdt_wdd);
 	iounmap(wdt->base);
 
-- 
2.5.0

Re: [RFC PATCH 04/13] watchdog: digicolor_wdt: use core restart handler

[Guenter Roeck]

On 11/02/2015 05:36 PM, Damien Riegel wrote:
> Get rid of the custom restart handler by using the one provided by the
> watchdog core.
>
> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/watchdog/digicolor_wdt.c | 18 +++++-------------
>   1 file changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/watchdog/digicolor_wdt.c b/drivers/watchdog/digicolor_wdt.c
> index 50abe1b..1ccb0b2 100644
> --- a/drivers/watchdog/digicolor_wdt.c
> +++ b/drivers/watchdog/digicolor_wdt.c
> @@ -15,7 +15,6 @@
>   #include <linux/delay.h>
>   #include <linux/clk.h>
>   #include <linux/watchdog.h>
> -#include <linux/reboot.h>
>   #include <linux/platform_device.h>
>   #include <linux/of_address.h>
>
> @@ -28,7 +27,6 @@
>   struct dc_wdt {
>   	void __iomem		*base;
>   	struct clk		*clk;
> -	struct notifier_block	restart_handler;
>   	spinlock_t		lock;
>   };
>
> @@ -50,16 +48,15 @@ static void dc_wdt_set(struct dc_wdt *wdt, u32 ticks)
>   	spin_unlock_irqrestore(&wdt->lock, flags);
>   }
>
> -static int dc_restart_handler(struct notifier_block *this, unsigned long mode,
> -			      void *cmd)
> +static int dc_wdt_restart(struct watchdog_device *wdog)
>   {
> -	struct dc_wdt *wdt = container_of(this, struct dc_wdt, restart_handler);
> +	struct dc_wdt *wdt = watchdog_get_drvdata(wdog);
>
>   	dc_wdt_set(wdt, 1);
>   	/* wait for reset to assert... */
>   	mdelay(500);
>
> -	return NOTIFY_DONE;
> +	return 0;
>   }
>
>   static int dc_wdt_start(struct watchdog_device *wdog)
> @@ -104,6 +101,7 @@ static struct watchdog_ops dc_wdt_ops = {
>   	.stop		= dc_wdt_stop,
>   	.set_timeout	= dc_wdt_set_timeout,
>   	.get_timeleft	= dc_wdt_get_timeleft,
> +	.restart        = dc_wdt_restart,
>   };
>
>   static struct watchdog_info dc_wdt_info = {
> @@ -148,6 +146,7 @@ static int dc_wdt_probe(struct platform_device *pdev)
>   	spin_lock_init(&wdt->lock);
>
>   	watchdog_set_drvdata(&dc_wdt_wdd, wdt);
> +	watchdog_set_restart_priority(&dc_wdt_wdd, 128);
>   	watchdog_init_timeout(&dc_wdt_wdd, timeout, dev);
>   	ret = watchdog_register_device(&dc_wdt_wdd);
>   	if (ret) {
> @@ -155,12 +154,6 @@ static int dc_wdt_probe(struct platform_device *pdev)
>   		goto err_iounmap;
>   	}
>
> -	wdt->restart_handler.notifier_call = dc_restart_handler;
> -	wdt->restart_handler.priority = 128;
> -	ret = register_restart_handler(&wdt->restart_handler);
> -	if (ret)
> -		dev_warn(&pdev->dev, "cannot register restart handler\n");
> -
>   	return 0;
>
>   err_iounmap:
> @@ -172,7 +165,6 @@ static int dc_wdt_remove(struct platform_device *pdev)
>   {
>   	struct dc_wdt *wdt = platform_get_drvdata(pdev);
>
> -	unregister_restart_handler(&wdt->restart_handler);
>   	watchdog_unregister_device(&dc_wdt_wdd);
>   	iounmap(wdt->base);
>
>

[RFC PATCH 05/13] watchdog: imgpdc_wdt: use core restart handler

[Damien Riegel]

Get rid of the custom restart handler by using the one provided by the
watchdog core.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/watchdog/imgpdc_wdt.c | 34 ++++++++++++----------------------
 1 file changed, 12 insertions(+), 22 deletions(-)

diff --git a/drivers/watchdog/imgpdc_wdt.c b/drivers/watchdog/imgpdc_wdt.c
index 15ab072..3679f2e 100644
--- a/drivers/watchdog/imgpdc_wdt.c
+++ b/drivers/watchdog/imgpdc_wdt.c
@@ -45,7 +45,6 @@
 #include <linux/log2.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/reboot.h>
 #include <linux/slab.h>
 #include <linux/watchdog.h>
 
@@ -87,7 +86,6 @@ struct pdc_wdt_dev {
 	struct clk *wdt_clk;
 	struct clk *sys_clk;
 	void __iomem *base;
-	struct notifier_block restart_handler;
 };
 
 static int pdc_wdt_keepalive(struct watchdog_device *wdt_dev)
@@ -152,6 +150,16 @@ static int pdc_wdt_start(struct watchdog_device *wdt_dev)
 	return 0;
 }
 
+static int pdc_wdt_restart(struct watchdog_device *wdt_dev)
+{
+	struct pdc_wdt_dev *wdt = watchdog_get_drvdata(wdt_dev);
+
+	/* Assert SOFT_RESET */
+	writel(0x1, wdt->base + PDC_WDT_SOFT_RESET);
+
+	return 0;
+}
+
 static struct watchdog_info pdc_wdt_info = {
 	.identity	= "IMG PDC Watchdog",
 	.options	= WDIOF_SETTIMEOUT |
@@ -165,20 +173,9 @@ static const struct watchdog_ops pdc_wdt_ops = {
 	.stop		= pdc_wdt_stop,
 	.ping		= pdc_wdt_keepalive,
 	.set_timeout	= pdc_wdt_set_timeout,
+	.restart        = pdc_wdt_restart,
 };
 
-static int pdc_wdt_restart(struct notifier_block *this, unsigned long mode,
-			   void *cmd)
-{
-	struct pdc_wdt_dev *wdt = container_of(this, struct pdc_wdt_dev,
-					       restart_handler);
-
-	/* Assert SOFT_RESET */
-	writel(0x1, wdt->base + PDC_WDT_SOFT_RESET);
-
-	return NOTIFY_OK;
-}
-
 static int pdc_wdt_probe(struct platform_device *pdev)
 {
 	u64 div;
@@ -282,6 +279,7 @@ static int pdc_wdt_probe(struct platform_device *pdev)
 	}
 
 	watchdog_set_nowayout(&pdc_wdt->wdt_dev, nowayout);
+	watchdog_set_restart_priority(&pdc_wdt->wdt_dev, 128);
 
 	platform_set_drvdata(pdev, pdc_wdt);
 
@@ -289,13 +287,6 @@ static int pdc_wdt_probe(struct platform_device *pdev)
 	if (ret)
 		goto disable_wdt_clk;
 
-	pdc_wdt->restart_handler.notifier_call = pdc_wdt_restart;
-	pdc_wdt->restart_handler.priority = 128;
-	ret = register_restart_handler(&pdc_wdt->restart_handler);
-	if (ret)
-		dev_warn(&pdev->dev, "failed to register restart handler: %d\n",
-			 ret);
-
 	return 0;
 
 disable_wdt_clk:
@@ -316,7 +307,6 @@ static int pdc_wdt_remove(struct platform_device *pdev)
 {
 	struct pdc_wdt_dev *pdc_wdt = platform_get_drvdata(pdev);
 
-	unregister_restart_handler(&pdc_wdt->restart_handler);
 	pdc_wdt_stop(&pdc_wdt->wdt_dev);
 	watchdog_unregister_device(&pdc_wdt->wdt_dev);
 	clk_disable_unprepare(pdc_wdt->wdt_clk);
-- 
2.5.0

Re: [RFC PATCH 05/13] watchdog: imgpdc_wdt: use core restart handler

[Guenter Roeck]

On 11/02/2015 05:36 PM, Damien Riegel wrote:
> Get rid of the custom restart handler by using the one provided by the
> watchdog core.
>
> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/watchdog/imgpdc_wdt.c | 34 ++++++++++++----------------------
>   1 file changed, 12 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/watchdog/imgpdc_wdt.c b/drivers/watchdog/imgpdc_wdt.c
> index 15ab072..3679f2e 100644
> --- a/drivers/watchdog/imgpdc_wdt.c
> +++ b/drivers/watchdog/imgpdc_wdt.c
> @@ -45,7 +45,6 @@
>   #include <linux/log2.h>
>   #include <linux/module.h>
>   #include <linux/platform_device.h>
> -#include <linux/reboot.h>
>   #include <linux/slab.h>
>   #include <linux/watchdog.h>
>
> @@ -87,7 +86,6 @@ struct pdc_wdt_dev {
>   	struct clk *wdt_clk;
>   	struct clk *sys_clk;
>   	void __iomem *base;
> -	struct notifier_block restart_handler;
>   };
>
>   static int pdc_wdt_keepalive(struct watchdog_device *wdt_dev)
> @@ -152,6 +150,16 @@ static int pdc_wdt_start(struct watchdog_device *wdt_dev)
>   	return 0;
>   }
>
> +static int pdc_wdt_restart(struct watchdog_device *wdt_dev)
> +{
> +	struct pdc_wdt_dev *wdt = watchdog_get_drvdata(wdt_dev);
> +
> +	/* Assert SOFT_RESET */
> +	writel(0x1, wdt->base + PDC_WDT_SOFT_RESET);
> +
> +	return 0;
> +}
> +
>   static struct watchdog_info pdc_wdt_info = {
>   	.identity	= "IMG PDC Watchdog",
>   	.options	= WDIOF_SETTIMEOUT |
> @@ -165,20 +173,9 @@ static const struct watchdog_ops pdc_wdt_ops = {
>   	.stop		= pdc_wdt_stop,
>   	.ping		= pdc_wdt_keepalive,
>   	.set_timeout	= pdc_wdt_set_timeout,
> +	.restart        = pdc_wdt_restart,
>   };
>
> -static int pdc_wdt_restart(struct notifier_block *this, unsigned long mode,
> -			   void *cmd)
> -{
> -	struct pdc_wdt_dev *wdt = container_of(this, struct pdc_wdt_dev,
> -					       restart_handler);
> -
> -	/* Assert SOFT_RESET */
> -	writel(0x1, wdt->base + PDC_WDT_SOFT_RESET);
> -
> -	return NOTIFY_OK;
> -}
> -
>   static int pdc_wdt_probe(struct platform_device *pdev)
>   {
>   	u64 div;
> @@ -282,6 +279,7 @@ static int pdc_wdt_probe(struct platform_device *pdev)
>   	}
>
>   	watchdog_set_nowayout(&pdc_wdt->wdt_dev, nowayout);
> +	watchdog_set_restart_priority(&pdc_wdt->wdt_dev, 128);
>
>   	platform_set_drvdata(pdev, pdc_wdt);
>
> @@ -289,13 +287,6 @@ static int pdc_wdt_probe(struct platform_device *pdev)
>   	if (ret)
>   		goto disable_wdt_clk;
>
> -	pdc_wdt->restart_handler.notifier_call = pdc_wdt_restart;
> -	pdc_wdt->restart_handler.priority = 128;
> -	ret = register_restart_handler(&pdc_wdt->restart_handler);
> -	if (ret)
> -		dev_warn(&pdev->dev, "failed to register restart handler: %d\n",
> -			 ret);
> -
>   	return 0;
>
>   disable_wdt_clk:
> @@ -316,7 +307,6 @@ static int pdc_wdt_remove(struct platform_device *pdev)
>   {
>   	struct pdc_wdt_dev *pdc_wdt = platform_get_drvdata(pdev);
>
> -	unregister_restart_handler(&pdc_wdt->restart_handler);
>   	pdc_wdt_stop(&pdc_wdt->wdt_dev);
>   	watchdog_unregister_device(&pdc_wdt->wdt_dev);
>   	clk_disable_unprepare(pdc_wdt->wdt_clk);
>

[RFC PATCH 06/13] watchdog: imx2_wdt: use core restart handler

[Damien Riegel]

Get rid of the custom restart handler by using the one provided by the
watchdog core.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/watchdog/imx2_wdt.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 29ef719..dda44f6 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -32,7 +32,6 @@
 #include <linux/notifier.h>
 #include <linux/of_address.h>
 #include <linux/platform_device.h>
-#include <linux/reboot.h>
 #include <linux/regmap.h>
 #include <linux/timer.h>
 #include <linux/watchdog.h>
@@ -64,7 +63,6 @@ struct imx2_wdt_device {
 	struct regmap *regmap;
 	struct timer_list timer;	/* Pings the watchdog when closed */
 	struct watchdog_device wdog;
-	struct notifier_block restart_handler;
 };
 
 static bool nowayout = WATCHDOG_NOWAYOUT;
@@ -83,13 +81,11 @@ static const struct watchdog_info imx2_wdt_info = {
 	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE,
 };
 
-static int imx2_restart_handler(struct notifier_block *this, unsigned long mode,
-				void *cmd)
+static int imx2_wdt_restart(struct watchdog_device *wdog)
 {
+	struct imx2_wdt_device *wdev = watchdog_get_drvdata(wdog);
 	unsigned int wcr_enable = IMX2_WDT_WCR_WDE;
-	struct imx2_wdt_device *wdev = container_of(this,
-						    struct imx2_wdt_device,
-						    restart_handler);
+
 	/* Assert SRS signal */
 	regmap_write(wdev->regmap, IMX2_WDT_WCR, wcr_enable);
 	/*
@@ -105,7 +101,7 @@ static int imx2_restart_handler(struct notifier_block *this, unsigned long mode,
 	/* wait for reset to assert... */
 	mdelay(500);
 
-	return NOTIFY_DONE;
+	return 0;
 }
 
 static inline void imx2_wdt_setup(struct watchdog_device *wdog)
@@ -213,6 +209,7 @@ static const struct watchdog_ops imx2_wdt_ops = {
 	.stop = imx2_wdt_stop,
 	.ping = imx2_wdt_ping,
 	.set_timeout = imx2_wdt_set_timeout,
+	.restart = imx2_wdt_restart,
 };
 
 static const struct regmap_config imx2_wdt_regmap_config = {
@@ -275,6 +272,7 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, wdog);
 	watchdog_set_drvdata(wdog, wdev);
 	watchdog_set_nowayout(wdog, nowayout);
+	watchdog_set_restart_priority(wdog, 128);
 	watchdog_init_timeout(wdog, timeout, &pdev->dev);
 
 	setup_timer(&wdev->timer, imx2_wdt_timer_ping, (unsigned long)wdog);
@@ -294,12 +292,6 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
 		goto disable_clk;
 	}
 
-	wdev->restart_handler.notifier_call = imx2_restart_handler;
-	wdev->restart_handler.priority = 128;
-	ret = register_restart_handler(&wdev->restart_handler);
-	if (ret)
-		dev_err(&pdev->dev, "cannot register restart handler\n");
-
 	dev_info(&pdev->dev, "timeout %d sec (nowayout=%d)\n",
 		 wdog->timeout, nowayout);
 
@@ -315,8 +307,6 @@ static int __exit imx2_wdt_remove(struct platform_device *pdev)
 	struct watchdog_device *wdog = platform_get_drvdata(pdev);
 	struct imx2_wdt_device *wdev = watchdog_get_drvdata(wdog);
 
-	unregister_restart_handler(&wdev->restart_handler);
-
 	watchdog_unregister_device(wdog);
 
 	if (imx2_wdt_is_running(wdev)) {
-- 
2.5.0

Re: [RFC PATCH 06/13] watchdog: imx2_wdt: use core restart handler

[Guenter Roeck]

On 11/02/2015 05:36 PM, Damien Riegel wrote:
> Get rid of the custom restart handler by using the one provided by the
> watchdog core.
>
> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/watchdog/imx2_wdt.c | 22 ++++++----------------
>   1 file changed, 6 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
> index 29ef719..dda44f6 100644
> --- a/drivers/watchdog/imx2_wdt.c
> +++ b/drivers/watchdog/imx2_wdt.c
> @@ -32,7 +32,6 @@
>   #include <linux/notifier.h>
>   #include <linux/of_address.h>
>   #include <linux/platform_device.h>
> -#include <linux/reboot.h>
>   #include <linux/regmap.h>
>   #include <linux/timer.h>
>   #include <linux/watchdog.h>
> @@ -64,7 +63,6 @@ struct imx2_wdt_device {
>   	struct regmap *regmap;
>   	struct timer_list timer;	/* Pings the watchdog when closed */
>   	struct watchdog_device wdog;
> -	struct notifier_block restart_handler;
>   };
>
>   static bool nowayout = WATCHDOG_NOWAYOUT;
> @@ -83,13 +81,11 @@ static const struct watchdog_info imx2_wdt_info = {
>   	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT | WDIOF_MAGICCLOSE,
>   };
>
> -static int imx2_restart_handler(struct notifier_block *this, unsigned long mode,
> -				void *cmd)
> +static int imx2_wdt_restart(struct watchdog_device *wdog)
>   {
> +	struct imx2_wdt_device *wdev = watchdog_get_drvdata(wdog);
>   	unsigned int wcr_enable = IMX2_WDT_WCR_WDE;
> -	struct imx2_wdt_device *wdev = container_of(this,
> -						    struct imx2_wdt_device,
> -						    restart_handler);
> +
>   	/* Assert SRS signal */
>   	regmap_write(wdev->regmap, IMX2_WDT_WCR, wcr_enable);
>   	/*
> @@ -105,7 +101,7 @@ static int imx2_restart_handler(struct notifier_block *this, unsigned long mode,
>   	/* wait for reset to assert... */
>   	mdelay(500);
>
> -	return NOTIFY_DONE;
> +	return 0;
>   }
>
>   static inline void imx2_wdt_setup(struct watchdog_device *wdog)
> @@ -213,6 +209,7 @@ static const struct watchdog_ops imx2_wdt_ops = {
>   	.stop = imx2_wdt_stop,
>   	.ping = imx2_wdt_ping,
>   	.set_timeout = imx2_wdt_set_timeout,
> +	.restart = imx2_wdt_restart,
>   };
>
>   static const struct regmap_config imx2_wdt_regmap_config = {
> @@ -275,6 +272,7 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
>   	platform_set_drvdata(pdev, wdog);
>   	watchdog_set_drvdata(wdog, wdev);
>   	watchdog_set_nowayout(wdog, nowayout);
> +	watchdog_set_restart_priority(wdog, 128);
>   	watchdog_init_timeout(wdog, timeout, &pdev->dev);
>
>   	setup_timer(&wdev->timer, imx2_wdt_timer_ping, (unsigned long)wdog);
> @@ -294,12 +292,6 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)
>   		goto disable_clk;
>   	}
>
> -	wdev->restart_handler.notifier_call = imx2_restart_handler;
> -	wdev->restart_handler.priority = 128;
> -	ret = register_restart_handler(&wdev->restart_handler);
> -	if (ret)
> -		dev_err(&pdev->dev, "cannot register restart handler\n");
> -
>   	dev_info(&pdev->dev, "timeout %d sec (nowayout=%d)\n",
>   		 wdog->timeout, nowayout);
>
> @@ -315,8 +307,6 @@ static int __exit imx2_wdt_remove(struct platform_device *pdev)
>   	struct watchdog_device *wdog = platform_get_drvdata(pdev);
>   	struct imx2_wdt_device *wdev = watchdog_get_drvdata(wdog);
>
> -	unregister_restart_handler(&wdev->restart_handler);
> -
>   	watchdog_unregister_device(wdog);
>
>   	if (imx2_wdt_is_running(wdev)) {
>

[RFC PATCH 07/13] watchdog: lpc18xx_wdt: use core restart handler

[Damien Riegel]

Get rid of the custom restart handler by using the one provided by the
watchdog core.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/watchdog/lpc18xx_wdt.c | 68 ++++++++++++++++++------------------------
 1 file changed, 29 insertions(+), 39 deletions(-)

diff --git a/drivers/watchdog/lpc18xx_wdt.c b/drivers/watchdog/lpc18xx_wdt.c
index ab7b8b1..6914c83 100644
--- a/drivers/watchdog/lpc18xx_wdt.c
+++ b/drivers/watchdog/lpc18xx_wdt.c
@@ -18,7 +18,6 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
-#include <linux/reboot.h>
 #include <linux/watchdog.h>
 
 /* Registers */
@@ -59,7 +58,6 @@ struct lpc18xx_wdt_dev {
 	unsigned long		clk_rate;
 	void __iomem		*base;
 	struct timer_list	timer;
-	struct notifier_block	restart_handler;
 	spinlock_t		lock;
 };
 
@@ -155,6 +153,33 @@ static int lpc18xx_wdt_start(struct watchdog_device *wdt_dev)
 	return 0;
 }
 
+static int lpc18xx_wdt_restart(struct watchdog_device *wdt_dev)
+{
+	struct lpc18xx_wdt_dev *lpc18xx_wdt = watchdog_get_drvdata(wdt_dev);
+	unsigned long flags;
+	int val;
+
+	/*
+	 * Incorrect feed sequence causes immediate watchdog reset if enabled.
+	 */
+	spin_lock_irqsave(&lpc18xx_wdt->lock, flags);
+
+	val = readl(lpc18xx_wdt->base + LPC18XX_WDT_MOD);
+	val |= LPC18XX_WDT_MOD_WDEN;
+	val |= LPC18XX_WDT_MOD_WDRESET;
+	writel(val, lpc18xx_wdt->base + LPC18XX_WDT_MOD);
+
+	writel(LPC18XX_WDT_FEED_MAGIC1, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
+	writel(LPC18XX_WDT_FEED_MAGIC2, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
+
+	writel(LPC18XX_WDT_FEED_MAGIC1, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
+	writel(LPC18XX_WDT_FEED_MAGIC1, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
+
+	spin_unlock_irqrestore(&lpc18xx_wdt->lock, flags);
+
+	return 0;
+}
+
 static struct watchdog_info lpc18xx_wdt_info = {
 	.identity	= "NXP LPC18xx Watchdog",
 	.options	= WDIOF_SETTIMEOUT |
@@ -169,37 +194,9 @@ static const struct watchdog_ops lpc18xx_wdt_ops = {
 	.ping		= lpc18xx_wdt_feed,
 	.set_timeout	= lpc18xx_wdt_set_timeout,
 	.get_timeleft	= lpc18xx_wdt_get_timeleft,
+	.restart        = lpc18xx_wdt_restart,
 };
 
-static int lpc18xx_wdt_restart(struct notifier_block *this, unsigned long mode,
-			       void *cmd)
-{
-	struct lpc18xx_wdt_dev *lpc18xx_wdt = container_of(this,
-				struct lpc18xx_wdt_dev, restart_handler);
-	unsigned long flags;
-	int val;
-
-	/*
-	 * Incorrect feed sequence causes immediate watchdog reset if enabled.
-	 */
-	spin_lock_irqsave(&lpc18xx_wdt->lock, flags);
-
-	val = readl(lpc18xx_wdt->base + LPC18XX_WDT_MOD);
-	val |= LPC18XX_WDT_MOD_WDEN;
-	val |= LPC18XX_WDT_MOD_WDRESET;
-	writel(val, lpc18xx_wdt->base + LPC18XX_WDT_MOD);
-
-	writel(LPC18XX_WDT_FEED_MAGIC1, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
-	writel(LPC18XX_WDT_FEED_MAGIC2, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
-
-	writel(LPC18XX_WDT_FEED_MAGIC1, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
-	writel(LPC18XX_WDT_FEED_MAGIC1, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
-
-	spin_unlock_irqrestore(&lpc18xx_wdt->lock, flags);
-
-	return NOTIFY_OK;
-}
-
 static int lpc18xx_wdt_probe(struct platform_device *pdev)
 {
 	struct lpc18xx_wdt_dev *lpc18xx_wdt;
@@ -273,6 +270,7 @@ static int lpc18xx_wdt_probe(struct platform_device *pdev)
 		    (unsigned long)&lpc18xx_wdt->wdt_dev);
 
 	watchdog_set_nowayout(&lpc18xx_wdt->wdt_dev, nowayout);
+	watchdog_set_restart_priority(&lpc18xx_wdt->wdt_dev, 128);
 
 	platform_set_drvdata(pdev, lpc18xx_wdt);
 
@@ -280,12 +278,6 @@ static int lpc18xx_wdt_probe(struct platform_device *pdev)
 	if (ret)
 		goto disable_wdt_clk;
 
-	lpc18xx_wdt->restart_handler.notifier_call = lpc18xx_wdt_restart;
-	lpc18xx_wdt->restart_handler.priority = 128;
-	ret = register_restart_handler(&lpc18xx_wdt->restart_handler);
-	if (ret)
-		dev_warn(dev, "failed to register restart handler: %d\n", ret);
-
 	return 0;
 
 disable_wdt_clk:
@@ -306,8 +298,6 @@ static int lpc18xx_wdt_remove(struct platform_device *pdev)
 {
 	struct lpc18xx_wdt_dev *lpc18xx_wdt = platform_get_drvdata(pdev);
 
-	unregister_restart_handler(&lpc18xx_wdt->restart_handler);
-
 	dev_warn(&pdev->dev, "I quit now, hardware will probably reboot!\n");
 	del_timer(&lpc18xx_wdt->timer);
 
-- 
2.5.0

Re: [RFC PATCH 07/13] watchdog: lpc18xx_wdt: use core restart handler

[Guenter Roeck]

On 11/02/2015 05:36 PM, Damien Riegel wrote:
> Get rid of the custom restart handler by using the one provided by the
> watchdog core.
>
> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/watchdog/lpc18xx_wdt.c | 68 ++++++++++++++++++------------------------
>   1 file changed, 29 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/watchdog/lpc18xx_wdt.c b/drivers/watchdog/lpc18xx_wdt.c
> index ab7b8b1..6914c83 100644
> --- a/drivers/watchdog/lpc18xx_wdt.c
> +++ b/drivers/watchdog/lpc18xx_wdt.c
> @@ -18,7 +18,6 @@
>   #include <linux/module.h>
>   #include <linux/of.h>
>   #include <linux/platform_device.h>
> -#include <linux/reboot.h>
>   #include <linux/watchdog.h>
>
>   /* Registers */
> @@ -59,7 +58,6 @@ struct lpc18xx_wdt_dev {
>   	unsigned long		clk_rate;
>   	void __iomem		*base;
>   	struct timer_list	timer;
> -	struct notifier_block	restart_handler;
>   	spinlock_t		lock;
>   };
>
> @@ -155,6 +153,33 @@ static int lpc18xx_wdt_start(struct watchdog_device *wdt_dev)
>   	return 0;
>   }
>
> +static int lpc18xx_wdt_restart(struct watchdog_device *wdt_dev)
> +{
> +	struct lpc18xx_wdt_dev *lpc18xx_wdt = watchdog_get_drvdata(wdt_dev);
> +	unsigned long flags;
> +	int val;
> +
> +	/*
> +	 * Incorrect feed sequence causes immediate watchdog reset if enabled.
> +	 */
> +	spin_lock_irqsave(&lpc18xx_wdt->lock, flags);
> +
> +	val = readl(lpc18xx_wdt->base + LPC18XX_WDT_MOD);
> +	val |= LPC18XX_WDT_MOD_WDEN;
> +	val |= LPC18XX_WDT_MOD_WDRESET;
> +	writel(val, lpc18xx_wdt->base + LPC18XX_WDT_MOD);
> +
> +	writel(LPC18XX_WDT_FEED_MAGIC1, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
> +	writel(LPC18XX_WDT_FEED_MAGIC2, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
> +
> +	writel(LPC18XX_WDT_FEED_MAGIC1, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
> +	writel(LPC18XX_WDT_FEED_MAGIC1, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
> +
> +	spin_unlock_irqrestore(&lpc18xx_wdt->lock, flags);
> +
> +	return 0;
> +}
> +
>   static struct watchdog_info lpc18xx_wdt_info = {
>   	.identity	= "NXP LPC18xx Watchdog",
>   	.options	= WDIOF_SETTIMEOUT |
> @@ -169,37 +194,9 @@ static const struct watchdog_ops lpc18xx_wdt_ops = {
>   	.ping		= lpc18xx_wdt_feed,
>   	.set_timeout	= lpc18xx_wdt_set_timeout,
>   	.get_timeleft	= lpc18xx_wdt_get_timeleft,
> +	.restart        = lpc18xx_wdt_restart,
>   };
>
> -static int lpc18xx_wdt_restart(struct notifier_block *this, unsigned long mode,
> -			       void *cmd)
> -{
> -	struct lpc18xx_wdt_dev *lpc18xx_wdt = container_of(this,
> -				struct lpc18xx_wdt_dev, restart_handler);
> -	unsigned long flags;
> -	int val;
> -
> -	/*
> -	 * Incorrect feed sequence causes immediate watchdog reset if enabled.
> -	 */
> -	spin_lock_irqsave(&lpc18xx_wdt->lock, flags);
> -
> -	val = readl(lpc18xx_wdt->base + LPC18XX_WDT_MOD);
> -	val |= LPC18XX_WDT_MOD_WDEN;
> -	val |= LPC18XX_WDT_MOD_WDRESET;
> -	writel(val, lpc18xx_wdt->base + LPC18XX_WDT_MOD);
> -
> -	writel(LPC18XX_WDT_FEED_MAGIC1, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
> -	writel(LPC18XX_WDT_FEED_MAGIC2, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
> -
> -	writel(LPC18XX_WDT_FEED_MAGIC1, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
> -	writel(LPC18XX_WDT_FEED_MAGIC1, lpc18xx_wdt->base + LPC18XX_WDT_FEED);
> -
> -	spin_unlock_irqrestore(&lpc18xx_wdt->lock, flags);
> -
> -	return NOTIFY_OK;
> -}
> -
>   static int lpc18xx_wdt_probe(struct platform_device *pdev)
>   {
>   	struct lpc18xx_wdt_dev *lpc18xx_wdt;
> @@ -273,6 +270,7 @@ static int lpc18xx_wdt_probe(struct platform_device *pdev)
>   		    (unsigned long)&lpc18xx_wdt->wdt_dev);
>
>   	watchdog_set_nowayout(&lpc18xx_wdt->wdt_dev, nowayout);
> +	watchdog_set_restart_priority(&lpc18xx_wdt->wdt_dev, 128);
>
>   	platform_set_drvdata(pdev, lpc18xx_wdt);
>
> @@ -280,12 +278,6 @@ static int lpc18xx_wdt_probe(struct platform_device *pdev)
>   	if (ret)
>   		goto disable_wdt_clk;
>
> -	lpc18xx_wdt->restart_handler.notifier_call = lpc18xx_wdt_restart;
> -	lpc18xx_wdt->restart_handler.priority = 128;
> -	ret = register_restart_handler(&lpc18xx_wdt->restart_handler);
> -	if (ret)
> -		dev_warn(dev, "failed to register restart handler: %d\n", ret);
> -
>   	return 0;
>
>   disable_wdt_clk:
> @@ -306,8 +298,6 @@ static int lpc18xx_wdt_remove(struct platform_device *pdev)
>   {
>   	struct lpc18xx_wdt_dev *lpc18xx_wdt = platform_get_drvdata(pdev);
>
> -	unregister_restart_handler(&lpc18xx_wdt->restart_handler);
> -
>   	dev_warn(&pdev->dev, "I quit now, hardware will probably reboot!\n");
>   	del_timer(&lpc18xx_wdt->timer);
>
>

[RFC PATCH 08/13] watchdog: meson_wdt: use core restart handler

[Damien Riegel]

Get rid of the custom restart handler by using the one provided by the
watchdog core.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/watchdog/meson_wdt.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/watchdog/meson_wdt.c b/drivers/watchdog/meson_wdt.c
index 1f4155e..9047c8a 100644
--- a/drivers/watchdog/meson_wdt.c
+++ b/drivers/watchdog/meson_wdt.c
@@ -20,7 +20,6 @@
 #include <linux/notifier.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
-#include <linux/reboot.h>
 #include <linux/types.h>
 #include <linux/watchdog.h>
 
@@ -45,23 +44,19 @@ static unsigned int timeout = MESON_WDT_TIMEOUT;
 struct meson_wdt_dev {
 	struct watchdog_device wdt_dev;
 	void __iomem *wdt_base;
-	struct notifier_block restart_handler;
 };
 
-static int meson_restart_handle(struct notifier_block *this, unsigned long mode,
-				void *cmd)
+static int meson_wdt_restart(struct watchdog_device *wdt_dev)
 {
+	struct meson_wdt_dev *meson_wdt = watchdog_get_drvdata(wdt_dev);
 	u32 tc_reboot = MESON_WDT_DC_RESET | MESON_WDT_TC_EN;
-	struct meson_wdt_dev *meson_wdt = container_of(this,
-						       struct meson_wdt_dev,
-						       restart_handler);
 
 	while (1) {
 		writel(tc_reboot, meson_wdt->wdt_base + MESON_WDT_TC);
 		mdelay(5);
 	}
 
-	return NOTIFY_DONE;
+	return 0;
 }
 
 static int meson_wdt_ping(struct watchdog_device *wdt_dev)
@@ -136,6 +131,7 @@ static const struct watchdog_ops meson_wdt_ops = {
 	.stop		= meson_wdt_stop,
 	.ping		= meson_wdt_ping,
 	.set_timeout	= meson_wdt_set_timeout,
+	.restart        = meson_wdt_restart,
 };
 
 static int meson_wdt_probe(struct platform_device *pdev)
@@ -164,6 +160,7 @@ static int meson_wdt_probe(struct platform_device *pdev)
 
 	watchdog_init_timeout(&meson_wdt->wdt_dev, timeout, &pdev->dev);
 	watchdog_set_nowayout(&meson_wdt->wdt_dev, nowayout);
+	watchdog_set_restart_priority(&meson_wdt->wdt_dev, 128);
 
 	meson_wdt_stop(&meson_wdt->wdt_dev);
 
@@ -173,13 +170,6 @@ static int meson_wdt_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, meson_wdt);
 
-	meson_wdt->restart_handler.notifier_call = meson_restart_handle;
-	meson_wdt->restart_handler.priority = 128;
-	err = register_restart_handler(&meson_wdt->restart_handler);
-	if (err)
-		dev_err(&pdev->dev,
-			"cannot register restart handler (err=%d)\n", err);
-
 	dev_info(&pdev->dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)",
 		 meson_wdt->wdt_dev.timeout, nowayout);
 
@@ -190,8 +180,6 @@ static int meson_wdt_remove(struct platform_device *pdev)
 {
 	struct meson_wdt_dev *meson_wdt = platform_get_drvdata(pdev);
 
-	unregister_restart_handler(&meson_wdt->restart_handler);
-
 	watchdog_unregister_device(&meson_wdt->wdt_dev);
 
 	return 0;
-- 
2.5.0

Re: [RFC PATCH 08/13] watchdog: meson_wdt: use core restart handler

[Guenter Roeck]

On 11/02/2015 05:36 PM, Damien Riegel wrote:
> Get rid of the custom restart handler by using the one provided by the
> watchdog core.
>
> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/watchdog/meson_wdt.c | 22 +++++-----------------
>   1 file changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/watchdog/meson_wdt.c b/drivers/watchdog/meson_wdt.c
> index 1f4155e..9047c8a 100644
> --- a/drivers/watchdog/meson_wdt.c
> +++ b/drivers/watchdog/meson_wdt.c
> @@ -20,7 +20,6 @@
>   #include <linux/notifier.h>
>   #include <linux/of.h>
>   #include <linux/platform_device.h>
> -#include <linux/reboot.h>
>   #include <linux/types.h>
>   #include <linux/watchdog.h>
>
> @@ -45,23 +44,19 @@ static unsigned int timeout = MESON_WDT_TIMEOUT;
>   struct meson_wdt_dev {
>   	struct watchdog_device wdt_dev;
>   	void __iomem *wdt_base;
> -	struct notifier_block restart_handler;
>   };
>
> -static int meson_restart_handle(struct notifier_block *this, unsigned long mode,
> -				void *cmd)
> +static int meson_wdt_restart(struct watchdog_device *wdt_dev)
>   {
> +	struct meson_wdt_dev *meson_wdt = watchdog_get_drvdata(wdt_dev);
>   	u32 tc_reboot = MESON_WDT_DC_RESET | MESON_WDT_TC_EN;
> -	struct meson_wdt_dev *meson_wdt = container_of(this,
> -						       struct meson_wdt_dev,
> -						       restart_handler);
>
>   	while (1) {
>   		writel(tc_reboot, meson_wdt->wdt_base + MESON_WDT_TC);
>   		mdelay(5);
>   	}
>
> -	return NOTIFY_DONE;
> +	return 0;
>   }
>
>   static int meson_wdt_ping(struct watchdog_device *wdt_dev)
> @@ -136,6 +131,7 @@ static const struct watchdog_ops meson_wdt_ops = {
>   	.stop		= meson_wdt_stop,
>   	.ping		= meson_wdt_ping,
>   	.set_timeout	= meson_wdt_set_timeout,
> +	.restart        = meson_wdt_restart,
>   };
>
>   static int meson_wdt_probe(struct platform_device *pdev)
> @@ -164,6 +160,7 @@ static int meson_wdt_probe(struct platform_device *pdev)
>
>   	watchdog_init_timeout(&meson_wdt->wdt_dev, timeout, &pdev->dev);
>   	watchdog_set_nowayout(&meson_wdt->wdt_dev, nowayout);
> +	watchdog_set_restart_priority(&meson_wdt->wdt_dev, 128);
>
>   	meson_wdt_stop(&meson_wdt->wdt_dev);
>
> @@ -173,13 +170,6 @@ static int meson_wdt_probe(struct platform_device *pdev)
>
>   	platform_set_drvdata(pdev, meson_wdt);
>
> -	meson_wdt->restart_handler.notifier_call = meson_restart_handle;
> -	meson_wdt->restart_handler.priority = 128;
> -	err = register_restart_handler(&meson_wdt->restart_handler);
> -	if (err)
> -		dev_err(&pdev->dev,
> -			"cannot register restart handler (err=%d)\n", err);
> -
>   	dev_info(&pdev->dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)",
>   		 meson_wdt->wdt_dev.timeout, nowayout);
>
> @@ -190,8 +180,6 @@ static int meson_wdt_remove(struct platform_device *pdev)
>   {
>   	struct meson_wdt_dev *meson_wdt = platform_get_drvdata(pdev);
>
> -	unregister_restart_handler(&meson_wdt->restart_handler);
> -
>   	watchdog_unregister_device(&meson_wdt->wdt_dev);
>
>   	return 0;
>

[RFC PATCH 09/13] watchdog: moxart_wdt: use core restart handler

[Damien Riegel]

Get rid of the custom restart handler by using the one provided by the
watchdog core.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/watchdog/moxart_wdt.c | 22 ++++++----------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
index 60b0605..c64e4f0 100644
--- a/drivers/watchdog/moxart_wdt.c
+++ b/drivers/watchdog/moxart_wdt.c
@@ -17,7 +17,6 @@
 #include <linux/kernel.h>
 #include <linux/notifier.h>
 #include <linux/platform_device.h>
-#include <linux/reboot.h>
 #include <linux/watchdog.h>
 #include <linux/moduleparam.h>
 
@@ -29,22 +28,19 @@ struct moxart_wdt_dev {
 	struct watchdog_device dev;
 	void __iomem *base;
 	unsigned int clock_frequency;
-	struct notifier_block restart_handler;
 };
 
 static int heartbeat;
 
-static int moxart_restart_handle(struct notifier_block *this,
-				 unsigned long mode, void *cmd)
+static int moxart_wdt_restart(struct watchdog_device *wdt_dev)
 {
-	struct moxart_wdt_dev *moxart_wdt = container_of(this,
-							 struct moxart_wdt_dev,
-							 restart_handler);
+	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);
 
-	return NOTIFY_DONE;
+	return 0;
 }
 
 static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
@@ -87,6 +83,7 @@ static const struct watchdog_ops moxart_wdt_ops = {
 	.start          = moxart_wdt_start,
 	.stop           = moxart_wdt_stop,
 	.set_timeout    = moxart_wdt_set_timeout,
+	.restart        = moxart_wdt_restart,
 };
 
 static int moxart_wdt_probe(struct platform_device *pdev)
@@ -134,6 +131,7 @@ static int moxart_wdt_probe(struct platform_device *pdev)
 
 	watchdog_init_timeout(&moxart_wdt->dev, heartbeat, dev);
 	watchdog_set_nowayout(&moxart_wdt->dev, nowayout);
+	watchdog_set_restart_priority(&moxart_wdt->dev, 128);
 
 	watchdog_set_drvdata(&moxart_wdt->dev, moxart_wdt);
 
@@ -141,13 +139,6 @@ static int moxart_wdt_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
-	moxart_wdt->restart_handler.notifier_call = moxart_restart_handle;
-	moxart_wdt->restart_handler.priority = 128;
-	err = register_restart_handler(&moxart_wdt->restart_handler);
-	if (err)
-		dev_err(dev, "cannot register restart notifier (err=%d)\n",
-			err);
-
 	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
 		moxart_wdt->dev.timeout, nowayout);
 
@@ -158,7 +149,6 @@ static int moxart_wdt_remove(struct platform_device *pdev)
 {
 	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
 
-	unregister_restart_handler(&moxart_wdt->restart_handler);
 	moxart_wdt_stop(&moxart_wdt->dev);
 
 	return 0;
-- 
2.5.0

Re: [RFC PATCH 09/13] watchdog: moxart_wdt: use core restart handler

[Guenter Roeck]

On 11/02/2015 05:36 PM, Damien Riegel wrote:
> Get rid of the custom restart handler by using the one provided by the
> watchdog core.
>
> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/watchdog/moxart_wdt.c | 22 ++++++----------------
>   1 file changed, 6 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/watchdog/moxart_wdt.c b/drivers/watchdog/moxart_wdt.c
> index 60b0605..c64e4f0 100644
> --- a/drivers/watchdog/moxart_wdt.c
> +++ b/drivers/watchdog/moxart_wdt.c
> @@ -17,7 +17,6 @@
>   #include <linux/kernel.h>
>   #include <linux/notifier.h>
>   #include <linux/platform_device.h>
> -#include <linux/reboot.h>
>   #include <linux/watchdog.h>
>   #include <linux/moduleparam.h>
>
> @@ -29,22 +28,19 @@ struct moxart_wdt_dev {
>   	struct watchdog_device dev;
>   	void __iomem *base;
>   	unsigned int clock_frequency;
> -	struct notifier_block restart_handler;
>   };
>
>   static int heartbeat;
>
> -static int moxart_restart_handle(struct notifier_block *this,
> -				 unsigned long mode, void *cmd)
> +static int moxart_wdt_restart(struct watchdog_device *wdt_dev)
>   {
> -	struct moxart_wdt_dev *moxart_wdt = container_of(this,
> -							 struct moxart_wdt_dev,
> -							 restart_handler);
> +	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);
>
> -	return NOTIFY_DONE;
> +	return 0;
>   }
>
>   static int moxart_wdt_stop(struct watchdog_device *wdt_dev)
> @@ -87,6 +83,7 @@ static const struct watchdog_ops moxart_wdt_ops = {
>   	.start          = moxart_wdt_start,
>   	.stop           = moxart_wdt_stop,
>   	.set_timeout    = moxart_wdt_set_timeout,
> +	.restart        = moxart_wdt_restart,
>   };
>
>   static int moxart_wdt_probe(struct platform_device *pdev)
> @@ -134,6 +131,7 @@ static int moxart_wdt_probe(struct platform_device *pdev)
>
>   	watchdog_init_timeout(&moxart_wdt->dev, heartbeat, dev);
>   	watchdog_set_nowayout(&moxart_wdt->dev, nowayout);
> +	watchdog_set_restart_priority(&moxart_wdt->dev, 128);
>
>   	watchdog_set_drvdata(&moxart_wdt->dev, moxart_wdt);
>
> @@ -141,13 +139,6 @@ static int moxart_wdt_probe(struct platform_device *pdev)
>   	if (err)
>   		return err;
>
> -	moxart_wdt->restart_handler.notifier_call = moxart_restart_handle;
> -	moxart_wdt->restart_handler.priority = 128;
> -	err = register_restart_handler(&moxart_wdt->restart_handler);
> -	if (err)
> -		dev_err(dev, "cannot register restart notifier (err=%d)\n",
> -			err);
> -
>   	dev_dbg(dev, "Watchdog enabled (heartbeat=%d sec, nowayout=%d)\n",
>   		moxart_wdt->dev.timeout, nowayout);
>
> @@ -158,7 +149,6 @@ static int moxart_wdt_remove(struct platform_device *pdev)
>   {
>   	struct moxart_wdt_dev *moxart_wdt = platform_get_drvdata(pdev);
>
> -	unregister_restart_handler(&moxart_wdt->restart_handler);
>   	moxart_wdt_stop(&moxart_wdt->dev);
>
>   	return 0;
>

[RFC PATCH 10/13] watchdog: mtk_wdt: use core restart handler

[Damien Riegel]

Get rid of the custom restart handler by using the one provided by the
watchdog core.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/watchdog/mtk_wdt.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
index 6ad9df9..277f588 100644
--- a/drivers/watchdog/mtk_wdt.c
+++ b/drivers/watchdog/mtk_wdt.c
@@ -29,7 +29,6 @@
 #include <linux/types.h>
 #include <linux/watchdog.h>
 #include <linux/notifier.h>
-#include <linux/reboot.h>
 #include <linux/delay.h>
 
 #define WDT_MAX_TIMEOUT		31
@@ -64,16 +63,13 @@ static unsigned int timeout = WDT_MAX_TIMEOUT;
 struct mtk_wdt_dev {
 	struct watchdog_device wdt_dev;
 	void __iomem *wdt_base;
-	struct notifier_block restart_handler;
 };
 
-static int mtk_reset_handler(struct notifier_block *this, unsigned long mode,
-				void *cmd)
+static int mtk_wdt_restart(struct watchdog_device *wdt_dev)
 {
-	struct mtk_wdt_dev *mtk_wdt;
+	struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdt_dev);
 	void __iomem *wdt_base;
 
-	mtk_wdt = container_of(this, struct mtk_wdt_dev, restart_handler);
 	wdt_base = mtk_wdt->wdt_base;
 
 	while (1) {
@@ -160,6 +156,7 @@ static const struct watchdog_ops mtk_wdt_ops = {
 	.stop		= mtk_wdt_stop,
 	.ping		= mtk_wdt_ping,
 	.set_timeout	= mtk_wdt_set_timeout,
+	.restart	= mtk_wdt_restart,
 };
 
 static int mtk_wdt_probe(struct platform_device *pdev)
@@ -188,6 +185,7 @@ static int mtk_wdt_probe(struct platform_device *pdev)
 
 	watchdog_init_timeout(&mtk_wdt->wdt_dev, timeout, &pdev->dev);
 	watchdog_set_nowayout(&mtk_wdt->wdt_dev, nowayout);
+	watchdog_set_restart_priority(&mtk_wdt->wdt_dev, 128);
 
 	watchdog_set_drvdata(&mtk_wdt->wdt_dev, mtk_wdt);
 
@@ -197,13 +195,6 @@ static int mtk_wdt_probe(struct platform_device *pdev)
 	if (unlikely(err))
 		return err;
 
-	mtk_wdt->restart_handler.notifier_call = mtk_reset_handler;
-	mtk_wdt->restart_handler.priority = 128;
-	err = register_restart_handler(&mtk_wdt->restart_handler);
-	if (err)
-		dev_warn(&pdev->dev,
-			"cannot register restart handler (err=%d)\n", err);
-
 	dev_info(&pdev->dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)\n",
 			mtk_wdt->wdt_dev.timeout, nowayout);
 
@@ -222,8 +213,6 @@ static int mtk_wdt_remove(struct platform_device *pdev)
 {
 	struct mtk_wdt_dev *mtk_wdt = platform_get_drvdata(pdev);
 
-	unregister_restart_handler(&mtk_wdt->restart_handler);
-
 	watchdog_unregister_device(&mtk_wdt->wdt_dev);
 
 	return 0;
-- 
2.5.0

Re: [RFC PATCH 10/13] watchdog: mtk_wdt: use core restart handler

[Guenter Roeck]

On 11/02/2015 05:36 PM, Damien Riegel wrote:
> Get rid of the custom restart handler by using the one provided by the
> watchdog core.
>
> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>   drivers/watchdog/mtk_wdt.c | 19 ++++---------------
>   1 file changed, 4 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/watchdog/mtk_wdt.c b/drivers/watchdog/mtk_wdt.c
> index 6ad9df9..277f588 100644
> --- a/drivers/watchdog/mtk_wdt.c
> +++ b/drivers/watchdog/mtk_wdt.c
> @@ -29,7 +29,6 @@
>   #include <linux/types.h>
>   #include <linux/watchdog.h>
>   #include <linux/notifier.h>
> -#include <linux/reboot.h>
>   #include <linux/delay.h>
>
>   #define WDT_MAX_TIMEOUT		31
> @@ -64,16 +63,13 @@ static unsigned int timeout = WDT_MAX_TIMEOUT;
>   struct mtk_wdt_dev {
>   	struct watchdog_device wdt_dev;
>   	void __iomem *wdt_base;
> -	struct notifier_block restart_handler;
>   };
>
> -static int mtk_reset_handler(struct notifier_block *this, unsigned long mode,
> -				void *cmd)
> +static int mtk_wdt_restart(struct watchdog_device *wdt_dev)
>   {
> -	struct mtk_wdt_dev *mtk_wdt;
> +	struct mtk_wdt_dev *mtk_wdt = watchdog_get_drvdata(wdt_dev);
>   	void __iomem *wdt_base;
>
> -	mtk_wdt = container_of(this, struct mtk_wdt_dev, restart_handler);
>   	wdt_base = mtk_wdt->wdt_base;
>
>   	while (1) {

Changed return value missing here ? Sure, it won't return, but still.

Ah, that reminds me - "#include <linux/notifier.h>" can probably be dropped
from most of the files. Can you look into that for all patches (including the
ones I already reviewed) ?

Thanks,
Guenter

> @@ -160,6 +156,7 @@ static const struct watchdog_ops mtk_wdt_ops = {
>   	.stop		= mtk_wdt_stop,
>   	.ping		= mtk_wdt_ping,
>   	.set_timeout	= mtk_wdt_set_timeout,
> +	.restart	= mtk_wdt_restart,
>   };
>
>   static int mtk_wdt_probe(struct platform_device *pdev)
> @@ -188,6 +185,7 @@ static int mtk_wdt_probe(struct platform_device *pdev)
>
>   	watchdog_init_timeout(&mtk_wdt->wdt_dev, timeout, &pdev->dev);
>   	watchdog_set_nowayout(&mtk_wdt->wdt_dev, nowayout);
> +	watchdog_set_restart_priority(&mtk_wdt->wdt_dev, 128);
>
>   	watchdog_set_drvdata(&mtk_wdt->wdt_dev, mtk_wdt);
>
> @@ -197,13 +195,6 @@ static int mtk_wdt_probe(struct platform_device *pdev)
>   	if (unlikely(err))
>   		return err;
>
> -	mtk_wdt->restart_handler.notifier_call = mtk_reset_handler;
> -	mtk_wdt->restart_handler.priority = 128;
> -	err = register_restart_handler(&mtk_wdt->restart_handler);
> -	if (err)
> -		dev_warn(&pdev->dev,
> -			"cannot register restart handler (err=%d)\n", err);
> -
>   	dev_info(&pdev->dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)\n",
>   			mtk_wdt->wdt_dev.timeout, nowayout);
>
> @@ -222,8 +213,6 @@ static int mtk_wdt_remove(struct platform_device *pdev)
>   {
>   	struct mtk_wdt_dev *mtk_wdt = platform_get_drvdata(pdev);
>
> -	unregister_restart_handler(&mtk_wdt->restart_handler);
> -
>   	watchdog_unregister_device(&mtk_wdt->wdt_dev);
>
>   	return 0;
>

[RFC PATCH 11/13] watchdog: qcom-wdt: use core restart handler

[Damien Riegel]

Get rid of the custom restart handler by using the one provided by the
watchdog core.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/watchdog/qcom-wdt.c | 63 +++++++++++++++++++--------------------------
 1 file changed, 26 insertions(+), 37 deletions(-)

diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
index 773dcfa..aa7105d 100644
--- a/drivers/watchdog/qcom-wdt.c
+++ b/drivers/watchdog/qcom-wdt.c
@@ -17,7 +17,6 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
-#include <linux/reboot.h>
 #include <linux/watchdog.h>
 
 #define WDT_RST		0x38
@@ -28,7 +27,6 @@ struct qcom_wdt {
 	struct watchdog_device	wdd;
 	struct clk		*clk;
 	unsigned long		rate;
-	struct notifier_block	restart_nb;
 	void __iomem		*base;
 };
 
@@ -72,11 +70,37 @@ static int qcom_wdt_set_timeout(struct watchdog_device *wdd,
 	return qcom_wdt_start(wdd);
 }
 
+static int qcom_wdt_restart(struct watchdog_device *wdd)
+{
+	struct qcom_wdt *wdt = to_qcom_wdt(wdd);
+	u32 timeout;
+
+	/*
+	 * Trigger watchdog bite:
+	 *    Setup BITE_TIME to be 128ms, and enable WDT.
+	 */
+	timeout = 128 * wdt->rate / 1000;
+
+	writel(0, wdt->base + WDT_EN);
+	writel(1, wdt->base + WDT_RST);
+	writel(timeout, wdt->base + WDT_BITE_TIME);
+	writel(1, wdt->base + WDT_EN);
+
+	/*
+	 * Actually make sure the above sequence hits hardware before sleeping.
+	 */
+	wmb();
+
+	msleep(150);
+	return 0;
+}
+
 static const struct watchdog_ops qcom_wdt_ops = {
 	.start		= qcom_wdt_start,
 	.stop		= qcom_wdt_stop,
 	.ping		= qcom_wdt_ping,
 	.set_timeout	= qcom_wdt_set_timeout,
+	.restart        = qcom_wdt_restart,
 	.owner		= THIS_MODULE,
 };
 
@@ -87,32 +111,6 @@ static const struct watchdog_info qcom_wdt_info = {
 	.identity	= KBUILD_MODNAME,
 };
 
-static int qcom_wdt_restart(struct notifier_block *nb, unsigned long action,
-			    void *data)
-{
-	struct qcom_wdt *wdt = container_of(nb, struct qcom_wdt, restart_nb);
-	u32 timeout;
-
-	/*
-	 * Trigger watchdog bite:
-	 *    Setup BITE_TIME to be 128ms, and enable WDT.
-	 */
-	timeout = 128 * wdt->rate / 1000;
-
-	writel(0, wdt->base + WDT_EN);
-	writel(1, wdt->base + WDT_RST);
-	writel(timeout, wdt->base + WDT_BITE_TIME);
-	writel(1, wdt->base + WDT_EN);
-
-	/*
-	 * Actually make sure the above sequence hits hardware before sleeping.
-	 */
-	wmb();
-
-	msleep(150);
-	return NOTIFY_DONE;
-}
-
 static int qcom_wdt_probe(struct platform_device *pdev)
 {
 	struct qcom_wdt *wdt;
@@ -187,14 +185,6 @@ static int qcom_wdt_probe(struct platform_device *pdev)
 		goto err_clk_unprepare;
 	}
 
-	/*
-	 * WDT restart notifier has priority 0 (use as a last resort)
-	 */
-	wdt->restart_nb.notifier_call = qcom_wdt_restart;
-	ret = register_restart_handler(&wdt->restart_nb);
-	if (ret)
-		dev_err(&pdev->dev, "failed to setup restart handler\n");
-
 	platform_set_drvdata(pdev, wdt);
 	return 0;
 
@@ -207,7 +197,6 @@ static int qcom_wdt_remove(struct platform_device *pdev)
 {
 	struct qcom_wdt *wdt = platform_get_drvdata(pdev);
 
-	unregister_restart_handler(&wdt->restart_nb);
 	watchdog_unregister_device(&wdt->wdd);
 	clk_disable_unprepare(wdt->clk);
 	return 0;
-- 
2.5.0

Re: [RFC PATCH 11/13] watchdog: qcom-wdt: use core restart handler

[Guenter Roeck]

On 11/02/2015 05:36 PM, Damien Riegel wrote:
> Get rid of the custom restart handler by using the one provided by the
> watchdog core.
>
> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/watchdog/qcom-wdt.c | 63 +++++++++++++++++++--------------------------
>   1 file changed, 26 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/watchdog/qcom-wdt.c b/drivers/watchdog/qcom-wdt.c
> index 773dcfa..aa7105d 100644
> --- a/drivers/watchdog/qcom-wdt.c
> +++ b/drivers/watchdog/qcom-wdt.c
> @@ -17,7 +17,6 @@
>   #include <linux/module.h>
>   #include <linux/of.h>
>   #include <linux/platform_device.h>
> -#include <linux/reboot.h>
>   #include <linux/watchdog.h>
>
>   #define WDT_RST		0x38
> @@ -28,7 +27,6 @@ struct qcom_wdt {
>   	struct watchdog_device	wdd;
>   	struct clk		*clk;
>   	unsigned long		rate;
> -	struct notifier_block	restart_nb;
>   	void __iomem		*base;
>   };
>
> @@ -72,11 +70,37 @@ static int qcom_wdt_set_timeout(struct watchdog_device *wdd,
>   	return qcom_wdt_start(wdd);
>   }
>
> +static int qcom_wdt_restart(struct watchdog_device *wdd)
> +{
> +	struct qcom_wdt *wdt = to_qcom_wdt(wdd);
> +	u32 timeout;
> +
> +	/*
> +	 * Trigger watchdog bite:
> +	 *    Setup BITE_TIME to be 128ms, and enable WDT.
> +	 */
> +	timeout = 128 * wdt->rate / 1000;
> +
> +	writel(0, wdt->base + WDT_EN);
> +	writel(1, wdt->base + WDT_RST);
> +	writel(timeout, wdt->base + WDT_BITE_TIME);
> +	writel(1, wdt->base + WDT_EN);
> +
> +	/*
> +	 * Actually make sure the above sequence hits hardware before sleeping.
> +	 */
> +	wmb();
> +
> +	msleep(150);
> +	return 0;
> +}
> +
>   static const struct watchdog_ops qcom_wdt_ops = {
>   	.start		= qcom_wdt_start,
>   	.stop		= qcom_wdt_stop,
>   	.ping		= qcom_wdt_ping,
>   	.set_timeout	= qcom_wdt_set_timeout,
> +	.restart        = qcom_wdt_restart,
>   	.owner		= THIS_MODULE,
>   };
>
> @@ -87,32 +111,6 @@ static const struct watchdog_info qcom_wdt_info = {
>   	.identity	= KBUILD_MODNAME,
>   };
>
> -static int qcom_wdt_restart(struct notifier_block *nb, unsigned long action,
> -			    void *data)
> -{
> -	struct qcom_wdt *wdt = container_of(nb, struct qcom_wdt, restart_nb);
> -	u32 timeout;
> -
> -	/*
> -	 * Trigger watchdog bite:
> -	 *    Setup BITE_TIME to be 128ms, and enable WDT.
> -	 */
> -	timeout = 128 * wdt->rate / 1000;
> -
> -	writel(0, wdt->base + WDT_EN);
> -	writel(1, wdt->base + WDT_RST);
> -	writel(timeout, wdt->base + WDT_BITE_TIME);
> -	writel(1, wdt->base + WDT_EN);
> -
> -	/*
> -	 * Actually make sure the above sequence hits hardware before sleeping.
> -	 */
> -	wmb();
> -
> -	msleep(150);
> -	return NOTIFY_DONE;
> -}
> -
>   static int qcom_wdt_probe(struct platform_device *pdev)
>   {
>   	struct qcom_wdt *wdt;
> @@ -187,14 +185,6 @@ static int qcom_wdt_probe(struct platform_device *pdev)
>   		goto err_clk_unprepare;
>   	}
>
> -	/*
> -	 * WDT restart notifier has priority 0 (use as a last resort)
> -	 */
> -	wdt->restart_nb.notifier_call = qcom_wdt_restart;
> -	ret = register_restart_handler(&wdt->restart_nb);
> -	if (ret)
> -		dev_err(&pdev->dev, "failed to setup restart handler\n");
> -
>   	platform_set_drvdata(pdev, wdt);
>   	return 0;
>
> @@ -207,7 +197,6 @@ static int qcom_wdt_remove(struct platform_device *pdev)
>   {
>   	struct qcom_wdt *wdt = platform_get_drvdata(pdev);
>
> -	unregister_restart_handler(&wdt->restart_nb);
>   	watchdog_unregister_device(&wdt->wdd);
>   	clk_disable_unprepare(wdt->clk);
>   	return 0;
>

[RFC PATCH 12/13] watchdog: s3c2410_wdt: use core restart handler

[Damien Riegel]

Get rid of the custom restart handler by using the one provided by the
watchdog core.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/watchdog/s3c2410_wdt.c | 60 ++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 35 deletions(-)

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index d781000..0093450 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -41,7 +41,6 @@
 #include <linux/of.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
-#include <linux/reboot.h>
 #include <linux/delay.h>
 
 #define S3C2410_WTCON		0x00
@@ -130,7 +129,6 @@ struct s3c2410_wdt {
 	unsigned long		wtdat_save;
 	struct watchdog_device	wdt_device;
 	struct notifier_block	freq_transition;
-	struct notifier_block	restart_handler;
 	struct s3c2410_wdt_variant *drv_data;
 	struct regmap *pmureg;
 };
@@ -351,6 +349,29 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd, unsigned timeou
 	return 0;
 }
 
+static int s3c2410wdt_restart(struct watchdog_device *wdd)
+{
+	struct s3c2410_wdt *wdt = watchdog_get_drvdata(wdd);
+	void __iomem *wdt_base = wdt->reg_base;
+
+	/* disable watchdog, to be safe  */
+	writel(0, wdt_base + S3C2410_WTCON);
+
+	/* put initial values into count and data */
+	writel(0x80, wdt_base + S3C2410_WTCNT);
+	writel(0x80, wdt_base + S3C2410_WTDAT);
+
+	/* set the watchdog to go and reset... */
+	writel(S3C2410_WTCON_ENABLE | S3C2410_WTCON_DIV16 |
+		S3C2410_WTCON_RSTEN | S3C2410_WTCON_PRESCALE(0x20),
+		wdt_base + S3C2410_WTCON);
+
+	/* wait for reset to assert... */
+	mdelay(500);
+
+	return 0;
+}
+
 #define OPTIONS (WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE)
 
 static const struct watchdog_info s3c2410_wdt_ident = {
@@ -365,6 +386,7 @@ static struct watchdog_ops s3c2410wdt_ops = {
 	.stop = s3c2410wdt_stop,
 	.ping = s3c2410wdt_keepalive,
 	.set_timeout = s3c2410wdt_set_heartbeat,
+	.restart = s3c2410wdt_restart,
 };
 
 static struct watchdog_device s3c2410_wdd = {
@@ -452,31 +474,6 @@ static inline void s3c2410wdt_cpufreq_deregister(struct s3c2410_wdt *wdt)
 }
 #endif
 
-static int s3c2410wdt_restart(struct notifier_block *this,
-			      unsigned long mode, void *cmd)
-{
-	struct s3c2410_wdt *wdt = container_of(this, struct s3c2410_wdt,
-					       restart_handler);
-	void __iomem *wdt_base = wdt->reg_base;
-
-	/* disable watchdog, to be safe  */
-	writel(0, wdt_base + S3C2410_WTCON);
-
-	/* put initial values into count and data */
-	writel(0x80, wdt_base + S3C2410_WTCNT);
-	writel(0x80, wdt_base + S3C2410_WTDAT);
-
-	/* set the watchdog to go and reset... */
-	writel(S3C2410_WTCON_ENABLE | S3C2410_WTCON_DIV16 |
-		S3C2410_WTCON_RSTEN | S3C2410_WTCON_PRESCALE(0x20),
-		wdt_base + S3C2410_WTCON);
-
-	/* wait for reset to assert... */
-	mdelay(500);
-
-	return NOTIFY_DONE;
-}
-
 static inline unsigned int s3c2410wdt_get_bootstatus(struct s3c2410_wdt *wdt)
 {
 	unsigned int rst_stat;
@@ -605,6 +602,7 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
 	}
 
 	watchdog_set_nowayout(&wdt->wdt_device, nowayout);
+	watchdog_set_restart_priority(&wdt->wdt_device, 128);
 
 	wdt->wdt_device.bootstatus = s3c2410wdt_get_bootstatus(wdt);
 	wdt->wdt_device.parent = &pdev->dev;
@@ -632,12 +630,6 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, wdt);
 
-	wdt->restart_handler.notifier_call = s3c2410wdt_restart;
-	wdt->restart_handler.priority = 128;
-	ret = register_restart_handler(&wdt->restart_handler);
-	if (ret)
-		pr_err("cannot register restart handler, %d\n", ret);
-
 	/* print out a statement of readiness */
 
 	wtcon = readl(wdt->reg_base + S3C2410_WTCON);
@@ -667,8 +659,6 @@ static int s3c2410wdt_remove(struct platform_device *dev)
 	int ret;
 	struct s3c2410_wdt *wdt = platform_get_drvdata(dev);
 
-	unregister_restart_handler(&wdt->restart_handler);
-
 	ret = s3c2410wdt_mask_and_disable_reset(wdt, true);
 	if (ret < 0)
 		return ret;
-- 
2.5.0

Re: [RFC PATCH 12/13] watchdog: s3c2410_wdt: use core restart handler

[Guenter Roeck]

On 11/02/2015 05:36 PM, Damien Riegel wrote:
> Get rid of the custom restart handler by using the one provided by the
> watchdog core.
>
> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/watchdog/s3c2410_wdt.c | 60 ++++++++++++++++++------------------------
>   1 file changed, 25 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index d781000..0093450 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -41,7 +41,6 @@
>   #include <linux/of.h>
>   #include <linux/mfd/syscon.h>
>   #include <linux/regmap.h>
> -#include <linux/reboot.h>
>   #include <linux/delay.h>
>
>   #define S3C2410_WTCON		0x00
> @@ -130,7 +129,6 @@ struct s3c2410_wdt {
>   	unsigned long		wtdat_save;
>   	struct watchdog_device	wdt_device;
>   	struct notifier_block	freq_transition;
> -	struct notifier_block	restart_handler;
>   	struct s3c2410_wdt_variant *drv_data;
>   	struct regmap *pmureg;
>   };
> @@ -351,6 +349,29 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd, unsigned timeou
>   	return 0;
>   }
>
> +static int s3c2410wdt_restart(struct watchdog_device *wdd)
> +{
> +	struct s3c2410_wdt *wdt = watchdog_get_drvdata(wdd);
> +	void __iomem *wdt_base = wdt->reg_base;
> +
> +	/* disable watchdog, to be safe  */
> +	writel(0, wdt_base + S3C2410_WTCON);
> +
> +	/* put initial values into count and data */
> +	writel(0x80, wdt_base + S3C2410_WTCNT);
> +	writel(0x80, wdt_base + S3C2410_WTDAT);
> +
> +	/* set the watchdog to go and reset... */
> +	writel(S3C2410_WTCON_ENABLE | S3C2410_WTCON_DIV16 |
> +		S3C2410_WTCON_RSTEN | S3C2410_WTCON_PRESCALE(0x20),
> +		wdt_base + S3C2410_WTCON);
> +
> +	/* wait for reset to assert... */
> +	mdelay(500);
> +
> +	return 0;
> +}
> +
>   #define OPTIONS (WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE)
>
>   static const struct watchdog_info s3c2410_wdt_ident = {
> @@ -365,6 +386,7 @@ static struct watchdog_ops s3c2410wdt_ops = {
>   	.stop = s3c2410wdt_stop,
>   	.ping = s3c2410wdt_keepalive,
>   	.set_timeout = s3c2410wdt_set_heartbeat,
> +	.restart = s3c2410wdt_restart,
>   };
>
>   static struct watchdog_device s3c2410_wdd = {
> @@ -452,31 +474,6 @@ static inline void s3c2410wdt_cpufreq_deregister(struct s3c2410_wdt *wdt)
>   }
>   #endif
>
> -static int s3c2410wdt_restart(struct notifier_block *this,
> -			      unsigned long mode, void *cmd)
> -{
> -	struct s3c2410_wdt *wdt = container_of(this, struct s3c2410_wdt,
> -					       restart_handler);
> -	void __iomem *wdt_base = wdt->reg_base;
> -
> -	/* disable watchdog, to be safe  */
> -	writel(0, wdt_base + S3C2410_WTCON);
> -
> -	/* put initial values into count and data */
> -	writel(0x80, wdt_base + S3C2410_WTCNT);
> -	writel(0x80, wdt_base + S3C2410_WTDAT);
> -
> -	/* set the watchdog to go and reset... */
> -	writel(S3C2410_WTCON_ENABLE | S3C2410_WTCON_DIV16 |
> -		S3C2410_WTCON_RSTEN | S3C2410_WTCON_PRESCALE(0x20),
> -		wdt_base + S3C2410_WTCON);
> -
> -	/* wait for reset to assert... */
> -	mdelay(500);
> -
> -	return NOTIFY_DONE;
> -}
> -
>   static inline unsigned int s3c2410wdt_get_bootstatus(struct s3c2410_wdt *wdt)
>   {
>   	unsigned int rst_stat;
> @@ -605,6 +602,7 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
>   	}
>
>   	watchdog_set_nowayout(&wdt->wdt_device, nowayout);
> +	watchdog_set_restart_priority(&wdt->wdt_device, 128);
>
>   	wdt->wdt_device.bootstatus = s3c2410wdt_get_bootstatus(wdt);
>   	wdt->wdt_device.parent = &pdev->dev;
> @@ -632,12 +630,6 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
>
>   	platform_set_drvdata(pdev, wdt);
>
> -	wdt->restart_handler.notifier_call = s3c2410wdt_restart;
> -	wdt->restart_handler.priority = 128;
> -	ret = register_restart_handler(&wdt->restart_handler);
> -	if (ret)
> -		pr_err("cannot register restart handler, %d\n", ret);
> -
>   	/* print out a statement of readiness */
>
>   	wtcon = readl(wdt->reg_base + S3C2410_WTCON);
> @@ -667,8 +659,6 @@ static int s3c2410wdt_remove(struct platform_device *dev)
>   	int ret;
>   	struct s3c2410_wdt *wdt = platform_get_drvdata(dev);
>
> -	unregister_restart_handler(&wdt->restart_handler);
> -
>   	ret = s3c2410wdt_mask_and_disable_reset(wdt, true);
>   	if (ret < 0)
>   		return ret;
>

[RFC PATCH 13/13] watchdog: sunxi_wdt: use core restart handler

[Damien Riegel]

Get rid of the custom restart handler by using the one provided by the
watchdog core.

Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
 drivers/watchdog/sunxi_wdt.c | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/drivers/watchdog/sunxi_wdt.c b/drivers/watchdog/sunxi_wdt.c
index 47bd8a1..06bf3c5 100644
--- a/drivers/watchdog/sunxi_wdt.c
+++ b/drivers/watchdog/sunxi_wdt.c
@@ -25,7 +25,6 @@
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
-#include <linux/reboot.h>
 #include <linux/types.h>
 #include <linux/watchdog.h>
 
@@ -60,7 +59,6 @@ struct sunxi_wdt_dev {
 	struct watchdog_device wdt_dev;
 	void __iomem *wdt_base;
 	const struct sunxi_wdt_reg *wdt_regs;
-	struct notifier_block restart_handler;
 };
 
 /*
@@ -86,12 +84,9 @@ static const int wdt_timeout_map[] = {
 };
 
 
-static int sunxi_restart_handle(struct notifier_block *this, unsigned long mode,
-				void *cmd)
+static int sunxi_wdt_restart(struct watchdog_device *wdt_dev)
 {
-	struct sunxi_wdt_dev *sunxi_wdt = container_of(this,
-						       struct sunxi_wdt_dev,
-						       restart_handler);
+	struct sunxi_wdt_dev *sunxi_wdt = watchdog_get_drvdata(wdt_dev);
 	void __iomem *wdt_base = sunxi_wdt->wdt_base;
 	const struct sunxi_wdt_reg *regs = sunxi_wdt->wdt_regs;
 	u32 val;
@@ -120,7 +115,7 @@ static int sunxi_restart_handle(struct notifier_block *this, unsigned long mode,
 		val |= WDT_MODE_EN;
 		writel(val, wdt_base + regs->wdt_mode);
 	}
-	return NOTIFY_DONE;
+	return 0;
 }
 
 static int sunxi_wdt_ping(struct watchdog_device *wdt_dev)
@@ -208,6 +203,7 @@ static const struct watchdog_ops sunxi_wdt_ops = {
 	.stop		= sunxi_wdt_stop,
 	.ping		= sunxi_wdt_ping,
 	.set_timeout	= sunxi_wdt_set_timeout,
+	.restart	= sunxi_wdt_restart,
 };
 
 static const struct sunxi_wdt_reg sun4i_wdt_reg = {
@@ -268,6 +264,7 @@ static int sunxi_wdt_probe(struct platform_device *pdev)
 
 	watchdog_init_timeout(&sunxi_wdt->wdt_dev, timeout, &pdev->dev);
 	watchdog_set_nowayout(&sunxi_wdt->wdt_dev, nowayout);
+	watchdog_set_restart_priority(&sunxi_wdt->wdt_dev, 128);
 
 	watchdog_set_drvdata(&sunxi_wdt->wdt_dev, sunxi_wdt);
 
@@ -277,13 +274,6 @@ static int sunxi_wdt_probe(struct platform_device *pdev)
 	if (unlikely(err))
 		return err;
 
-	sunxi_wdt->restart_handler.notifier_call = sunxi_restart_handle;
-	sunxi_wdt->restart_handler.priority = 128;
-	err = register_restart_handler(&sunxi_wdt->restart_handler);
-	if (err)
-		dev_err(&pdev->dev,
-			"cannot register restart handler (err=%d)\n", err);
-
 	dev_info(&pdev->dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)",
 			sunxi_wdt->wdt_dev.timeout, nowayout);
 
@@ -294,8 +284,6 @@ static int sunxi_wdt_remove(struct platform_device *pdev)
 {
 	struct sunxi_wdt_dev *sunxi_wdt = platform_get_drvdata(pdev);
 
-	unregister_restart_handler(&sunxi_wdt->restart_handler);
-
 	watchdog_unregister_device(&sunxi_wdt->wdt_dev);
 	watchdog_set_drvdata(&sunxi_wdt->wdt_dev, NULL);
 
-- 
2.5.0

Re: [RFC PATCH 13/13] watchdog: sunxi_wdt: use core restart handler

[Guenter Roeck]

On 11/02/2015 05:36 PM, Damien Riegel wrote:
> Get rid of the custom restart handler by using the one provided by the
> watchdog core.
>
> Signed-off-by: Damien Riegel <damien.riegel@savoirfairelinux.com>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
>   drivers/watchdog/sunxi_wdt.c | 22 +++++-----------------
>   1 file changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/watchdog/sunxi_wdt.c b/drivers/watchdog/sunxi_wdt.c
> index 47bd8a1..06bf3c5 100644
> --- a/drivers/watchdog/sunxi_wdt.c
> +++ b/drivers/watchdog/sunxi_wdt.c
> @@ -25,7 +25,6 @@
>   #include <linux/of.h>
>   #include <linux/of_device.h>
>   #include <linux/platform_device.h>
> -#include <linux/reboot.h>
>   #include <linux/types.h>
>   #include <linux/watchdog.h>

Should be able to drop the notifier.h include.
Assuming you'll fix that,

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

>
> @@ -60,7 +59,6 @@ struct sunxi_wdt_dev {
>   	struct watchdog_device wdt_dev;
>   	void __iomem *wdt_base;
>   	const struct sunxi_wdt_reg *wdt_regs;
> -	struct notifier_block restart_handler;
>   };
>
>   /*
> @@ -86,12 +84,9 @@ static const int wdt_timeout_map[] = {
>   };
>
>
> -static int sunxi_restart_handle(struct notifier_block *this, unsigned long mode,
> -				void *cmd)
> +static int sunxi_wdt_restart(struct watchdog_device *wdt_dev)
>   {
> -	struct sunxi_wdt_dev *sunxi_wdt = container_of(this,
> -						       struct sunxi_wdt_dev,
> -						       restart_handler);
> +	struct sunxi_wdt_dev *sunxi_wdt = watchdog_get_drvdata(wdt_dev);
>   	void __iomem *wdt_base = sunxi_wdt->wdt_base;
>   	const struct sunxi_wdt_reg *regs = sunxi_wdt->wdt_regs;
>   	u32 val;
> @@ -120,7 +115,7 @@ static int sunxi_restart_handle(struct notifier_block *this, unsigned long mode,
>   		val |= WDT_MODE_EN;
>   		writel(val, wdt_base + regs->wdt_mode);
>   	}
> -	return NOTIFY_DONE;
> +	return 0;
>   }
>
>   static int sunxi_wdt_ping(struct watchdog_device *wdt_dev)
> @@ -208,6 +203,7 @@ static const struct watchdog_ops sunxi_wdt_ops = {
>   	.stop		= sunxi_wdt_stop,
>   	.ping		= sunxi_wdt_ping,
>   	.set_timeout	= sunxi_wdt_set_timeout,
> +	.restart	= sunxi_wdt_restart,
>   };
>
>   static const struct sunxi_wdt_reg sun4i_wdt_reg = {
> @@ -268,6 +264,7 @@ static int sunxi_wdt_probe(struct platform_device *pdev)
>
>   	watchdog_init_timeout(&sunxi_wdt->wdt_dev, timeout, &pdev->dev);
>   	watchdog_set_nowayout(&sunxi_wdt->wdt_dev, nowayout);
> +	watchdog_set_restart_priority(&sunxi_wdt->wdt_dev, 128);
>
>   	watchdog_set_drvdata(&sunxi_wdt->wdt_dev, sunxi_wdt);
>
> @@ -277,13 +274,6 @@ static int sunxi_wdt_probe(struct platform_device *pdev)
>   	if (unlikely(err))
>   		return err;
>
> -	sunxi_wdt->restart_handler.notifier_call = sunxi_restart_handle;
> -	sunxi_wdt->restart_handler.priority = 128;
> -	err = register_restart_handler(&sunxi_wdt->restart_handler);
> -	if (err)
> -		dev_err(&pdev->dev,
> -			"cannot register restart handler (err=%d)\n", err);
> -
>   	dev_info(&pdev->dev, "Watchdog enabled (timeout=%d sec, nowayout=%d)",
>   			sunxi_wdt->wdt_dev.timeout, nowayout);
>
> @@ -294,8 +284,6 @@ static int sunxi_wdt_remove(struct platform_device *pdev)
>   {
>   	struct sunxi_wdt_dev *sunxi_wdt = platform_get_drvdata(pdev);
>
> -	unregister_restart_handler(&sunxi_wdt->restart_handler);
> -
>   	watchdog_unregister_device(&sunxi_wdt->wdt_dev);
>   	watchdog_set_drvdata(&sunxi_wdt->wdt_dev, NULL);
>
>