* [PATCH v5 1/5] watchdog: w83627hf_wdt: Replace magic numbers with descriptive macros
2026-10-20 8:11 [PATCH v5 0/5] watchdog: w83627hf_wdt: Convert driver to the driver model and add minute support Paul Louvel
@ 2026-10-20 8:11 ` Paul Louvel
2026-10-20 8:11 ` [PATCH v5 2/5] watchdog: w83627hf_wdt: Convert to platform driver model Paul Louvel
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Paul Louvel @ 2026-10-20 8:11 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: Tzung-Bi Shih, linux-watchdog, linux-kernel, Thomas Petazzoni,
Paul Louvel
Raw hex values for Super I/O register addresses are difficult to
understand without constantly consulting the datasheet. Use named macros
instead.
Also, replace macros value that contains bit position with the BIT()
macro.
Remove comments that doesn't help understanding the code.
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/watchdog/w83627hf_wdt.c | 41 +++++++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index db77599e43a0..fa1808a4c6c8 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -27,6 +27,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/bits.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/types.h>
@@ -71,6 +72,12 @@ MODULE_PARM_DESC(early_disable, "Disable watchdog at boot time (default=0)");
* Kernel methods.
*/
+#define SIO_REG_LDSEL 0x07 /* Logical device select */
+#define SIO_REG_DEVID 0x20 /* Device ID (1 or 2 bytes) */
+#define SIO_REG_ENABLE 0x30 /* Logical device enable */
+#define SIO_CONF_PORT0 0x2E
+#define SIO_CONF_PORT1 0x4E
+
#define WDT_EFER (wdt_io+0) /* Extended Function Enable Registers */
#define WDT_EFIR (wdt_io+0) /* Extended Function Index Register
(same as EFER) */
@@ -115,9 +122,12 @@ MODULE_PARM_DESC(early_disable, "Disable watchdog at boot time (default=0)");
#define W836X7HF_WDT_CSR 0xf7
#define NCT6102D_WDT_CSR 0xf2
-#define WDT_CSR_STATUS 0x10
-#define WDT_CSR_KBD 0x40
-#define WDT_CSR_MOUSE 0x80
+#define WDT_CSR_STATUS BIT(4)
+#define WDT_CSR_KBD_INT_RESET BIT(6)
+#define WDT_CSR_MOUSE_INT_RESET BIT(7)
+
+#define WDT_CTRL_RISING_EDGE_KBD_RESET BIT(2)
+#define WDT_CTRL_MINUTE_MODE BIT(3)
static void superio_outb(int reg, int val)
{
@@ -144,7 +154,7 @@ static int superio_enter(void)
static void superio_select(int ld)
{
- superio_outb(0x07, ld);
+ superio_outb(SIO_REG_LDSEL, ld);
}
static void superio_exit(void)
@@ -165,9 +175,9 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
superio_select(W83627HF_LD_WDT);
/* set CR30 bit 0 to activate GPIO2 */
- t = superio_inb(0x30);
+ t = superio_inb(SIO_REG_ENABLE);
if (!(t & 0x01))
- superio_outb(0x30, t | 0x01);
+ superio_outb(SIO_REG_ENABLE, t | 0x01);
switch (chip) {
case w83627hf:
@@ -248,16 +258,15 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
}
}
- /* set second mode & disable keyboard turning off watchdog */
- t = superio_inb(cr_wdt_control) & ~0x0C;
+ t = superio_inb(cr_wdt_control) &
+ ~(WDT_CTRL_MINUTE_MODE | WDT_CTRL_RISING_EDGE_KBD_RESET);
superio_outb(cr_wdt_control, t);
t = superio_inb(cr_wdt_csr);
if (t & WDT_CSR_STATUS)
wdog->bootstatus |= WDIOF_CARDRESET;
- /* reset status, disable keyboard & mouse turning off watchdog */
- t &= ~(WDT_CSR_STATUS | WDT_CSR_KBD | WDT_CSR_MOUSE);
+ t &= ~(WDT_CSR_STATUS | WDT_CSR_KBD_INT_RESET | WDT_CSR_MOUSE_INT_RESET);
superio_outb(cr_wdt_csr, t);
superio_exit();
@@ -355,7 +364,7 @@ static int wdt_find(int addr)
if (ret)
return ret;
superio_select(W83627HF_LD_WDT);
- val = superio_inb(0x20);
+ val = superio_inb(SIO_REG_DEVID);
switch (val) {
case W83627HF_ID:
ret = w83627hf;
@@ -431,7 +440,7 @@ static int wdt_find(int addr)
cr_wdt_csr = NCT6102D_WDT_CSR;
break;
case NCT6116_ID:
- val = superio_inb(0x21);
+ val = superio_inb(SIO_REG_DEVID + 1);
if (val == NCT6126_VER_A_LOW_ID || val == NCT6126_VER_B_LOW_ID)
ret = nct6126;
else
@@ -513,11 +522,11 @@ static int __init wdt_init(void)
/* Apply system-specific quirks */
dmi_check_system(wdt_dmi_table);
- wdt_io = 0x2e;
- chip = wdt_find(0x2e);
+ wdt_io = SIO_CONF_PORT0;
+ chip = wdt_find(SIO_CONF_PORT0);
if (chip < 0) {
- wdt_io = 0x4e;
- chip = wdt_find(0x4e);
+ wdt_io = SIO_CONF_PORT1;
+ chip = wdt_find(SIO_CONF_PORT1);
if (chip < 0)
return chip;
}
--
2.56.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v5 2/5] watchdog: w83627hf_wdt: Convert to platform driver model
2026-10-20 8:11 [PATCH v5 0/5] watchdog: w83627hf_wdt: Convert driver to the driver model and add minute support Paul Louvel
2026-10-20 8:11 ` [PATCH v5 1/5] watchdog: w83627hf_wdt: Replace magic numbers with descriptive macros Paul Louvel
@ 2026-10-20 8:11 ` Paul Louvel
2026-10-20 13:29 ` Tzung-Bi Shih
2026-10-20 8:11 ` [PATCH v5 3/5] watchdog: w83627hf_wdt: Move register offsets into driver data Paul Louvel
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Paul Louvel @ 2026-10-20 8:11 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: Tzung-Bi Shih, linux-watchdog, linux-kernel, Thomas Petazzoni,
Paul Louvel
This driver is not yet converted to the driver model.
Convert it and add missing error messages in probe.
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
drivers/watchdog/w83627hf_wdt.c | 165 ++++++++++++++++++++++++++--------------
1 file changed, 106 insertions(+), 59 deletions(-)
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index fa1808a4c6c8..96ea278e45c4 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -30,6 +30,7 @@
#include <linux/bits.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
+#include <linux/platform_device.h>
#include <linux/types.h>
#include <linux/watchdog.h>
#include <linux/ioport.h>
@@ -129,6 +130,11 @@ MODULE_PARM_DESC(early_disable, "Disable watchdog at boot time (default=0)");
#define WDT_CTRL_RISING_EDGE_KBD_RESET BIT(2)
#define WDT_CTRL_MINUTE_MODE BIT(3)
+struct w83627hf_data {
+ struct watchdog_device wdd;
+ struct watchdog_info info;
+};
+
static void superio_outb(int reg, int val)
{
outb(reg, WDT_EFER);
@@ -326,10 +332,6 @@ static unsigned int wdt_get_time(struct watchdog_device *wdog)
* Kernel Interfaces
*/
-static struct watchdog_info wdt_info = {
- .options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
-};
-
static const struct watchdog_ops wdt_ops = {
.owner = THIS_MODULE,
.start = wdt_start,
@@ -338,14 +340,6 @@ static const struct watchdog_ops wdt_ops = {
.get_timeleft = wdt_get_time,
};
-static struct watchdog_device wdt_dev = {
- .info = &wdt_info,
- .ops = &wdt_ops,
- .timeout = WATCHDOG_TIMEOUT,
- .min_timeout = 1,
- .max_timeout = 255,
-};
-
/*
* The WDT needs to learn about soft shutdowns in order to
* turn the timebomb registers off.
@@ -462,6 +456,58 @@ static int wdt_find(int addr)
return ret;
}
+static int wdt_probe(struct platform_device *pdev)
+{
+ const struct platform_device_id *id = platform_get_device_id(pdev);
+ struct device *dev = &pdev->dev;
+ struct watchdog_device *wdd;
+ struct w83627hf_data *data;
+ enum chips chip;
+ int ret;
+
+ dev_info(dev, "WDT driver initialising\n");
+
+ if (!id)
+ return dev_err_probe(dev, -EINVAL, "failed to get chip id\n");
+
+ chip = id->driver_data;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ data->info.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE;
+ snprintf(data->info.identity, sizeof(data->info.identity), "%s Watchdog",
+ id->name);
+
+ wdd = &data->wdd;
+
+ wdd->parent = dev;
+ wdd->info = &data->info;
+ wdd->ops = &wdt_ops;
+ wdd->timeout = WATCHDOG_TIMEOUT;
+ wdd->min_timeout = 1;
+ wdd->max_timeout = 255;
+
+ watchdog_set_drvdata(wdd, data);
+ watchdog_init_timeout(wdd, timeout, NULL);
+ watchdog_set_nowayout(wdd, nowayout);
+ watchdog_stop_on_reboot(wdd);
+
+ ret = w83627hf_init(wdd, chip);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to initialize watchdog\n");
+
+ ret = devm_watchdog_register_device(dev, wdd);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to register watchdog\n");
+
+ dev_info(dev, "initialized. timeout=%d sec (nowayout=%d)\n",
+ wdd->timeout, nowayout);
+
+ return 0;
+}
+
/*
* On some systems, the NCT6791D comes with a companion chip and the
* watchdog function is in this companion chip. We must use a different
@@ -488,36 +534,48 @@ static const struct dmi_system_id wdt_dmi_table[] __initconst = {
{}
};
+static const struct platform_device_id wdt_ids[] = {
+ { .name = "W83627HF", .driver_data = w83627hf },
+ { .name = "W83627S", .driver_data = w83627s },
+ { .name = "W83697HF", .driver_data = w83697hf },
+ { .name = "W83697UG", .driver_data = w83697ug },
+ { .name = "W83637HF", .driver_data = w83637hf },
+ { .name = "W83627THF", .driver_data = w83627thf },
+ { .name = "W83687THF", .driver_data = w83687thf },
+ { .name = "W83627EHF", .driver_data = w83627ehf },
+ { .name = "W83627DHG", .driver_data = w83627dhg },
+ { .name = "W83627UHG", .driver_data = w83627uhg },
+ { .name = "W83667HG", .driver_data = w83667hg },
+ { .name = "W83667DHG-P", .driver_data = w83627dhg_p },
+ { .name = "W83667HG-B", .driver_data = w83667hg_b },
+ { .name = "NCT6775", .driver_data = nct6775 },
+ { .name = "NCT6776", .driver_data = nct6776 },
+ { .name = "NCT6779", .driver_data = nct6779 },
+ { .name = "NCT6791", .driver_data = nct6791 },
+ { .name = "NCT6792", .driver_data = nct6792 },
+ { .name = "NCT6793", .driver_data = nct6793 },
+ { .name = "NCT6795", .driver_data = nct6795 },
+ { .name = "NCT6796", .driver_data = nct6796 },
+ { .name = "NCT6102", .driver_data = nct6102 },
+ { .name = "NCT6116", .driver_data = nct6116 },
+ { .name = "NCT6126", .driver_data = nct6126 },
+ {}
+};
+
+static struct platform_driver wdt_driver = {
+ .probe = wdt_probe,
+ .id_table = wdt_ids,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ },
+};
+
+static struct platform_device *wdt_pdev;
+
static int __init wdt_init(void)
{
int ret;
int chip;
- static const char * const chip_name[] = {
- "W83627HF",
- "W83627S",
- "W83697HF",
- "W83697UG",
- "W83637HF",
- "W83627THF",
- "W83687THF",
- "W83627EHF",
- "W83627DHG",
- "W83627UHG",
- "W83667HG",
- "W83667DHG-P",
- "W83667HG-B",
- "NCT6775",
- "NCT6776",
- "NCT6779",
- "NCT6791",
- "NCT6792",
- "NCT6793",
- "NCT6795",
- "NCT6796",
- "NCT6102",
- "NCT6116",
- "NCT6126"
- };
/* Apply system-specific quirks */
dmi_check_system(wdt_dmi_table);
@@ -531,35 +589,24 @@ static int __init wdt_init(void)
return chip;
}
- pr_info("WDT driver for %s Super I/O chip initialising\n",
- chip_name[chip]);
-
- snprintf(wdt_info.identity, sizeof(wdt_info.identity), "%s Watchdog",
- chip_name[chip]);
-
- watchdog_init_timeout(&wdt_dev, timeout, NULL);
- watchdog_set_nowayout(&wdt_dev, nowayout);
- watchdog_stop_on_reboot(&wdt_dev);
-
- ret = w83627hf_init(&wdt_dev, chip);
- if (ret) {
- pr_err("failed to initialize watchdog (err=%d)\n", ret);
- return ret;
- }
-
- ret = watchdog_register_device(&wdt_dev);
+ ret = platform_driver_register(&wdt_driver);
if (ret)
return ret;
- pr_info("initialized. timeout=%d sec (nowayout=%d)\n",
- wdt_dev.timeout, nowayout);
+ wdt_pdev = platform_device_register_data(NULL, wdt_ids[chip].name,
+ PLATFORM_DEVID_NONE, NULL, 0);
+ if (IS_ERR(wdt_pdev)) {
+ platform_driver_unregister(&wdt_driver);
+ return PTR_ERR(wdt_pdev);
+ }
- return ret;
+ return 0;
}
static void __exit wdt_exit(void)
{
- watchdog_unregister_device(&wdt_dev);
+ platform_device_unregister(wdt_pdev);
+ platform_driver_unregister(&wdt_driver);
}
module_init(wdt_init);
--
2.56.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v5 2/5] watchdog: w83627hf_wdt: Convert to platform driver model
2026-10-20 8:11 ` [PATCH v5 2/5] watchdog: w83627hf_wdt: Convert to platform driver model Paul Louvel
@ 2026-10-20 13:29 ` Tzung-Bi Shih
0 siblings, 0 replies; 11+ messages in thread
From: Tzung-Bi Shih @ 2026-10-20 13:29 UTC (permalink / raw)
To: Paul Louvel
Cc: Wim Van Sebroeck, Guenter Roeck, linux-watchdog, linux-kernel,
Thomas Petazzoni
On Tue, Oct 20, 2026 at 10:11:55AM +0200, Paul Louvel wrote:
> This driver is not yet converted to the driver model.
> Convert it and add missing error messages in probe.
>
> Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
I'm not sure if the W=1 warning reported by 0day bot [1] (also flagged by
Sashiko [2]) is still relevant as the truncation can never actually happen
for now. Even if the truncation really happened, it should be harmless.
Given that,
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
[1] https://lore.kernel.org/oe-kbuild-all/202610080027.3OsJDwfo-lkp@intel.com
[2] https://lore.kernel.org/all/sashiko-outbox-178668@kernel.org
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v5 3/5] watchdog: w83627hf_wdt: Move register offsets into driver data
2026-10-20 8:11 [PATCH v5 0/5] watchdog: w83627hf_wdt: Convert driver to the driver model and add minute support Paul Louvel
2026-10-20 8:11 ` [PATCH v5 1/5] watchdog: w83627hf_wdt: Replace magic numbers with descriptive macros Paul Louvel
2026-10-20 8:11 ` [PATCH v5 2/5] watchdog: w83627hf_wdt: Convert to platform driver model Paul Louvel
@ 2026-10-20 8:11 ` Paul Louvel
2026-10-20 13:44 ` Tzung-Bi Shih
2026-10-20 8:11 ` [PATCH v5 4/5] watchdog: w83627hf_wdt: Store Super I/O configurations in platform data Paul Louvel
2026-10-20 8:11 ` [PATCH v5 5/5] watchdog: w83627hf_wdt: Add minute mode counting Paul Louvel
4 siblings, 1 reply; 11+ messages in thread
From: Paul Louvel @ 2026-10-20 8:11 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: Tzung-Bi Shih, linux-watchdog, linux-kernel, Thomas Petazzoni,
Paul Louvel
Register offsets for the watchdog timer, control, and status registers
differ across Super I/O chip variants. Store them in the driver data
instead of global variables.
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
drivers/watchdog/w83627hf_wdt.c | 93 ++++++++++++++++++++++++++---------------
1 file changed, 59 insertions(+), 34 deletions(-)
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 96ea278e45c4..fc3703b38141 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -42,9 +42,6 @@
#define WATCHDOG_TIMEOUT 60 /* 60 sec default timeout */
static int wdt_io;
-static int cr_wdt_timeout; /* WDT timeout register */
-static int cr_wdt_control; /* WDT control register */
-static int cr_wdt_csr; /* WDT control & status register */
static int wdt_cfg_enter = 0x87;/* key to unlock configuration space */
static int wdt_cfg_leave = 0xAA;/* key to lock configuration space */
@@ -130,9 +127,16 @@ MODULE_PARM_DESC(early_disable, "Disable watchdog at boot time (default=0)");
#define WDT_CTRL_RISING_EDGE_KBD_RESET BIT(2)
#define WDT_CTRL_MINUTE_MODE BIT(3)
+struct w83627hf_reg {
+ int control;
+ int timeout;
+ int csr;
+};
+
struct w83627hf_data {
struct watchdog_device wdd;
struct watchdog_info info;
+ const struct w83627hf_reg *reg;
};
static void superio_outb(int reg, int val)
@@ -171,6 +175,7 @@ static void superio_exit(void)
static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
{
+ struct w83627hf_data *data = watchdog_get_drvdata(wdog);
int ret;
unsigned char t;
@@ -210,10 +215,10 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
case w83627dhg_p:
t = superio_inb(0x2D) & ~0x01; /* PIN77 -> WDT0# */
superio_outb(0x2D, t); /* set GPIO5 to WDT0 */
- t = superio_inb(cr_wdt_control);
+ t = superio_inb(data->reg->control);
t |= 0x02; /* enable the WDTO# output low pulse
* to the KBRST# pin */
- superio_outb(cr_wdt_control, t);
+ superio_outb(data->reg->control, t);
break;
case w83637hf:
break;
@@ -242,46 +247,47 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
* Don't touch its configuration, and hope the BIOS
* does the right thing.
*/
- t = superio_inb(cr_wdt_control);
+ t = superio_inb(data->reg->control);
t |= 0x02; /* enable the WDTO# output low pulse
* to the KBRST# pin */
- superio_outb(cr_wdt_control, t);
+ superio_outb(data->reg->control, t);
break;
default:
break;
}
- t = superio_inb(cr_wdt_timeout);
+ t = superio_inb(data->reg->timeout);
if (t != 0) {
if (early_disable) {
pr_warn("Stopping previously enabled watchdog until userland kicks in\n");
- superio_outb(cr_wdt_timeout, 0);
+ superio_outb(data->reg->timeout, 0);
} else {
pr_info("Watchdog already running. Resetting timeout to %d sec\n",
wdog->timeout);
- superio_outb(cr_wdt_timeout, wdog->timeout);
+ superio_outb(data->reg->timeout, wdog->timeout);
set_bit(WDOG_HW_RUNNING, &wdog->status);
}
}
- t = superio_inb(cr_wdt_control) &
+ t = superio_inb(data->reg->control) &
~(WDT_CTRL_MINUTE_MODE | WDT_CTRL_RISING_EDGE_KBD_RESET);
- superio_outb(cr_wdt_control, t);
+ superio_outb(data->reg->control, t);
- t = superio_inb(cr_wdt_csr);
+ t = superio_inb(data->reg->csr);
if (t & WDT_CSR_STATUS)
wdog->bootstatus |= WDIOF_CARDRESET;
t &= ~(WDT_CSR_STATUS | WDT_CSR_KBD_INT_RESET | WDT_CSR_MOUSE_INT_RESET);
- superio_outb(cr_wdt_csr, t);
+ superio_outb(data->reg->csr, t);
superio_exit();
return 0;
}
-static int wdt_set_time(unsigned int timeout)
+static int wdt_set_time(struct watchdog_device *wdog, unsigned int timeout)
{
+ struct w83627hf_data *data = watchdog_get_drvdata(wdog);
int ret;
ret = superio_enter();
@@ -289,7 +295,7 @@ static int wdt_set_time(unsigned int timeout)
return ret;
superio_select(W83627HF_LD_WDT);
- superio_outb(cr_wdt_timeout, timeout);
+ superio_outb(data->reg->timeout, timeout);
superio_exit();
return 0;
@@ -297,12 +303,12 @@ static int wdt_set_time(unsigned int timeout)
static int wdt_start(struct watchdog_device *wdog)
{
- return wdt_set_time(wdog->timeout);
+ return wdt_set_time(wdog, wdog->timeout);
}
static int wdt_stop(struct watchdog_device *wdog)
{
- return wdt_set_time(0);
+ return wdt_set_time(wdog, 0);
}
static int wdt_set_timeout(struct watchdog_device *wdog, unsigned int timeout)
@@ -314,6 +320,7 @@ static int wdt_set_timeout(struct watchdog_device *wdog, unsigned int timeout)
static unsigned int wdt_get_time(struct watchdog_device *wdog)
{
+ struct w83627hf_data *data = watchdog_get_drvdata(wdog);
unsigned int timeleft;
int ret;
@@ -322,7 +329,7 @@ static unsigned int wdt_get_time(struct watchdog_device *wdog)
return 0;
superio_select(W83627HF_LD_WDT);
- timeleft = superio_inb(cr_wdt_timeout);
+ timeleft = superio_inb(data->reg->timeout);
superio_exit();
return timeleft;
@@ -350,10 +357,6 @@ static int wdt_find(int addr)
u8 val;
int ret;
- cr_wdt_timeout = W83627HF_WDT_TIMEOUT;
- cr_wdt_control = W83627HF_WDT_CONTROL;
- cr_wdt_csr = W836X7HF_WDT_CSR;
-
ret = superio_enter();
if (ret)
return ret;
@@ -368,13 +371,9 @@ static int wdt_find(int addr)
break;
case W83697HF_ID:
ret = w83697hf;
- cr_wdt_timeout = W83697HF_WDT_TIMEOUT;
- cr_wdt_control = W83697HF_WDT_CONTROL;
break;
case W83697UG_ID:
ret = w83697ug;
- cr_wdt_timeout = W83697HF_WDT_TIMEOUT;
- cr_wdt_control = W83697HF_WDT_CONTROL;
break;
case W83637HF_ID:
ret = w83637hf;
@@ -429,9 +428,6 @@ static int wdt_find(int addr)
break;
case NCT6102_ID:
ret = nct6102;
- cr_wdt_timeout = NCT6102D_WDT_TIMEOUT;
- cr_wdt_control = NCT6102D_WDT_CONTROL;
- cr_wdt_csr = NCT6102D_WDT_CSR;
break;
case NCT6116_ID:
val = superio_inb(SIO_REG_DEVID + 1);
@@ -439,10 +435,6 @@ static int wdt_find(int addr)
ret = nct6126;
else
ret = nct6116;
-
- cr_wdt_timeout = NCT6102D_WDT_TIMEOUT;
- cr_wdt_control = NCT6102D_WDT_CONTROL;
- cr_wdt_csr = NCT6102D_WDT_CSR;
break;
case 0xff:
ret = -ENODEV;
@@ -456,6 +448,24 @@ static int wdt_find(int addr)
return ret;
}
+static const struct w83627hf_reg w83627hf_default_regs = {
+ .timeout = W83627HF_WDT_TIMEOUT,
+ .control = W83627HF_WDT_CONTROL,
+ .csr = W836X7HF_WDT_CSR
+};
+
+static const struct w83627hf_reg w83627hf_nct61xx_regs = {
+ .timeout = NCT6102D_WDT_TIMEOUT,
+ .control = NCT6102D_WDT_CONTROL,
+ .csr = NCT6102D_WDT_CSR
+};
+
+static const struct w83627hf_reg w83627hf_w83697xx_regs = {
+ .timeout = W83697HF_WDT_TIMEOUT,
+ .control = W83697HF_WDT_CONTROL,
+ .csr = W836X7HF_WDT_CSR
+};
+
static int wdt_probe(struct platform_device *pdev)
{
const struct platform_device_id *id = platform_get_device_id(pdev);
@@ -489,6 +499,21 @@ static int wdt_probe(struct platform_device *pdev)
wdd->min_timeout = 1;
wdd->max_timeout = 255;
+ switch (chip) {
+ case nct6102:
+ case nct6116:
+ case nct6126:
+ data->reg = &w83627hf_nct61xx_regs;
+ break;
+ case w83697hf:
+ case w83697ug:
+ data->reg = &w83627hf_w83697xx_regs;
+ break;
+ default:
+ data->reg = &w83627hf_default_regs;
+ break;
+ }
+
watchdog_set_drvdata(wdd, data);
watchdog_init_timeout(wdd, timeout, NULL);
watchdog_set_nowayout(wdd, nowayout);
--
2.56.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v5 3/5] watchdog: w83627hf_wdt: Move register offsets into driver data
2026-10-20 8:11 ` [PATCH v5 3/5] watchdog: w83627hf_wdt: Move register offsets into driver data Paul Louvel
@ 2026-10-20 13:44 ` Tzung-Bi Shih
0 siblings, 0 replies; 11+ messages in thread
From: Tzung-Bi Shih @ 2026-10-20 13:44 UTC (permalink / raw)
To: Paul Louvel
Cc: Wim Van Sebroeck, Guenter Roeck, linux-watchdog, linux-kernel,
Thomas Petazzoni
On Tue, Oct 20, 2026 at 10:11:56AM +0200, Paul Louvel wrote:
> Register offsets for the watchdog timer, control, and status registers
> differ across Super I/O chip variants. Store them in the driver data
> instead of global variables.
>
> Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
With a nit fixed,
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
> static int wdt_io;
> -static int cr_wdt_timeout; /* WDT timeout register */
> -static int cr_wdt_control; /* WDT control register */
> -static int cr_wdt_csr; /* WDT control & status register */
> static int wdt_cfg_enter = 0x87;/* key to unlock configuration space */
> static int wdt_cfg_leave = 0xAA;/* key to lock configuration space */
...
> +struct w83627hf_reg {
> + int control;
> + int timeout;
> + int csr;
> +};
...
> > +static const struct w83627hf_reg w83627hf_default_regs = {
> + .timeout = W83627HF_WDT_TIMEOUT,
> + .control = W83627HF_WDT_CONTROL,
> + .csr = W836X7HF_WDT_CSR
> +};
> +
> +static const struct w83627hf_reg w83627hf_nct61xx_regs = {
> + .timeout = NCT6102D_WDT_TIMEOUT,
> + .control = NCT6102D_WDT_CONTROL,
> + .csr = NCT6102D_WDT_CSR
> +};
> +
> +static const struct w83627hf_reg w83627hf_w83697xx_regs = {
> + .timeout = W83697HF_WDT_TIMEOUT,
> + .control = W83697HF_WDT_CONTROL,
> + .csr = W836X7HF_WDT_CSR
> +};
To be consistent and neat, I'd expect the order in the declaration of
struct w83627hf_reg to also be: timeout -> control -> csr.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v5 4/5] watchdog: w83627hf_wdt: Store Super I/O configurations in platform data
2026-10-20 8:11 [PATCH v5 0/5] watchdog: w83627hf_wdt: Convert driver to the driver model and add minute support Paul Louvel
` (2 preceding siblings ...)
2026-10-20 8:11 ` [PATCH v5 3/5] watchdog: w83627hf_wdt: Move register offsets into driver data Paul Louvel
@ 2026-10-20 8:11 ` Paul Louvel
2026-10-20 8:11 ` [PATCH v5 5/5] watchdog: w83627hf_wdt: Add minute mode counting Paul Louvel
4 siblings, 0 replies; 11+ messages in thread
From: Paul Louvel @ 2026-10-20 8:11 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: Tzung-Bi Shih, linux-watchdog, linux-kernel, Thomas Petazzoni,
Paul Louvel
Instead of using global variables, store Super I/O related
configurations in platform data.
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
drivers/watchdog/w83627hf_wdt.c | 161 ++++++++++++++++++++++------------------
1 file changed, 87 insertions(+), 74 deletions(-)
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index fc3703b38141..3d6f35e79631 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -41,10 +41,6 @@
#define WATCHDOG_NAME "w83627hf/thf/hg/dhg WDT"
#define WATCHDOG_TIMEOUT 60 /* 60 sec default timeout */
-static int wdt_io;
-static int wdt_cfg_enter = 0x87;/* key to unlock configuration space */
-static int wdt_cfg_leave = 0xAA;/* key to lock configuration space */
-
enum chips { w83627hf, w83627s, w83697hf, w83697ug, w83637hf, w83627thf,
w83687thf, w83627ehf, w83627dhg, w83627uhg, w83667hg, w83627dhg_p,
w83667hg_b, nct6775, nct6776, nct6779, nct6791, nct6792, nct6793,
@@ -76,9 +72,8 @@ MODULE_PARM_DESC(early_disable, "Disable watchdog at boot time (default=0)");
#define SIO_CONF_PORT0 0x2E
#define SIO_CONF_PORT1 0x4E
-#define WDT_EFER (wdt_io+0) /* Extended Function Enable Registers */
-#define WDT_EFIR (wdt_io+0) /* Extended Function Index Register
- (same as EFER) */
+#define WDT_EFER (base+0) /* Extended Function Enable Registers */
+#define WDT_EFIR (base+0) /* Extended Function Index Register (same as EFER) */
#define WDT_EFDR (WDT_EFIR+1) /* Extended Function Data Register */
#define W83627HF_LD_WDT 0x08
@@ -133,44 +128,51 @@ struct w83627hf_reg {
int csr;
};
+struct w83627hf_sio_pdata {
+ int addr;
+ int cfg_enter;
+ int cfg_leave;
+};
+
struct w83627hf_data {
struct watchdog_device wdd;
struct watchdog_info info;
const struct w83627hf_reg *reg;
+ const struct w83627hf_sio_pdata *sio;
};
-static void superio_outb(int reg, int val)
+static void superio_outb(int base, int reg, int val)
{
outb(reg, WDT_EFER);
outb(val, WDT_EFDR);
}
-static inline int superio_inb(int reg)
+static inline int superio_inb(int base, int reg)
{
outb(reg, WDT_EFER);
return inb(WDT_EFDR);
}
-static int superio_enter(void)
+static int superio_enter(int base, int enter)
{
- if (!request_muxed_region(wdt_io, 2, WATCHDOG_NAME))
+ if (!request_muxed_region(base, 2, WATCHDOG_NAME))
return -EBUSY;
- outb_p(wdt_cfg_enter, WDT_EFER); /* Enter extended function mode */
- outb_p(wdt_cfg_enter, WDT_EFER); /* Again according to manual */
+ outb_p(enter, WDT_EFER); /* Enter extended function mode */
+ outb_p(enter, WDT_EFER); /* Again according to manual */
return 0;
}
-static void superio_select(int ld)
+static void superio_select(int base, int ld)
{
- superio_outb(SIO_REG_LDSEL, ld);
+ superio_outb(base, SIO_REG_LDSEL, ld);
}
-static void superio_exit(void)
+static void superio_exit(int base, int leave)
{
- outb_p(wdt_cfg_leave, WDT_EFER); /* Leave extended function mode */
- release_region(wdt_io, 2);
+ outb_p(leave, WDT_EFER); /* Leave extended function mode */
+ release_region(base, 2);
}
static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
@@ -179,52 +181,52 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
int ret;
unsigned char t;
- ret = superio_enter();
+ ret = superio_enter(data->sio->addr, data->sio->cfg_enter);
if (ret)
return ret;
- superio_select(W83627HF_LD_WDT);
+ superio_select(data->sio->addr, W83627HF_LD_WDT);
/* set CR30 bit 0 to activate GPIO2 */
- t = superio_inb(SIO_REG_ENABLE);
+ t = superio_inb(data->sio->addr, SIO_REG_ENABLE);
if (!(t & 0x01))
- superio_outb(SIO_REG_ENABLE, t | 0x01);
+ superio_outb(data->sio->addr, SIO_REG_ENABLE, t | 0x01);
switch (chip) {
case w83627hf:
case w83627s:
- t = superio_inb(0x2B) & ~0x10;
- superio_outb(0x2B, t); /* set GPIO24 to WDT0 */
+ t = superio_inb(data->sio->addr, 0x2B) & ~0x10;
+ superio_outb(data->sio->addr, 0x2B, t); /* set GPIO24 to WDT0 */
break;
case w83697hf:
/* Set pin 119 to WDTO# mode (= CR29, WDT0) */
- t = superio_inb(0x29) & ~0x60;
+ t = superio_inb(data->sio->addr, 0x29) & ~0x60;
t |= 0x20;
- superio_outb(0x29, t);
+ superio_outb(data->sio->addr, 0x29, t);
break;
case w83697ug:
/* Set pin 118 to WDTO# mode */
- t = superio_inb(0x2b) & ~0x04;
- superio_outb(0x2b, t);
+ t = superio_inb(data->sio->addr, 0x2b) & ~0x04;
+ superio_outb(data->sio->addr, 0x2b, t);
break;
case w83627thf:
- t = (superio_inb(0x2B) & ~0x08) | 0x04;
- superio_outb(0x2B, t); /* set GPIO3 to WDT0 */
+ t = (superio_inb(data->sio->addr, 0x2B) & ~0x08) | 0x04;
+ superio_outb(data->sio->addr, 0x2B, t); /* set GPIO3 to WDT0 */
break;
case w83627dhg:
case w83627dhg_p:
- t = superio_inb(0x2D) & ~0x01; /* PIN77 -> WDT0# */
- superio_outb(0x2D, t); /* set GPIO5 to WDT0 */
- t = superio_inb(data->reg->control);
+ t = superio_inb(data->sio->addr, 0x2D) & ~0x01; /* PIN77 -> WDT0# */
+ superio_outb(data->sio->addr, 0x2D, t); /* set GPIO5 to WDT0 */
+ t = superio_inb(data->sio->addr, data->reg->control);
t |= 0x02; /* enable the WDTO# output low pulse
* to the KBRST# pin */
- superio_outb(data->reg->control, t);
+ superio_outb(data->sio->addr, data->reg->control, t);
break;
case w83637hf:
break;
case w83687thf:
- t = superio_inb(0x2C) & ~0x80; /* PIN47 -> WDT0# */
- superio_outb(0x2C, t);
+ t = superio_inb(data->sio->addr, 0x2C) & ~0x80; /* PIN47 -> WDT0# */
+ superio_outb(data->sio->addr, 0x2C, t);
break;
case w83627ehf:
case w83627uhg:
@@ -247,40 +249,40 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
* Don't touch its configuration, and hope the BIOS
* does the right thing.
*/
- t = superio_inb(data->reg->control);
+ t = superio_inb(data->sio->addr, data->reg->control);
t |= 0x02; /* enable the WDTO# output low pulse
* to the KBRST# pin */
- superio_outb(data->reg->control, t);
+ superio_outb(data->sio->addr, data->reg->control, t);
break;
default:
break;
}
- t = superio_inb(data->reg->timeout);
+ t = superio_inb(data->sio->addr, data->reg->timeout);
if (t != 0) {
if (early_disable) {
pr_warn("Stopping previously enabled watchdog until userland kicks in\n");
- superio_outb(data->reg->timeout, 0);
+ superio_outb(data->sio->addr, data->reg->timeout, 0);
} else {
pr_info("Watchdog already running. Resetting timeout to %d sec\n",
wdog->timeout);
- superio_outb(data->reg->timeout, wdog->timeout);
+ superio_outb(data->sio->addr, data->reg->timeout, wdog->timeout);
set_bit(WDOG_HW_RUNNING, &wdog->status);
}
}
- t = superio_inb(data->reg->control) &
+ t = superio_inb(data->sio->addr, data->reg->control) &
~(WDT_CTRL_MINUTE_MODE | WDT_CTRL_RISING_EDGE_KBD_RESET);
- superio_outb(data->reg->control, t);
+ superio_outb(data->sio->addr, data->reg->control, t);
- t = superio_inb(data->reg->csr);
+ t = superio_inb(data->sio->addr, data->reg->csr);
if (t & WDT_CSR_STATUS)
wdog->bootstatus |= WDIOF_CARDRESET;
t &= ~(WDT_CSR_STATUS | WDT_CSR_KBD_INT_RESET | WDT_CSR_MOUSE_INT_RESET);
- superio_outb(data->reg->csr, t);
+ superio_outb(data->sio->addr, data->reg->csr, t);
- superio_exit();
+ superio_exit(data->sio->addr, data->sio->cfg_leave);
return 0;
}
@@ -290,13 +292,13 @@ static int wdt_set_time(struct watchdog_device *wdog, unsigned int timeout)
struct w83627hf_data *data = watchdog_get_drvdata(wdog);
int ret;
- ret = superio_enter();
+ ret = superio_enter(data->sio->addr, data->sio->cfg_enter);
if (ret)
return ret;
- superio_select(W83627HF_LD_WDT);
- superio_outb(data->reg->timeout, timeout);
- superio_exit();
+ superio_select(data->sio->addr, W83627HF_LD_WDT);
+ superio_outb(data->sio->addr, data->reg->timeout, timeout);
+ superio_exit(data->sio->addr, data->sio->cfg_leave);
return 0;
}
@@ -324,13 +326,13 @@ static unsigned int wdt_get_time(struct watchdog_device *wdog)
unsigned int timeleft;
int ret;
- ret = superio_enter();
+ ret = superio_enter(data->sio->addr, data->sio->cfg_enter);
if (ret)
return 0;
- superio_select(W83627HF_LD_WDT);
- timeleft = superio_inb(data->reg->timeout);
- superio_exit();
+ superio_select(data->sio->addr, W83627HF_LD_WDT);
+ timeleft = superio_inb(data->sio->addr, data->reg->timeout);
+ superio_exit(data->sio->addr, data->sio->cfg_leave);
return timeleft;
}
@@ -352,16 +354,16 @@ static const struct watchdog_ops wdt_ops = {
* turn the timebomb registers off.
*/
-static int wdt_find(int addr)
+static int wdt_find(int addr, int enter, int leave)
{
u8 val;
int ret;
- ret = superio_enter();
+ ret = superio_enter(addr, enter);
if (ret)
return ret;
- superio_select(W83627HF_LD_WDT);
- val = superio_inb(SIO_REG_DEVID);
+ superio_select(addr, W83627HF_LD_WDT);
+ val = superio_inb(addr, SIO_REG_DEVID);
switch (val) {
case W83627HF_ID:
ret = w83627hf;
@@ -430,7 +432,7 @@ static int wdt_find(int addr)
ret = nct6102;
break;
case NCT6116_ID:
- val = superio_inb(SIO_REG_DEVID + 1);
+ val = superio_inb(addr, SIO_REG_DEVID + 1);
if (val == NCT6126_VER_A_LOW_ID || val == NCT6126_VER_B_LOW_ID)
ret = nct6126;
else
@@ -444,7 +446,7 @@ static int wdt_find(int addr)
pr_err("Unsupported chip ID: 0x%02x\n", val);
break;
}
- superio_exit();
+ superio_exit(addr, leave);
return ret;
}
@@ -468,6 +470,7 @@ static const struct w83627hf_reg w83627hf_w83697xx_regs = {
static int wdt_probe(struct platform_device *pdev)
{
+ const struct w83627hf_sio_pdata *pdata = dev_get_platdata(&pdev->dev);
const struct platform_device_id *id = platform_get_device_id(pdev);
struct device *dev = &pdev->dev;
struct watchdog_device *wdd;
@@ -480,12 +483,16 @@ static int wdt_probe(struct platform_device *pdev)
if (!id)
return dev_err_probe(dev, -EINVAL, "failed to get chip id\n");
+ if (!pdata)
+ return dev_err_probe(dev, -EINVAL, "failed to get pdata\n");
+
chip = id->driver_data;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
+ data->sio = pdata;
data->info.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE;
snprintf(data->info.identity, sizeof(data->info.identity), "%s Watchdog",
id->name);
@@ -533,18 +540,20 @@ static int wdt_probe(struct platform_device *pdev)
return 0;
}
+static const struct w83627hf_sio_pdata w83627hf_sio_pdata_default = {
+ .cfg_enter = 0x87,
+ .cfg_leave = 0xAA
+};
+
/*
* On some systems, the NCT6791D comes with a companion chip and the
* watchdog function is in this companion chip. We must use a different
* unlocking sequence to access the companion chip.
*/
-static int __init wdt_use_alt_key(const struct dmi_system_id *d)
-{
- wdt_cfg_enter = 0x88;
- wdt_cfg_leave = 0xBB;
-
- return 0;
-}
+static const struct w83627hf_sio_pdata w83627hf_sio_pdata_alt = {
+ .cfg_enter = 0x88,
+ .cfg_leave = 0xBB
+};
static const struct dmi_system_id wdt_dmi_table[] __initconst = {
{
@@ -554,7 +563,7 @@ static const struct dmi_system_id wdt_dmi_table[] __initconst = {
DMI_EXACT_MATCH(DMI_BOARD_VENDOR, "INVES"),
DMI_EXACT_MATCH(DMI_BOARD_NAME, "SHARKBAY"),
},
- .callback = wdt_use_alt_key,
+ .driver_data = (void *)&w83627hf_sio_pdata_alt
},
{}
};
@@ -599,17 +608,21 @@ static struct platform_device *wdt_pdev;
static int __init wdt_init(void)
{
+ struct w83627hf_sio_pdata pdata = w83627hf_sio_pdata_default;
+ const struct dmi_system_id *match;
int ret;
int chip;
/* Apply system-specific quirks */
- dmi_check_system(wdt_dmi_table);
+ match = dmi_first_match(wdt_dmi_table);
+ if (match)
+ pdata = *(const struct w83627hf_sio_pdata *)match->driver_data;
- wdt_io = SIO_CONF_PORT0;
- chip = wdt_find(SIO_CONF_PORT0);
+ pdata.addr = SIO_CONF_PORT0;
+ chip = wdt_find(pdata.addr, pdata.cfg_enter, pdata.cfg_leave);
if (chip < 0) {
- wdt_io = SIO_CONF_PORT1;
- chip = wdt_find(SIO_CONF_PORT1);
+ pdata.addr = SIO_CONF_PORT1;
+ chip = wdt_find(pdata.addr, pdata.cfg_enter, pdata.cfg_leave);
if (chip < 0)
return chip;
}
@@ -619,7 +632,7 @@ static int __init wdt_init(void)
return ret;
wdt_pdev = platform_device_register_data(NULL, wdt_ids[chip].name,
- PLATFORM_DEVID_NONE, NULL, 0);
+ PLATFORM_DEVID_NONE, &pdata, sizeof(pdata));
if (IS_ERR(wdt_pdev)) {
platform_driver_unregister(&wdt_driver);
return PTR_ERR(wdt_pdev);
--
2.56.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v5 5/5] watchdog: w83627hf_wdt: Add minute mode counting
2026-10-20 8:11 [PATCH v5 0/5] watchdog: w83627hf_wdt: Convert driver to the driver model and add minute support Paul Louvel
` (3 preceding siblings ...)
2026-10-20 8:11 ` [PATCH v5 4/5] watchdog: w83627hf_wdt: Store Super I/O configurations in platform data Paul Louvel
@ 2026-10-20 8:11 ` Paul Louvel
2026-10-20 14:08 ` Tzung-Bi Shih
4 siblings, 1 reply; 11+ messages in thread
From: Paul Louvel @ 2026-10-20 8:11 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: Tzung-Bi Shih, linux-watchdog, linux-kernel, Thomas Petazzoni,
Paul Louvel
All of the chips supported by the driver can be set to minute mode,
allowing timeout value up to 255 minutes.
Add it to the driver.
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
drivers/watchdog/w83627hf_wdt.c | 68 +++++++++++++++++++++++++++++++----------
1 file changed, 52 insertions(+), 16 deletions(-)
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 3d6f35e79631..c45d5b9b97b7 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -40,6 +40,7 @@
#define WATCHDOG_NAME "w83627hf/thf/hg/dhg WDT"
#define WATCHDOG_TIMEOUT 60 /* 60 sec default timeout */
+#define WATCHDOG_MAX_TIMEOUT (255 * 60)
enum chips { w83627hf, w83627s, w83697hf, w83697ug, w83637hf, w83627thf,
w83687thf, w83627ehf, w83627dhg, w83627uhg, w83667hg, w83627dhg_p,
@@ -49,7 +50,8 @@ enum chips { w83627hf, w83627s, w83697hf, w83697ug, w83637hf, w83627thf,
static int timeout; /* in seconds */
module_param(timeout, int, 0);
MODULE_PARM_DESC(timeout,
- "Watchdog timeout in seconds. 1 <= timeout <= 255, default="
+ "Watchdog timeout in seconds. 1 <= timeout <= "
+ __MODULE_STRING(WATCHDOG_MAX_TIMEOUT) ", default="
__MODULE_STRING(WATCHDOG_TIMEOUT) ".");
static bool nowayout = WATCHDOG_NOWAYOUT;
@@ -139,6 +141,9 @@ struct w83627hf_data {
struct watchdog_info info;
const struct w83627hf_reg *reg;
const struct w83627hf_sio_pdata *sio;
+ bool minute_mode;
+ u8 early_timer_val;
+ u8 timer_val;
};
static void superio_outb(int base, int reg, int val)
@@ -258,21 +263,10 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
break;
}
- t = superio_inb(data->sio->addr, data->reg->timeout);
- if (t != 0) {
- if (early_disable) {
- pr_warn("Stopping previously enabled watchdog until userland kicks in\n");
- superio_outb(data->sio->addr, data->reg->timeout, 0);
- } else {
- pr_info("Watchdog already running. Resetting timeout to %d sec\n",
- wdog->timeout);
- superio_outb(data->sio->addr, data->reg->timeout, wdog->timeout);
- set_bit(WDOG_HW_RUNNING, &wdog->status);
- }
- }
+ data->early_timer_val = superio_inb(data->sio->addr, data->reg->timeout);
t = superio_inb(data->sio->addr, data->reg->control) &
- ~(WDT_CTRL_MINUTE_MODE | WDT_CTRL_RISING_EDGE_KBD_RESET);
+ ~WDT_CTRL_RISING_EDGE_KBD_RESET;
superio_outb(data->sio->addr, data->reg->control, t);
t = superio_inb(data->sio->addr, data->reg->csr);
@@ -290,6 +284,7 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
static int wdt_set_time(struct watchdog_device *wdog, unsigned int timeout)
{
struct w83627hf_data *data = watchdog_get_drvdata(wdog);
+ unsigned char ctrl;
int ret;
ret = superio_enter(data->sio->addr, data->sio->cfg_enter);
@@ -297,6 +292,15 @@ static int wdt_set_time(struct watchdog_device *wdog, unsigned int timeout)
return ret;
superio_select(data->sio->addr, W83627HF_LD_WDT);
+
+ ctrl = superio_inb(data->sio->addr, data->reg->control);
+
+ if (data->minute_mode)
+ ctrl |= WDT_CTRL_MINUTE_MODE;
+ else
+ ctrl &= ~WDT_CTRL_MINUTE_MODE;
+
+ superio_outb(data->sio->addr, data->reg->control, ctrl);
superio_outb(data->sio->addr, data->reg->timeout, timeout);
superio_exit(data->sio->addr, data->sio->cfg_leave);
@@ -305,7 +309,9 @@ static int wdt_set_time(struct watchdog_device *wdog, unsigned int timeout)
static int wdt_start(struct watchdog_device *wdog)
{
- return wdt_set_time(wdog, wdog->timeout);
+ struct w83627hf_data *data = watchdog_get_drvdata(wdog);
+
+ return wdt_set_time(wdog, data->timer_val);
}
static int wdt_stop(struct watchdog_device *wdog)
@@ -315,6 +321,17 @@ static int wdt_stop(struct watchdog_device *wdog)
static int wdt_set_timeout(struct watchdog_device *wdog, unsigned int timeout)
{
+ struct w83627hf_data *data = watchdog_get_drvdata(wdog);
+
+ if (timeout > 255) {
+ data->minute_mode = true;
+ data->timer_val = DIV_ROUND_UP(timeout, 60);
+ timeout = data->timer_val * 60;
+ } else {
+ data->minute_mode = false;
+ data->timer_val = timeout;
+ }
+
wdog->timeout = timeout;
return 0;
@@ -332,6 +349,8 @@ static unsigned int wdt_get_time(struct watchdog_device *wdog)
superio_select(data->sio->addr, W83627HF_LD_WDT);
timeleft = superio_inb(data->sio->addr, data->reg->timeout);
+ if (data->minute_mode)
+ timeleft *= 60;
superio_exit(data->sio->addr, data->sio->cfg_leave);
return timeleft;
@@ -504,7 +523,7 @@ static int wdt_probe(struct platform_device *pdev)
wdd->ops = &wdt_ops;
wdd->timeout = WATCHDOG_TIMEOUT;
wdd->min_timeout = 1;
- wdd->max_timeout = 255;
+ wdd->max_timeout = WATCHDOG_MAX_TIMEOUT;
switch (chip) {
case nct6102:
@@ -526,10 +545,27 @@ static int wdt_probe(struct platform_device *pdev)
watchdog_set_nowayout(wdd, nowayout);
watchdog_stop_on_reboot(wdd);
+ wdt_set_timeout(wdd, wdd->timeout);
+
ret = w83627hf_init(wdd, chip);
if (ret)
return dev_err_probe(dev, ret, "failed to initialize watchdog\n");
+ if (data->early_timer_val) {
+ if (early_disable) {
+ dev_warn(dev, "Stopping previously enabled watchdog until userland kicks in\n");
+ ret = wdt_stop(wdd);
+ } else {
+ dev_info(dev, "Watchdog already running. Resetting timeout to %d sec\n",
+ wdd->timeout);
+ ret = wdt_start(wdd);
+ set_bit(WDOG_HW_RUNNING, &wdd->status);
+ }
+
+ if (ret)
+ return ret;
+ }
+
ret = devm_watchdog_register_device(dev, wdd);
if (ret)
return dev_err_probe(dev, ret, "failed to register watchdog\n");
--
2.56.0
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v5 5/5] watchdog: w83627hf_wdt: Add minute mode counting
2026-10-20 8:11 ` [PATCH v5 5/5] watchdog: w83627hf_wdt: Add minute mode counting Paul Louvel
@ 2026-10-20 14:08 ` Tzung-Bi Shih
2026-10-20 15:42 ` Guenter Roeck
2026-10-20 16:28 ` Paul Louvel
0 siblings, 2 replies; 11+ messages in thread
From: Tzung-Bi Shih @ 2026-10-20 14:08 UTC (permalink / raw)
To: Paul Louvel
Cc: Wim Van Sebroeck, Guenter Roeck, linux-watchdog, linux-kernel,
Thomas Petazzoni
On Tue, Oct 20, 2026 at 10:11:58AM +0200, Paul Louvel wrote:
> All of the chips supported by the driver can be set to minute mode,
> allowing timeout value up to 255 minutes.
> Add it to the driver.
I didn't review the patch carefully as I was thinking about a fundamental
question: isn't 255 seconds already more than enough for a hardware
watchdog? Why does it need to support minute mode for timeouts larger than
that?
> @@ -49,7 +50,8 @@ enum chips { w83627hf, w83627s, w83697hf, w83697ug, w83637hf, w83627thf,
> static int timeout; /* in seconds */
> module_param(timeout, int, 0);
> MODULE_PARM_DESC(timeout,
> - "Watchdog timeout in seconds. 1 <= timeout <= 255, default="
> + "Watchdog timeout in seconds. 1 <= timeout <= "
> + __MODULE_STRING(WATCHDOG_MAX_TIMEOUT) ", default="
> __MODULE_STRING(WATCHDOG_TIMEOUT) ".");
The patch is obviously not based on the watchdog-next branch (see commit
b152ca999c5f ("watchdog: Replace __MODULE_STRING() with __stringify()")).
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v5 5/5] watchdog: w83627hf_wdt: Add minute mode counting
2026-10-20 14:08 ` Tzung-Bi Shih
@ 2026-10-20 15:42 ` Guenter Roeck
2026-10-20 16:28 ` Paul Louvel
1 sibling, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2026-10-20 15:42 UTC (permalink / raw)
To: Tzung-Bi Shih, Paul Louvel
Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel, Thomas Petazzoni
On 10/20/26 07:08, Tzung-Bi Shih wrote:
> On Tue, Oct 20, 2026 at 10:11:58AM +0200, Paul Louvel wrote:
>> All of the chips supported by the driver can be set to minute mode,
>> allowing timeout value up to 255 minutes.
>> Add it to the driver.
>
> I didn't review the patch carefully as I was thinking about a fundamental
> question: isn't 255 seconds already more than enough for a hardware
> watchdog? Why does it need to support minute mode for timeouts larger than
> that?
>
One would think so, but oddly enough there are people and even large
companies (like the one you and me work for) using such large timeouts.
Guenter
>> @@ -49,7 +50,8 @@ enum chips { w83627hf, w83627s, w83697hf, w83697ug, w83637hf, w83627thf,
>> static int timeout; /* in seconds */
>> module_param(timeout, int, 0);
>> MODULE_PARM_DESC(timeout,
>> - "Watchdog timeout in seconds. 1 <= timeout <= 255, default="
>> + "Watchdog timeout in seconds. 1 <= timeout <= "
>> + __MODULE_STRING(WATCHDOG_MAX_TIMEOUT) ", default="
>> __MODULE_STRING(WATCHDOG_TIMEOUT) ".");
>
> The patch is obviously not based on the watchdog-next branch (see commit
> b152ca999c5f ("watchdog: Replace __MODULE_STRING() with __stringify()")).
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v5 5/5] watchdog: w83627hf_wdt: Add minute mode counting
2026-10-20 14:08 ` Tzung-Bi Shih
2026-10-20 15:42 ` Guenter Roeck
@ 2026-10-20 16:28 ` Paul Louvel
1 sibling, 0 replies; 11+ messages in thread
From: Paul Louvel @ 2026-10-20 16:28 UTC (permalink / raw)
To: Tzung-Bi Shih, Paul Louvel
Cc: Wim Van Sebroeck, Guenter Roeck, linux-watchdog, linux-kernel,
Thomas Petazzoni
On Tue Oct 20, 2026 at 4:08 PM CEST, Tzung-Bi Shih wrote:
> On Tue, Oct 20, 2026 at 10:11:58AM +0200, Paul Louvel wrote:
>> All of the chips supported by the driver can be set to minute mode,
>> allowing timeout value up to 255 minutes.
>> Add it to the driver.
>
> I didn't review the patch carefully as I was thinking about a fundamental
> question: isn't 255 seconds already more than enough for a hardware
> watchdog? Why does it need to support minute mode for timeouts larger than
> that?
>
>> @@ -49,7 +50,8 @@ enum chips { w83627hf, w83627s, w83697hf, w83697ug, w83637hf, w83627thf,
>> static int timeout; /* in seconds */
>> module_param(timeout, int, 0);
>> MODULE_PARM_DESC(timeout,
>> - "Watchdog timeout in seconds. 1 <= timeout <= 255, default="
>> + "Watchdog timeout in seconds. 1 <= timeout <= "
>> + __MODULE_STRING(WATCHDOG_MAX_TIMEOUT) ", default="
>> __MODULE_STRING(WATCHDOG_TIMEOUT) ".");
>
> The patch is obviously not based on the watchdog-next branch (see commit
> b152ca999c5f ("watchdog: Replace __MODULE_STRING() with __stringify()")).
Thanks. Indeed, the series was based on v7.3-rc7 tag.
--
Paul Louvel, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 11+ messages in thread