Re: [PATCH v3] watchdog: sunxi_wdt: Allow watchdog to remain enabled after probe
From: Guenter Roeck <linux@roeck-us.net>
Date: 2025-02-25 21:51:36
Also in:
linux-sunxi, linux-watchdog, lkml
On 2/25/25 06:36, Regis Dargent wrote:
quoted hunk ↗ jump to hunk
If the watchdog is already running during probe, let it run on, read its configured timeout, and set its status so that it is correctly handled by the kernel. Signed-off-by: Regis Dargent <redacted> -- Changelog v1..v2: - add sunxi_wdt_read_timeout function - add signed-off-by tag Changelog v2..v3: - WDIOF_SETTIMEOUT was set twice, and other code cleanup --- drivers/watchdog/sunxi_wdt.c | 45 ++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-)diff --git a/drivers/watchdog/sunxi_wdt.c b/drivers/watchdog/sunxi_wdt.c index b85354a99582..d509dbcb77ce 100644 --- a/drivers/watchdog/sunxi_wdt.c +++ b/drivers/watchdog/sunxi_wdt.c@@ -140,6 +140,7 @@ static int sunxi_wdt_set_timeout(struct watchdog_device *wdt_dev, timeout++; sunxi_wdt->wdt_dev.timeout = timeout; + sunxi_wdt->wdt_dev.max_hw_heartbeat_ms = 0; reg = readl(wdt_base + regs->wdt_mode); reg &= ~(WDT_TIMEOUT_MASK << regs->wdt_timeout_shift);@@ -152,6 +153,32 @@ static int sunxi_wdt_set_timeout(struct watchdog_device *wdt_dev, return 0; } +static int sunxi_wdt_read_timeout(struct watchdog_device *wdt_dev) +{ + struct sunxi_wdt_dev *sunxi_wdt = watchdog_get_drvdata(wdt_dev); + void __iomem *wdt_base = sunxi_wdt->wdt_base; + const struct sunxi_wdt_reg *regs = sunxi_wdt->wdt_regs; + int i; + u32 reg; + + reg = readl(wdt_base + regs->wdt_mode); + reg >>= regs->wdt_timeout_shift; + reg &= WDT_TIMEOUT_MASK; + + /* Start at 0 which actually means 0.5s */ + for (i = 0; (i < WDT_MAX_TIMEOUT) && (wdt_timeout_map[i] != reg); i++)
Unnecessary (). On top of that, its complexity is unnecessary. The timeout in seconds, except for reg == 0, is wdt_timeout_map[reg], with values of 0x0c..0x0f undefined. Worse, the above code can access beyond the size of wdt_timeout_map[] if reg >= 0x0c.
+ ;
+ if (i == 0) {
+ wdt_dev->timeout = 1;
+ wdt_dev->max_hw_heartbeat_ms = 500;This is an unacceptable API abuse. max_hw_heartbeat_ms, if set, should be 16000, not 500. You could set the timeout to 1 second instead. Guenter