[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <43dcd981-806a-dda5-39f6-6a0e42081d76@ti.com>
Date: Mon, 13 Jul 2020 15:45:19 +0300
From: Tero Kristo <t-kristo@...com>
To: Guenter Roeck <linux@...ck-us.net>, <wim@...ux-watchdog.org>,
<linux-watchdog@...r.kernel.org>
CC: <linux-kernel@...r.kernel.org>, <jan.kiszka@...mens.com>
Subject: Re: [PATCHv2 2/5] watchdog: add support for adjusting last known HW
keepalive time
On 05/07/2020 17:58, Guenter Roeck wrote:
> On 7/3/20 5:04 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@...com>
>> ---
>> drivers/watchdog/watchdog_dev.c | 23 +++++++++++++++++++++++
>> include/linux/watchdog.h | 2 ++
>> 2 files changed, 25 insertions(+)
>>
>> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
>> index bc1cfa288553..5848551cf29d 100644
>> --- a/drivers/watchdog/watchdog_dev.c
>> +++ b/drivers/watchdog/watchdog_dev.c
>> @@ -1138,6 +1138,29 @@ void watchdog_dev_unregister(struct watchdog_device *wdd)
>> watchdog_cdev_unregister(wdd);
>> }
>>
>> +/*
>> + * watchdog_set_last_hw_keepalive: set last HW keepalive time for watchdog
>> + *
>> + * Adjusts the last known HW keepalive time for a watchdog timer.
>> + * This is needed in case where watchdog has been started before
>> + * kernel by someone like bootloader, and it can't be pinged
>
> ... needed if the watchdog is already running when the probe function
> is called, and ...
>
>> + * immediately. This adjusts the watchdog ping period to match
>> + * the currently running timer.
>
> It doesn't adjust the ping period.
>
>> + */
>
> last_ping_ms needs to be documented (the last heartbeat was last_ping_ms
> milliseconds ago ?), both here and in Documentation/watchdog/watchdog-kernel-api.rst.
> It needs to be documented that the function must be called immediately
> after watchdog registration, and that min_hw_heartbeat_ms must
> be set for it to be useful.
>
>> +int watchdog_set_last_hw_keepalive(struct watchdog_device *wdd,
>> + unsigned int last_ping_ms)
>> +{
>> + struct watchdog_core_data *wd_data = wdd->wd_data;
>
> This needs a NULL check, in case it is called before watchdog driver
> registration.
Ok will fix all the above in next revision.
-Tero
>
>> + ktime_t now;
>> +
>> + 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 *);
>>
>>
>
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Powered by blists - more mailing lists