From 2165bf524da5f5e496d1cdb8c5afae1345ecce1e Mon Sep 17 00:00:00 2001 From: Damien Riegel Date: Mon, 16 Nov 2015 12:27:59 -0500 Subject: watchdog: core: add restart handler support 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. Suggested-by: Vivien Didelot Signed-off-by: Damien Riegel Reviewed-by: Guenter Roeck Reviewed-by: Vivien Didelot Signed-off-by: Guenter Roeck Signed-off-by: Wim Van Sebroeck --- include/linux/watchdog.h | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'include/linux') diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h index 027b1f43f12d..5b52c834f7aa 100644 --- a/include/linux/watchdog.h +++ b/include/linux/watchdog.h @@ -12,6 +12,7 @@ #include #include #include +#include #include struct watchdog_ops; @@ -26,6 +27,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 (in seconds). * @get_timeleft:The routine that gets the time left before a reset (in seconds). + * @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 +47,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); @@ -62,6 +65,7 @@ struct watchdog_ops { * @timeout: The watchdog devices timeout value (in seconds). * @min_timeout:The watchdog devices minimum timeout value (in seconds). * @max_timeout:The watchdog devices maximum timeout value (in seconds). + * @restart_nb: The notifier block to register a restart function. * @driver-data:Pointer to the drivers private data. * @lock: Lock for watchdog core internal use only. * @status: Field that contains the devices internal status bits. @@ -88,6 +92,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 +147,7 @@ static inline void *watchdog_get_drvdata(struct watchdog_device *wdd) } /* drivers/watchdog/watchdog_core.c */ +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 *); -- cgit v1.2.3 From 65a4a1dc31ad9d73918971f1b89c617812c494bf Mon Sep 17 00:00:00 2001 From: Damien Riegel Date: Mon, 16 Nov 2015 12:28:00 -0500 Subject: watchdog: bcm47xx_wdt: use core restart handler Get rid of the custom restart handler by using the one provided by the watchdog core. Signed-off-by: Damien Riegel Signed-off-by: Guenter Roeck Signed-off-by: Wim Van Sebroeck --- include/linux/bcm47xx_wdt.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include/linux') diff --git a/include/linux/bcm47xx_wdt.h b/include/linux/bcm47xx_wdt.h index 5582c211f594..b708786d4cbf 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; -- cgit v1.2.3 From e131319669e0ef5e6fcd75174daeffa40492135c Mon Sep 17 00:00:00 2001 From: Damien Riegel Date: Fri, 20 Nov 2015 16:54:51 -0500 Subject: watchdog: core: add reboot notifier support Many watchdog drivers register a reboot notifier in order to stop the watchdog on system reboot. Thus we can factorize this code in the watchdog core. For that purpose, a new notifier block is added in watchdog_device for internal use only, as well as a new watchdog_stop_on_reboot helper function. If this helper is called, watchdog core registers the related notifier block and will stop the watchdog when SYS_HALT or SYS_DOWN is received. Since this operation can be critical on some platforms, abort the device registration if the reboot notifier registration fails. Suggested-by: Vivien Didelot Signed-off-by: Damien Riegel Reviewed-by: Vivien Didelot Signed-off-by: Guenter Roeck Signed-off-by: Wim Van Sebroeck --- include/linux/watchdog.h | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'include/linux') diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h index 5b52c834f7aa..a88f955fde92 100644 --- a/include/linux/watchdog.h +++ b/include/linux/watchdog.h @@ -65,6 +65,7 @@ struct watchdog_ops { * @timeout: The watchdog devices timeout value (in seconds). * @min_timeout:The watchdog devices minimum timeout value (in seconds). * @max_timeout:The watchdog devices maximum timeout value (in seconds). + * @reboot_nb: The notifier block to stop watchdog on reboot. * @restart_nb: The notifier block to register a restart function. * @driver-data:Pointer to the drivers private data. * @lock: Lock for watchdog core internal use only. @@ -92,6 +93,7 @@ struct watchdog_device { unsigned int timeout; unsigned int min_timeout; unsigned int max_timeout; + struct notifier_block reboot_nb; struct notifier_block restart_nb; void *driver_data; struct mutex lock; @@ -102,6 +104,7 @@ struct watchdog_device { #define WDOG_ALLOW_RELEASE 2 /* Did we receive the magic char ? */ #define WDOG_NO_WAY_OUT 3 /* Is 'nowayout' feature set ? */ #define WDOG_UNREGISTERED 4 /* Has the device been unregistered */ +#define WDOG_STOP_ON_REBOOT 5 /* Should be stopped on reboot */ struct list_head deferred; }; @@ -121,6 +124,12 @@ static inline void watchdog_set_nowayout(struct watchdog_device *wdd, bool noway set_bit(WDOG_NO_WAY_OUT, &wdd->status); } +/* Use the following function to stop the watchdog on reboot */ +static inline void watchdog_stop_on_reboot(struct watchdog_device *wdd) +{ + set_bit(WDOG_STOP_ON_REBOOT, &wdd->status); +} + /* Use the following function to check if a timeout value is invalid */ static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigned int t) { -- cgit v1.2.3 From 2786aadeab263609eb690ca37e7dfd3b9ffa3625 Mon Sep 17 00:00:00 2001 From: Damien Riegel Date: Fri, 20 Nov 2015 16:54:52 -0500 Subject: watchdog: bcm47xx_wdt: use core reboot notifier Get rid of the custom reboot notifier block registration and use the one provided by the watchdog core. Signed-off-by: Damien Riegel Reviewed-by: Guenter Roeck Reviewed-by: Vivien Didelot Signed-off-by: Guenter Roeck Signed-off-by: Wim Van Sebroeck --- include/linux/bcm47xx_wdt.h | 2 -- 1 file changed, 2 deletions(-) (limited to 'include/linux') diff --git a/include/linux/bcm47xx_wdt.h b/include/linux/bcm47xx_wdt.h index b708786d4cbf..8d9d07ec22a5 100644 --- a/include/linux/bcm47xx_wdt.h +++ b/include/linux/bcm47xx_wdt.h @@ -1,7 +1,6 @@ #ifndef LINUX_BCM47XX_WDT_H_ #define LINUX_BCM47XX_WDT_H_ -#include #include #include #include @@ -15,7 +14,6 @@ struct bcm47xx_wdt { void *driver_data; struct watchdog_device wdd; - struct notifier_block notifier; struct timer_list soft_timer; atomic_t soft_ticks; -- cgit v1.2.3 From b4ffb1909843b28f3b1b60197d517b123b7a9b66 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Fri, 25 Dec 2015 16:01:42 -0800 Subject: watchdog: Separate and maintain variables based on variable lifetime All variables required by the watchdog core to manage a watchdog are currently stored in struct watchdog_device. The lifetime of those variables is determined by the watchdog driver. However, the lifetime of variables used by the watchdog core differs from the lifetime of struct watchdog_device. To remedy this situation, watchdog drivers can implement ref and unref callbacks, to be used by the watchdog core to lock struct watchdog_device in memory. While this solves the immediate problem, it depends on watchdog drivers to actually implement the ref/unref callbacks. This is error prone, often not implemented in the first place, or not implemented correctly. To solve the problem without requiring driver support, split the variables in struct watchdog_device into two data structures - one for variables associated with the watchdog driver, one for variables associated with the watchdog core. With this approach, the watchdog core can keep track of its variable lifetime and no longer depends on ref/unref callbacks in the driver. As a side effect, some of the variables originally in struct watchdog_driver are now private to the watchdog core and no longer visible in watchdog drivers. As a side effect of the changes made, an ioctl will now always fail with -ENODEV after a watchdog device was unregistered with the character device still open. Previously, it would only fail with -ENODEV in some situations. Also, ioctl operations are now atomic from driver perspective. With this change, it is now guaranteed that the driver will not unregister a watchdog between a timeout change and the subsequent ping. The 'ref' and 'unref' callbacks in struct watchdog_driver are no longer used and marked as deprecated. Signed-off-by: Guenter Roeck Signed-off-by: Wim Van Sebroeck --- include/linux/watchdog.h | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) (limited to 'include/linux') diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h index a88f955fde92..850af04fe0c7 100644 --- a/include/linux/watchdog.h +++ b/include/linux/watchdog.h @@ -17,6 +17,7 @@ struct watchdog_ops; struct watchdog_device; +struct watchdog_core_data; /** struct watchdog_ops - The watchdog-devices operations * @@ -28,8 +29,6 @@ struct watchdog_device; * @set_timeout:The routine for setting the watchdog devices timeout value (in seconds). * @get_timeleft:The routine that gets the time left before a reset (in seconds). * @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. * * The watchdog_ops structure contains a list of low-level operations @@ -48,15 +47,14 @@ struct watchdog_ops { 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 *); + void (*ref)(struct watchdog_device *) __deprecated; + void (*unref)(struct watchdog_device *) __deprecated; long (*ioctl)(struct watchdog_device *, unsigned int, unsigned long); }; /** struct watchdog_device - The structure that defines a watchdog device * * @id: The watchdog's ID. (Allocated by watchdog_register_device) - * @cdev: The watchdog's Character device. * @dev: The device for our watchdog * @parent: The parent bus device * @info: Pointer to a watchdog_info structure. @@ -67,8 +65,8 @@ struct watchdog_ops { * @max_timeout:The watchdog devices maximum timeout value (in seconds). * @reboot_nb: The notifier block to stop watchdog on reboot. * @restart_nb: The notifier block to register a restart function. - * @driver-data:Pointer to the drivers private data. - * @lock: Lock for watchdog core internal use only. + * @driver_data:Pointer to the drivers private data. + * @wd_data: Pointer to watchdog core internal data. * @status: Field that contains the devices internal status bits. * @deferred: entry in wtd_deferred_reg_list which is used to * register early initialized watchdogs. @@ -84,7 +82,6 @@ struct watchdog_ops { */ struct watchdog_device { int id; - struct cdev cdev; struct device *dev; struct device *parent; const struct watchdog_info *info; @@ -96,15 +93,12 @@ struct watchdog_device { struct notifier_block reboot_nb; struct notifier_block restart_nb; void *driver_data; - struct mutex lock; + struct watchdog_core_data *wd_data; unsigned long status; /* Bit numbers for status flags */ #define WDOG_ACTIVE 0 /* Is the watchdog running/active */ -#define WDOG_DEV_OPEN 1 /* Opened via /dev/watchdog ? */ -#define WDOG_ALLOW_RELEASE 2 /* Did we receive the magic char ? */ -#define WDOG_NO_WAY_OUT 3 /* Is 'nowayout' feature set ? */ -#define WDOG_UNREGISTERED 4 /* Has the device been unregistered */ -#define WDOG_STOP_ON_REBOOT 5 /* Should be stopped on reboot */ +#define WDOG_NO_WAY_OUT 1 /* Is 'nowayout' feature set ? */ +#define WDOG_STOP_ON_REBOOT 2 /* Should be stopped on reboot */ struct list_head deferred; }; -- cgit v1.2.3 From 62cd1c40ce1c7c16835b599751c7a002eb5bbdf5 Mon Sep 17 00:00:00 2001 From: Tomas Winkler Date: Sun, 3 Jan 2016 13:32:37 +0200 Subject: watchdog: kill unref/ref ops ref/unref ops are not called at all so even marked them as deprecated is misleading, we need to just drop the API. Signed-off-by: Tomas Winkler Signed-off-by: Guenter Roeck Signed-off-by: Wim Van Sebroeck --- include/linux/watchdog.h | 2 -- 1 file changed, 2 deletions(-) (limited to 'include/linux') diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h index 850af04fe0c7..aaabd4703b46 100644 --- a/include/linux/watchdog.h +++ b/include/linux/watchdog.h @@ -47,8 +47,6 @@ struct watchdog_ops { 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 *) __deprecated; - void (*unref)(struct watchdog_device *) __deprecated; long (*ioctl)(struct watchdog_device *, unsigned int, unsigned long); }; -- cgit v1.2.3 From faa584757b63aad42d19f1c6a6eac2c848618f83 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Sun, 3 Jan 2016 15:11:56 -0800 Subject: watchdog: Add support for creating driver specific sysfs attributes The Zodiac watchdog driver attaches additional sysfs attributes to the watchdog device. This has a number of problems: The watchdog device lifetime differs from the driver lifetime, and the device structure should therefore not be accessed from drivers. Also, creating sysfs attributes after driver registration results in a potential race condition if user space expects the attributes to exist but they don't exist yet. Add support for creating driver specific sysfs attributes to the watchdog core to solve the problems. Signed-off-by: Guenter Roeck Signed-off-by: Guenter Roeck Signed-off-by: Wim Van Sebroeck --- include/linux/watchdog.h | 3 +++ 1 file changed, 3 insertions(+) (limited to 'include/linux') diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h index aaabd4703b46..076df50ea0da 100644 --- a/include/linux/watchdog.h +++ b/include/linux/watchdog.h @@ -55,6 +55,8 @@ struct watchdog_ops { * @id: The watchdog's ID. (Allocated by watchdog_register_device) * @dev: The device for our watchdog * @parent: The parent bus device + * @groups: List of sysfs attribute groups to create when creating the + * watchdog device. * @info: Pointer to a watchdog_info structure. * @ops: Pointer to the list of watchdog operations. * @bootstatus: Status of the watchdog device at boot. @@ -82,6 +84,7 @@ struct watchdog_device { int id; struct device *dev; struct device *parent; + const struct attribute_group **groups; const struct watchdog_info *info; const struct watchdog_ops *ops; unsigned int bootstatus; -- cgit v1.2.3 From 0254e953537c92df3e7d0176f401a211e944fd61 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Sun, 3 Jan 2016 15:11:58 -0800 Subject: watchdog: Drop pointer to watchdog device from struct watchdog_device The lifetime of the watchdog device pointer is different from the lifetime of its character device. Remove it entirely to avoid race conditions. Signed-off-by: Guenter Roeck Signed-off-by: Guenter Roeck Signed-off-by: Wim Van Sebroeck --- include/linux/watchdog.h | 2 -- 1 file changed, 2 deletions(-) (limited to 'include/linux') diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h index 076df50ea0da..b585fa2507ee 100644 --- a/include/linux/watchdog.h +++ b/include/linux/watchdog.h @@ -53,7 +53,6 @@ struct watchdog_ops { /** struct watchdog_device - The structure that defines a watchdog device * * @id: The watchdog's ID. (Allocated by watchdog_register_device) - * @dev: The device for our watchdog * @parent: The parent bus device * @groups: List of sysfs attribute groups to create when creating the * watchdog device. @@ -82,7 +81,6 @@ struct watchdog_ops { */ struct watchdog_device { int id; - struct device *dev; struct device *parent; const struct attribute_group **groups; const struct watchdog_info *info; -- cgit v1.2.3