[PATCH] watchdog: rti: drop hack manipulating WDT clock rate
From: Alexander Sverdlin alexander.sverdlin@siemens.com
The hack itself seems to be copied from Linux rti_wdt.c, but the WDT reset principle is different in U-Boot. While Linux relies on correct frequencies and timers and doesn't check the actual WDT counter value U-Boot driver seems to be more rubust: it does compare RTIDWDCNTR vs RTIDWDPRLD.
Now the root cause of the original motivation to manipulate the clock rate is said to be understood and fixed in Linux commit cae58516534e ("watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate a safety margin") which simultaneously removed the hack itself.
While is fix part of the mentioned patch is neither applicable nor requied for the U-Boot driver just drop the hack setting WDT clock rate to 90% of the real rate. This has a nice effect that the WDT timeout is now as requested and not 10% shorter.
Signed-off-by: Alexander Sverdlin alexander.sverdlin@siemens.com --- drivers/watchdog/rti_wdt.c | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c index 99168d0cad03..f806e24e53da 100644 --- a/drivers/watchdog/rti_wdt.c +++ b/drivers/watchdog/rti_wdt.c @@ -186,14 +186,6 @@ static int rti_wdt_probe(struct udevice *dev)
priv->clk_hz = clk_get_rate(&clk);
- /* - * If watchdog is running at 32k clock, it is not accurate. - * Adjust frequency down in this case so that it does not expire - * earlier than expected. - */ - if (priv->clk_hz < 32768) - priv->clk_hz = priv->clk_hz * 9 / 10; - return 0; }
On 20.11.24 23:24, A. Sverdlin wrote:
From: Alexander Sverdlin alexander.sverdlin@siemens.com
The hack itself seems to be copied from Linux rti_wdt.c, but the WDT reset principle is different in U-Boot. While Linux relies on correct frequencies and timers and doesn't check the actual WDT counter value U-Boot driver seems to be more rubust: it does compare RTIDWDCNTR vs RTIDWDPRLD.
robust
Now the root cause of the original motivation to manipulate the clock rate is said to be understood and fixed in Linux commit cae58516534e ("watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate a safety margin") which simultaneously removed the hack itself.
While is fix part of the mentioned patch is neither applicable nor requied
required
for the U-Boot driver just drop the hack setting WDT clock rate to 90% of the real rate. This has a nice effect that the WDT timeout is now as requested and not 10% shorter.
Signed-off-by: Alexander Sverdlin alexander.sverdlin@siemens.com
drivers/watchdog/rti_wdt.c | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c index 99168d0cad03..f806e24e53da 100644 --- a/drivers/watchdog/rti_wdt.c +++ b/drivers/watchdog/rti_wdt.c @@ -186,14 +186,6 @@ static int rti_wdt_probe(struct udevice *dev)
priv->clk_hz = clk_get_rate(&clk);
- /*
* If watchdog is running at 32k clock, it is not accurate.* Adjust frequency down in this case so that it does not expire* earlier than expected.*/- if (priv->clk_hz < 32768)
priv->clk_hz = priv->clk_hz * 9 / 10;- return 0;
}
Makes sense when the kernel is updated to a stable version that contains "watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate a safety margin" because the intention of this change was "to have comparable preset values for both drivers".
Heads up for my colleagues (you already use such a kernel) and a
Reviewed-by: Jan Kiszka jan.kiszka@siemens.com
for this here.
Jan
Thanks for quick feedback, Jan!
On Thu, 2024-11-21 at 07:16 +0100, Jan Kiszka wrote:
On 20.11.24 23:24, A. Sverdlin wrote:
From: Alexander Sverdlin alexander.sverdlin@siemens.com
The hack itself seems to be copied from Linux rti_wdt.c, but the WDT reset principle is different in U-Boot. While Linux relies on correct frequencies and timers and doesn't check the actual WDT counter value U-Boot driver seems to be more rubust: it does compare RTIDWDCNTR vs RTIDWDPRLD.
robust
Now the root cause of the original motivation to manipulate the clock rate is said to be understood and fixed in Linux commit cae58516534e ("watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate a safety margin") which simultaneously removed the hack itself.
While is fix part of the mentioned patch is neither applicable nor requied
required
I'll re-spin with those two typos corrected.
for the U-Boot driver just drop the hack setting WDT clock rate to 90% of the real rate. This has a nice effect that the WDT timeout is now as requested and not 10% shorter.
Signed-off-by: Alexander Sverdlin alexander.sverdlin@siemens.com
drivers/watchdog/rti_wdt.c | 8 -------- 1 file changed, 8 deletions(-)
diff --git a/drivers/watchdog/rti_wdt.c b/drivers/watchdog/rti_wdt.c index 99168d0cad03..f806e24e53da 100644 --- a/drivers/watchdog/rti_wdt.c +++ b/drivers/watchdog/rti_wdt.c @@ -186,14 +186,6 @@ static int rti_wdt_probe(struct udevice *dev) priv->clk_hz = clk_get_rate(&clk);
- /*
* If watchdog is running at 32k clock, it is not accurate.* Adjust frequency down in this case so that it does not expire* earlier than expected.*/- if (priv->clk_hz < 32768)
priv->clk_hz = priv->clk_hz * 9 / 10;return 0; }
Makes sense when the kernel is updated to a stable version that contains "watchdog: rti_wdt: Set min_hw_heartbeat_ms to accommodate a safety margin" because the intention of this change was "to have comparable preset values for both drivers".
Actually, I believe, this doesn't make any change on the kernel side, because it takes the configured RTIDWDPRLD as the base for its calculations of the period, but there is either "hack+no safety margin" or "no hack, but 2% safety margin" driver version.
Heads up for my colleagues (you already use such a kernel) and a
Reviewed-by: Jan Kiszka jan.kiszka@siemens.com
for this here.
participants (3)
-
A. Sverdlin -
Jan Kiszka -
Sverdlin, Alexander