| Message ID | 20250225143638.1989755-2-regis.dargent@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] watchdog: sunxi_wdt: Allow watchdog to remain enabled after probe | expand |
On 2/25/25 06:36, Regis Dargent wrote: > 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 <regis.dargent@gmail.com> > > -- > > 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
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++) + ; + if (i == 0) { + wdt_dev->timeout = 1; + wdt_dev->max_hw_heartbeat_ms = 500; + } else { + wdt_dev->timeout = i; + wdt_dev->max_hw_heartbeat_ms = 0; + } + + return 0; +} + static int sunxi_wdt_stop(struct watchdog_device *wdt_dev) { struct sunxi_wdt_dev *sunxi_wdt = watchdog_get_drvdata(wdt_dev); @@ -192,6 +219,16 @@ static int sunxi_wdt_start(struct watchdog_device *wdt_dev) return 0; } +static bool sunxi_wdt_enabled(struct sunxi_wdt_dev *wdt) +{ + void __iomem *wdt_base = wdt->wdt_base; + const struct sunxi_wdt_reg *regs = wdt->wdt_regs; + u32 reg; + + reg = readl(wdt_base + regs->wdt_mode); + return !!(reg & WDT_MODE_EN); +} + static const struct watchdog_info sunxi_wdt_info = { .identity = DRV_NAME, .options = WDIOF_SETTIMEOUT | @@ -275,8 +312,12 @@ static int sunxi_wdt_probe(struct platform_device *pdev) watchdog_set_drvdata(&sunxi_wdt->wdt_dev, sunxi_wdt); - sunxi_wdt_stop(&sunxi_wdt->wdt_dev); - + if (sunxi_wdt_enabled(sunxi_wdt)) { + sunxi_wdt_read_timeout(&sunxi_wdt->wdt_dev); + set_bit(WDOG_HW_RUNNING, &sunxi_wdt->wdt_dev.status); + } else { + sunxi_wdt_stop(&sunxi_wdt->wdt_dev); + } watchdog_stop_on_reboot(&sunxi_wdt->wdt_dev); err = devm_watchdog_register_device(dev, &sunxi_wdt->wdt_dev); if (unlikely(err))
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 <regis.dargent@gmail.com> -- 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(-)