diff mbox series

[v3] watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate a safety margin

Message ID 20240417205700.3947408-1-jm@ti.com (mailing list archive)
State New
Headers show
Series [v3] watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate a safety margin | expand

Commit Message

Judith Mendez April 17, 2024, 8:57 p.m. UTC
On AM62x, the watchdog is pet before the valid window is open. Fix
min_hw_heartbeat and accommodate a 2% + static offset safety margin.
The static offset accounts for max hardware error.

Remove the hack in the driver which shifts the open window boundary,
since it is no longer necessary due to the fix mentioned above.

cc: stable@vger.kernel.org
Fixes: 5527483f8f7c ("watchdog: rti-wdt: attach to running watchdog during probe")
Signed-off-by: Judith Mendez <jm@ti.com>
---
Changes since v2:
- Change safety margin to 2% + max hardware error
- remove hack that shifts open window boundary
---
 drivers/watchdog/rti_wdt.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)


base-commit: 860bbe8e618fd62446309e286ab4a83d38201c0a

Comments

Guenter Roeck April 17, 2024, 9:10 p.m. UTC | #1
On Wed, Apr 17, 2024 at 03:57:00PM -0500, Judith Mendez wrote:
> On AM62x, the watchdog is pet before the valid window is open. Fix
> min_hw_heartbeat and accommodate a 2% + static offset safety margin.
> The static offset accounts for max hardware error.
> 
> Remove the hack in the driver which shifts the open window boundary,
> since it is no longer necessary due to the fix mentioned above.
> 
> 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>
Judith Mendez May 8, 2024, 6:50 p.m. UTC | #2
Hi all,

On 4/17/24 3:57 PM, Judith Mendez wrote:
> On AM62x, the watchdog is pet before the valid window is open. Fix
> min_hw_heartbeat and accommodate a 2% + static offset safety margin.
> The static offset accounts for max hardware error.
> 
> Remove the hack in the driver which shifts the open window boundary,
> since it is no longer necessary due to the fix mentioned above.
> 

This patch has been sitting and there are no comments.
Please let me know if there is any issue with this patch, since I would
like to get this fix merged (:

Thanks,
Judith
Saulnier, Nick May 8, 2024, 8:32 p.m. UTC | #3
Tested on my side with the default 50% window across a range of periods. Works as expected. Details at 
https://e2e.ti.com/support/processors-group/processors/f/processors-forum/1338070/am625-how-to-control-the-watchdog/5166032#5166032 

Regards,
Nick Saulnier

-----Original Message-----
From: Mendez, Judith 
Sent: Wednesday, May 08, 2024 1:50 PM
To: Wim Van Sebroeck <wim@linux-watchdog.org>; Guenter Roeck <linux@roeck-us.net>
Cc: linux-watchdog@vger.kernel.org; linux-kernel@vger.kernel.org; Rafael Beims <rafael.beims@toradex.com>; Raghavendra, Vignesh <vigneshr@ti.com>; Saulnier, Nick <nsaulnier@ti.com>
Subject: Re: [PATCH v3] watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate a safety margin

Hi all,

On 4/17/24 3:57 PM, Judith Mendez wrote:
> On AM62x, the watchdog is pet before the valid window is open. Fix 
> min_hw_heartbeat and accommodate a 2% + static offset safety margin.
> The static offset accounts for max hardware error.
> 
> Remove the hack in the driver which shifts the open window boundary, 
> since it is no longer necessary due to the fix mentioned above.
> 

This patch has been sitting and there are no comments.
Please let me know if there is any issue with this patch, since I would like to get this fix merged (:

Thanks,
Judith
diff mbox series

Patch

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index 8e1be7ba01039..c7d4de28e1891 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -59,6 +59,8 @@ 
 #define PON_REASON_EOF_NUM	0xCCCCBBBB
 #define RESERVED_MEM_MIN_SIZE	12
 
+#define MAX_HW_ERROR		250
+
 static int heartbeat = DEFAULT_HEARTBEAT;
 
 /*
@@ -92,7 +94,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 = 520 * wdd->timeout + MAX_HW_ERROR;
 
 	/* Generate NMI when wdt expires */
 	writel_relaxed(RTIWWDRX_NMI, wdt->base + RTIWWDRXCTRL);
@@ -126,31 +128,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 2% + max hardware error
+	 * safety margin.
 	 */
 	switch (wsize) {
 	case RTIWWDSIZE_50P:
-		/* 50% open window => 50% min heartbeat */
-		wdd->min_hw_heartbeat_ms = 500 * heartbeat;
+		/* 50% open window => 52% min heartbeat */
+		wdd->min_hw_heartbeat_ms = 520 * heartbeat + MAX_HW_ERROR;
 		break;
 
 	case RTIWWDSIZE_25P:
-		/* 25% open window => 75% min heartbeat */
-		wdd->min_hw_heartbeat_ms = 750 * heartbeat;
+		/* 25% open window => 77% min heartbeat */
+		wdd->min_hw_heartbeat_ms = 770 * heartbeat + MAX_HW_ERROR;
 		break;
 
 	case RTIWWDSIZE_12P5:
-		/* 12.5% open window => 87.5% min heartbeat */
-		wdd->min_hw_heartbeat_ms = 875 * heartbeat;
+		/* 12.5% open window => 89.5% min heartbeat */
+		wdd->min_hw_heartbeat_ms = 895 * heartbeat + MAX_HW_ERROR;
 		break;
 
 	case RTIWWDSIZE_6P25:
-		/* 6.5% open window => 93.5% min heartbeat */
-		wdd->min_hw_heartbeat_ms = 935 * heartbeat;
+		/* 6.5% open window => 95.5% min heartbeat */
+		wdd->min_hw_heartbeat_ms = 955 * heartbeat + MAX_HW_ERROR;
 		break;
 
 	case RTIWWDSIZE_3P125:
-		/* 3.125% open window => 96.9% min heartbeat */
-		wdd->min_hw_heartbeat_ms = 969 * heartbeat;
+		/* 3.125% open window => 98.9% min heartbeat */
+		wdd->min_hw_heartbeat_ms = 989 * heartbeat + MAX_HW_ERROR;
 		break;
 
 	default:
@@ -228,14 +232,6 @@  static int rti_wdt_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	/*
-	 * If watchdog is running at 32k clock, it is not accurate.
-	 * Adjust frequency down in this case so that we don't pet
-	 * the watchdog too often.
-	 */
-	if (wdt->freq < 32768)
-		wdt->freq = wdt->freq * 9 / 10;
-
 	pm_runtime_enable(dev);
 	ret = pm_runtime_resume_and_get(dev);
 	if (ret < 0) {