Re: [PATCH v2] watchdog: sunxi_wdt: Allow watchdog to remain enabled after probe
From: Jernej Škrabec
Date: Thu Feb 20 2025 - 13:55:14 EST
Hi,
please send new versions as separate e-mail thread.
Dne petek, 14. februar 2025 ob 12:22:54 Srednjeevropski standardni čas je Regis Dargent napisal(a):
> 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@xxxxxxxxx>
> ---
> drivers/watchdog/sunxi_wdt.c | 48 +++++++++++++++++++++++++++++++++---
> 1 file changed, 45 insertions(+), 3 deletions(-)
You should add changelog here.
>
> diff --git a/drivers/watchdog/sunxi_wdt.c b/drivers/watchdog/sunxi_wdt.c
> index b85354a99582..0094bcd063c5 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++)
Drop parenthesis in condition.
> + ;
> + 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,11 +219,22 @@ static int sunxi_wdt_start(struct watchdog_device *wdt_dev)
> return 0;
> }
>
> +static bool sunxi_wdt_enabled(struct sunxi_wdt_dev *wdt)
> +{
> + u32 reg;
> + void __iomem *wdt_base = wdt->wdt_base;
> + const struct sunxi_wdt_reg *regs = wdt->wdt_regs;
Inverse christmas tree.
> +
> + 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 |
> WDIOF_KEEPALIVEPING |
> - WDIOF_MAGICCLOSE,
> + WDIOF_MAGICCLOSE |
> + WDIOF_SETTIMEOUT,
Drop this change. WDIOF_SETTIMEOUT is already defined.
> };
>
> static const struct watchdog_ops sunxi_wdt_ops = {
> @@ -275,8 +313,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)) {
Also check for IS_ENABLED(CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED). This will
allow user to select behaviour via Kconfig.
Best regards,
Jernej
> + 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))
>