* [PATCH v3 1/6] watchdog: w83627hf_wdt: Replace magic numbers with descriptive macros
2026-10-04 12:12 [PATCH v3 0/6] watchdog: w83627hf_wdt: Convert driver to the driver model and add minute support Paul Louvel
@ 2026-10-04 12:12 ` Paul Louvel
2026-10-06 14:24 ` Tzung-Bi Shih
2026-10-04 12:12 ` [PATCH v3 2/6] watchdog: w83627hf_wdt: Convert to platform driver model Paul Louvel
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Paul Louvel @ 2026-10-04 12:12 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: 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.
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
drivers/watchdog/w83627hf_wdt.c | 43 ++++++++++++++++++++++++++---------------
1 file changed, 27 insertions(+), 16 deletions(-)
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index db77599e43a0..1529a4e16820 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_REG_CONF_ADDR0 0x2E
+#define SIO_REG_CONF_ADDR1 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,17 @@ 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;
+ /* set second mode & disable keyboard reset turning off watchdog */
+ 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);
+ /* reset status, disable keyboard & mouse interrupt turning off watchdog */
+ t &= ~(WDT_CSR_STATUS | WDT_CSR_KBD_INT_RESET | WDT_CSR_MOUSE_INT_RESET);
superio_outb(cr_wdt_csr, t);
superio_exit();
@@ -355,7 +366,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 +442,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 +524,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_REG_CONF_ADDR0;
+ chip = wdt_find(SIO_REG_CONF_ADDR0);
if (chip < 0) {
- wdt_io = 0x4e;
- chip = wdt_find(0x4e);
+ wdt_io = SIO_REG_CONF_ADDR1;
+ chip = wdt_find(SIO_REG_CONF_ADDR1);
if (chip < 0)
return chip;
}
--
2.55.0
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3 1/6] watchdog: w83627hf_wdt: Replace magic numbers with descriptive macros
2026-10-04 12:12 ` [PATCH v3 1/6] watchdog: w83627hf_wdt: Replace magic numbers with descriptive macros Paul Louvel
@ 2026-10-06 14:24 ` Tzung-Bi Shih
0 siblings, 0 replies; 12+ messages in thread
From: Tzung-Bi Shih @ 2026-10-06 14:24 UTC (permalink / raw)
To: Paul Louvel
Cc: Wim Van Sebroeck, Guenter Roeck, linux-watchdog, linux-kernel,
Thomas Petazzoni
On Sun, Oct 04, 2026 at 02:12:49PM +0200, Paul Louvel wrote:
> @@ -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_REG_CONF_ADDR0 0x2E
> +#define SIO_REG_CONF_ADDR1 0x4E
They are actually I/O ports rather than SIO registers. SIO_PORT_2E and
SIO_PORT_4E (or SIO_CONF_PORT_*) would be better names to distinguish them
from SIO_REG_*.
> @@ -248,16 +258,17 @@ 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;
> + /* set second mode & disable keyboard reset turning off watchdog */
This is confusing. How about:
/* set second mode & disable watchdog reload on keyboard reset */
> + 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);
> + /* reset status, disable keyboard & mouse interrupt turning off watchdog */
Same here. How about:
/* reset status & disable watchdog reload on keyboard/mouse interrupts */
> @@ -513,11 +524,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_REG_CONF_ADDR0;
> + chip = wdt_find(SIO_REG_CONF_ADDR0);
> if (chip < 0) {
> - wdt_io = 0x4e;
> - chip = wdt_find(0x4e);
> + wdt_io = SIO_REG_CONF_ADDR1;
> + chip = wdt_find(SIO_REG_CONF_ADDR1);
I noticed the 'addr' parameter in wdt_find() is unused. We could consider
removing it in a later cleanup patch.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 2/6] watchdog: w83627hf_wdt: Convert to platform driver model
2026-10-04 12:12 [PATCH v3 0/6] watchdog: w83627hf_wdt: Convert driver to the driver model and add minute support Paul Louvel
2026-10-04 12:12 ` [PATCH v3 1/6] watchdog: w83627hf_wdt: Replace magic numbers with descriptive macros Paul Louvel
@ 2026-10-04 12:12 ` Paul Louvel
2026-10-06 14:24 ` Tzung-Bi Shih
2026-10-04 12:12 ` [PATCH v3 3/6] watchdog: w83627hf_wdt: Move register offsets into driver data Paul Louvel
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Paul Louvel @ 2026-10-04 12:12 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: linux-watchdog, linux-kernel, Thomas Petazzoni, Paul Louvel
Convert the driver to the Linux driver model with a platform driver /
device.
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 1529a4e16820..57f5c000f276 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -29,6 +29,7 @@
#include <linux/bits.h>
#include <linux/module.h>
+#include <linux/platform_device.h>
#include <linux/moduleparam.h>
#include <linux/types.h>
#include <linux/watchdog.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);
@@ -328,10 +334,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,
@@ -340,14 +342,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.
@@ -464,6 +458,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(&pdev->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 ret;
+
+ dev_info(dev, "initialized. timeout=%d sec (nowayout=%d)\n",
+ wdd->timeout, nowayout);
+
+ return ret;
+}
+
/*
* On some systems, the NCT6791D comes with a companion chip and the
* watchdog function is in this companion chip. We must use a different
@@ -490,36 +536,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);
@@ -533,35 +591,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.55.0
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3 2/6] watchdog: w83627hf_wdt: Convert to platform driver model
2026-10-04 12:12 ` [PATCH v3 2/6] watchdog: w83627hf_wdt: Convert to platform driver model Paul Louvel
@ 2026-10-06 14:24 ` Tzung-Bi Shih
0 siblings, 0 replies; 12+ messages in thread
From: Tzung-Bi Shih @ 2026-10-06 14:24 UTC (permalink / raw)
To: Paul Louvel
Cc: Wim Van Sebroeck, Guenter Roeck, linux-watchdog, linux-kernel,
Thomas Petazzoni
On Sun, Oct 04, 2026 at 02:12:50PM +0200, Paul Louvel wrote:
> @@ -29,6 +29,7 @@
>
> #include <linux/bits.h>
> #include <linux/module.h>
> +#include <linux/platform_device.h>
> #include <linux/moduleparam.h>
> #include <linux/types.h>
> #include <linux/watchdog.h>
Keep it sorted.
> +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(&pdev->dev, sizeof(*data), GFP_KERNEL);
^^^^^^^^^^
To be neat: `dev`.
> + ret = devm_watchdog_register_device(dev, wdd);
> + if (ret)
> + return ret;
> +
> + dev_info(dev, "initialized. timeout=%d sec (nowayout=%d)\n",
> + wdd->timeout, nowayout);
> +
> + return ret;
To be neat: `return 0;`
> +static const struct platform_device_id wdt_ids[] = {
> + { .name = "W83627HF", .driver_data = w83627hf },
...
> + {},
This is the sentinel; the last comma can/should be dropped.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 3/6] watchdog: w83627hf_wdt: Move register offsets into driver data
2026-10-04 12:12 [PATCH v3 0/6] watchdog: w83627hf_wdt: Convert driver to the driver model and add minute support Paul Louvel
2026-10-04 12:12 ` [PATCH v3 1/6] watchdog: w83627hf_wdt: Replace magic numbers with descriptive macros Paul Louvel
2026-10-04 12:12 ` [PATCH v3 2/6] watchdog: w83627hf_wdt: Convert to platform driver model Paul Louvel
@ 2026-10-04 12:12 ` Paul Louvel
2026-10-06 14:25 ` Tzung-Bi Shih
2026-10-04 12:12 ` [PATCH v3 4/6] watchdog: w83627hf_wdt: Store Super I/O configurations in platform data Paul Louvel
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Paul Louvel @ 2026-10-04 12:12 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: 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 | 73 ++++++++++++++++++++++-------------------
1 file changed, 39 insertions(+), 34 deletions(-)
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 57f5c000f276..44ec2f2e23ea 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 */
@@ -133,6 +130,11 @@ MODULE_PARM_DESC(early_disable, "Disable watchdog at boot time (default=0)");
struct w83627hf_data {
struct watchdog_device wdd;
struct watchdog_info info;
+ struct {
+ int control;
+ int timeout;
+ int csr;
+ } reg;
};
static void superio_outb(int reg, int val)
@@ -171,6 +173,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 +213,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,48 +245,49 @@ 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);
}
}
/* set second mode & disable keyboard reset turning off watchdog */
- 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;
/* reset status, disable keyboard & mouse interrupt turning off watchdog */
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();
@@ -291,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;
@@ -299,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)
@@ -316,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;
@@ -324,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;
@@ -352,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;
@@ -370,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;
@@ -431,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);
@@ -441,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;
@@ -491,6 +481,21 @@ static int wdt_probe(struct platform_device *pdev)
wdd->min_timeout = 1;
wdd->max_timeout = 255;
+ data->reg.timeout = W83627HF_WDT_TIMEOUT;
+ data->reg.control = W83627HF_WDT_CONTROL;
+ data->reg.csr = W836X7HF_WDT_CSR;
+
+ if (chip == nct6102 || chip == nct6116 || chip == nct6126) {
+ data->reg.timeout = NCT6102D_WDT_TIMEOUT;
+ data->reg.control = NCT6102D_WDT_CONTROL;
+ data->reg.csr = NCT6102D_WDT_CSR;
+ }
+
+ if (chip == w83697hf || chip == w83697ug) {
+ data->reg.timeout = W83697HF_WDT_TIMEOUT;
+ data->reg.control = W83697HF_WDT_CONTROL;
+ }
+
watchdog_set_drvdata(wdd, data);
watchdog_init_timeout(wdd, timeout, NULL);
watchdog_set_nowayout(wdd, nowayout);
--
2.55.0
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3 3/6] watchdog: w83627hf_wdt: Move register offsets into driver data
2026-10-04 12:12 ` [PATCH v3 3/6] watchdog: w83627hf_wdt: Move register offsets into driver data Paul Louvel
@ 2026-10-06 14:25 ` Tzung-Bi Shih
0 siblings, 0 replies; 12+ messages in thread
From: Tzung-Bi Shih @ 2026-10-06 14:25 UTC (permalink / raw)
To: Paul Louvel
Cc: Wim Van Sebroeck, Guenter Roeck, linux-watchdog, linux-kernel,
Thomas Petazzoni
On Sun, Oct 04, 2026 at 02:12:51PM +0200, Paul Louvel wrote:
> @@ -491,6 +481,21 @@ static int wdt_probe(struct platform_device *pdev)
> wdd->min_timeout = 1;
> wdd->max_timeout = 255;
>
> + data->reg.timeout = W83627HF_WDT_TIMEOUT;
> + data->reg.control = W83627HF_WDT_CONTROL;
> + data->reg.csr = W836X7HF_WDT_CSR;
> +
> + if (chip == nct6102 || chip == nct6116 || chip == nct6126) {
> + data->reg.timeout = NCT6102D_WDT_TIMEOUT;
> + data->reg.control = NCT6102D_WDT_CONTROL;
> + data->reg.csr = NCT6102D_WDT_CSR;
> + }
> +
> + if (chip == w83697hf || chip == w83697ug) {
> + data->reg.timeout = W83697HF_WDT_TIMEOUT;
> + data->reg.control = W83697HF_WDT_CONTROL;
> + }
> +
A switch statement would be cleaner here.
Also, there are only 3 distinct register configurations: default, nct61xx,
and w83697xx. Rather than storing and copying mutable integer fields in
every driver data instance, consider defining static const register tables
and holding a pointer to them, e.g.:
struct w83627hf_regs {
u8 timeout;
u8 control;
u8 csr;
};
static const struct w83627hf_regs w83627hf_default_regs = {
.timeout = W83627HF_WDT_TIMEOUT,
.control = W83627HF_WDT_CONTROL,
.csr = W836X7HF_WDT_CSR,
};
...
struct w83627hf_data {
...
const struct w83627hf_regs *regs;
};
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 4/6] watchdog: w83627hf_wdt: Store Super I/O configurations in platform data
2026-10-04 12:12 [PATCH v3 0/6] watchdog: w83627hf_wdt: Convert driver to the driver model and add minute support Paul Louvel
` (2 preceding siblings ...)
2026-10-04 12:12 ` [PATCH v3 3/6] watchdog: w83627hf_wdt: Move register offsets into driver data Paul Louvel
@ 2026-10-04 12:12 ` Paul Louvel
2026-10-06 14:25 ` Tzung-Bi Shih
2026-10-04 12:12 ` [PATCH v3 5/6] watchdog: w83627hf_wdt: Add minute mode counting Paul Louvel
2026-10-04 12:12 ` [PATCH v3 6/6] watchdog: w83627hf_wdt: Report all initialization failures in probe Paul Louvel
5 siblings, 1 reply; 12+ messages in thread
From: Paul Louvel @ 2026-10-04 12:12 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: 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>
---
drivers/watchdog/w83627hf_wdt.c | 156 +++++++++++++++++++++++-----------------
1 file changed, 89 insertions(+), 67 deletions(-)
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 44ec2f2e23ea..1ef7b61d7e55 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_REG_CONF_ADDR0 0x2E
#define SIO_REG_CONF_ADDR1 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
@@ -127,6 +122,12 @@ 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 wdt_pdata {
+ int sioaddr;
+ int siocfg_enter;
+ int siocfg_leave;
+};
+
struct w83627hf_data {
struct watchdog_device wdd;
struct watchdog_info info;
@@ -135,40 +136,43 @@ struct w83627hf_data {
int timeout;
int csr;
} reg;
+ int sioaddr;
+ int siocfg_enter;
+ int siocfg_leave;
};
-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)
@@ -177,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->sioaddr, data->siocfg_enter);
if (ret)
return ret;
- superio_select(W83627HF_LD_WDT);
+ superio_select(data->sioaddr, W83627HF_LD_WDT);
/* set CR30 bit 0 to activate GPIO2 */
- t = superio_inb(SIO_REG_ENABLE);
+ t = superio_inb(data->sioaddr, SIO_REG_ENABLE);
if (!(t & 0x01))
- superio_outb(SIO_REG_ENABLE, t | 0x01);
+ superio_outb(data->sioaddr, 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->sioaddr, 0x2B) & ~0x10;
+ superio_outb(data->sioaddr, 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->sioaddr, 0x29) & ~0x60;
t |= 0x20;
- superio_outb(0x29, t);
+ superio_outb(data->sioaddr, 0x29, t);
break;
case w83697ug:
/* Set pin 118 to WDTO# mode */
- t = superio_inb(0x2b) & ~0x04;
- superio_outb(0x2b, t);
+ t = superio_inb(data->sioaddr, 0x2b) & ~0x04;
+ superio_outb(data->sioaddr, 0x2b, t);
break;
case w83627thf:
- t = (superio_inb(0x2B) & ~0x08) | 0x04;
- superio_outb(0x2B, t); /* set GPIO3 to WDT0 */
+ t = (superio_inb(data->sioaddr, 0x2B) & ~0x08) | 0x04;
+ superio_outb(data->sioaddr, 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->sioaddr, 0x2D) & ~0x01; /* PIN77 -> WDT0# */
+ superio_outb(data->sioaddr, 0x2D, t); /* set GPIO5 to WDT0 */
+ t = superio_inb(data->sioaddr, data->reg.control);
t |= 0x02; /* enable the WDTO# output low pulse
* to the KBRST# pin */
- superio_outb(data->reg.control, t);
+ superio_outb(data->sioaddr, 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->sioaddr, 0x2C) & ~0x80; /* PIN47 -> WDT0# */
+ superio_outb(data->sioaddr, 0x2C, t);
break;
case w83627ehf:
case w83627uhg:
@@ -245,42 +249,42 @@ 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->sioaddr, data->reg.control);
t |= 0x02; /* enable the WDTO# output low pulse
* to the KBRST# pin */
- superio_outb(data->reg.control, t);
+ superio_outb(data->sioaddr, data->reg.control, t);
break;
default:
break;
}
- t = superio_inb(data->reg.timeout);
+ t = superio_inb(data->sioaddr, 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->sioaddr, 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->sioaddr, data->reg.timeout, wdog->timeout);
set_bit(WDOG_HW_RUNNING, &wdog->status);
}
}
/* set second mode & disable keyboard reset turning off watchdog */
- t = superio_inb(data->reg.control) &
+ t = superio_inb(data->sioaddr, data->reg.control) &
~(WDT_CTRL_MINUTE_MODE | WDT_CTRL_RISING_EDGE_KBD_RESET);
- superio_outb(data->reg.control, t);
+ superio_outb(data->sioaddr, data->reg.control, t);
- t = superio_inb(data->reg.csr);
+ t = superio_inb(data->sioaddr, data->reg.csr);
if (t & WDT_CSR_STATUS)
wdog->bootstatus |= WDIOF_CARDRESET;
/* reset status, disable keyboard & mouse interrupt turning off watchdog */
t &= ~(WDT_CSR_STATUS | WDT_CSR_KBD_INT_RESET | WDT_CSR_MOUSE_INT_RESET);
- superio_outb(data->reg.csr, t);
+ superio_outb(data->sioaddr, data->reg.csr, t);
- superio_exit();
+ superio_exit(data->sioaddr, data->siocfg_leave);
return 0;
}
@@ -290,13 +294,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->sioaddr, data->siocfg_enter);
if (ret)
return ret;
- superio_select(W83627HF_LD_WDT);
- superio_outb(data->reg.timeout, timeout);
- superio_exit();
+ superio_select(data->sioaddr, W83627HF_LD_WDT);
+ superio_outb(data->sioaddr, data->reg.timeout, timeout);
+ superio_exit(data->sioaddr, data->siocfg_leave);
return 0;
}
@@ -324,13 +328,13 @@ static unsigned int wdt_get_time(struct watchdog_device *wdog)
unsigned int timeleft;
int ret;
- ret = superio_enter();
+ ret = superio_enter(data->sioaddr, data->siocfg_enter);
if (ret)
return 0;
- superio_select(W83627HF_LD_WDT);
- timeleft = superio_inb(data->reg.timeout);
- superio_exit();
+ superio_select(data->sioaddr, W83627HF_LD_WDT);
+ timeleft = superio_inb(data->sioaddr, data->reg.timeout);
+ superio_exit(data->sioaddr, data->siocfg_leave);
return timeleft;
}
@@ -352,16 +356,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 +434,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,13 +448,14 @@ static int wdt_find(int addr)
pr_err("Unsupported chip ID: 0x%02x\n", val);
break;
}
- superio_exit();
+ superio_exit(addr, leave);
return ret;
}
static int wdt_probe(struct platform_device *pdev)
{
const struct platform_device_id *id = platform_get_device_id(pdev);
+ const struct wdt_pdata *pdata = pdev->dev.platform_data;
struct device *dev = &pdev->dev;
struct watchdog_device *wdd;
struct w83627hf_data *data;
@@ -462,6 +467,9 @@ 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(&pdev->dev, sizeof(*data), GFP_KERNEL);
@@ -481,6 +489,9 @@ static int wdt_probe(struct platform_device *pdev)
wdd->min_timeout = 1;
wdd->max_timeout = 255;
+ data->sioaddr = pdata->sioaddr;
+ data->siocfg_enter = pdata->siocfg_enter;
+ data->siocfg_leave = pdata->siocfg_leave;
data->reg.timeout = W83627HF_WDT_TIMEOUT;
data->reg.control = W83627HF_WDT_CONTROL;
data->reg.csr = W836X7HF_WDT_CSR;
@@ -522,12 +533,16 @@ static int wdt_probe(struct platform_device *pdev)
*/
static int __init wdt_use_alt_key(const struct dmi_system_id *d)
{
- wdt_cfg_enter = 0x88;
- wdt_cfg_leave = 0xBB;
+ struct wdt_pdata *pdata = d->driver_data;
+
+ pdata->siocfg_enter = 0x88;
+ pdata->siocfg_leave = 0xBB;
return 0;
}
+static struct wdt_pdata pdata;
+
static const struct dmi_system_id wdt_dmi_table[] __initconst = {
{
.matches = {
@@ -537,6 +552,7 @@ static const struct dmi_system_id wdt_dmi_table[] __initconst = {
DMI_EXACT_MATCH(DMI_BOARD_NAME, "SHARKBAY"),
},
.callback = wdt_use_alt_key,
+ .driver_data = &pdata,
},
{}
};
@@ -581,27 +597,33 @@ static struct platform_device *wdt_pdev;
static int __init wdt_init(void)
{
+ int sioaddr;
int ret;
int chip;
+ pdata.siocfg_enter = 0x87;
+ pdata.siocfg_leave = 0xAA;
+
/* Apply system-specific quirks */
dmi_check_system(wdt_dmi_table);
- wdt_io = SIO_REG_CONF_ADDR0;
- chip = wdt_find(SIO_REG_CONF_ADDR0);
+ sioaddr = SIO_REG_CONF_ADDR0;
+ chip = wdt_find(sioaddr, pdata.siocfg_enter, pdata.siocfg_leave);
if (chip < 0) {
- wdt_io = SIO_REG_CONF_ADDR1;
- chip = wdt_find(SIO_REG_CONF_ADDR1);
+ sioaddr = SIO_REG_CONF_ADDR1;
+ chip = wdt_find(sioaddr, pdata.siocfg_enter, pdata.siocfg_leave);
if (chip < 0)
return chip;
}
+ pdata.sioaddr = sioaddr;
+
ret = platform_driver_register(&wdt_driver);
if (ret)
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.55.0
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3 4/6] watchdog: w83627hf_wdt: Store Super I/O configurations in platform data
2026-10-04 12:12 ` [PATCH v3 4/6] watchdog: w83627hf_wdt: Store Super I/O configurations in platform data Paul Louvel
@ 2026-10-06 14:25 ` Tzung-Bi Shih
0 siblings, 0 replies; 12+ messages in thread
From: Tzung-Bi Shih @ 2026-10-06 14:25 UTC (permalink / raw)
To: Paul Louvel
Cc: Wim Van Sebroeck, Guenter Roeck, linux-watchdog, linux-kernel,
Thomas Petazzoni
On Sun, Oct 04, 2026 at 02:12:52PM +0200, Paul Louvel wrote:
> static int wdt_probe(struct platform_device *pdev)
> {
> const struct platform_device_id *id = platform_get_device_id(pdev);
> + const struct wdt_pdata *pdata = pdev->dev.platform_data;
Use dev_get_platdata().
> @@ -522,12 +533,16 @@ static int wdt_probe(struct platform_device *pdev)
> */
> static int __init wdt_use_alt_key(const struct dmi_system_id *d)
> {
> - wdt_cfg_enter = 0x88;
> - wdt_cfg_leave = 0xBB;
> + struct wdt_pdata *pdata = d->driver_data;
> +
> + pdata->siocfg_enter = 0x88;
> + pdata->siocfg_leave = 0xBB;
>
> return 0;
> }
>
> +static struct wdt_pdata pdata;
> +
> static const struct dmi_system_id wdt_dmi_table[] __initconst = {
> {
> .matches = {
> @@ -537,6 +552,7 @@ static const struct dmi_system_id wdt_dmi_table[] __initconst = {
> DMI_EXACT_MATCH(DMI_BOARD_NAME, "SHARKBAY"),
> },
> .callback = wdt_use_alt_key,
> + .driver_data = &pdata,
> },
> {}
> };
> @@ -581,27 +597,33 @@ static struct platform_device *wdt_pdev;
>
> static int __init wdt_init(void)
> {
> + int sioaddr;
> int ret;
> int chip;
>
> + pdata.siocfg_enter = 0x87;
> + pdata.siocfg_leave = 0xAA;
> +
> /* Apply system-specific quirks */
> dmi_check_system(wdt_dmi_table);
>
> - wdt_io = SIO_REG_CONF_ADDR0;
> - chip = wdt_find(SIO_REG_CONF_ADDR0);
> + sioaddr = SIO_REG_CONF_ADDR0;
> + chip = wdt_find(sioaddr, pdata.siocfg_enter, pdata.siocfg_leave);
> if (chip < 0) {
> - wdt_io = SIO_REG_CONF_ADDR1;
> - chip = wdt_find(SIO_REG_CONF_ADDR1);
> + sioaddr = SIO_REG_CONF_ADDR1;
> + chip = wdt_find(sioaddr, pdata.siocfg_enter, pdata.siocfg_leave);
> if (chip < 0)
> return chip;
> }
>
> + pdata.sioaddr = sioaddr;
> +
`pdata` is still global. Given that there are only a few settings, maybe
we can use the same method I proposed in [3/6] patch, and use
`dmi_first_match()` to avoid the global variable. E.g.:
struct w83627hf_sio_pdata {
u16 sioaddr;
u8 siocfg_enter;
u8 siocfg_leave;
};
static const struct w83627hf_sio_pdata default_pdata = {
.siocfg_enter = 0x87,
.siocfg_leave = 0xaa,
};
static const struct w83627hf_sio_pdata alt_pdata = {
.siocfg_enter = 0x88,
.siocfg_leave = 0xbb,
};
static const struct dmi_system_id wdt_dmi_table[] __initconst = {
{
.matches = {
DMI_EXACT_MATCH(DMI_SYS_VENDOR, "INVENTEC"),
DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "Symphony"),
DMI_EXACT_MATCH(DMI_BOARD_NAME, "SHARKBAY"),
},
.driver_data = (void *)&alt_pdata,
},
{}
};
Then, in `wdt_init()`, it can define a local `pdata` and initialize it
based on the DMI match:
const struct dmi_system_id *match;
struct w83627hf_sio_pdata pdata;
match = dmi_first_match(wdt_dmi_table);
if (match)
pdata = *(const struct w83627hf_sio_pdata *)match->driver_data;
else
pdata = default_pdata;
pdata.sioaddr = SIO_REG_CONF_ADDR0;
chip = wdt_find(pdata.sioaddr, pdata.siocfg_enter, pdata.siocfg_leave);
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 5/6] watchdog: w83627hf_wdt: Add minute mode counting
2026-10-04 12:12 [PATCH v3 0/6] watchdog: w83627hf_wdt: Convert driver to the driver model and add minute support Paul Louvel
` (3 preceding siblings ...)
2026-10-04 12:12 ` [PATCH v3 4/6] watchdog: w83627hf_wdt: Store Super I/O configurations in platform data Paul Louvel
@ 2026-10-04 12:12 ` Paul Louvel
2026-10-04 12:12 ` [PATCH v3 6/6] watchdog: w83627hf_wdt: Report all initialization failures in probe Paul Louvel
5 siblings, 0 replies; 12+ messages in thread
From: Paul Louvel @ 2026-10-04 12:12 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: 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 | 70 +++++++++++++++++++++++++++++++----------
1 file changed, 53 insertions(+), 17 deletions(-)
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 1ef7b61d7e55..1aba9efa27fe 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 {
int sioaddr;
int siocfg_enter;
int siocfg_leave;
+ bool minute_mode;
+ u8 early_timer_val;
+ u8 timer_val;
};
static void superio_outb(int base, int reg, int val)
@@ -258,22 +263,11 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
break;
}
- t = superio_inb(data->sioaddr, data->reg.timeout);
- if (t != 0) {
- if (early_disable) {
- pr_warn("Stopping previously enabled watchdog until userland kicks in\n");
- superio_outb(data->sioaddr, data->reg.timeout, 0);
- } else {
- pr_info("Watchdog already running. Resetting timeout to %d sec\n",
- wdog->timeout);
- superio_outb(data->sioaddr, data->reg.timeout, wdog->timeout);
- set_bit(WDOG_HW_RUNNING, &wdog->status);
- }
- }
+ data->early_timer_val = superio_inb(data->sioaddr, data->reg.timeout);
- /* set second mode & disable keyboard reset turning off watchdog */
+ /* disable keyboard reset turning off watchdog */
t = superio_inb(data->sioaddr, data->reg.control) &
- ~(WDT_CTRL_MINUTE_MODE | WDT_CTRL_RISING_EDGE_KBD_RESET);
+ ~WDT_CTRL_RISING_EDGE_KBD_RESET;
superio_outb(data->sioaddr, data->reg.control, t);
t = superio_inb(data->sioaddr, data->reg.csr);
@@ -292,6 +286,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->sioaddr, data->siocfg_enter);
@@ -299,6 +294,15 @@ static int wdt_set_time(struct watchdog_device *wdog, unsigned int timeout)
return ret;
superio_select(data->sioaddr, W83627HF_LD_WDT);
+
+ ctrl = superio_inb(data->sioaddr, data->reg.control);
+
+ if (data->minute_mode)
+ ctrl |= WDT_CTRL_MINUTE_MODE;
+ else
+ ctrl &= ~WDT_CTRL_MINUTE_MODE;
+
+ superio_outb(data->sioaddr, data->reg.control, ctrl);
superio_outb(data->sioaddr, data->reg.timeout, timeout);
superio_exit(data->sioaddr, data->siocfg_leave);
@@ -307,7 +311,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)
@@ -317,6 +323,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;
@@ -334,6 +351,8 @@ static unsigned int wdt_get_time(struct watchdog_device *wdog)
superio_select(data->sioaddr, W83627HF_LD_WDT);
timeleft = superio_inb(data->sioaddr, data->reg.timeout);
+ if (data->minute_mode)
+ timeleft *= 60;
superio_exit(data->sioaddr, data->siocfg_leave);
return timeleft;
@@ -487,7 +506,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;
data->sioaddr = pdata->sioaddr;
data->siocfg_enter = pdata->siocfg_enter;
@@ -512,10 +531,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 ret;
--
2.55.0
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v3 6/6] watchdog: w83627hf_wdt: Report all initialization failures in probe
2026-10-04 12:12 [PATCH v3 0/6] watchdog: w83627hf_wdt: Convert driver to the driver model and add minute support Paul Louvel
` (4 preceding siblings ...)
2026-10-04 12:12 ` [PATCH v3 5/6] watchdog: w83627hf_wdt: Add minute mode counting Paul Louvel
@ 2026-10-04 12:12 ` Paul Louvel
2026-10-06 14:25 ` Tzung-Bi Shih
5 siblings, 1 reply; 12+ messages in thread
From: Paul Louvel @ 2026-10-04 12:12 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: linux-watchdog, linux-kernel, Thomas Petazzoni, Paul Louvel
The driver currently logs an error only if the chip initialization
fails. Extend the error reporting to all failure paths in probe to
improve diagnostics.
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
drivers/watchdog/w83627hf_wdt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 1aba9efa27fe..bba7882b254e 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -549,12 +549,12 @@ static int wdt_probe(struct platform_device *pdev)
}
if (ret)
- return ret;
+ return dev_err_probe(dev, ret, "failed to set watchdog timeout\n");
}
ret = devm_watchdog_register_device(dev, wdd);
if (ret)
- return 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);
--
2.55.0
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3 6/6] watchdog: w83627hf_wdt: Report all initialization failures in probe
2026-10-04 12:12 ` [PATCH v3 6/6] watchdog: w83627hf_wdt: Report all initialization failures in probe Paul Louvel
@ 2026-10-06 14:25 ` Tzung-Bi Shih
0 siblings, 0 replies; 12+ messages in thread
From: Tzung-Bi Shih @ 2026-10-06 14:25 UTC (permalink / raw)
To: Paul Louvel
Cc: Wim Van Sebroeck, Guenter Roeck, linux-watchdog, linux-kernel,
Thomas Petazzoni
On Sun, Oct 04, 2026 at 02:12:54PM +0200, Paul Louvel wrote:
> @@ -549,12 +549,12 @@ static int wdt_probe(struct platform_device *pdev)
> }
>
> if (ret)
> - return ret;
> + return dev_err_probe(dev, ret, "failed to set watchdog timeout\n");
> }
>
> ret = devm_watchdog_register_device(dev, wdd);
> if (ret)
> - return ret;
> + return dev_err_probe(dev, ret, "failed to register watchdog\n");
Could this patch be split and squashed into the earlier patches in the
series?
^ permalink raw reply [flat|nested] 12+ messages in thread