[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <56A785B8.8080601@roeck-us.net>
Date: Tue, 26 Jan 2016 06:42:00 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
Guenter Roeck <patchwork@...chwork.roeck-us.net>
Cc: linux-watchdog@...r.kernel.org, Wim Van Sebroeck <wim@...ana.be>,
linux-kernel@...r.kernel.org,
Timo Kokkonen <timo.kokkonen@...code.fi>,
linux-doc@...r.kernel.org, Doug Anderson <dianders@...omium.org>,
Jonathan Corbet <corbet@....net>
Subject: Re: [PATCH v7 4/9] watchdog: Add support for minimum time between
heartbeats
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@...ck-us.net>
>>
>> Some watchdogs require a minimum time between heartbeats.
>> Examples are the watchdogs in DA9062 and AT91SAM9x.
>>
>> Signed-off-by: Guenter Roeck <linux@...ck-us.net>
>> ---
>> 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
Powered by blists - more mailing lists