diff mbox series

watchdog: sunxi_wdt: preserve boot-enabled watchdog

Message ID 20260827-submit-sunxi-wdt-boot-enabled-v1-v1-1-d16f934cc809@gmail.com (mailing list archive)
State New
Headers
Series watchdog: sunxi_wdt: preserve boot-enabled watchdog |

Commit Message

James Hilliard Aug. 27, 2026, 11:42 p.m. UTC 
sunxi_wdt_probe() unconditionally stops the watchdog even when firmware
left it running. This opens an unprotected interval during boot and
prevents CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED from taking over the active
watchdog.

Detect an enabled watchdog and decode its programmed interval. Preserve
that timeout while setting the Linux reset mode and pinging the watchdog;
use the configured timeout only when the hardware interval cannot be
represented. The enable bit remains set throughout. Mark it as
hardware-running before registration so the watchdog core services it
until userspace takes control. Keep the existing stop operation when the
watchdog was disabled.

Fixes: d00680ed0026 ("watchdog: sunxi: New watchdog driver for Allwinner A10/A13")
Cc: stable@vger.kernel.org
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/watchdog/sunxi_wdt.c | 47 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)


---
base-commit: 1b78070aaef63512688aebfbc82365ef9d6660f1
change-id: 20260827-submit-sunxi-wdt-boot-enabled-v1-94cd6be885d2

Best regards,
--  
James Hilliard <james.hilliard1@gmail.com>

Comments

Guenter Roeck Aug. 28, 2026, 4:25 a.m. UTC | #1 
On 8/27/26 16:42, James Hilliard wrote:
> sunxi_wdt_probe() unconditionally stops the watchdog even when firmware
> left it running. This opens an unprotected interval during boot and
> prevents CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED from taking over the active
> watchdog.
> 
> Detect an enabled watchdog and decode its programmed interval. Preserve
> that timeout while setting the Linux reset mode and pinging the watchdog;
> use the configured timeout only when the hardware interval cannot be
> represented. The enable bit remains set throughout. Mark it as
> hardware-running before registration so the watchdog core services it
> until userspace takes control. Keep the existing stop operation when the
> watchdog was disabled.
> 
> Fixes: d00680ed0026 ("watchdog: sunxi: New watchdog driver for Allwinner A10/A13")
> Cc: stable@vger.kernel.org
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>   drivers/watchdog/sunxi_wdt.c | 47 +++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 46 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/sunxi_wdt.c b/drivers/watchdog/sunxi_wdt.c
> index b6c761acc3de..1f3da1d47fa5 100644
> --- a/drivers/watchdog/sunxi_wdt.c
> +++ b/drivers/watchdog/sunxi_wdt.c
> @@ -128,6 +128,38 @@ static int sunxi_wdt_ping(struct watchdog_device *wdt_dev)
>   	return 0;
>   }
>   
> +static bool sunxi_wdt_is_running(struct watchdog_device *wdt_dev)
> +{
> +	struct sunxi_wdt_dev *sunxi_wdt = watchdog_get_drvdata(wdt_dev);
> +	const struct sunxi_wdt_reg *regs = sunxi_wdt->wdt_regs;
> +
> +	return readl(sunxi_wdt->wdt_base + regs->wdt_mode) & WDT_MODE_EN;
> +}
> +
> +static unsigned int sunxi_wdt_get_timeout(struct watchdog_device *wdt_dev)
> +{
> +	struct sunxi_wdt_dev *sunxi_wdt = watchdog_get_drvdata(wdt_dev);
> +	const struct sunxi_wdt_reg *regs = sunxi_wdt->wdt_regs;
> +	unsigned int timeout;
> +	u32 interval;
> +
> +	interval = readl(sunxi_wdt->wdt_base + regs->wdt_mode);
> +	interval >>= regs->wdt_timeout_shift;
> +	interval &= WDT_TIMEOUT_MASK;
> +	/* The 0.5-second interval cannot be represented by wdt_dev->timeout. */
> +	if (!interval)
> +		return 0;
> +

Why 0 and not 1 ? If I understand correctly, this sets the timeout
to  WDT_MAX_TIMEOUT or 16 seconds. It is not immediately obvious why
that would be desirable.

> +	for (timeout = WDT_MIN_TIMEOUT;
> +	     timeout < ARRAY_SIZE(wdt_timeout_map); timeout++) {
> +		if (wdt_timeout_map[timeout] == interval)
> +			return timeout;
> +	}
> +
> +	/* Reserved interval encoding. */
> +	return 0;
> +}
> +
>   static int sunxi_wdt_set_timeout(struct watchdog_device *wdt_dev,
>   		unsigned int timeout)
>   {
> @@ -259,6 +291,7 @@ static int sunxi_wdt_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct sunxi_wdt_dev *sunxi_wdt;
> +	unsigned int running_timeout;
>   	int err;
>   
>   	sunxi_wdt = devm_kzalloc(dev, sizeof(*sunxi_wdt), GFP_KERNEL);
> @@ -286,7 +319,19 @@ 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_is_running(&sunxi_wdt->wdt_dev)) {
> +		running_timeout = sunxi_wdt_get_timeout(&sunxi_wdt->wdt_dev);
> +		if (running_timeout)
> +			sunxi_wdt->wdt_dev.timeout = running_timeout;
> +
> +		err = sunxi_wdt_start(&sunxi_wdt->wdt_dev);
> +		if (err)
> +			return err;
> +
> +		set_bit(WDOG_HW_RUNNING, &sunxi_wdt->wdt_dev.status);
> +	} else {
> +		sunxi_wdt_stop(&sunxi_wdt->wdt_dev);

Why is this necessary ? The watchdog is known to be stopped, why
stop it again ?

Thanks,
Guenter

> +	}
>   
>   	watchdog_stop_on_reboot(&sunxi_wdt->wdt_dev);
>   	err = devm_watchdog_register_device(dev, &sunxi_wdt->wdt_dev);
> 
> ---
> base-commit: 1b78070aaef63512688aebfbc82365ef9d6660f1
> change-id: 20260827-submit-sunxi-wdt-boot-enabled-v1-94cd6be885d2
> 
> Best regards,
> --
> James Hilliard <james.hilliard1@gmail.com>
diff mbox series

Patch

diff --git a/drivers/watchdog/sunxi_wdt.c b/drivers/watchdog/sunxi_wdt.c
index b6c761acc3de..1f3da1d47fa5 100644
--- a/drivers/watchdog/sunxi_wdt.c
+++ b/drivers/watchdog/sunxi_wdt.c
@@ -128,6 +128,38 @@  static int sunxi_wdt_ping(struct watchdog_device *wdt_dev)
 	return 0;
 }
 
+static bool sunxi_wdt_is_running(struct watchdog_device *wdt_dev)
+{
+	struct sunxi_wdt_dev *sunxi_wdt = watchdog_get_drvdata(wdt_dev);
+	const struct sunxi_wdt_reg *regs = sunxi_wdt->wdt_regs;
+
+	return readl(sunxi_wdt->wdt_base + regs->wdt_mode) & WDT_MODE_EN;
+}
+
+static unsigned int sunxi_wdt_get_timeout(struct watchdog_device *wdt_dev)
+{
+	struct sunxi_wdt_dev *sunxi_wdt = watchdog_get_drvdata(wdt_dev);
+	const struct sunxi_wdt_reg *regs = sunxi_wdt->wdt_regs;
+	unsigned int timeout;
+	u32 interval;
+
+	interval = readl(sunxi_wdt->wdt_base + regs->wdt_mode);
+	interval >>= regs->wdt_timeout_shift;
+	interval &= WDT_TIMEOUT_MASK;
+	/* The 0.5-second interval cannot be represented by wdt_dev->timeout. */
+	if (!interval)
+		return 0;
+
+	for (timeout = WDT_MIN_TIMEOUT;
+	     timeout < ARRAY_SIZE(wdt_timeout_map); timeout++) {
+		if (wdt_timeout_map[timeout] == interval)
+			return timeout;
+	}
+
+	/* Reserved interval encoding. */
+	return 0;
+}
+
 static int sunxi_wdt_set_timeout(struct watchdog_device *wdt_dev,
 		unsigned int timeout)
 {
@@ -259,6 +291,7 @@  static int sunxi_wdt_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct sunxi_wdt_dev *sunxi_wdt;
+	unsigned int running_timeout;
 	int err;
 
 	sunxi_wdt = devm_kzalloc(dev, sizeof(*sunxi_wdt), GFP_KERNEL);
@@ -286,7 +319,19 @@  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_is_running(&sunxi_wdt->wdt_dev)) {
+		running_timeout = sunxi_wdt_get_timeout(&sunxi_wdt->wdt_dev);
+		if (running_timeout)
+			sunxi_wdt->wdt_dev.timeout = running_timeout;
+
+		err = sunxi_wdt_start(&sunxi_wdt->wdt_dev);
+		if (err)
+			return err;
+
+		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);