Am 27.10.24 um 14:36 schrieb Guenter Roeck:
On 10/27/24 03:53, Stefan Wahren wrote:
The WDOG Timeout Value (TOVAL) is a 16 bit value, which is stored
at the beginning of a 32 bit register. So add a range check to
prevent writing in the reserved register area.
Signed-off-by: Stefan Wahren <wahrenst@xxxxxxx>
---
drivers/watchdog/imx7ulp_wdt.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/watchdog/imx7ulp_wdt.c
b/drivers/watchdog/imx7ulp_wdt.c
index 0f92d2217088..a7574f9c9150 100644
--- a/drivers/watchdog/imx7ulp_wdt.c
+++ b/drivers/watchdog/imx7ulp_wdt.c
@@ -48,6 +48,8 @@
#define RETRY_MAX 5
+#define TOVAL_MAX 0xFFFF
+
static bool nowayout = WATCHDOG_NOWAYOUT;
module_param(nowayout, bool, 0000);
MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started
(default="
@@ -192,6 +194,9 @@ static int imx7ulp_wdt_set_timeout(struct
watchdog_device *wdog,
int ret;
u32 loop = RETRY_MAX;
+ if (toval > TOVAL_MAX)
+ return -EINVAL;
+
The whole idea of having max_timeout in struct watchdog_device is to
avoid the need
for this check. max_timeout should be set to 0xffff /
wdt->hw->wdog_clock_rate.
It is currently set to 128. With wdt->hw->wdog_clock_rate set to
either 125 or 1000,
it can indeed overflow. However, checking the value above is wrong.
max_timeout should
be initialized correctly instead.
Even better would be to set max_hw_heartbeat_ms and let the watchdog
core handle
larger timeouts.
It's funny because I tried this on a i.MX93 board but it didn't work for
me. But I must confess that I didn't spend much time in the investigation.
Another question is why the driver enables a clock but doesn't use its
actual
frequency.
Yes, this would be better
Regards
Guenter
