watchdog: Add support for keepalives triggered by infrastructure
Subsystems (4)
Guenter Roeck
maintainerv8 cover watchdog: Add support for keepalives triggered by infrastructure
The watchdog infrastructure is currently purely passive, meaning it only passes information from user space to drivers and vice versa.
Since watchdog hardware tends to have its own quirks, this can result in quite complex watchdog drivers. A number of scanarios are especially common.
- A watchdog is always active and can not be disabled, or can not be disabled once enabled. To support such hardware, watchdog drivers have to implement their own timers and use those timers to trigger watchdog keepalives while the watchdog device is not or not yet opened.
- A variant of this is the desire to enable a watchdog as soon as its driver has been instantiated, to protect the system while it is still booting up, but the watchdog daemon is not yet running.
- Some watchdogs have a very short maximum timeout, in the range of just a few seconds. Such low timeouts are difficult if not impossible to support from user space. Drivers supporting such watchdog hardware need to implement a timer function to augment heartbeats from user space.
This patch set solves the above problems while keeping changes to the watchdog core minimal.
- A new status flag, WDOG_HW_RUNNING, informs the watchdog subsystem that a watchdog is running, and that the watchdog subsystem needs to generate heartbeat requests while the associated watchdog device is closed.
- A new parameter in the watchdog data structure, max_hw_heartbeat_ms, informs the watchdog subsystem about a maximum hardware heartbeat. The watchdog subsystem uses this information together with the configured timeout and the maximum permitted timeout to determine if it needs to generate additional heartbeat requests.
As part of this patchset, the semantics of the 'timeout' variable and of the WDOG_ACTIVE flag are changed slightly.
Per the current watchdog kernel API, the 'timeout' variable is supposed to reflect the actual hardware watchdog timeout. WDOG_ACTIVE is supposed to reflect if the hardware watchdog is running or not.
Unfortunately, this does not always reflect reality. In drivers which solve the above mentioned problems internally, 'timeout' is the watchdog timeout as seen from user space, and WDOG_ACTIVE reflects that user space is expected to send keepalive requests to the watchdog driver.
After this patch set is applied, this so far inofficial interpretation is the 'official' semantics for the timeout variable and the WDOG_ACTIVE flag. In other words, both values no longer reflect the hardware watchdog status, but its status as seen from user space.
Patch #1 makes the set_timeout function optional.
Patch #2 adds timer functionality to the watchdog core. It solves the problem of short maximum hardware heartbeats by augmenting heartbeats triggered from user space with internally triggered heartbeats.
Patch #3 adds functionality to generate heartbeats while the watchdog device is closed. It handles situation where where the watchdog is running after the driver has been instantiated, but the device is not yet opened, and post-close situations necessary if a watchdog can not be stopped.
Patch #4 makes the stop function optional.
Patch #5 adds code to ensure that the minimum time between heartbeats meets constraints provided by the watchdog driver.
Patch #6 to #10 are example conversions of some watchdog drivers. Out of those, patches #6 (dw_wdt) and #7 (imx2) have been tested. Patches #8 to #10 will require testing and are marked as RFT.
The patch series is also available in branch watchdog-timer ofgit://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git.
The series is curently based on top of v4.5-rc5.
The patch series was inspired by an earlier patch set from Timo Kokonnen.
v8:
- max_hw_timeout_ms -> max_hw_heartbeat_ms
- Rebased to v4.5-rc5
- The patch to make the set_timeout function optional is now the first patch of the series.
- Separate code making the stop function optional into a separate patch.
- Dropped all Acked-by: and Tested-by: tags. Even though there are no functional changes, I concluded that the changes are too substantial to warrant maintaining previously provided tags.
Note: while the patch series was rearranged, correctness was ensured by implementing the max_hw_timeout_ms -> max_hw_heartbeat_ms changes separately on top of the original series and comparing the results.
v7:
- Rebased to v4.5-rc1.
- Moved new variables to local data structure
- Fixed typos in documentation (hw_max_timeout_ms -> max_hw_timeout_ms).
- Enforce that max_hw_timeout_ms must be set if the stop function is not implemented by a driver.
- Set max_hw_timeout_ms in converted drivers.
v6:
- Added patch #9, converting the dw_wdt driver.
- Set last_keepalive more accurately when starting the watchdog.
- Rebased to v4.4-rc2.
- Renamed WDOG_RUNNING to WDOG_HW_RUNNING and watchdog_running() to watchdog_hw_running().
v5:
- Patches #1 and #2 of the original patch series are now in mainline and have been dropped.
- Rebased to v4.4-rc1.
- Added patch to simplify watchdog_update_worker().
v4:
- Rebased to v4.3-rc3
- Rearranged patch sequence
- Dropped gpio driver patch. The driver was changed since v4.2, and merging the changes turned out to be too difficult.
- Various other cleanups as listed in individual patches
v3:
- Rebased to v4.2-rc8
- Reworked and cleaned up some of the functions.
- No longer call the worker update function if all that is needed is to stop the worker.
- max_timeout will now be ignored if max_hw_timeout_ms is provided.
- Added patch 9/9.
v2:
- Rebased to v4.2-rc5
- Improved and hopefully clarified documentation.
- Rearranged variables in struct watchdog_device such that internal variables come last.
- The code now ensures that the watchdog times out <timeout> seconds after the most recent keepalive sent from user space.
- The internal keepalive now stops silently and no longer generates a warning message. Reason is that it will now stop early, while there may still be a substantial amount of time for keepalives from user space to arrive. If such keepalives arrive late (for example if user space is configured to send keepalives just a few seconds before the watchdog times out), the message would just be noise and not provide any value.
Guenter Roeck
maintainerPATCH v8 1/10 watchdog: Make set_timeout function optional
For some watchdogs, the watchdog driver handles timeout changes without explicitly setting any registers. In this situation, the watchdog driver might only set the 'timeout' variable but do nothing else. This can as well be handled by the infrastructure, so make the set_timeout callback optional. If WDIOF_SETTIMEOUT is configured but the .set_timeout callback is not available, update the timeout variable in the infrastructure code.
---
v8: Moved to be first patch of series
Rebased to v4.5-rc5
v7: Rebased to v4.5-rc1
v6: Rebased to v4.4-rc2
v5: Rebased to v4.4-rc1
v4: No changes
v3: No changes
v2: No changes
--- Documentation/watchdog/watchdog-kernel-api.txt | 5 +++++ drivers/watchdog/watchdog_dev.c | 11 +++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-)
Documentation/watchdog/watchdog-kernel-api.txt +5 -0
@@ -156,6 +156,11 @@ they are supported. These optional routines/operations are: because the watchdog does not necessarily has a 1 second resolution). (Note: the WDIOF_SETTIMEOUT needs to be set in the options field of the watchdog's info structure). + If the watchdog driver does not have to perform any action but setting the + watchdog_device.timeout, this callback can be omitted. + If set_timeout is not provided but, WDIOF_SETTIMEOUT is set, the watchdog + infrastructure updates the timeout value of the watchdog_device internally + to the requested value. * get_timeleft: this routines returns the time that's left before a reset. * restart: this routine restarts the machine. It returns 0 on success or a negative errno code for failure.
drivers/watchdog/watchdog_dev.c +9 -2
@@ -183,13 +183,20 @@ static unsigned int watchdog_get_status(struct watchdog_device *wdd)static int watchdog_set_timeout (struct watchdog_device *wdd,unsigned int timeout) { -if (!wdd->ops->set_timeout || !(wdd->info->options &WDIOF_SETTIMEOUT )) +int err =0 ; + +if (!(wdd->info->options &WDIOF_SETTIMEOUT ))return -EOPNOTSUPP ;if (watchdog_timeout_invalid (wdd, timeout))return -EINVAL ; -return wdd->ops->set_timeout (wdd, timeout); +if (wdd->ops->set_timeout) + err = wdd->ops->set_timeout (wdd, timeout); +else + wdd->timeout = timeout; + +return err; }/*
Signature · 1 line
2.5.0
Guenter Roeck
maintainerPATCH v8 2/10 watchdog: Introduce hardware maximum heartbeat in watchdog core
In reply to Guenter RoeckIntroduce an optional hardware maximum heartbeat in the watchdog core. The hardware maximum heartbeat can be lower than the maximum timeout.
Drivers can set the maximum hardware heartbeat value in the watchdog data structure. If the configured timeout exceeds the maximum hardware heartbeat, the watchdog core enables a timer function to assist sending keepalive requests to the watchdog driver.
---
v8:
- Rebased to v4.5-rc5
- Now second patch of series
- max_hw_timeout_ms -> max_hw_heartbeat_ms
- Merged 'watchdog: Simplify update_worker' into this patch
v7:
- Rebased to v4.5-rc1
- Moved new variables to local data structure
- Dropped Uwe's Acked-by: due to substantial changes
v6:
- Set last_keepalive more accurately when starting the watchdog
- Rebased to v4.4-rc2
- Added Uwe's Acked-by:
v5:
- Rebased to v4.4-rc1
v4:
- Improved and fixed documentation
- Split hw_timeout_ms variable to timeout_ms, hw_timeout_ms for clarity
- Dropped redundant comments
- Added comments explaining failure conditions in watchdog_timeout_invalid().
- Moved the call to watchdog_update_worker() into _watchdog_ping(). v3:
- Reworked and cleaned up some of the functions.
- No longer call the worker update function if all that is needed is to stop the worker.
- max_timeout will now be ignored if max_hw_timeout_ms is provided. v2:
- Improved and hopefully clarified documentation.
- Rearranged variables in struct watchdog_device such that internal variables come last.
- The code now ensures that the watchdog times out <timeout> seconds after the most recent keepalive sent from user space.
- The internal keepalive now stops silently and no longer generates a warning message. Reason is that it will now stop early, while there may still be a substantial amount of time for keepalives from user space to arrive. If such keepalives arrive late (for example if user space is configured to send keepalives just a few seconds before the watchdog times out), the message would just be noise and not provide any value.
--- Documentation/watchdog/watchdog-kernel-api.txt | 19 +++- drivers/watchdog/watchdog_dev.c | 129 +++++++++++++++++++++++-- include/linux/watchdog.h | 28 ++++-- 3 files changed, 158 insertions(+), 18 deletions(-)
Documentation/watchdog/watchdog-kernel-api.txt +17 -2
@@ -52,6 +52,7 @@ struct watchdog_device { unsigned int timeout; unsigned int min_timeout; unsigned int max_timeout; + unsigned int max_hw_heartbeat_ms; struct notifier_block reboot_nb; struct notifier_block restart_nb; void *driver_data;
@@ -73,8 +74,18 @@ It contains following fields: additional information about the watchdog timer itself. (Like it's unique name) * ops: a pointer to the list of watchdog operations that the watchdog supports. * timeout: the watchdog timer's timeout value (in seconds). + This is the time after which the system will reboot if user space does + not send a heartbeat request if WDOG_ACTIVE is set. * min_timeout: the watchdog timer's minimum timeout value (in seconds). -* max_timeout: the watchdog timer's maximum timeout value (in seconds). + If set, the minimum configurable value for 'timeout'. +* max_timeout: the watchdog timer's maximum timeout value (in seconds), + as seen from userspace. If set, the maximum configurable value for + 'timeout'. Not used if max_hw_heartbeat_ms is non-zero. +* max_hw_heartbeat_ms: Maximum hardware heartbeat, in milli-seconds. + If set, the infrastructure will send heartbeats to the watchdog driver + if 'timeout' is larger than max_hw_heartbeat_ms, unless WDOG_ACTIVE + is set and userspace failed to send a heartbeat for at least 'timeout' + seconds. * reboot_nb: notifier block that is registered for reboot notifications, for internal use only. If the driver calls watchdog_stop_on_reboot, watchdog core will stop the watchdog on such notifications.
@@ -153,7 +164,11 @@ they are supported. These optional routines/operations are: and -EIO for "could not write value to the watchdog". On success this routine should set the timeout value of the watchdog_device to the achieved timeout value (which may be different from the requested one - because the watchdog does not necessarily has a 1 second resolution). + because the watchdog does not necessarily have a 1 second resolution). + Drivers implementing max_hw_heartbeat_ms set the hardware watchdog heartbeat + to the minimum of timeout and max_hw_heartbeat_ms. Those drivers set the + timeout value of the watchdog_device either to the requested timeout value + (if it is larger than max_hw_heartbeat_ms), or to the achieved timeout value. (Note: the WDIOF_SETTIMEOUT needs to be set in the options field of the watchdog's info structure). If the watchdog driver does not have to perform any action but setting the
drivers/watchdog/watchdog_dev.c +121 -8
224 lines not on this page · raw message
include/linux/watchdog.h +20 -8
@@ -10,8 +10,9 @@#include <linux/bitops.h> -#include <linux/device.h> #include <linux/cdev.h> +#include <linux/device.h> +#include <linux/kernel.h> #include <linux/notifier.h> #include <uapi/linux/watchdog.h>
@@ -61,14 +62,19 @@ struct watchdog_ops {* @bootstatus: Status of the watchdog device at boot. * @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). +* @max_timeout:The watchdog devices maximum timeout value (in seconds) +* as configurable from user space. Only relevant if +* max_hw_heartbeat_ms is not provided. +* @max_hw_heartbeat_ms: +* Hardware limit for maximum timeout, in milli-seconds. +* Replaces max_timeout if specified. * @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. * @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. +* @deferred: Entry in wtd_deferred_reg_list which is used to +* register early initialized watchdogs. * * The watchdog_device structure contains all information about a * watchdog timer device.
@@ -89,6 +95,7 @@ struct watchdog_device {unsigned int timeout;unsigned int min_timeout;unsigned int max_timeout; +unsigned int max_hw_heartbeat_ms;struct notifier_block reboot_nb;struct notifier_block restart_nb;void *driver_data;
@@ -128,13 +135,18 @@ static inline bool watchdog_timeout_invalid(struct watchdog_device *wdd, unsigne {/* * The timeout is invalid if +* - the requested value is larger than UINT_MAX / 1000 +* (since internal calculations are done in milli-seconds), +* or * - the requested value is smaller than the configured minimum timeout, * or -* - a maximum timeout is configured, and the requested value is larger -* than the maximum timeout. +* - a maximum hardware timeout is not configured, a maximum timeout +* is configured, and the requested value is larger than the +* configured maximum timeout. */ -return t < wdd->min_timeout || - (wdd->max_timeout && t > wdd->max_timeout); +return t >UINT_MAX /1000 || t < wdd->min_timeout || + (!wdd->max_hw_heartbeat_ms && wdd->max_timeout && + t > wdd->max_timeout); }/* Use the following functions to manipulate watchdog driver specific data */
Signature · 1 line
2.5.0
Guenter Roeck
maintainerPATCH v8 3/10 watchdog: Introduce WDOG_HW_RUNNING flag
In reply to Guenter RoeckThe WDOG_HW_RUNNING flag is expected to be set by watchdog drivers if the hardware watchdog is running. If the flag is set, the watchdog subsystem will ping the watchdog even if the watchdog device is closed.
The watchdog driver stop function is now optional and may be omitted if the watchdog can not be stopped. If stopping the watchdog is not possible but the driver implements a stop function, it is responsible to set the WDOG_HW_RUNNING flag in its stop function.
---
v8: Rebased to v4.5-rc5
max_hw_timeout_ms -> max_hw_heartbeat_ms
Split out making stop function optional
v7: Fix typos in documentation (hw_max_timeout_ms -> max_hw_timeout_ms).
Enforce that max_hw_timeout_ms must be set if the stop function is not implemented by a driver.
Rebased to v4.5-rc1
v6: Fixed typo in documentation.
Renamed WDOG_RUNNING to WDOG_HW_RUNNING and watchdog_running() to watchdog_hw_running().
Rebased to v4.4-rc2.
v5: Rebased to v4.4-rc1.
v4: No change.
v3: Clarified meaning of WDOG_ACTIVE.
Do not call cancel_delayed_work_sync() from watchdog_update_worker(). Call it directly where needed instead, to keep the common code simple. Do not (re-)start an already running watchdog when opening the watchdog device. Send a heartbeat request instead.
v2: Improved documentation.
--- Documentation/watchdog/watchdog-kernel-api.txt | 22 +++++++---- drivers/watchdog/watchdog_dev.c | 52 +++++++++++++++++++------- include/linux/watchdog.h | 10 +++++ 3 files changed, 64 insertions(+), 20 deletions(-)
Documentation/watchdog/watchdog-kernel-api.txt +15 -7
@@ -137,10 +137,10 @@ are: * stop: with this routine the watchdog timer device is being stopped. The routine needs a pointer to the watchdog timer device structure as a parameter. It returns zero on success or a negative errno code for failure. - Some watchdog timer hardware can only be started and not be stopped. The - driver supporting this hardware needs to make sure that a start and stop - routine is being provided. This can be done by using a timer in the driver - that regularly sends a keepalive ping to the watchdog timer hardware. + Some watchdog timer hardware can only be started and not be stopped. + If a watchdog can not be stopped, the watchdog driver must set the + WDOG_HW_RUNNING flag in its stop function to inform the watchdog core that + the watchdog is still running. Not all watchdog timer hardware supports the same functionality. That's why all other routines/operations are optional. They only need to be provided if
@@ -189,11 +189,19 @@ The 'ref' and 'unref' operations are no longer used and deprecated. The status bits should (preferably) be set with the set_bit and clear_bit alike bit-operations. The status bits that are defined are: * WDOG_ACTIVE: this status bit indicates whether or not a watchdog timer device - is active or not. When the watchdog is active after booting, then you should - set this status bit (Note: when you register the watchdog timer device with - this bit set, then opening /dev/watchdog will skip the start operation) + is active or not from user perspective. User space is expected to send + heartbeat requests to the driver while this flag is set. * WDOG_NO_WAY_OUT: this bit stores the nowayout setting for the watchdog. If this bit is set then the watchdog timer will not be able to stop. +* WDOG_HW_RUNNING: Set by the watchdog driver if the hardware watchdog is + running. The bit must be set if the watchdog timer hardware can not be + stopped. The bit may also be set if the watchdog timer is running after + booting, before the watchdog device is opened. If set, the watchdog + infrastructure will send keepalives to the watchdog hardware while + WDOG_ACTIVE is not set. + Note: when you register the watchdog timer device with this bit set, + then opening /dev/watchdog will skip the start operation but send a keepalive + request instead. To set the WDOG_NO_WAY_OUT status bit (before registering your watchdog timer device) you can either:
drivers/watchdog/watchdog_dev.c +39 -13
125 lines not on this page · raw message
include/linux/watchdog.h +10 -0
@@ -105,6 +105,7 @@ struct watchdog_device {#define WDOG_ACTIVE 0/* Is the watchdog running/active */ #define WDOG_NO_WAY_OUT 1/* Is 'nowayout' feature set ? */ #define WDOG_STOP_ON_REBOOT 2/* Should be stopped on reboot */ +#define WDOG_HW_RUNNING 3/* True if HW watchdog running */ struct list_head deferred; };
@@ -117,6 +118,15 @@ static inline bool watchdog_active(struct watchdog_device *wdd)return test_bit (WDOG_ACTIVE , &wdd->status); } +/* +* Use the following function to check whether or not the hardware watchdog +* is running +*/ +static inline bool watchdog_hw_running (struct watchdog_device *wdd) +{ +return test_bit (WDOG_HW_RUNNING , &wdd->status); +} +/* Use the following function to set the nowayout feature */ static inline void watchdog_set_nowayout (struct watchdog_device *wdd,bool nowayout) {
Signature · 1 line
2.5.0
Guenter Roeck
maintainerPATCH v8 4/10 watchdog: Make stop function optional
In reply to Guenter RoeckNot all hardware watchdogs can be stopped. The driver for such watchdogs would typically only set the WATCHDOG_HW_RUNNING flag in its stop function. Make the stop function optional and set WATCHDOG_HW_RUNNING in the watchdog core if it is not provided.
---
v8: Split functionality from patch introducing WATCHDOG_HW_RUNNING
--- Documentation/watchdog/watchdog-kernel-api.txt | 20 ++++++++++++-------- drivers/watchdog/watchdog_core.c | 2 +- drivers/watchdog/watchdog_dev.c | 6 +++++- 3 files changed, 18 insertions(+), 10 deletions(-)
Documentation/watchdog/watchdog-kernel-api.txt +12 -8
@@ -85,7 +85,8 @@ It contains following fields: If set, the infrastructure will send heartbeats to the watchdog driver if 'timeout' is larger than max_hw_heartbeat_ms, unless WDOG_ACTIVE is set and userspace failed to send a heartbeat for at least 'timeout' - seconds. + seconds. max_hw_heartbeat_ms must be set if a driver does not implement + the stop function. * reboot_nb: notifier block that is registered for reboot notifications, for internal use only. If the driver calls watchdog_stop_on_reboot, watchdog core will stop the watchdog on such notifications.
@@ -134,17 +135,20 @@ are: device. The routine needs a pointer to the watchdog timer device structure as a parameter. It returns zero on success or a negative errno code for failure. -* stop: with this routine the watchdog timer device is being stopped. - The routine needs a pointer to the watchdog timer device structure as a - parameter. It returns zero on success or a negative errno code for failure. - Some watchdog timer hardware can only be started and not be stopped. - If a watchdog can not be stopped, the watchdog driver must set the - WDOG_HW_RUNNING flag in its stop function to inform the watchdog core that - the watchdog is still running. Not all watchdog timer hardware supports the same functionality. That's why all other routines/operations are optional. They only need to be provided if they are supported. These optional routines/operations are: +* stop: with this routine the watchdog timer device is being stopped. + The routine needs a pointer to the watchdog timer device structure as a + parameter. It returns zero on success or a negative errno code for failure. + Some watchdog timer hardware can only be started and not be stopped. A + driver supporting such hardware does not have to implement the stop routine. + If a driver has no stop function, the watchdog core will set WDOG_HW_RUNNING + and start calling the driver's keepalive pings function after the watchdog + device is closed. + If a watchdog driver does not implement the stop function, it must set + max_hw_heartbeat_ms. * ping: this is the routine that sends a keepalive ping to the watchdog timer hardware. The routine needs a pointer to the watchdog timer device structure as a
drivers/watchdog/watchdog_core.c +1 -1
@@ -199,7 +199,7 @@ static int __watchdog_register_device(struct watchdog_device *wdd)return -EINVAL ;/* Mandatory operations need to be supported */ -if (wdd->ops->start ==NULL || wdd->ops->stop ==NULL ) +if (!wdd->ops->start || (!wdd->ops->stop && !wdd->max_hw_heartbeat_ms))return -EINVAL ;watchdog_check_min_max_timeout (wdd);
drivers/watchdog/watchdog_dev.c +5 -1
@@ -246,7 +246,11 @@ static int watchdog_stop(struct watchdog_device *wdd)return -EBUSY ; } - err = wdd->ops->stop (wdd); +if (wdd->ops->stop) + err = wdd->ops->stop (wdd); +else +set_bit (WDOG_HW_RUNNING , &wdd->status); +if (err ==0 ) {clear_bit (WDOG_ACTIVE , &wdd->status);watchdog_update_worker (wdd);
Signature · 1 line
2.5.0
Guenter Roeck
maintainerPATCH v8 5/10 watchdog: Add support for minimum time between heartbeats
In reply to Guenter RoeckSome watchdogs require a minimum time between heartbeats. Examples are the watchdogs in DA9062 and AT91SAM9x.
---
v8: Rebased to v4.5-rc5
v7: Rebased to v4.5-rc1
v6: Rebased to v4.4-rc2
v5: Rebased to v4.4-rc1
Fixed typo in documentation.
v4: Added patch
--- Documentation/watchdog/watchdog-kernel-api.txt | 3 +++ drivers/watchdog/watchdog_dev.c | 15 +++++++++++++++ include/linux/watchdog.h | 3 +++ 3 files changed, 21 insertions(+)
Documentation/watchdog/watchdog-kernel-api.txt +3 -0
@@ -52,6 +52,7 @@ struct watchdog_device { unsigned int timeout; unsigned int min_timeout; unsigned int max_timeout; + unsigned int min_hw_heartbeat_ms; unsigned int max_hw_heartbeat_ms; struct notifier_block reboot_nb; struct notifier_block restart_nb;
@@ -81,6 +82,8 @@ It contains following fields: * max_timeout: the watchdog timer's maximum timeout value (in seconds), as seen from userspace. If set, the maximum configurable value for 'timeout'. Not used if max_hw_heartbeat_ms is non-zero. +* min_hw_heartbeat_ms: Minimum time between heartbeats sent to the chip, + in milli-seconds. * max_hw_heartbeat_ms: Maximum hardware heartbeat, in milli-seconds. If set, the infrastructure will send heartbeats to the watchdog driver if 'timeout' is larger than max_hw_heartbeat_ms, unless WDOG_ACTIVE
drivers/watchdog/watchdog_dev.c +15 -0
@@ -64,6 +64,7 @@ struct watchdog_core_data {struct watchdog_device *wdd;struct mutex lock;unsigned long last_keepalive; +unsigned long last_hw_keepalive;struct delayed_work work;unsigned long status;/* Internal status bits */ #define _WDOG_DEV_OPEN 0/* Opened ? */
@@ -137,8 +138,19 @@ static inline void watchdog_update_worker(struct watchdog_device *wdd)static int __watchdog_ping (struct watchdog_device *wdd) { +struct watchdog_core_data *wd_data = wdd->wd_data; +unsigned long earliest_keepalive = wd_data->last_hw_keepalive + +msecs_to_jiffies (wdd->min_hw_heartbeat_ms);int err; +if (time_is_after_jiffies (earliest_keepalive)) { +mod_delayed_work (watchdog_wq, &wd_data->work, + earliest_keepalive - jiffies); +return 0 ; + } + + wd_data->last_hw_keepalive = jiffies; +if (wdd->ops->ping) err = wdd->ops->ping (wdd);/* ping the watchdog */ else
@@ -819,6 +831,9 @@ static int watchdog_cdev_register(struct watchdog_device *wdd, dev_t devno)return err; } +/* Record time of most recent heartbeat as 'just before now'. */ + wd_data->last_hw_keepalive = jiffies -1 ; +/* * If the watchdog is running, prevent its driver from being unloaded, * and schedule an immediate ping.
include/linux/watchdog.h +3 -0
@@ -65,6 +65,8 @@ struct watchdog_ops {* @max_timeout:The watchdog devices maximum timeout value (in seconds) * as configurable from user space. Only relevant if * max_hw_heartbeat_ms is not provided. +* @min_hw_heartbeat_ms: +* Minimum time between heartbeats, in milli-seconds. * @max_hw_heartbeat_ms: * Hardware limit for maximum timeout, in milli-seconds. * Replaces max_timeout if specified.
@@ -95,6 +97,7 @@ struct watchdog_device {unsigned int timeout;unsigned int min_timeout;unsigned int max_timeout; +unsigned int min_hw_heartbeat_ms;unsigned int max_hw_heartbeat_ms;struct notifier_block reboot_nb;struct notifier_block restart_nb;
Signature · 1 line
2.5.0
Guenter Roeck
maintainerPATCH v8 6/10 watchdog: dw_wdt: Convert to use watchdog infrastructure
In reply to Guenter RoeckConvert driver to use watchdog infrastructure. This includes infrastructure support to handle watchdog keepalive if the watchdog is running while the watchdog device is closed.
---
v8: max_hw_timeout_ms -> max_hw_heartbeat_ms
Rebased to v4.5-rc5
v7: Set max_hw_timeout_ms
Rebased to v4.5-rc1
v6: Added patch
---
drivers/watchdog/Kconfig | 1 +
drivers/watchdog/dw_wdt.c | 323 +++++++++++++++++-----------------------------
2 files changed, 117 insertions(+), 207 deletions(-)drivers/watchdog/Kconfig +1 -0
@@ -330,6 +330,7 @@ config SA1100_WATCHDOG config DW_WATCHDOG tristate "Synopsys DesignWare watchdog" depends on HAS_IOMEM + select WATCHDOG_CORE help Say Y here if to include support for the Synopsys DesignWare watchdog timer found in many chips.
drivers/watchdog/dw_wdt.c +116 -207
457 lines not on this page · raw message
Signature · 1 line
2.5.0
Re: [PATCH v8 06/10] watchdog: dw_wdt: Convert to use watchdog infrastructure
Guenter,
Quoted 8 lines · Guenter Roeck
Convert driver to use watchdog infrastructure. This includes infrastructure support to handle watchdog keepalive if the watchdog is running while the watchdog device is closed.
Signed-off-by Guenter Roeck <[email protected]>---
v8:max_hw_timeout_ms->max_hw_heartbeat_ms
Quoted 8 lines
Rebased to v4.5-rc5
v7: Set max_hw_timeout_ms
Rebased to v4.5-rc1
v6: Added patch
---
drivers/watchdog/Kconfig | 1 +
drivers/watchdog/dw_wdt.c | 323 +++++++++++++++++-----------------------------
2 files changed, 117 insertions(+), 207 deletions(-)I re-ran the same set of tests I ran against v7. They all still pass.
Guenter Roeck
maintainerRe: [PATCH v8 06/10] watchdog: dw_wdt: Convert to use watchdog infrastructure
Quoted 20 lines · Doug Anderson
Guenter,
Quoted 8 lines · Guenter Roeck
Convert driver to use watchdog infrastructure. This includes infrastructure support to handle watchdog keepalive if the watchdog is running while the watchdog device is closed.
Signed-off-by Guenter Roeck <[email protected]>---
v8:max_hw_timeout_ms->max_hw_heartbeat_msQuoted 8 lines
Rebased to v4.5-rc5 v7: Set max_hw_timeout_ms Rebased to v4.5-rc1 v6: Added patch --- drivers/watchdog/Kconfig | 1 + drivers/watchdog/dw_wdt.c | 323 +++++++++++++++++----------------------------- 2 files changed, 117 insertions(+), 207 deletions(-)
Quoted 3 lines
I re-ran the same set of tests I ran against v7. They all still pass.
Tested-by Douglas Anderson <[email protected]>
Thanks a lot!
Guenter
Guenter Roeck
maintainerPATCH v8 7/10 watchdog: imx2: Convert to use infrastructure triggered keepalives
In reply to Guenter RoeckThe watchdog infrastructure now supports handling watchdog keepalive if the watchdog is running while the watchdog device is closed. Convert the driver to use this infrastructure.
---
v8: max_hw_timeout_ms -> max_hw_heartbeat_ms
Rebased to v4.5-rc5
v7: Set max_hw_timeout_ms
Rebased to v4.5-rc1
v6: Rename WDOG_RUNNING to WDOG_HW_RUNNING
Rebased to v4.4-rc2v5: Rebased to v4.4-rc1
v4: No changes
v3: No changes
v2: No changes
--- drivers/watchdog/imx2_wdt.c | 74 ++++++++------------------------------------- 1 file changed, 13 insertions(+), 61 deletions(-)
drivers/watchdog/imx2_wdt.c +13 -61
@@ -25,14 +25,12 @@#include <linux/delay.h> #include <linux/init.h> #include <linux/io.h> -#include <linux/jiffies.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/moduleparam.h> #include <linux/of_address.h> #include <linux/platform_device.h> #include <linux/regmap.h> -#include <linux/timer.h> #include <linux/watchdog.h> #define DRIVER_NAME "imx2-wdt"
@@ -60,7 +58,6 @@struct imx2_wdt_device {struct clk *clk;struct regmap *regmap; -struct timer_list timer;/* Pings the watchdog when closed */ struct watchdog_device wdog; };
@@ -146,16 +143,6 @@ static int imx2_wdt_ping(struct watchdog_device *wdog)return 0 ; } -static void imx2_wdt_timer_ping (unsigned long arg) -{ -struct watchdog_device *wdog = (struct watchdog_device *)arg; -struct imx2_wdt_device *wdev =watchdog_get_drvdata (wdog); - -/* ping it every wdog->timeout / 2 seconds to prevent reboot */ -imx2_wdt_ping (wdog); -mod_timer (&wdev->timer, jiffies + wdog->timeout *HZ /2 ); -} -static int imx2_wdt_set_timeout (struct watchdog_device *wdog,unsigned int new_timeout) {
@@ -172,40 +159,19 @@ static int imx2_wdt_start(struct watchdog_device *wdog) {struct imx2_wdt_device *wdev =watchdog_get_drvdata (wdog); -if (imx2_wdt_is_running (wdev)) { -/* delete the timer that pings the watchdog after close */ -del_timer_sync (&wdev->timer); +if (imx2_wdt_is_running (wdev))imx2_wdt_set_timeout (wdog, wdog->timeout); - }else +else imx2_wdt_setup (wdog); -return imx2_wdt_ping (wdog); -} - -static int imx2_wdt_stop (struct watchdog_device *wdog) -{ -/* -* We don't need a clk_disable, it cannot be disabled once started. -* We use a timer to ping the watchdog while /dev/watchdog is closed -*/ -imx2_wdt_timer_ping ((unsigned long )wdog); -return 0 ; -} - -static inline void imx2_wdt_ping_if_active (struct watchdog_device *wdog) -{ -struct imx2_wdt_device *wdev =watchdog_get_drvdata (wdog); +set_bit (WDOG_HW_RUNNING , &wdog->status); -if (imx2_wdt_is_running (wdev)) { -imx2_wdt_set_timeout (wdog, wdog->timeout); -imx2_wdt_timer_ping ((unsigned long )wdog); - } +return imx2_wdt_ping (wdog); }static const struct watchdog_ops imx2_wdt_ops = { .owner =THIS_MODULE , .start = imx2_wdt_start, - .stop = imx2_wdt_stop, .ping = imx2_wdt_ping, .set_timeout = imx2_wdt_set_timeout, .restart = imx2_wdt_restart,
@@ -253,7 +219,7 @@ static int __init imx2_wdt_probe(struct platform_device *pdev) wdog->info = &imx2_wdt_info; wdog->ops = &imx2_wdt_ops; wdog->min_timeout =1 ; - wdog->max_timeout =IMX2_WDT_MAX_TIME ; + wdog->max_hw_heartbeat_ms =IMX2_WDT_MAX_TIME *1000 ; wdog->parent = &pdev->dev; ret =clk_prepare_enable (wdev->clk);
@@ -274,9 +240,10 @@ static int __init imx2_wdt_probe(struct platform_device *pdev)watchdog_set_restart_priority (wdog,128 );watchdog_init_timeout (wdog, timeout, &pdev->dev); -setup_timer (&wdev->timer, imx2_wdt_timer_ping, (unsigned long )wdog); - -imx2_wdt_ping_if_active (wdog); +if (imx2_wdt_is_running (wdev)) { +imx2_wdt_set_timeout (wdog, wdog->timeout); +set_bit (WDOG_HW_RUNNING , &wdog->status); + }/* * Disable the watchdog power down counter at boot. Otherwise the power
@@ -309,7 +276,6 @@ static int __exit imx2_wdt_remove(struct platform_device *pdev)watchdog_unregister_device (wdog);if (imx2_wdt_is_running (wdev)) { -del_timer_sync (&wdev->timer);imx2_wdt_ping (wdog);dev_crit (&pdev->dev,"Device removed: Expect reboot!\n" ); }
@@ -323,10 +289,9 @@ static void imx2_wdt_shutdown(struct platform_device *pdev)if (imx2_wdt_is_running (wdev)) {/* -* We are running, we need to delete the timer but will -* give max timeout before reboot will take place +* We are running, configure max timeout before reboot +* will take place. */ -del_timer_sync (&wdev->timer);imx2_wdt_set_timeout (wdog,IMX2_WDT_MAX_TIME );imx2_wdt_ping (wdog);dev_crit (&pdev->dev,"Device shutdown: Expect reboot!\n" );
@@ -344,10 +309,6 @@ static int imx2_wdt_suspend(struct device *dev)if (imx2_wdt_is_running (wdev)) {imx2_wdt_set_timeout (wdog,IMX2_WDT_MAX_TIME );imx2_wdt_ping (wdog); - -/* The watchdog is not active */ -if (!watchdog_active (wdog)) -del_timer_sync (&wdev->timer); }clk_disable_unprepare (wdev->clk);
@@ -373,19 +334,10 @@ static int imx2_wdt_resume(struct device *dev)* watchdog again. */ imx2_wdt_setup (wdog); + } +if (imx2_wdt_is_running (wdev)) {imx2_wdt_set_timeout (wdog, wdog->timeout);imx2_wdt_ping (wdog); - }else if (imx2_wdt_is_running (wdev)) { -/* Resuming from non-deep sleep state. */ -imx2_wdt_set_timeout (wdog, wdog->timeout); -imx2_wdt_ping (wdog); -/* -* But the watchdog is not active, then start -* the timer again. -*/ -if (!watchdog_active (wdog)) -mod_timer (&wdev->timer, - jiffies + wdog->timeout *HZ /2 ); }return 0 ;
Signature · 1 line
2.5.0
Guenter Roeck
maintainerRFC PATCH v8 8/10 watchdog: retu: Convert to use infrastructure triggered keepalives
In reply to Guenter RoeckThe watchdog infrastructure now supports handling watchdog keepalive if the watchdog is running while the watchdog device is closed. Convert the driver to use this infrastructure.
---
v8: max_hw_timeout_ms -> max_hw_heartbeat_ms
Rebased to v4.5-rc5
v7: Set max_hw_timeout_ms
Rebased to v4.5-rc1
v6: Rename WDOG_RUNNING to WDOG_HW_RUNNING
Rebased to v4.4-rc2v5: Rebased to v4.4-rc1
v4: No changes
v3: No changes
v2: No changes
--- drivers/watchdog/retu_wdt.c | 80 +++++---------------------------------------- 1 file changed, 8 insertions(+), 72 deletions(-)
drivers/watchdog/retu_wdt.c +8 -72
@@ -28,69 +28,22 @@/* Watchdog timer values in seconds */ #define RETU_WDT_MAX_TIMER 63 -struct retu_wdt_dev { -struct retu_dev *rdev; -struct device *dev; -struct delayed_work ping_work; -}; - -/* -* Since Retu watchdog cannot be disabled in hardware, we must kick it -* with a timer until userspace watchdog software takes over. If -* CONFIG_WATCHDOG_NOWAYOUT is set, we never start the feeding. -*/ -static void retu_wdt_ping_enable (struct retu_wdt_dev *wdev) -{ -retu_write (wdev->rdev,RETU_REG_WATCHDOG ,RETU_WDT_MAX_TIMER ); -schedule_delayed_work (&wdev->ping_work, -round_jiffies_relative (RETU_WDT_MAX_TIMER *HZ /2 )); -} - -static void retu_wdt_ping_disable (struct retu_wdt_dev *wdev) -{ -retu_write (wdev->rdev,RETU_REG_WATCHDOG ,RETU_WDT_MAX_TIMER ); -cancel_delayed_work_sync (&wdev->ping_work); -} - -static void retu_wdt_ping_work (struct work_struct *work) -{ -struct retu_wdt_dev *wdev =container_of (to_delayed_work (work), - struct retu_wdt_dev, ping_work); -retu_wdt_ping_enable (wdev); -} -static int retu_wdt_start (struct watchdog_device *wdog) { -struct retu_wdt_dev *wdev =watchdog_get_drvdata (wdog); +struct retu_dev *rdev =watchdog_get_drvdata (wdog); -retu_wdt_ping_disable (wdev); +set_bit (WDOG_HW_RUNNING , &wdog->status); -return retu_write (wdev->rdev,RETU_REG_WATCHDOG , wdog->timeout); -} - -static int retu_wdt_stop (struct watchdog_device *wdog) -{ -struct retu_wdt_dev *wdev =watchdog_get_drvdata (wdog); - -retu_wdt_ping_enable (wdev); - -return 0 ; -} - -static int retu_wdt_ping (struct watchdog_device *wdog) -{ -struct retu_wdt_dev *wdev =watchdog_get_drvdata (wdog); - -return retu_write (wdev->rdev,RETU_REG_WATCHDOG , wdog->timeout); +return retu_write (rdev,RETU_REG_WATCHDOG , wdog->timeout); }static int retu_wdt_set_timeout (struct watchdog_device *wdog,unsigned int timeout) { -struct retu_wdt_dev *wdev =watchdog_get_drvdata (wdog); +struct retu_dev *rdev =watchdog_get_drvdata (wdog); wdog->timeout = timeout; -return retu_write (wdev->rdev,RETU_REG_WATCHDOG , wdog->timeout); +return retu_write (rdev,RETU_REG_WATCHDOG , wdog->timeout); }static const struct watchdog_info retu_wdt_info = {
@@ -101,8 +54,6 @@ static const struct watchdog_info retu_wdt_info = {static const struct watchdog_ops retu_wdt_ops = { .owner =THIS_MODULE , .start = retu_wdt_start, - .stop = retu_wdt_stop, - .ping = retu_wdt_ping, .set_timeout = retu_wdt_set_timeout, };
@@ -111,40 +62,27 @@ static int retu_wdt_probe(struct platform_device *pdev)struct retu_dev *rdev =dev_get_drvdata (pdev->dev.parent);bool nowayout =WATCHDOG_NOWAYOUT ;struct watchdog_device *retu_wdt; -struct retu_wdt_dev *wdev;int ret; retu_wdt =devm_kzalloc (&pdev->dev,sizeof (*retu_wdt),GFP_KERNEL );if (!retu_wdt)return -ENOMEM ; - wdev =devm_kzalloc (&pdev->dev,sizeof (*wdev),GFP_KERNEL ); -if (!wdev) -return -ENOMEM ; - retu_wdt->info = &retu_wdt_info; retu_wdt->ops = &retu_wdt_ops; retu_wdt->timeout =RETU_WDT_MAX_TIMER ; retu_wdt->min_timeout =0 ; - retu_wdt->max_timeout =RETU_WDT_MAX_TIMER ; + retu_wdt->max_hw_heartbeat_ms =RETU_WDT_MAX_TIMER *1000 ; retu_wdt->parent = &pdev->dev; -watchdog_set_drvdata (retu_wdt, wdev); +watchdog_set_drvdata (retu_wdt, rdev);watchdog_set_nowayout (retu_wdt, nowayout); - wdev->rdev = rdev; - wdev->dev = &pdev->dev; - -INIT_DELAYED_WORK (&wdev->ping_work, retu_wdt_ping_work); - ret =watchdog_register_device (retu_wdt);if (ret <0 )return ret; -if (nowayout) -retu_wdt_ping (retu_wdt); -else -retu_wdt_ping_enable (wdev); +retu_wdt_start (retu_wdt);platform_set_drvdata (pdev, retu_wdt);
@@ -154,10 +92,8 @@ static int retu_wdt_probe(struct platform_device *pdev)static int retu_wdt_remove (struct platform_device *pdev) {struct watchdog_device *wdog =platform_get_drvdata (pdev); -struct retu_wdt_dev *wdev =watchdog_get_drvdata (wdog);watchdog_unregister_device (wdog); -cancel_delayed_work_sync (&wdev->ping_work);return 0 ; }
Signature · 1 line
2.5.0
Guenter Roeck
maintainerRFC PATCH v8 9/10 watchdog: at91sam9: Convert to use infrastructure triggered keepalives
In reply to Guenter RoeckThe watchdog infrastructure now supports handling watchdog keepalive if the watchdog is running while the watchdog device is closed. The infrastructure now also supports generating additional heartbeats if the maximum hardware timeout is smaller than or close to the configured timeout. Convert the driver to use this infrastructure.
---
v8: max_hw_timeout_ms -> max_hw_heartbeat_ms
Rebased to v4.5-rc5
v7: Rebased to v4.5-rc1
v6: Rebased to v4.4-rc2
v5: Rebased to v4.4-rc1
v4: No changes
v3: No changes
v2: No changes
--- drivers/watchdog/at91sam9_wdt.c | 103 +++++----------------------------------- 1 file changed, 12 insertions(+), 91 deletions(-)
drivers/watchdog/at91sam9_wdt.c +12 -91
@@ -30,7 +30,6 @@#include <linux/types.h> #include <linux/watchdog.h> #include <linux/jiffies.h> -#include <linux/timer.h> #include <linux/bitops.h> #include <linux/uaccess.h> #include <linux/of.h>
@@ -49,8 +48,8 @@* use this to convert a watchdog * value from/to milliseconds. */ -#define ticks_to_hz_rounddown (t) ((((t) + 1) * HZ) >> 8) -#define ticks_to_hz_roundup (t) (((((t) + 1) * HZ) + 255) >> 8) +#define ticks_to_ms_rounddown (t) ((((t) + 1) * 1000) >> 8) +#define ticks_to_ms_roundup (t) (((((t) + 1) * 1000) + 255) >> 8)#define ticks_to_secs (t) (((t) + 1) >> 8)#define secs_to_ticks (s) ((s) ? (((s) << 8) - 1) : 0)
@@ -65,9 +64,6 @@/* Hardware timeout in seconds */ #define WDT_HW_TIMEOUT 2 -/* Timer heartbeat (500ms) */ -#define WDT_TIMEOUT (HZ/2) -/* User land timeout */ #define WDT_HEARTBEAT 15static int heartbeat;
@@ -84,11 +80,8 @@ MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started "struct at91wdt {struct watchdog_device wdd;void __iomem *base; -unsigned long next_heartbeat;/* the next_heartbeat for the timer */ -struct timer_list timer;/* The timer that pings the watchdog */ u32 mr;u32 mr_mask; -unsigned long heartbeat;/* WDT heartbeat in jiffies */ bool nowayout;unsigned int irq;struct clk *sclk;
@@ -109,47 +102,13 @@ static irqreturn_t wdt_interrupt(int irq, void *dev_id)return IRQ_HANDLED ; } -/* -* Reload the watchdog timer. (ie, pat the watchdog) -*/ -static inline void at91_wdt_reset (struct at91wdt *wdt) -{ -wdt_write (wdt,AT91_WDT_CR ,AT91_WDT_KEY |AT91_WDT_WDRSTT ); -} - -/* -* Timer tick -*/ -static void at91_ping (unsigned long data) -{ -struct at91wdt *wdt = (struct at91wdt *)data; -if (time_before (jiffies, wdt->next_heartbeat) || - !watchdog_active (&wdt->wdd)) { -at91_wdt_reset (wdt); -mod_timer (&wdt->timer, jiffies + wdt->heartbeat); - }else { -pr_crit ("I will reset your machine !\n" ); - } -} -static int at91_wdt_start (struct watchdog_device *wdd) {struct at91wdt *wdt =to_wdt (wdd); -/* calculate when the next userspace timeout will be */ - wdt->next_heartbeat = jiffies + wdd->timeout *HZ ; -return 0 ; -} -static int at91_wdt_stop (struct watchdog_device *wdd) -{ -/* The watchdog timer hardware can not be stopped... */ -return 0 ; -} +wdt_write (wdt,AT91_WDT_CR ,AT91_WDT_KEY |AT91_WDT_WDRSTT ); -static int at91_wdt_set_timeout (struct watchdog_device *wdd,unsigned int new_timeout) -{ - wdd->timeout = new_timeout; -return at91_wdt_start (wdd); +return 0 ; }static int at91_wdt_init (struct platform_device *pdev,struct at91wdt *wdt)
@@ -159,8 +118,8 @@ static int at91_wdt_init(struct platform_device *pdev, struct at91wdt *wdt)u32 value;int err;u32 mask = wdt->mr_mask; -unsigned long min_heartbeat =1 ; -unsigned long max_heartbeat; +unsigned int min_heartbeat =jiffies_to_msecs (1 ); +unsigned int hw_heartbeat;struct device *dev = &pdev->dev; tmp =wdt_read (wdt,AT91_WDT_MR );
@@ -182,31 +141,15 @@ static int at91_wdt_init(struct platform_device *pdev, struct at91wdt *wdt) delta = (tmp &AT91_WDT_WDD ) >>16 ;if (delta < value) - min_heartbeat =ticks_to_hz_roundup (value - delta); + min_heartbeat =ticks_to_ms_roundup (value - delta); - max_heartbeat =ticks_to_hz_rounddown (value); -if (!max_heartbeat) { + hw_heartbeat =ticks_to_ms_rounddown (value); +if (hw_heartbeat < min_heartbeat *2 ) {dev_err (dev,"heartbeat is too small for the system to handle it correctly\n" );return -EINVAL ; } - -/* -* Try to reset the watchdog counter 4 or 2 times more often than -* actually requested, to avoid spurious watchdog reset. -* If this is not possible because of the min_heartbeat value, reset -* it at the min_heartbeat period. -*/ -if ((max_heartbeat /4 ) >= min_heartbeat) - wdt->heartbeat = max_heartbeat /4 ; -else if ((max_heartbeat /2 ) >= min_heartbeat) - wdt->heartbeat = max_heartbeat /2 ; -else - wdt->heartbeat = min_heartbeat; - -if (max_heartbeat < min_heartbeat +4 ) -dev_warn (dev, -"min heartbeat and max heartbeat might be too close for the system to handle it correctly\n" ); + wdt->wdd.max_hw_heartbeat_ms = hw_heartbeat;if ((tmp &AT91_WDT_WDFIEN ) && wdt->irq) { err = request_irq(wdt->irq, wdt_interrupt,
@@ -222,32 +165,12 @@ static int at91_wdt_init(struct platform_device *pdev, struct at91wdt *wdt)"watchdog already configured differently (mr = %x expecting %x)\n" , tmp & wdt->mr_mask, wdt->mr & wdt->mr_mask); -setup_timer (&wdt->timer, at91_ping, (unsigned long )wdt); - -/* -* Use min_heartbeat the first time to avoid spurious watchdog reset: -* we don't know for how long the watchdog counter is running, and -* - resetting it right now might trigger a watchdog fault reset -* - waiting for heartbeat time might lead to a watchdog timeout -* reset -*/ -mod_timer (&wdt->timer, jiffies + min_heartbeat); -/* Try to set timeout from device tree first */ if (watchdog_init_timeout (&wdt->wdd,0 , dev))watchdog_init_timeout (&wdt->wdd, heartbeat, dev);watchdog_set_nowayout (&wdt->wdd, wdt->nowayout); - err =watchdog_register_device (&wdt->wdd); -if (err) - goto out_stop_timer; - - wdt->next_heartbeat = jiffies + wdt->wdd.timeout *HZ ; -return 0 ; - -out_stop_timer: -del_timer (&wdt->timer); -return err; +return watchdog_register_device (&wdt->wdd); }/* ......................................................................... */
@@ -261,8 +184,6 @@ static const struct watchdog_info at91_wdt_info = {static const struct watchdog_ops at91_wdt_ops = { .owner =THIS_MODULE , .start = at91_wdt_start, - .stop = at91_wdt_stop, - .set_timeout = at91_wdt_set_timeout, };#if defined(CONFIG_OF )
@@ -393,7 +314,7 @@ static int __exit at91wdt_remove(struct platform_device *pdev)watchdog_unregister_device (&wdt->wdd);pr_warn ("I quit now, hardware will probably reboot!\n" ); -del_timer (&wdt->timer); +clk_disable_unprepare (wdt->sclk);return 0 ;
Signature · 1 line
2.5.0
Guenter Roeck
maintainerRFC PATCH v8 10/10 watchdog: gpio: Convert to use infrastructure triggered keepalives
In reply to Guenter RoeckThe watchdog infrastructure now supports handling watchdog keepalive if the watchdog is running while the watchdog device is closed. The infrastructure now also supports generating additional heartbeats if the maximum hardware timeout is smaller than or close to the configured timeout. Convert the driver to use this infrastructure.
---
v8: Added patch to series
--- drivers/watchdog/gpio_wdt.c | 73 ++++++++------------------------------------- 1 file changed, 13 insertions(+), 60 deletions(-)
drivers/watchdog/gpio_wdt.c +13 -60
@@ -18,7 +18,6 @@#define SOFT_TIMEOUT_MIN 1#define SOFT_TIMEOUT_DEF 60 -#define SOFT_TIMEOUT_MAX 0xffffenum {HW_ALGO_TOGGLE ,
@@ -30,11 +29,7 @@ struct gpio_wdt_priv {bool active_low;bool state;bool always_running; -bool armed;unsigned int hw_algo; -unsigned int hw_margin; -unsigned long last_jiffies; -struct timer_list timer;struct watchdog_device wdd; };
@@ -47,21 +42,10 @@ static void gpio_wdt_disable(struct gpio_wdt_priv *priv)gpio_direction_input (priv->gpio); } -static void gpio_wdt_hwping (unsigned long data) +static int gpio_wdt_ping (struct watchdog_device *wdd) { -struct watchdog_device *wdd = (struct watchdog_device *)data;struct gpio_wdt_priv *priv =watchdog_get_drvdata (wdd); -if (priv->armed &&time_after (jiffies, priv->last_jiffies + -msecs_to_jiffies (wdd->timeout *1000 ))) { -dev_crit (wdd->parent, -"Timer expired. System will reboot soon!\n" ); -return ; - } - -/* Restart timer */ -mod_timer (&priv->timer, jiffies + priv->hw_margin); -switch (priv->hw_algo) {case HW_ALGO_TOGGLE :/* Toggle output pin */
@@ -75,55 +59,33 @@ static void gpio_wdt_hwping(unsigned long data)gpio_set_value_cansleep (priv->gpio, priv->active_low);break ; } -} - -static void gpio_wdt_start_impl (struct gpio_wdt_priv *priv) -{ - priv->state = priv->active_low; -gpio_direction_output (priv->gpio, priv->state); - priv->last_jiffies = jiffies; -gpio_wdt_hwping ((unsigned long )&priv->wdd); +return 0 ; }static int gpio_wdt_start (struct watchdog_device *wdd) {struct gpio_wdt_priv *priv =watchdog_get_drvdata (wdd); -gpio_wdt_start_impl (priv); - priv->armed = true; + priv->state = priv->active_low; +gpio_direction_output (priv->gpio, priv->state); -return 0 ; +set_bit (WDOG_HW_RUNNING , &wdd->status); + +return gpio_wdt_ping (wdd); }static int gpio_wdt_stop (struct watchdog_device *wdd) {struct gpio_wdt_priv *priv =watchdog_get_drvdata (wdd); - priv->armed = false;if (!priv->always_running) { -mod_timer (&priv->timer,0 );gpio_wdt_disable (priv); +clear_bit (WDOG_HW_RUNNING , &wdd->status); }return 0 ; } -static int gpio_wdt_ping (struct watchdog_device *wdd) -{ -struct gpio_wdt_priv *priv =watchdog_get_drvdata (wdd); - - priv->last_jiffies = jiffies; - -return 0 ; -} - -static int gpio_wdt_set_timeout (struct watchdog_device *wdd,unsigned int t) -{ - wdd->timeout = t; - -return gpio_wdt_ping (wdd); -} -static const struct watchdog_info gpio_wdt_ident = { .options =WDIOF_MAGICCLOSE |WDIOF_KEEPALIVEPING |WDIOF_SETTIMEOUT ,
@@ -135,7 +97,6 @@ static const struct watchdog_ops gpio_wdt_ops = { .start = gpio_wdt_start, .stop = gpio_wdt_stop, .ping = gpio_wdt_ping, - .set_timeout = gpio_wdt_set_timeout, };static int gpio_wdt_probe (struct platform_device *pdev)
@@ -183,9 +144,6 @@ static int gpio_wdt_probe(struct platform_device *pdev)if (hw_margin <2 || hw_margin >65535 )return -EINVAL ; -/* Use safe value (1/2 of real timeout) */ - priv->hw_margin =msecs_to_jiffies (hw_margin /2 ); - priv->always_running =of_property_read_bool (pdev->dev.of_node,"always-running" );
@@ -194,31 +152,26 @@ static int gpio_wdt_probe(struct platform_device *pdev) priv->wdd.info = &gpio_wdt_ident; priv->wdd.ops = &gpio_wdt_ops; priv->wdd.min_timeout =SOFT_TIMEOUT_MIN ; - priv->wdd.max_timeout =SOFT_TIMEOUT_MAX ; + priv->wdd.max_hw_heartbeat_ms = hw_margin; priv->wdd.parent = &pdev->dev;if (watchdog_init_timeout (&priv->wdd,0 , &pdev->dev) <0 ) priv->wdd.timeout =SOFT_TIMEOUT_DEF ; -setup_timer (&priv->timer, gpio_wdt_hwping, (unsigned long )&priv->wdd); -watchdog_stop_on_reboot (&priv->wdd); - ret =watchdog_register_device (&priv->wdd); -if (ret) -return ret; -if (priv->always_running) -gpio_wdt_start_impl (priv); +gpio_wdt_start (&priv->wdd); -return 0 ; + ret =watchdog_register_device (&priv->wdd); + +return ret; }static int gpio_wdt_remove (struct platform_device *pdev) {struct gpio_wdt_priv *priv =platform_get_drvdata (pdev); -del_timer_sync (&priv->timer);watchdog_unregister_device (&priv->wdd);return 0 ;
Signature · 1 line
2.5.0
Hi Guenter,
Quoted 121 lines
The watchdog infrastructure is currently purely passive, meaning it only passes information from user space to drivers and vice versa.
Since watchdog hardware tends to have its own quirks, this can result in quite complex watchdog drivers. A number of scanarios are especially common.
- A watchdog is always active and can not be disabled, or can not be disabled once enabled. To support such hardware, watchdog drivers have to implement their own timers and use those timers to trigger watchdog keepalives while the watchdog device is not or not yet opened.
- A variant of this is the desire to enable a watchdog as soon as its driver has been instantiated, to protect the system while it is still booting up, but the watchdog daemon is not yet running.
- Some watchdogs have a very short maximum timeout, in the range of just a few seconds. Such low timeouts are difficult if not impossible to support from user space. Drivers supporting such watchdog hardware need to implement a timer function to augment heartbeats from user space.This patch set solves the above problems while keeping changes to the watchdog core minimal.
- A new status flag,
WDOG_HW_RUNNING, informs the watchdog subsystem that a watchdog is running, and that the watchdog subsystem needs to generate heartbeat requests while the associated watchdog device is closed.
- A new parameter in the watchdog data structure,max_hw_heartbeat_ms, informs the watchdog subsystem about a maximum hardware heartbeat. The watchdog subsystem uses this information together with the configured timeout and the maximum permitted timeout to determine if it needs to generate additional heartbeat requests.As part of this patchset, the semantics of the 'timeout' variable and of the
WDOG_ACTIVEflag are changed slightly.Per the current watchdog kernel API, the 'timeout' variable is supposed to reflect the actual hardware watchdog timeout.
WDOG_ACTIVEis supposed to reflect if the hardware watchdog is running or not.Unfortunately, this does not always reflect reality. In drivers which solve the above mentioned problems internally, 'timeout' is the watchdog timeout as seen from user space, and
WDOG_ACTIVEreflects that user space is expected to send keepalive requests to the watchdog driver.After this patch set is applied, this so far inofficial interpretation is the 'official' semantics for the timeout variable and the
WDOG_ACTIVEflag. In other words, both values no longer reflect the hardware watchdog status, but its status as seen from user space.Patch #1 makes the
set_timeoutfunction optional.Patch #2 adds timer functionality to the watchdog core. It solves the problem of short maximum hardware heartbeats by augmenting heartbeats triggered from user space with internally triggered heartbeats.
Patch #3 adds functionality to generate heartbeats while the watchdog device is closed. It handles situation where where the watchdog is running after the driver has been instantiated, but the device is not yet opened, and post-close situations necessary if a watchdog can not be stopped.
Patch #4 makes the stop function optional.
Patch #5 adds code to ensure that the minimum time between heartbeats meets constraints provided by the watchdog driver.
Patch #6 to #10 are example conversions of some watchdog drivers. Out of those, patches #6 (
dw_wdt) and #7 (imx2) have been tested. Patches #8 to #10 will require testing and are marked as RFT.The patch series is also available in branch watchdog-timer of
git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git.
The series is curently based on top of v4.5-rc5.The patch series was inspired by an earlier patch set from Timo Kokonnen.
v8:
-max_hw_timeout_ms->max_hw_heartbeat_ms
- Rebased to v4.5-rc5
- The patch to make theset_timeoutfunction optional is now the first patch of the series.
- Separate code making the stop function optional into a separate patch.
- Dropped all Acked-by: and Tested-by: tags. Even though there are no functional changes, I concluded that the changes are too substantial to warrant maintaining previously provided tags.
Note: while the patch series was rearranged, correctness was ensured by implementing themax_hw_timeout_ms->max_hw_heartbeat_mschanges separately on top of the original series and comparing the results.
v7:
- Rebased to v4.5-rc1.
- Moved new variables to local data structure
- Fixed typos in documentation (hw_max_timeout_ms->max_hw_timeout_ms).
- Enforce thatmax_hw_timeout_msmust be set if the stop function is not implemented by a driver.
- Setmax_hw_timeout_msin converted drivers.
v6:
- Added patch #9, converting thedw_wdtdriver.
- Setlast_keepalivemore accurately when starting the watchdog.
- Rebased to v4.4-rc2.
- RenamedWDOG_RUNNINGtoWDOG_HW_RUNNINGandwatchdog_running()towatchdog_hw_running().
v5:
- Patches #1 and #2 of the original patch series are now in mainline and have been dropped.
- Rebased to v4.4-rc1.
- Added patch to simplifywatchdog_update_worker().
v4:
- Rebased to v4.3-rc3
- Rearranged patch sequence
- Dropped gpio driver patch. The driver was changed since v4.2, and merging the changes turned out to be too difficult.
- Various other cleanups as listed in individual patches
v3:
- Rebased to v4.2-rc8
- Reworked and cleaned up some of the functions.
- No longer call the worker update function if all that is needed is to stop the worker.
-max_timeoutwill now be ignored ifmax_hw_timeout_msis provided.
- Added patch 9/9.
v2:
- Rebased to v4.2-rc5
- Improved and hopefully clarified documentation.
- Rearranged variables in structwatchdog_devicesuch that internal variables come last.
Quoted 8 lines
- The code now ensures that the watchdog times out <timeout> seconds after the most recent keepalive sent from user space.
- The internal keepalive now stops silently and no longer generates a warning message. Reason is that it will now stop early, while there may still be a substantial amount of time for keepalives from user space to arrive. If such keepalives arrive late (for example if user space is configured to send keepalives just a few seconds before the watchdog times out), the message would just be noise and not provide any value.
Patches 1 till 7 of this series has been added to linux-watchdog-next.
Kind regards,
Wim.
Guenter Roeck
maintainerHi Wim,
Quoted 5 lines · Wim Van Sebroeck
Hi Guenter,
Quoted 3 lines
The watchdog infrastructure is currently purely passive, meaning it only passes information from user space to drivers and vice versa.
[ ... ]
Quoted 3 lines
Patches 1 till 7 of this series has been added to linux-watchdog-next.
Thanks a lot!
Guenter
Hi Guenter,
Quoted 5 lines
Hi Wim,
Quoted 2 lines · Wim Van Sebroeck
Hi Guenter,
Quoted 8 lines
Quoted 3 lines
Quoted 3 lines
The watchdog infrastructure is currently purely passive, meaning it only passes information from user space to drivers and vice versa.
[ ... ]
Quoted 3 lines
Patches 1 till 7 of this series has been added to linux-watchdog-next.
Thanks a lot!
Did you receive any feedback/test-results on patches 8 till 10 ?
Kind regards,
Wim.
Guenter Roeck
maintainerQuoted 10 lines · Wim Van Sebroeck
Hi Guenter,
Quoted 8 lines
Hi Wim,
Quoted 5 lines · Wim Van Sebroeck
Hi Guenter,
Quoted 3 lines
The watchdog infrastructure is currently purely passive, meaning it only passes information from user space to drivers and vice versa.
Quoted 8 lines
Quoted 5 lines
[ ... ]
Quoted 3 lines
Patches 1 till 7 of this series has been added to linux-watchdog-next.
Thanks a lot!
Did you receive any feedback/test-results on patches 8 till 10 ?
No, unfortunately not. I think I'll ping some of those involved in the driver development, but I'll do it after the -rc is out. No need to rush with those now.
Thanks,
Guenter
Guenter Roeck
maintainerHi Wim,
any chance to get this series into the next release ?
Thanks,
Guenter
Quoted 134 lines · Guenter Roeck
The watchdog infrastructure is currently purely passive, meaning it only passes information from user space to drivers and vice versa.
Since watchdog hardware tends to have its own quirks, this can result in quite complex watchdog drivers. A number of scanarios are especially common.
- A watchdog is always active and can not be disabled, or can not be disabled once enabled. To support such hardware, watchdog drivers have to implement their own timers and use those timers to trigger watchdog keepalives while the watchdog device is not or not yet opened.
- A variant of this is the desire to enable a watchdog as soon as its driver has been instantiated, to protect the system while it is still booting up, but the watchdog daemon is not yet running.
- Some watchdogs have a very short maximum timeout, in the range of just a few seconds. Such low timeouts are difficult if not impossible to support from user space. Drivers supporting such watchdog hardware need to implement a timer function to augment heartbeats from user space.This patch set solves the above problems while keeping changes to the watchdog core minimal.
- A new status flag,
WDOG_HW_RUNNING, informs the watchdog subsystem that a watchdog is running, and that the watchdog subsystem needs to generate heartbeat requests while the associated watchdog device is closed.
- A new parameter in the watchdog data structure,max_hw_heartbeat_ms, informs the watchdog subsystem about a maximum hardware heartbeat. The watchdog subsystem uses this information together with the configured timeout and the maximum permitted timeout to determine if it needs to generate additional heartbeat requests.As part of this patchset, the semantics of the 'timeout' variable and of the
WDOG_ACTIVEflag are changed slightly.Per the current watchdog kernel API, the 'timeout' variable is supposed to reflect the actual hardware watchdog timeout.
WDOG_ACTIVEis supposed to reflect if the hardware watchdog is running or not.Unfortunately, this does not always reflect reality. In drivers which solve the above mentioned problems internally, 'timeout' is the watchdog timeout as seen from user space, and
WDOG_ACTIVEreflects that user space is expected to send keepalive requests to the watchdog driver.After this patch set is applied, this so far inofficial interpretation is the 'official' semantics for the timeout variable and the
WDOG_ACTIVEflag. In other words, both values no longer reflect the hardware watchdog status, but its status as seen from user space.Patch #1 makes the
set_timeoutfunction optional.Patch #2 adds timer functionality to the watchdog core. It solves the problem of short maximum hardware heartbeats by augmenting heartbeats triggered from user space with internally triggered heartbeats.
Patch #3 adds functionality to generate heartbeats while the watchdog device is closed. It handles situation where where the watchdog is running after the driver has been instantiated, but the device is not yet opened, and post-close situations necessary if a watchdog can not be stopped.
Patch #4 makes the stop function optional.
Patch #5 adds code to ensure that the minimum time between heartbeats meets constraints provided by the watchdog driver.
Patch #6 to #10 are example conversions of some watchdog drivers. Out of those, patches #6 (
dw_wdt) and #7 (imx2) have been tested. Patches #8 to #10 will require testing and are marked as RFT.The patch series is also available in branch watchdog-timer of
git://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git.
The series is curently based on top of v4.5-rc5.The patch series was inspired by an earlier patch set from Timo Kokonnen.
v8:
-max_hw_timeout_ms->max_hw_heartbeat_ms
- Rebased to v4.5-rc5
- The patch to make theset_timeoutfunction optional is now the first patch of the series.
- Separate code making the stop function optional into a separate patch.
- Dropped all Acked-by: and Tested-by: tags. Even though there are no functional changes, I concluded that the changes are too substantial to warrant maintaining previously provided tags.
Note: while the patch series was rearranged, correctness was ensured by implementing themax_hw_timeout_ms->max_hw_heartbeat_mschanges separately on top of the original series and comparing the results.
v7:
- Rebased to v4.5-rc1.
- Moved new variables to local data structure
- Fixed typos in documentation (hw_max_timeout_ms->max_hw_timeout_ms).
- Enforce thatmax_hw_timeout_msmust be set if the stop function is not implemented by a driver.
- Setmax_hw_timeout_msin converted drivers.
v6:
- Added patch #9, converting thedw_wdtdriver.
- Setlast_keepalivemore accurately when starting the watchdog.
- Rebased to v4.4-rc2.
- RenamedWDOG_RUNNINGtoWDOG_HW_RUNNINGandwatchdog_running()towatchdog_hw_running().
v5:
- Patches #1 and #2 of the original patch series are now in mainline and have been dropped.
- Rebased to v4.4-rc1.
- Added patch to simplifywatchdog_update_worker().
v4:
- Rebased to v4.3-rc3
- Rearranged patch sequence
- Dropped gpio driver patch. The driver was changed since v4.2, and merging the changes turned out to be too difficult.
- Various other cleanups as listed in individual patches
v3:
- Rebased to v4.2-rc8
- Reworked and cleaned up some of the functions.
- No longer call the worker update function if all that is needed is to stop the worker.
-max_timeoutwill now be ignored ifmax_hw_timeout_msis provided.
- Added patch 9/9.
v2:
- Rebased to v4.2-rc5
- Improved and hopefully clarified documentation.
- Rearranged variables in structwatchdog_devicesuch that internal variables come last.
- The code now ensures that the watchdog times out <timeout> seconds after the most recent keepalive sent from user space.
- The internal keepalive now stops silently and no longer generates a warning message. Reason is that it will now stop early, while there may still be a substantial amount of time for keepalives from user space to arrive. If such keepalives arrive late (for example if user space is configured to send keepalives just a few seconds before the watchdog times out), the message would just be noise and not provide any value.
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html