Re: [PATCHv4 2/4] watchdog: add support for adjusting last known HW keepalive time

[Guenter Roeck <linux@roeck-us.net>]

On 7/17/20 6:29 AM, Tero Kristo wrote:
> Certain watchdogs require the watchdog only to be pinged within a
> specific time window, pinging too early or too late cause the watchdog
> to fire. In cases where this sort of watchdog has been started before
> kernel comes up, we must adjust the watchdog keepalive window to match
> the actually running timer, so add a new driver API for this purpose.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  .../watchdog/watchdog-kernel-api.rst          | 12 ++++++++
>  drivers/watchdog/watchdog_dev.c               | 30 +++++++++++++++++++
>  include/linux/watchdog.h                      |  2 ++
>  3 files changed, 44 insertions(+)
> 
> diff --git a/Documentation/watchdog/watchdog-kernel-api.rst b/Documentation/watchdog/watchdog-kernel-api.rst
> index 068a55ee0d4a..baf44e986b07 100644
> --- a/Documentation/watchdog/watchdog-kernel-api.rst
> +++ b/Documentation/watchdog/watchdog-kernel-api.rst
> @@ -336,3 +336,15 @@ an action is taken by a preconfigured pretimeout governor preassigned to
>  the watchdog device. If watchdog pretimeout governor framework is not
>  enabled, watchdog_notify_pretimeout() prints a notification message to
>  the kernel log buffer.
> +
> +To set the last known HW keepalive time for a watchdog, the following function
> +should be used::
> +
> +  int watchdog_set_last_hw_keepalive(struct watchdog_device *wdd,
> +                                     unsigned int last_ping_ms)
> +
> +This function must be called immediately after watchdog registration. It
> +sets the last known hardware heartbeat to have happened last_ping_ms before
> +current time. Calling this is only needed if the watchdog is already running
> +when probe is called, and the watchdog can only be pinged after the
> +min_hw_heartbeat_ms time has passed from the last ping.
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index bc1cfa288553..e74a0c6811b5 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -1138,6 +1138,36 @@ void watchdog_dev_unregister(struct watchdog_device *wdd)
>  	watchdog_cdev_unregister(wdd);
>  }
>  
> +/*
> + *	watchdog_set_last_hw_keepalive: set last HW keepalive time for watchdog
> + *	@wdd: watchdog device
> + *	@last_ping_ms: time since last HW heartbeat
> + *
> + *	Adjusts the last known HW keepalive time for a watchdog timer.
> + *	This is needed if the watchdog is already running when the probe
> + *	function is called, and it can't be pinged immediately. This
> + *	function must be called immediately after watchdog registration,
> + *	and min_hw_heartbeat_ms must be set for this to be useful.
> + */
> +int watchdog_set_last_hw_keepalive(struct watchdog_device *wdd,
> +				   unsigned int last_ping_ms)
> +{
> +	struct watchdog_core_data *wd_data;
> +	ktime_t now;
> +
> +	if (!wdd)
> +		return -EINVAL;
> +
> +	wd_data = wdd->wd_data;
> +
> +	now = ktime_get();
> +
> +	wd_data->last_hw_keepalive = ktime_sub(now, ms_to_ktime(last_ping_ms));
> +
> +	return __watchdog_ping(wdd);
> +}
> +EXPORT_SYMBOL_GPL(watchdog_set_last_hw_keepalive);
> +
>  /*
>   *	watchdog_dev_init: init dev part of watchdog core
>   *
> diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h
> index 1464ce6ffa31..9b19e6bb68b5 100644
> --- a/include/linux/watchdog.h
> +++ b/include/linux/watchdog.h
> @@ -210,6 +210,8 @@ extern int watchdog_init_timeout(struct watchdog_device *wdd,
>  extern int watchdog_register_device(struct watchdog_device *);
>  extern void watchdog_unregister_device(struct watchdog_device *);
>  
> +int watchdog_set_last_hw_keepalive(struct watchdog_device *, unsigned int);
> +
>  /* devres register variant */
>  int devm_watchdog_register_device(struct device *dev, struct watchdog_device *);
>  
>