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

From: Uwe Kleine-König
Date: Tue Jan 26 2016 - 03:07:54 EST


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?

> if (wdd->ops->ping)
> err = wdd->ops->ping(wdd); /* ping the watchdog */
> else

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |