Re: [PATCH v7 4/9] watchdog: Add support for minimum time between heartbeats

From: Guenter Roeck
Date: Tue Jan 26 2016 - 09:42:11 EST


Hi Uwe,

On 01/26/2016 12:07 AM, Uwe Kleine-König wrote:
Hello Guenter,

On Mon, Jan 25, 2016 at 06:53:11PM -0800, Guenter Roeck wrote:
From: Guenter Roeck <linux@xxxxxxxxxxxx>

Some watchdogs require a minimum time between heartbeats.
Examples are the watchdogs in DA9062 and AT91SAM9x.

Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
---
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(+)

diff --git a/Documentation/watchdog/watchdog-kernel-api.txt b/Documentation/watchdog/watchdog-kernel-api.txt
index 62d49d6a7ff5..2221fb4f2739 100644
--- a/Documentation/watchdog/watchdog-kernel-api.txt
+++ b/Documentation/watchdog/watchdog-kernel-api.txt
@@ -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_timeout_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_timeout_ms is non-zero.
+* min_hw_heartbeat_ms: Minimum time between heartbeats sent to the chip,
+ in milli-seconds.
* max_hw_timeout_ms: Maximum hardware timeout, in milli-seconds.
If set, the infrastructure will send heartbeats to the watchdog driver
if 'timeout' is larger than max_hw_timeout_ms, unless WDOG_ACTIVE
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 1a8f3861fe92..ff03cbc8e081 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -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 ? */
@@ -138,8 +139,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;
+

Do you need to undo this assignment when ping fails?


I thought about it, but decided against it. I concluded that it is better
in this situation to only try again after the minimum delay between
heartbeats.

Thanks,
Guenter