linux-watchdog.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate 5% safety margin
@ 2024-04-04 15:33 Judith Mendez
  2024-04-05 13:11 ` Rafael Beims
  2024-04-06 13:01 ` Guenter Roeck
  0 siblings, 2 replies; 6+ messages in thread
From: Judith Mendez @ 2024-04-04 15:33 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck
  Cc: linux-watchdog, linux-kernel, Francesco Dolcini

On AM62x, the watchdog is pet before the valid window
is open. Fix min_hw_heartbeat and accommodate a 5% safety
margin with the exception of open window size < 10%,
which shall use <5% due to the smaller open window size.

cc: stable@vger.kernel.org
Fixes: 5527483f8f7c (" watchdog: rti-wdt: attach to running watchdog during probe")
Signed-off-by: Judith Mendez <jm@ti.com>
---
 drivers/watchdog/rti_wdt.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index 8e1be7ba0103..0b16ada659cc 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -92,7 +92,7 @@ static int rti_wdt_start(struct watchdog_device *wdd)
 	 * to be 50% or less than that; we obviouly want to configure the open
 	 * window as large as possible so we select the 50% option.
 	 */
-	wdd->min_hw_heartbeat_ms = 500 * wdd->timeout;
+	wdd->min_hw_heartbeat_ms = 550 * wdd->timeout;
 
 	/* Generate NMI when wdt expires */
 	writel_relaxed(RTIWWDRX_NMI, wdt->base + RTIWWDRXCTRL);
@@ -126,31 +126,33 @@ static int rti_wdt_setup_hw_hb(struct watchdog_device *wdd, u32 wsize)
 	 * be petted during the open window; not too early or not too late.
 	 * The HW configuration options only allow for the open window size
 	 * to be 50% or less than that.
+	 * To avoid any glitches, we accommodate 5% safety margin, with the
+	 * exception of open window size < 10%.
 	 */
 	switch (wsize) {
 	case RTIWWDSIZE_50P:
-		/* 50% open window => 50% min heartbeat */
-		wdd->min_hw_heartbeat_ms = 500 * heartbeat;
+		/* 50% open window => 55% min heartbeat */
+		wdd->min_hw_heartbeat_ms = 550 * heartbeat;
 		break;
 
 	case RTIWWDSIZE_25P:
-		/* 25% open window => 75% min heartbeat */
-		wdd->min_hw_heartbeat_ms = 750 * heartbeat;
+		/* 25% open window => 80% min heartbeat */
+		wdd->min_hw_heartbeat_ms = 800 * heartbeat;
 		break;
 
 	case RTIWWDSIZE_12P5:
-		/* 12.5% open window => 87.5% min heartbeat */
-		wdd->min_hw_heartbeat_ms = 875 * heartbeat;
+		/* 12.5% open window => 92.5% min heartbeat */
+		wdd->min_hw_heartbeat_ms = 925 * heartbeat;
 		break;
 
 	case RTIWWDSIZE_6P25:
-		/* 6.5% open window => 93.5% min heartbeat */
-		wdd->min_hw_heartbeat_ms = 935 * heartbeat;
+		/* 6.5% open window => 96.5% min heartbeat */
+		wdd->min_hw_heartbeat_ms = 965 * heartbeat;
 		break;
 
 	case RTIWWDSIZE_3P125:
-		/* 3.125% open window => 96.9% min heartbeat */
-		wdd->min_hw_heartbeat_ms = 969 * heartbeat;
+		/* 3.125% open window => 97.9% min heartbeat */
+		wdd->min_hw_heartbeat_ms = 979 * heartbeat;
 		break;
 
 	default:

base-commit: 860bbe8e618fd62446309e286ab4a83d38201c0a
-- 
2.43.2


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

* Re: [PATCH v2] watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate 5% safety margin
  2024-04-04 15:33 [PATCH v2] watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate 5% safety margin Judith Mendez
@ 2024-04-05 13:11 ` Rafael Beims
  2024-04-06 13:01 ` Guenter Roeck
  1 sibling, 0 replies; 6+ messages in thread
From: Rafael Beims @ 2024-04-05 13:11 UTC (permalink / raw)
  To: Judith Mendez, Wim Van Sebroeck, Guenter Roeck
  Cc: linux-watchdog, linux-kernel, Francesco Dolcini

On 04/04/2024 12:33, Judith Mendez wrote:
> On AM62x, the watchdog is pet before the valid window
> is open. Fix min_hw_heartbeat and accommodate a 5% safety
> margin with the exception of open window size < 10%,
> which shall use <5% due to the smaller open window size.
>
> cc: stable@vger.kernel.org
> Fixes: 5527483f8f7c (" watchdog: rti-wdt: attach to running watchdog during probe")
> Signed-off-by: Judith Mendez <jm@ti.com>
> ---
>   drivers/watchdog/rti_wdt.c | 24 +++++++++++++-----------
>   1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
> index 8e1be7ba0103..0b16ada659cc 100644
> --- a/drivers/watchdog/rti_wdt.c
> +++ b/drivers/watchdog/rti_wdt.c
> @@ -92,7 +92,7 @@ static int rti_wdt_start(struct watchdog_device *wdd)
>   	 * to be 50% or less than that; we obviouly want to configure the open
>   	 * window as large as possible so we select the 50% option.
>   	 */
> -	wdd->min_hw_heartbeat_ms = 500 * wdd->timeout;
> +	wdd->min_hw_heartbeat_ms = 550 * wdd->timeout;
>   
>   	/* Generate NMI when wdt expires */
>   	writel_relaxed(RTIWWDRX_NMI, wdt->base + RTIWWDRXCTRL);
> @@ -126,31 +126,33 @@ static int rti_wdt_setup_hw_hb(struct watchdog_device *wdd, u32 wsize)
>   	 * be petted during the open window; not too early or not too late.
>   	 * The HW configuration options only allow for the open window size
>   	 * to be 50% or less than that.
> +	 * To avoid any glitches, we accommodate 5% safety margin, with the
> +	 * exception of open window size < 10%.
>   	 */
>   	switch (wsize) {
>   	case RTIWWDSIZE_50P:
> -		/* 50% open window => 50% min heartbeat */
> -		wdd->min_hw_heartbeat_ms = 500 * heartbeat;
> +		/* 50% open window => 55% min heartbeat */
> +		wdd->min_hw_heartbeat_ms = 550 * heartbeat;
>   		break;
>   
>   	case RTIWWDSIZE_25P:
> -		/* 25% open window => 75% min heartbeat */
> -		wdd->min_hw_heartbeat_ms = 750 * heartbeat;
> +		/* 25% open window => 80% min heartbeat */
> +		wdd->min_hw_heartbeat_ms = 800 * heartbeat;
>   		break;
>   
>   	case RTIWWDSIZE_12P5:
> -		/* 12.5% open window => 87.5% min heartbeat */
> -		wdd->min_hw_heartbeat_ms = 875 * heartbeat;
> +		/* 12.5% open window => 92.5% min heartbeat */
> +		wdd->min_hw_heartbeat_ms = 925 * heartbeat;
>   		break;
>   
>   	case RTIWWDSIZE_6P25:
> -		/* 6.5% open window => 93.5% min heartbeat */
> -		wdd->min_hw_heartbeat_ms = 935 * heartbeat;
> +		/* 6.5% open window => 96.5% min heartbeat */
> +		wdd->min_hw_heartbeat_ms = 965 * heartbeat;
>   		break;
>   
>   	case RTIWWDSIZE_3P125:
> -		/* 3.125% open window => 96.9% min heartbeat */
> -		wdd->min_hw_heartbeat_ms = 969 * heartbeat;
> +		/* 3.125% open window => 97.9% min heartbeat */
> +		wdd->min_hw_heartbeat_ms = 979 * heartbeat;
>   		break;
>   
>   	default:
>
> base-commit: 860bbe8e618fd62446309e286ab4a83d38201c0a


Tested-by: Rafael Beims <rafael.beims@toradex.com>


Rafael


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

* Re: [PATCH v2] watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate 5% safety margin
  2024-04-04 15:33 [PATCH v2] watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate 5% safety margin Judith Mendez
  2024-04-05 13:11 ` Rafael Beims
@ 2024-04-06 13:01 ` Guenter Roeck
  2024-04-09 19:37   ` Judith Mendez
  1 sibling, 1 reply; 6+ messages in thread
From: Guenter Roeck @ 2024-04-06 13:01 UTC (permalink / raw)
  To: Judith Mendez
  Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel, Francesco Dolcini

On Thu, Apr 04, 2024 at 10:33:19AM -0500, Judith Mendez wrote:
> On AM62x, the watchdog is pet before the valid window
> is open. Fix min_hw_heartbeat and accommodate a 5% safety
> margin with the exception of open window size < 10%,
> which shall use <5% due to the smaller open window size.
> 
> cc: stable@vger.kernel.org
> Fixes: 5527483f8f7c (" watchdog: rti-wdt: attach to running watchdog during probe")
> Signed-off-by: Judith Mendez <jm@ti.com>

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

> ---
>  drivers/watchdog/rti_wdt.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
> index 8e1be7ba0103..0b16ada659cc 100644
> --- a/drivers/watchdog/rti_wdt.c
> +++ b/drivers/watchdog/rti_wdt.c
> @@ -92,7 +92,7 @@ static int rti_wdt_start(struct watchdog_device *wdd)
>  	 * to be 50% or less than that; we obviouly want to configure the open
>  	 * window as large as possible so we select the 50% option.
>  	 */
> -	wdd->min_hw_heartbeat_ms = 500 * wdd->timeout;
> +	wdd->min_hw_heartbeat_ms = 550 * wdd->timeout;
>  
>  	/* Generate NMI when wdt expires */
>  	writel_relaxed(RTIWWDRX_NMI, wdt->base + RTIWWDRXCTRL);
> @@ -126,31 +126,33 @@ static int rti_wdt_setup_hw_hb(struct watchdog_device *wdd, u32 wsize)
>  	 * be petted during the open window; not too early or not too late.
>  	 * The HW configuration options only allow for the open window size
>  	 * to be 50% or less than that.
> +	 * To avoid any glitches, we accommodate 5% safety margin, with the
> +	 * exception of open window size < 10%.
>  	 */
>  	switch (wsize) {
>  	case RTIWWDSIZE_50P:
> -		/* 50% open window => 50% min heartbeat */
> -		wdd->min_hw_heartbeat_ms = 500 * heartbeat;
> +		/* 50% open window => 55% min heartbeat */
> +		wdd->min_hw_heartbeat_ms = 550 * heartbeat;
>  		break;
>  
>  	case RTIWWDSIZE_25P:
> -		/* 25% open window => 75% min heartbeat */
> -		wdd->min_hw_heartbeat_ms = 750 * heartbeat;
> +		/* 25% open window => 80% min heartbeat */
> +		wdd->min_hw_heartbeat_ms = 800 * heartbeat;
>  		break;
>  
>  	case RTIWWDSIZE_12P5:
> -		/* 12.5% open window => 87.5% min heartbeat */
> -		wdd->min_hw_heartbeat_ms = 875 * heartbeat;
> +		/* 12.5% open window => 92.5% min heartbeat */
> +		wdd->min_hw_heartbeat_ms = 925 * heartbeat;
>  		break;
>  
>  	case RTIWWDSIZE_6P25:
> -		/* 6.5% open window => 93.5% min heartbeat */
> -		wdd->min_hw_heartbeat_ms = 935 * heartbeat;
> +		/* 6.5% open window => 96.5% min heartbeat */
> +		wdd->min_hw_heartbeat_ms = 965 * heartbeat;
>  		break;
>  
>  	case RTIWWDSIZE_3P125:
> -		/* 3.125% open window => 96.9% min heartbeat */
> -		wdd->min_hw_heartbeat_ms = 969 * heartbeat;
> +		/* 3.125% open window => 97.9% min heartbeat */
> +		wdd->min_hw_heartbeat_ms = 979 * heartbeat;
>  		break;
>  
>  	default:
> 
> base-commit: 860bbe8e618fd62446309e286ab4a83d38201c0a
> -- 
> 2.43.2
> 
> 

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

* Re: [PATCH v2] watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate 5% safety margin
  2024-04-06 13:01 ` Guenter Roeck
@ 2024-04-09 19:37   ` Judith Mendez
  2024-04-09 19:52     ` Francesco Dolcini
  0 siblings, 1 reply; 6+ messages in thread
From: Judith Mendez @ 2024-04-09 19:37 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel, Francesco Dolcini

Hi all,

On 4/6/24 8:01 AM, Guenter Roeck wrote:
> On Thu, Apr 04, 2024 at 10:33:19AM -0500, Judith Mendez wrote:
>> On AM62x, the watchdog is pet before the valid window
>> is open. Fix min_hw_heartbeat and accommodate a 5% safety
>> margin with the exception of open window size < 10%,
>> which shall use <5% due to the smaller open window size.

Please do not merge this patch, I will add an additional
patch removing the hack in this driver.

~ Judith

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

* Re: [PATCH v2] watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate 5% safety margin
  2024-04-09 19:37   ` Judith Mendez
@ 2024-04-09 19:52     ` Francesco Dolcini
  2024-04-09 21:44       ` Judith Mendez
  0 siblings, 1 reply; 6+ messages in thread
From: Francesco Dolcini @ 2024-04-09 19:52 UTC (permalink / raw)
  To: Judith Mendez
  Cc: Guenter Roeck, Wim Van Sebroeck, linux-watchdog, linux-kernel,
	Francesco Dolcini

On Tue, Apr 09, 2024 at 02:37:15PM -0500, Judith Mendez wrote:
> Hi all,
> 
> On 4/6/24 8:01 AM, Guenter Roeck wrote:
> > On Thu, Apr 04, 2024 at 10:33:19AM -0500, Judith Mendez wrote:
> > > On AM62x, the watchdog is pet before the valid window
> > > is open. Fix min_hw_heartbeat and accommodate a 5% safety
> > > margin with the exception of open window size < 10%,
> > > which shall use <5% due to the smaller open window size.
> 
> Please do not merge this patch, I will add an additional
> patch removing the hack in this driver.

Is the patch buggy, or you are just talking about an additional clean-up?
If it is an additional patch and this code is fine, why holding it back?

Thanks,
Francesco


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

* Re: [PATCH v2] watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate 5% safety margin
  2024-04-09 19:52     ` Francesco Dolcini
@ 2024-04-09 21:44       ` Judith Mendez
  0 siblings, 0 replies; 6+ messages in thread
From: Judith Mendez @ 2024-04-09 21:44 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Guenter Roeck, Wim Van Sebroeck, linux-watchdog, linux-kernel

Hi Francesco,

On 4/9/24 2:52 PM, Francesco Dolcini wrote:
> On Tue, Apr 09, 2024 at 02:37:15PM -0500, Judith Mendez wrote:
>> Hi all,
>>
>> On 4/6/24 8:01 AM, Guenter Roeck wrote:
>>> On Thu, Apr 04, 2024 at 10:33:19AM -0500, Judith Mendez wrote:
>>>> On AM62x, the watchdog is pet before the valid window
>>>> is open. Fix min_hw_heartbeat and accommodate a 5% safety
>>>> margin with the exception of open window size < 10%,
>>>> which shall use <5% due to the smaller open window size.
>>
>> Please do not merge this patch, I will add an additional
>> patch removing the hack in this driver.
> 
> Is the patch buggy, or you are just talking about an additional clean-up?
> If it is an additional patch and this code is fine, why holding it back?


1. If we leave the hack, the hack shifts the valid window. This
is not desirable behavior. It is better to add a safety margin
that works in all possible scenarios.

2. 5% safety margin works in almost all cases except when the
timeout is < 5s. This due to a requirement of a min of 0.25s for the
safety margin due max possible error. At < 5s timeout, 5% is not
enough.

I am not 100% sure if using < 5s timeout is a real use-case
but I would rather use a safety margin that works for all cases.

~ Judith



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

end of thread, other threads:[~2024-04-09 21:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-04 15:33 [PATCH v2] watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate 5% safety margin Judith Mendez
2024-04-05 13:11 ` Rafael Beims
2024-04-06 13:01 ` Guenter Roeck
2024-04-09 19:37   ` Judith Mendez
2024-04-09 19:52     ` Francesco Dolcini
2024-04-09 21:44       ` Judith Mendez

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).