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

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

 



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@xxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Vivien Didelot <vivien.didelot@xxxxxxxxxxxxxxxxxxxx>

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 *);


--
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