Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCHv2 2/5] watchdog: add support for adjusting last known HW
 keepalive time
To:     Guenter Roeck <linux@roeck-us.net>, <wim@linux-watchdog.org>,
        <linux-watchdog@vger.kernel.org>
CC:     <linux-kernel@vger.kernel.org>, <jan.kiszka@siemens.com>
References: <20200703120406.7092-1-t-kristo@ti.com>
 <20200703120406.7092-3-t-kristo@ti.com>
 <faeebfbb-cb8c-e31e-50ef-af42e1b11e0e@roeck-us.net>
From:   Tero Kristo <t-kristo@ti.com>
Message-ID: <43dcd981-806a-dda5-39f6-6a0e42081d76@ti.com>
Date:   Mon, 13 Jul 2020 15:45:19 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.8.0
MIME-Version: 1.0
In-Reply-To: <faeebfbb-cb8c-e31e-50ef-af42e1b11e0e@roeck-us.net>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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@ti.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