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