Re: [PATCH] watchdog: core: module param to activate watchdog

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/28/2014 08:16 AM, Markus Pargmann wrote:
Many watchdog driver reset the watchdog device on initialization. This
is a problem if the watchdog is activated by the bootloader and should
be active the whole time until the userspace can write to it.


Shouldn't those watchdog drivers be fixed to keep the watchdog running
if it was already active ? Otherwise there is still the time between
driver initialization, which disables the watchdog, and it being re-enabled
by watchdog registration.

Also, does this affect or impact drivers which _do_ keep the watchdog
enabled ?

This patch adds a module parameter (watchdog.activate_first) that
activates the first registered watchdog. Using this parameter it is
possible to have an active watchdog during the whole boot process.

Signed-off-by: Markus Pargmann <mpa@xxxxxxxxxxxxxx>
---
  drivers/watchdog/watchdog_core.c |  4 ++++
  drivers/watchdog/watchdog_core.h |  4 ++++
  drivers/watchdog/watchdog_dev.c  | 21 ++++++++++++++++++++-
  3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index cec9b55..6db16eb 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -43,6 +43,10 @@
  static DEFINE_IDA(watchdog_ida);
  static struct class *watchdog_class;

+unsigned int activate_first = 0;

Bad variable name for a global variable.

+module_param(activate_first, uint, 0);
+MODULE_PARM_DESC(activate_first, "Activation timeout for first watchdog registered. 0 means no activation.");
+

Does that _have_ to be in this file ? Would be much easier
if it was in watchdog_dev.c, where it is used.

  static void watchdog_check_min_max_timeout(struct watchdog_device *wdd)
  {
  	/*
diff --git a/drivers/watchdog/watchdog_core.h b/drivers/watchdog/watchdog_core.h
index 6c95141..4526170 100644
--- a/drivers/watchdog/watchdog_core.h
+++ b/drivers/watchdog/watchdog_core.h
@@ -28,6 +28,10 @@

  #define MAX_DOGS	32	/* Maximum number of watchdog devices */

+/* Activate first registered watchdog and set this as the timeout value
+ * (only != 0) */

Coding style.

+extern unsigned int activate_first;
+
  /*
   *	Functions/procedures to be called by the core
   */
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 6aaefba..fe60fdc 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -553,8 +553,27 @@ int watchdog_dev_register(struct watchdog_device *watchdog)
  			misc_deregister(&watchdog_miscdev);
  			old_wdd = NULL;
  		}
+

Is that extra empty line really necessary ?

+		return err;
  	}
-	return err;
+
+	if (activate_first && watchdog->id == 0) {
+		err = watchdog_set_timeout(watchdog, activate_first);
+		if (err && err != -EOPNOTSUPP) {
+			pr_err("watchdog%d failed to set timeout for first activation\n",
+					watchdog->id);
+			return err;
+		}
+
+		err = watchdog_start(watchdog);
+		if (err) {
+			pr_err("watchdog%d failed to start first watchdog\n",
+					watchdog->id);
+			return err;
+		}
+	}
+
+	return 0;
  }

  /*


--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux