On 04/11/2017 09:06 AM, Andrey Smirnov wrote:
Save a bit of cleanup code by leveraging newly added
devm_register_reboot_notifier().

Cc: cphe...@gmail.com
Cc: linux-kernel@vger.kernel.org
Cc: Wim Van Sebroeck <w...@iguana.be>
Cc: Guenter Roeck <li...@roeck-us.net>
Cc: Andy Shevchenko <andy.shevche...@gmail.com>
Cc: Andrew Morton <a...@linux-foundation.org>
Signed-off-by: Andrey Smirnov <andrew.smir...@gmail.com>

Acked-by: Guenter Roeck <li...@roeck-us.net>

---

Guenter, Andy:

Here's the second version of the patch originally submitted in
[v1]. This time I tried to follow Guenter's suggestion of moving this
bit on initialization in watchdog_dev_register().

If I missed something obvious why this patch wouldn't work, again,
please let me know.

Patch introducting devm_register_reboot_notifier() can be found in
[other-patch]

Thanks,
Andrey Smirnov

[v1] https://lkml.org/lkml/2017/4/4/388
[other-patch] https://lkml.org/lkml/2017/3/20/671


 drivers/watchdog/watchdog_core.c | 35 -----------------------------------
 drivers/watchdog/watchdog_dev.c  | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 35 deletions(-)

diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index 74265b2..8a8d952 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -137,25 +137,6 @@ int watchdog_init_timeout(struct watchdog_device *wdd,
 }
 EXPORT_SYMBOL_GPL(watchdog_init_timeout);

-static int watchdog_reboot_notifier(struct notifier_block *nb,
-                                   unsigned long code, void *data)
-{
-       struct watchdog_device *wdd = container_of(nb, struct watchdog_device,
-                                                  reboot_nb);
-
-       if (code == SYS_DOWN || code == SYS_HALT) {
-               if (watchdog_active(wdd)) {
-                       int ret;
-
-                       ret = wdd->ops->stop(wdd);
-                       if (ret)
-                               return NOTIFY_BAD;
-               }
-       }
-
-       return NOTIFY_DONE;
-}
-
 static int watchdog_restart_notifier(struct notifier_block *nb,
                                     unsigned long action, void *data)
 {
@@ -244,19 +225,6 @@ static int __watchdog_register_device(struct 
watchdog_device *wdd)
                }
        }

-       if (test_bit(WDOG_STOP_ON_REBOOT, &wdd->status)) {
-               wdd->reboot_nb.notifier_call = watchdog_reboot_notifier;
-
-               ret = register_reboot_notifier(&wdd->reboot_nb);
-               if (ret) {
-                       pr_err("watchdog%d: Cannot register reboot notifier 
(%d)\n",
-                              wdd->id, ret);
-                       watchdog_dev_unregister(wdd);
-                       ida_simple_remove(&watchdog_ida, wdd->id);
-                       return ret;
-               }
-       }
-
        if (wdd->ops->restart) {
                wdd->restart_nb.notifier_call = watchdog_restart_notifier;

@@ -302,9 +270,6 @@ static void __watchdog_unregister_device(struct 
watchdog_device *wdd)
        if (wdd->ops->restart)
                unregister_restart_handler(&wdd->restart_nb);

-       if (test_bit(WDOG_STOP_ON_REBOOT, &wdd->status))
-               unregister_reboot_notifier(&wdd->reboot_nb);
-
        watchdog_dev_unregister(wdd);
        ida_simple_remove(&watchdog_ida, wdd->id);
 }
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 32930a0..9a52990 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -42,6 +42,7 @@
 #include <linux/miscdevice.h>    /* For handling misc devices */
 #include <linux/module.h>        /* For module stuff/... */
 #include <linux/mutex.h> /* For mutexes */
+#include <linux/reboot.h>        /* For reboot notifier */
 #include <linux/slab.h>          /* For memory functions */
 #include <linux/types.h> /* For standard types (like size_t) */
 #include <linux/watchdog.h>      /* For watchdog specific items */
@@ -998,6 +999,25 @@ static struct class watchdog_class = {
        .dev_groups =   wdt_groups,
 };

+static int watchdog_reboot_notifier(struct notifier_block *nb,
+                                   unsigned long code, void *data)
+{
+       struct watchdog_device *wdd = container_of(nb, struct watchdog_device,
+                                                  reboot_nb);
+
+       if (code == SYS_DOWN || code == SYS_HALT) {
+               if (watchdog_active(wdd)) {
+                       int ret;
+
+                       ret = wdd->ops->stop(wdd);
+                       if (ret)
+                               return NOTIFY_BAD;
+               }
+       }
+
+       return NOTIFY_DONE;
+}
+
 /*
  *     watchdog_dev_register: register a watchdog device
  *     @wdd: watchdog device
@@ -1031,6 +1051,18 @@ int watchdog_dev_register(struct watchdog_device *wdd)
        if (ret) {
                device_destroy(&watchdog_class, devno);
                watchdog_cdev_unregister(wdd);
+               return ret;
+       }
+
+       if (test_bit(WDOG_STOP_ON_REBOOT, &wdd->status)) {
+               wdd->reboot_nb.notifier_call = watchdog_reboot_notifier;
+
+               ret = devm_register_reboot_notifier(dev, &wdd->reboot_nb);
+               if (ret) {
+                       pr_err("watchdog%d: Cannot register reboot notifier 
(%d)\n",
+                              wdd->id, ret);
+                       watchdog_dev_unregister(wdd);
+               }
        }

        return ret;


Reply via email to