Linux-Watchdog Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] watchdog: make DesignWare watchdog allow users to set bigger timeout value
@ 2019-11-20 10:07 Wang, Peng 1. (NSB - CN/Hangzhou)
  2019-11-20 17:15 ` Guenter Roeck
  0 siblings, 1 reply; 8+ messages in thread
From: Wang, Peng 1. (NSB - CN/Hangzhou) @ 2019-11-20 10:07 UTC (permalink / raw)
  To: wim, linux, linux-watchdog, linux-kernel

From 1d051b7c081083751dc0bab97d3ab9efbba0f4a7 Mon Sep 17 00:00:00 2001
From: Peng Wang <peng.1.wang@nokia-sbell.com>
Date: Wed, 20 Nov 2019 15:12:59 +0800
Subject: [PATCH] watchdog: make DesignWare watchdog allow users to set bigger
 timeout value

watchdog_dev.c provides means to allow users to set bigger timeout value
than HW can support, make DesignWare watchdog align with this.

Signed-off-by: Peng Wang <peng.1.wang@nokia-sbell.com>
---
 drivers/watchdog/dw_wdt.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index fef7c61..8911e5e 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -113,8 +113,15 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
 	 */
 	writel(top_val | top_val << WDOG_TIMEOUT_RANGE_TOPINIT_SHIFT,
 	       dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
-
-	wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
+    
+    /*
+     * In case users set bigger timeout value than HW can support,
+     * kernel(watchdog_dev.c) helps to feed watchdog before 
+     * wdd->timeout
+     */
+    if ( wdd->timeout * 1000 <= wdd->max_hw_heartbeat_ms ) {
+	    wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
+    }
 
 	return 0;
 }
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] watchdog: make DesignWare watchdog allow users to set bigger timeout value
  2019-11-20 10:07 [PATCH] watchdog: make DesignWare watchdog allow users to set bigger timeout value Wang, Peng 1. (NSB - CN/Hangzhou)
@ 2019-11-20 17:15 ` Guenter Roeck
  2019-11-21  1:29   ` Wang, Peng 1. (NSB - CN/Hangzhou)
  0 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2019-11-20 17:15 UTC (permalink / raw)
  To: Wang, Peng 1. (NSB - CN/Hangzhou); +Cc: wim, linux-watchdog, linux-kernel

On Wed, Nov 20, 2019 at 10:07:57AM +0000, Wang, Peng 1. (NSB - CN/Hangzhou) wrote:
> From 1d051b7c081083751dc0bab97d3ab9efbba0f4a7 Mon Sep 17 00:00:00 2001
> From: Peng Wang <peng.1.wang@nokia-sbell.com>
> Date: Wed, 20 Nov 2019 15:12:59 +0800
> Subject: [PATCH] watchdog: make DesignWare watchdog allow users to set bigger
>  timeout value
> 
> watchdog_dev.c provides means to allow users to set bigger timeout value
> than HW can support, make DesignWare watchdog align with this.
> 
> Signed-off-by: Peng Wang <peng.1.wang@nokia-sbell.com>
> ---
>  drivers/watchdog/dw_wdt.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> index fef7c61..8911e5e 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -113,8 +113,15 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
>  	 */
>  	writel(top_val | top_val << WDOG_TIMEOUT_RANGE_TOPINIT_SHIFT,
>  	       dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
> -
> -	wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
> +    
> +    /*
> +     * In case users set bigger timeout value than HW can support,
> +     * kernel(watchdog_dev.c) helps to feed watchdog before 
> +     * wdd->timeout
> +     */
> +    if ( wdd->timeout * 1000 <= wdd->max_hw_heartbeat_ms ) {
> +	    wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
> +    }

{ } is unnecessary here. Also, the above code compares the _old_
timeout againt the maximum supported timeout, which doesn't look
correct.

Thanks,
Guenter

>  
>  	return 0;
>  }
> -- 
> 1.8.3.1
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] watchdog: make DesignWare watchdog allow users to set bigger timeout value
  2019-11-20 17:15 ` Guenter Roeck
@ 2019-11-21  1:29   ` Wang, Peng 1. (NSB - CN/Hangzhou)
  2019-11-21  3:40     ` Guenter Roeck
  0 siblings, 1 reply; 8+ messages in thread
From: Wang, Peng 1. (NSB - CN/Hangzhou) @ 2019-11-21  1:29 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: wim, linux-watchdog, linux-kernel

Hi Guenter, 

Thank you for your time.
- I will remove the unnecessary {}
- wdd->max_hw_heartbeat_ms is the max timeout value which HW can support, this value is limited according to the input clock, say. It only supports 20 seconds, if users requires to set timeout to be say. 60 seconds, the watchdog device driver 'watchdog_dev.c' checks if wdd->timeout is bigger than wdd->max_hw_heartbeat_ms, if yes, watchdog_dev.c feeds the watchdog by a worker queue itself to help to feed the watchdog before 60 seconds elapse. Here the issue of dw_wdt.c is that, the original codes update wdd->timeout to the value which HW can support, which means if users requires 60 seconds to be the timeout, then dw_wdt.c updates the timeout value to 20 seconds, this makes the "feeding helper" mechanism in watchdog_dev.c not take effect. That's why I add this check.

Thanks,
Peng Wang

-----Original Message-----
From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter Roeck
Sent: Thursday, November 21, 2019 1:15 AM
To: Wang, Peng 1. (NSB - CN/Hangzhou) <peng.1.wang@nokia-sbell.com>
Cc: wim@linux-watchdog.org; linux-watchdog@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] watchdog: make DesignWare watchdog allow users to set bigger timeout value

On Wed, Nov 20, 2019 at 10:07:57AM +0000, Wang, Peng 1. (NSB - CN/Hangzhou) wrote:
> From 1d051b7c081083751dc0bab97d3ab9efbba0f4a7 Mon Sep 17 00:00:00 2001
> From: Peng Wang <peng.1.wang@nokia-sbell.com>
> Date: Wed, 20 Nov 2019 15:12:59 +0800
> Subject: [PATCH] watchdog: make DesignWare watchdog allow users to set 
> bigger  timeout value
> 
> watchdog_dev.c provides means to allow users to set bigger timeout 
> value than HW can support, make DesignWare watchdog align with this.
> 
> Signed-off-by: Peng Wang <peng.1.wang@nokia-sbell.com>
> ---
>  drivers/watchdog/dw_wdt.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c 
> index fef7c61..8911e5e 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -113,8 +113,15 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
>  	 */
>  	writel(top_val | top_val << WDOG_TIMEOUT_RANGE_TOPINIT_SHIFT,
>  	       dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
> -
> -	wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
> +    
> +    /*
> +     * In case users set bigger timeout value than HW can support,
> +     * kernel(watchdog_dev.c) helps to feed watchdog before 
> +     * wdd->timeout
> +     */
> +    if ( wdd->timeout * 1000 <= wdd->max_hw_heartbeat_ms ) {
> +	    wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
> +    }

{ } is unnecessary here. Also, the above code compares the _old_ timeout againt the maximum supported timeout, which doesn't look correct.

Thanks,
Guenter

>  
>  	return 0;
>  }
> --
> 1.8.3.1
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] watchdog: make DesignWare watchdog allow users to set bigger timeout value
  2019-11-21  1:29   ` Wang, Peng 1. (NSB - CN/Hangzhou)
@ 2019-11-21  3:40     ` Guenter Roeck
  2019-11-21  8:07       ` Wang, Peng 1. (NSB - CN/Hangzhou)
  0 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2019-11-21  3:40 UTC (permalink / raw)
  To: Wang, Peng 1. (NSB - CN/Hangzhou); +Cc: wim, linux-watchdog, linux-kernel

On 11/20/19 5:29 PM, Wang, Peng 1. (NSB - CN/Hangzhou) wrote:
> Hi Guenter,
> 
> Thank you for your time.
> - I will remove the unnecessary {}
> - wdd->max_hw_heartbeat_ms is the max timeout value which HW can support, this value is limited according to the input clock, say. It only supports 20 seconds, if users requires to set timeout to be say. 60 seconds, the watchdog device driver 'watchdog_dev.c' checks if wdd->timeout is bigger than wdd->max_hw_heartbeat_ms, if yes, watchdog_dev.c feeds the watchdog by a worker queue itself to help to feed the watchdog before 60 seconds elapse. Here the issue of dw_wdt.c is that, the original codes update wdd->timeout to the value which HW can support, which means if users requires 60 seconds to be the timeout, then dw_wdt.c updates the timeout value to 20 seconds, this makes the "feeding helper" mechanism in watchdog_dev.c not take effect. That's why I add this check.
> 

Yes, I understand you need a check. What I am saying is that the check is wrong.

You need something like

	if (top_s > DW_WDT_MAX_TOP)
		wdt->timeout = top_s;
	else
		wdt->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);

Guenter

> Thanks,
> Peng Wang
> 
> -----Original Message-----
> From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter Roeck
> Sent: Thursday, November 21, 2019 1:15 AM
> To: Wang, Peng 1. (NSB - CN/Hangzhou) <peng.1.wang@nokia-sbell.com>
> Cc: wim@linux-watchdog.org; linux-watchdog@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] watchdog: make DesignWare watchdog allow users to set bigger timeout value
> 
> On Wed, Nov 20, 2019 at 10:07:57AM +0000, Wang, Peng 1. (NSB - CN/Hangzhou) wrote:
>>  From 1d051b7c081083751dc0bab97d3ab9efbba0f4a7 Mon Sep 17 00:00:00 2001
>> From: Peng Wang <peng.1.wang@nokia-sbell.com>
>> Date: Wed, 20 Nov 2019 15:12:59 +0800
>> Subject: [PATCH] watchdog: make DesignWare watchdog allow users to set
>> bigger  timeout value
>>
>> watchdog_dev.c provides means to allow users to set bigger timeout
>> value than HW can support, make DesignWare watchdog align with this.
>>
>> Signed-off-by: Peng Wang <peng.1.wang@nokia-sbell.com>
>> ---
>>   drivers/watchdog/dw_wdt.c | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
>> index fef7c61..8911e5e 100644
>> --- a/drivers/watchdog/dw_wdt.c
>> +++ b/drivers/watchdog/dw_wdt.c
>> @@ -113,8 +113,15 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
>>   	 */
>>   	writel(top_val | top_val << WDOG_TIMEOUT_RANGE_TOPINIT_SHIFT,
>>   	       dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
>> -
>> -	wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
>> +
>> +    /*
>> +     * In case users set bigger timeout value than HW can support,
>> +     * kernel(watchdog_dev.c) helps to feed watchdog before
>> +     * wdd->timeout
>> +     */
>> +    if ( wdd->timeout * 1000 <= wdd->max_hw_heartbeat_ms ) {
>> +	    wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
>> +    }
> 
> { } is unnecessary here. Also, the above code compares the _old_ timeout againt the maximum supported timeout, which doesn't look correct.
> 
> Thanks,
> Guenter
> 
>>   
>>   	return 0;
>>   }
>> --
>> 1.8.3.1
>>
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] watchdog: make DesignWare watchdog allow users to set bigger timeout value
  2019-11-21  3:40     ` Guenter Roeck
@ 2019-11-21  8:07       ` Wang, Peng 1. (NSB - CN/Hangzhou)
  2019-11-21  9:45         ` Guenter Roeck
  0 siblings, 1 reply; 8+ messages in thread
From: Wang, Peng 1. (NSB - CN/Hangzhou) @ 2019-11-21  8:07 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: wim, linux-watchdog, linux-kernel

Actually, this function is used by watchdog_dev.c, the timeout value in wdd is already modified there. 
but yes, you are right, decide the actual timeout value here is more reasonable. :)

thanks,
Peng Wang
-----Original Message-----
From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter Roeck
Sent: Thursday, November 21, 2019 11:41 AM
To: Wang, Peng 1. (NSB - CN/Hangzhou) <peng.1.wang@nokia-sbell.com>
Cc: wim@linux-watchdog.org; linux-watchdog@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH] watchdog: make DesignWare watchdog allow users to set bigger timeout value

On 11/20/19 5:29 PM, Wang, Peng 1. (NSB - CN/Hangzhou) wrote:
> Hi Guenter,
> 
> Thank you for your time.
> - I will remove the unnecessary {}
> - wdd->max_hw_heartbeat_ms is the max timeout value which HW can support, this value is limited according to the input clock, say. It only supports 20 seconds, if users requires to set timeout to be say. 60 seconds, the watchdog device driver 'watchdog_dev.c' checks if wdd->timeout is bigger than wdd->max_hw_heartbeat_ms, if yes, watchdog_dev.c feeds the watchdog by a worker queue itself to help to feed the watchdog before 60 seconds elapse. Here the issue of dw_wdt.c is that, the original codes update wdd->timeout to the value which HW can support, which means if users requires 60 seconds to be the timeout, then dw_wdt.c updates the timeout value to 20 seconds, this makes the "feeding helper" mechanism in watchdog_dev.c not take effect. That's why I add this check.
> 

Yes, I understand you need a check. What I am saying is that the check is wrong.

You need something like

	if (top_s > DW_WDT_MAX_TOP)
		wdt->timeout = top_s;
	else
		wdt->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);

Guenter

> Thanks,
> Peng Wang
> 
> -----Original Message-----
> From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter 
> Roeck
> Sent: Thursday, November 21, 2019 1:15 AM
> To: Wang, Peng 1. (NSB - CN/Hangzhou) <peng.1.wang@nokia-sbell.com>
> Cc: wim@linux-watchdog.org; linux-watchdog@vger.kernel.org; 
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] watchdog: make DesignWare watchdog allow users to 
> set bigger timeout value
> 
> On Wed, Nov 20, 2019 at 10:07:57AM +0000, Wang, Peng 1. (NSB - CN/Hangzhou) wrote:
>>  From 1d051b7c081083751dc0bab97d3ab9efbba0f4a7 Mon Sep 17 00:00:00 
>> 2001
>> From: Peng Wang <peng.1.wang@nokia-sbell.com>
>> Date: Wed, 20 Nov 2019 15:12:59 +0800
>> Subject: [PATCH] watchdog: make DesignWare watchdog allow users to 
>> set bigger  timeout value
>>
>> watchdog_dev.c provides means to allow users to set bigger timeout 
>> value than HW can support, make DesignWare watchdog align with this.
>>
>> Signed-off-by: Peng Wang <peng.1.wang@nokia-sbell.com>
>> ---
>>   drivers/watchdog/dw_wdt.c | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c 
>> index fef7c61..8911e5e 100644
>> --- a/drivers/watchdog/dw_wdt.c
>> +++ b/drivers/watchdog/dw_wdt.c
>> @@ -113,8 +113,15 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
>>   	 */
>>   	writel(top_val | top_val << WDOG_TIMEOUT_RANGE_TOPINIT_SHIFT,
>>   	       dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
>> -
>> -	wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
>> +
>> +    /*
>> +     * In case users set bigger timeout value than HW can support,
>> +     * kernel(watchdog_dev.c) helps to feed watchdog before
>> +     * wdd->timeout
>> +     */
>> +    if ( wdd->timeout * 1000 <= wdd->max_hw_heartbeat_ms ) {
>> +	    wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
>> +    }
> 
> { } is unnecessary here. Also, the above code compares the _old_ timeout againt the maximum supported timeout, which doesn't look correct.
> 
> Thanks,
> Guenter
> 
>>   
>>   	return 0;
>>   }
>> --
>> 1.8.3.1
>>
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] watchdog: make DesignWare watchdog allow users to set bigger timeout value
  2019-11-21  8:07       ` Wang, Peng 1. (NSB - CN/Hangzhou)
@ 2019-11-21  9:45         ` Guenter Roeck
  0 siblings, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2019-11-21  9:45 UTC (permalink / raw)
  To: Wang, Peng 1. (NSB - CN/Hangzhou); +Cc: wim, linux-watchdog, linux-kernel

On 11/21/19 12:07 AM, Wang, Peng 1. (NSB - CN/Hangzhou) wrote:
> Actually, this function is used by watchdog_dev.c, the timeout value in wdd is already modified there.
> but yes, you are right, decide the actual timeout value here is more reasonable. :)
> 

Please avoid top-posting.

watchdog_dev.c only sets wdd->timeout if there is no driver function
to set it.

Guenter

> thanks,
> Peng Wang
> -----Original Message-----
> From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter Roeck
> Sent: Thursday, November 21, 2019 11:41 AM
> To: Wang, Peng 1. (NSB - CN/Hangzhou) <peng.1.wang@nokia-sbell.com>
> Cc: wim@linux-watchdog.org; linux-watchdog@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] watchdog: make DesignWare watchdog allow users to set bigger timeout value
> 
> On 11/20/19 5:29 PM, Wang, Peng 1. (NSB - CN/Hangzhou) wrote:
>> Hi Guenter,
>>
>> Thank you for your time.
>> - I will remove the unnecessary {}
>> - wdd->max_hw_heartbeat_ms is the max timeout value which HW can support, this value is limited according to the input clock, say. It only supports 20 seconds, if users requires to set timeout to be say. 60 seconds, the watchdog device driver 'watchdog_dev.c' checks if wdd->timeout is bigger than wdd->max_hw_heartbeat_ms, if yes, watchdog_dev.c feeds the watchdog by a worker queue itself to help to feed the watchdog before 60 seconds elapse. Here the issue of dw_wdt.c is that, the original codes update wdd->timeout to the value which HW can support, which means if users requires 60 seconds to be the timeout, then dw_wdt.c updates the timeout value to 20 seconds, this makes the "feeding helper" mechanism in watchdog_dev.c not take effect. That's why I add this check.
>>
> 
> Yes, I understand you need a check. What I am saying is that the check is wrong.
> 
> You need something like
> 
> 	if (top_s > DW_WDT_MAX_TOP)
> 		wdt->timeout = top_s;
> 	else
> 		wdt->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
> 
> Guenter
> 
>> Thanks,
>> Peng Wang
>>
>> -----Original Message-----
>> From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter
>> Roeck
>> Sent: Thursday, November 21, 2019 1:15 AM
>> To: Wang, Peng 1. (NSB - CN/Hangzhou) <peng.1.wang@nokia-sbell.com>
>> Cc: wim@linux-watchdog.org; linux-watchdog@vger.kernel.org;
>> linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH] watchdog: make DesignWare watchdog allow users to
>> set bigger timeout value
>>
>> On Wed, Nov 20, 2019 at 10:07:57AM +0000, Wang, Peng 1. (NSB - CN/Hangzhou) wrote:
>>>   From 1d051b7c081083751dc0bab97d3ab9efbba0f4a7 Mon Sep 17 00:00:00
>>> 2001
>>> From: Peng Wang <peng.1.wang@nokia-sbell.com>
>>> Date: Wed, 20 Nov 2019 15:12:59 +0800
>>> Subject: [PATCH] watchdog: make DesignWare watchdog allow users to
>>> set bigger  timeout value
>>>
>>> watchdog_dev.c provides means to allow users to set bigger timeout
>>> value than HW can support, make DesignWare watchdog align with this.
>>>
>>> Signed-off-by: Peng Wang <peng.1.wang@nokia-sbell.com>
>>> ---
>>>    drivers/watchdog/dw_wdt.c | 11 +++++++++--
>>>    1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
>>> index fef7c61..8911e5e 100644
>>> --- a/drivers/watchdog/dw_wdt.c
>>> +++ b/drivers/watchdog/dw_wdt.c
>>> @@ -113,8 +113,15 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
>>>    	 */
>>>    	writel(top_val | top_val << WDOG_TIMEOUT_RANGE_TOPINIT_SHIFT,
>>>    	       dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
>>> -
>>> -	wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
>>> +
>>> +    /*
>>> +     * In case users set bigger timeout value than HW can support,
>>> +     * kernel(watchdog_dev.c) helps to feed watchdog before
>>> +     * wdd->timeout
>>> +     */
>>> +    if ( wdd->timeout * 1000 <= wdd->max_hw_heartbeat_ms ) {
>>> +	    wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
>>> +    }
>>
>> { } is unnecessary here. Also, the above code compares the _old_ timeout againt the maximum supported timeout, which doesn't look correct.
>>
>> Thanks,
>> Guenter
>>
>>>    
>>>    	return 0;
>>>    }
>>> --
>>> 1.8.3.1
>>>
>>
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] watchdog: make DesignWare watchdog allow users to set bigger timeout value
  2019-11-21  8:21 Wang, Peng 1. (NSB - CN/Hangzhou)
@ 2019-11-21  9:56 ` Guenter Roeck
  0 siblings, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2019-11-21  9:56 UTC (permalink / raw)
  To: Wang, Peng 1. (NSB - CN/Hangzhou), Guenter Roeck
  Cc: wim, linux-watchdog, linux-kernel

On 11/21/19 12:21 AM, Wang, Peng 1. (NSB - CN/Hangzhou) wrote:
>>From d21d084122d08816454a1e338f0946a9da1f81e3 Mon Sep 17 00:00:00 2001
> From: Peng Wang <peng.1.wang@nokia-sbell.com>
> Date: Wed, 20 Nov 2019 15:12:59 +0800
> Subject: [PATCH] watchdog: make DesignWare watchdog allow users to set bigger
>   timeout value
> 
> watchdog_dev.c provides means to allow users to set bigger timeout value
> than HW can support, make DesignWare watchdog align with this.
> 
> Signed-off-by: Peng Wang <peng.1.wang@nokia-sbell.com>
> ---

Please version your patches, and add a change log here.

>   drivers/watchdog/dw_wdt.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> index fef7c61..f1a431c 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -114,7 +114,15 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
>   	writel(top_val | top_val << WDOG_TIMEOUT_RANGE_TOPINIT_SHIFT,
>   	       dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
>   
> -	wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
> +	/*
> +	 * In case users set bigger timeout value than HW can support,
> +	 * kernel(watchdog_dev.c) helps to feed watchdog before
> +	 * wdd->timeout

No, before wdd->max_hw_heartbeat_ms.

> +	 */
> +	if ( top_s * 1000 <= wdd->max_hw_heartbeat_ms )
> +		wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
> +	else
> +		wdd->timeout = top_s;
>   
>   	return 0;
>   }
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH] watchdog: make DesignWare watchdog allow users to set bigger timeout value
@ 2019-11-21  8:21 Wang, Peng 1. (NSB - CN/Hangzhou)
  2019-11-21  9:56 ` Guenter Roeck
  0 siblings, 1 reply; 8+ messages in thread
From: Wang, Peng 1. (NSB - CN/Hangzhou) @ 2019-11-21  8:21 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: wim, linux-watchdog, linux-kernel

From d21d084122d08816454a1e338f0946a9da1f81e3 Mon Sep 17 00:00:00 2001
From: Peng Wang <peng.1.wang@nokia-sbell.com>
Date: Wed, 20 Nov 2019 15:12:59 +0800
Subject: [PATCH] watchdog: make DesignWare watchdog allow users to set bigger
 timeout value

watchdog_dev.c provides means to allow users to set bigger timeout value
than HW can support, make DesignWare watchdog align with this.

Signed-off-by: Peng Wang <peng.1.wang@nokia-sbell.com>
---
 drivers/watchdog/dw_wdt.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
index fef7c61..f1a431c 100644
--- a/drivers/watchdog/dw_wdt.c
+++ b/drivers/watchdog/dw_wdt.c
@@ -114,7 +114,15 @@ static int dw_wdt_set_timeout(struct watchdog_device *wdd, unsigned int top_s)
 	writel(top_val | top_val << WDOG_TIMEOUT_RANGE_TOPINIT_SHIFT,
 	       dw_wdt->regs + WDOG_TIMEOUT_RANGE_REG_OFFSET);
 
-	wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
+	/*
+	 * In case users set bigger timeout value than HW can support,
+	 * kernel(watchdog_dev.c) helps to feed watchdog before 
+	 * wdd->timeout
+	 */
+	if ( top_s * 1000 <= wdd->max_hw_heartbeat_ms )
+		wdd->timeout = dw_wdt_top_in_seconds(dw_wdt, top_val);
+	else
+		wdd->timeout = top_s;
 
 	return 0;
 }
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-20 10:07 [PATCH] watchdog: make DesignWare watchdog allow users to set bigger timeout value Wang, Peng 1. (NSB - CN/Hangzhou)
2019-11-20 17:15 ` Guenter Roeck
2019-11-21  1:29   ` Wang, Peng 1. (NSB - CN/Hangzhou)
2019-11-21  3:40     ` Guenter Roeck
2019-11-21  8:07       ` Wang, Peng 1. (NSB - CN/Hangzhou)
2019-11-21  9:45         ` Guenter Roeck
2019-11-21  8:21 Wang, Peng 1. (NSB - CN/Hangzhou)
2019-11-21  9:56 ` Guenter Roeck

Linux-Watchdog Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \
		linux-watchdog@vger.kernel.org
	public-inbox-index linux-watchdog

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-watchdog


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git