Linux Watchdog driver development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Paul Louvel" <paul.louvel@bootlin.com>
Cc: linux-watchdog@vger.kernel.org
Subject: Re: [PATCH v3 2/2] watchdog: w83627hf_wdt: Use WDOG_HW_RUNNING for running chip on boot
Date: Mon, 06 Jul 2026 10:40:08 +0000	[thread overview]
Message-ID: <20260706104010.A345B1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260706-w83627hf_wdt-nct6126d-v3-2-fa5ad8d486bc@bootlin.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the manual hardware timeout refresh while still unconditionally switching the hardware timer base from minutes to seconds drastically truncates the remaining timeout if the BIOS originally configured it in minutes, risking unexpected system reboots.
--

commit 0014048ec369e2a09098aa033aeb725449e2fd67
Author: Paul Louvel <paul.louvel@bootlin.com>

watchdog: w83627hf_wdt: Use WDOG_HW_RUNNING for running chip on boot

This patch updates the w83627hf watchdog driver to rely on the watchdog
core's WDOG_HW_RUNNING flag to keep a running watchdog alive during boot.
It replaces the previous behavior of manually issuing a single keepalive
during initialization.

> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c
[ ... ]
> @@ -241,9 +241,8 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
>  			pr_warn("Stopping previously enabled watchdog until userland kicks in\n");
>  			superio_outb(cr_wdt_timeout, 0);
>  		} else {
> -			pr_info("Watchdog already running. Resetting timeout to %d sec\n",
> -				wdog->timeout);
> -			superio_outb(cr_wdt_timeout, wdog->timeout);
> +			pr_info("Watchdog already running.\n");
> +			set_bit(WDOG_HW_RUNNING, &wdog->status);
>  		}
>  	}
>  

[Severity: High]
Does removing this explicit timeout reset create a risk of unexpected reboots?

If the BIOS originally configured the watchdog with a timeout in minutes
(for example, a timeout of 5 minutes), the hardware counter cr_wdt_timeout
will hold the value 5.

Because this patch removes the explicit write of wdog->timeout (which is
typically 60) to cr_wdt_timeout, the counter retains the raw value 5.
Just after this block in w83627hf_init(), the driver unconditionally
switches the timebase to seconds:

w83627hf_init() {
    ...
    /* set second mode & disable keyboard turning off watchdog */
    t = superio_inb(cr_wdt_control) & ~0x0C;
    superio_outb(cr_wdt_control, t);
    ...
}

This immediately shrinks the remaining timeout from 5 minutes to 5 seconds.

If the watchdog core's kworker is delayed, or if the system is configured
not to ping the watchdog before user space takes over, could this unusually
short timeout cause the system to reboot unexpectedly during early boot?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706-w83627hf_wdt-nct6126d-v3-0-fa5ad8d486bc@bootlin.com?part=2

  reply	other threads:[~2026-07-06 10:40 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-06 10:29 [PATCH v3 0/2] watchdog: w83627hf_wdt: Add NCT6126 support and improve handling of running watchdog Paul Louvel
2026-07-06 10:29 ` [PATCH v3 1/2] watchdog: w83627hf_wdt: Add support for Nuvoton NCT6126 Paul Louvel
2026-07-06 14:14   ` Guenter Roeck
2026-07-06 15:37     ` Paul Louvel
2026-07-06 10:29 ` [PATCH v3 2/2] watchdog: w83627hf_wdt: Use WDOG_HW_RUNNING for running chip on boot Paul Louvel
2026-07-06 10:40   ` sashiko-bot [this message]
2026-07-06 11:15     ` Paul Louvel
2026-07-06 14:22       ` Guenter Roeck
2026-07-06 15:44         ` Paul Louvel
2026-07-06 16:12           ` Guenter Roeck
2026-07-06 21:42             ` Paul Louvel
2026-07-06 21:48               ` Guenter Roeck
2026-07-07  7:47                 ` Paul Louvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260706104010.A345B1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=paul.louvel@bootlin.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox