Re: [PATCH 2/2] watchdog: rti: tweak min_hw_heartbeat_ms to match initial allowed window

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 25/06/2020 16:35, Guenter Roeck wrote:
On 6/25/20 1:32 AM, Tero Kristo wrote:
On 24/06/2020 18:24, Jan Kiszka wrote:
On 24.06.20 13:45, Tero Kristo wrote:
If the RTI watchdog has been started by someone (like bootloader) when
the driver probes, we must adjust the initial ping timeout to match the
currently running watchdog window to avoid generating watchdog reset.

Signed-off-by: Tero Kristo <t-kristo@xxxxxx>
---
   drivers/watchdog/rti_wdt.c | 25 +++++++++++++++++++++++++
   1 file changed, 25 insertions(+)

diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c
index d456dd72d99a..02ea2b2435f5 100644
--- a/drivers/watchdog/rti_wdt.c
+++ b/drivers/watchdog/rti_wdt.c
@@ -55,11 +55,13 @@ static int heartbeat;
    * @base - base io address of WD device
    * @freq - source clock frequency of WDT
    * @wdd  - hold watchdog device as is in WDT core
+ * @min_hw_heartbeat_save - save of the min hw heartbeat value
    */
   struct rti_wdt_device {
       void __iomem        *base;
       unsigned long        freq;
       struct watchdog_device    wdd;
+    unsigned int        min_hw_heartbeat_save;
   };
   static int rti_wdt_start(struct watchdog_device *wdd)
@@ -107,6 +109,11 @@ static int rti_wdt_ping(struct watchdog_device *wdd)
       /* put watchdog in active state */
       writel_relaxed(WDKEY_SEQ1, wdt->base + RTIWDKEY);
+    if (wdt->min_hw_heartbeat_save) {
+        wdd->min_hw_heartbeat_ms = wdt->min_hw_heartbeat_save;
+        wdt->min_hw_heartbeat_save = 0;
+    }
+
       return 0;
   }
@@ -201,6 +208,24 @@ static int rti_wdt_probe(struct platform_device *pdev)
           goto err_iomap;
       }
+    if (readl(wdt->base + RTIDWDCTRL) == WDENABLE_KEY) {
+        u32 time_left;
+        u32 heartbeat;
+
+        set_bit(WDOG_HW_RUNNING, &wdd->status);
+        time_left = rti_wdt_get_timeleft(wdd);
+        heartbeat = readl(wdt->base + RTIDWDPRLD);
+        heartbeat <<= WDT_PRELOAD_SHIFT;
+        heartbeat /= wdt->freq;
+        if (time_left < heartbeat / 2)
+            wdd->min_hw_heartbeat_ms = 0;
+        else
+            wdd->min_hw_heartbeat_ms =
+                (time_left - heartbeat / 2 + 1) * 1000;
+
+        wdt->min_hw_heartbeat_save = 11 * heartbeat * 1000 / 20;
+    }
+
       ret = watchdog_register_device(wdd);
       if (ret) {
           dev_err(dev, "cannot register watchdog device\n");


This assumes that the bootloader also programmed a 50% window, right? The pending U-Boot patch will do that, but what if that may chance or someone uses a different setup?

Yes, we assume 50%. I think based on the hw design, 50% is the only sane value to be used, otherwise you just shrink the open window too much and for no apparent reason.


Not sure if that is a valid assumption. Someone who designs a watchdog
with such a narrow ping window might as well also use it. The question
is if you want to rely on that assumption, or check and change it if needed.

Right, if that is a blocker, I can modify the code. Should be maybe couple of lines addition.

Also, I wonder if we should add an API function such as
"set_last_hw_keepalive()" to avoid all that complexity.

I can try adding that also if it is desirable.

-Tero
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux