[PATCH v4 0/6] watchdog: w83627hf_wdt: Convert driver to the driver model and add minute support

[PATCH v4 0/6] watchdog: w83627hf_wdt: Convert driver to the driver model and add minute support

[Paul Louvel]

Hi,

This series aims to improve driver readability and convert it to the
Linux driver model by means of a platform driver.
Furthermore, support for minute-based counting has been added to allow
timeout values longer than 255 seconds. When setting a timeout value
that is not a multiple of 60 seconds, the value is rounded up to the
nearest multiple.

Notes:

  - PATCH #2: I could not introduce a macro for each hard-coded value in
    this driver, since some datasheets are not available (to my
    knowledge) on the internet.

  - PATCH #7: I have moved the code that refreshes or stops the watchdog
    timer out of the w83627hf_init() function and into the wdt_probe()
    function. Keeping it there would have led to code duplication.

    Since the timer has a granularity of at most one second, this does
    not introduce an unexpected reboot: the timer cannot expire in
    between, as at most a few milliseconds would have elapsed (the
    margin is < 1s anyway).

This series was tested on a x86-64 board featuring a Nuvoton NCT6126
Super I/O chip.

Best regards,

To: Wim Van Sebroeck <wim@linux-watchdog.org>
To: Guenter Roeck <linux@roeck-us.net>
Cc: Tzung-Bi Shih <tzungbi@kernel.org>
Cc: linux-watchdog@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
Changes in v4:
- Squashed last commit into patch #2.
- Applied suggestions of Tzung-Bi.
- Added one commit in the beginning of the series, fixing an issue
  reported by the kernel test robot that is present in an older commit.
- Link to v3: https://patch.msgid.link/20261004-w83627hf_wdt-improvements-v3-0-8e27b518595e@bootlin.com

Changes in v3:

- Drop the platform device resource for the Super I/O configuration port:
  claiming the shared 0x2e/0x4e port could break concurrent
  request_muxed_region() users on module unload (Sashiko). The port
  address is now passed via platform data together with the
  configuration keys.
- Patches 5 and 6 are merged into "Store Super I/O configurations in platform data".
- Validate platform_get_device_id() and platform_data in probe before
  use.
- struct watchdog_device parent member is now assigned to pdev device
  instance.
- Re-parent this series on 7.3-rc5.
- Remove the dependency in the cover letter since it was merged.

- Link to v2: https://patch.msgid.link/20260726-w83627hf_wdt-improvements-v2-0-3645a2a6c022@bootlin.com

Changes in v2:
- Initialize res stack variable in PATCH #5.
- Reworded PATCH #1. Describe the introduction of BIT().
- Remove the usage in PATCH #8 of goto in error path.
- Dropped PATCH #9. Squashed with PATCH #2.
- Link to v1: https://patch.msgid.link/20260725-w83627hf_wdt-improvements-v1-0-4e9a1b4e8297@bootlin.com

---
Paul Louvel (6):
      watchdog: w83627hf_wdt: Fix watchdog identity truncation
      watchdog: w83627hf_wdt: Replace magic numbers with descriptive macros
      watchdog: w83627hf_wdt: Convert to platform driver model
      watchdog: w83627hf_wdt: Move register offsets into driver data
      watchdog: w83627hf_wdt: Store Super I/O configurations in platform data
      watchdog: w83627hf_wdt: Add minute mode counting

 drivers/watchdog/w83627hf_wdt.c | 467 +++++++++++++++++++++++++---------------
 1 file changed, 299 insertions(+), 168 deletions(-)
---
base-commit: 028ef9c96e96197026887c0f092424679298aae8
change-id: 20260713-w83627hf_wdt-improvements-ba2364157d6e
prerequisite-change-id: 20260705-w83627hf_wdt-nct6126d-9a016bf7c936:v4
prerequisite-patch-id: 1162062ebf31e7c2c70ff52b79118b67b00c396b
prerequisite-patch-id: 9638243b3ade2a4e9c3e94664343b235e5cb806d
prerequisite-patch-id: 73803f726f2fab91516b429f79a59c2d3a574cfe

Best regards,
--  
Paul Louvel, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCH v4 1/6] watchdog: w83627hf_wdt: Fix watchdog identity truncation

[Paul Louvel]

The identity field in struct watchdog_info is only 32 bytes in size, but
the identity could be at most 33 bytes in this driver, resulting in
truncation.
Remove the "Watchdog" suffix, as this is implied that the device is a
watchdog if a user request his identity.

Cc: <stable@vger.kernel.org>
Fixes: 3d6757a640f8 ("watchdog: w83627hf_wdt: Report the probed chip name via WDIOC_GETSUPPORT")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202610080027.3OsJDwfo-lkp@intel.com/
Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
 drivers/watchdog/w83627hf_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index db77599e43a0..0fc7648ef337 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -525,7 +525,7 @@ static int __init wdt_init(void)
 	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",
+	snprintf(wdt_info.identity, sizeof(wdt_info.identity), "%s",
 		 chip_name[chip]);
 
 	watchdog_init_timeout(&wdt_dev, timeout, NULL);

-- 
2.56.0

[PATCH v4 2/6] watchdog: w83627hf_wdt: Replace magic numbers with descriptive macros

[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>
---
 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 0fc7648ef337..c9ad1545d525 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

[PATCH v4 3/6] watchdog: w83627hf_wdt: Convert to platform driver model

[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 c9ad1545d525..cc72d2f5e56a 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 dev_err_probe(dev, -ENOMEM, "failed to allocate memory\n");
+
+	data->info.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE;
+	snprintf(data->info.identity, sizeof(data->info.identity), "%s",
+		 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",
-		 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

[PATCH v4 4/6] watchdog: w83627hf_wdt: Move register offsets into driver data

[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 cc72d2f5e56a..4943ae164771 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 = {
+	.control = W83627HF_WDT_CONTROL,
+	.timeout = W83627HF_WDT_TIMEOUT,
+	.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

[PATCH v4 5/6] watchdog: w83627hf_wdt: Store Super I/O configurations in platform data

[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 | 160 ++++++++++++++++++++++------------------
 1 file changed, 87 insertions(+), 73 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 4943ae164771..0b9195e7e95e 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 dev_err_probe(dev, -ENOMEM, "failed to allocate memory\n");
 
+	data->sio = pdata;
 	data->info.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE;
 	snprintf(data->info.identity, sizeof(data->info.identity), "%s",
 		 id->name);
@@ -533,18 +540,21 @@ 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 +564,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 +609,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 +633,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

[PATCH v4 6/6] watchdog: w83627hf_wdt: Add minute mode counting

[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 0b9195e7e95e..8f78fe64acde 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

Re: [PATCH v4 6/6] watchdog: w83627hf_wdt: Add minute mode counting

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] watchdog: spurious hardware reboot during timeout mode transition
--

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

watchdog: w83627hf_wdt: Add minute mode counting

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.

> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> index 0b9195e7e95ef..8f78fe64acde9 100644
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c

[ ... ]

> @@ -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);

[Severity: High]
Can this sequence trigger a spurious hardware reboot during a timeout mode
transition?

In wdt_set_time(), the hardware control register is updated first to change
between minute and second modes, immediately followed by the timeout register:

	superio_outb(data->sio->addr, data->reg->control, ctrl);
	superio_outb(data->sio->addr, data->reg->timeout, timeout);

When transitioning from minute mode to second mode (for example, from 1 minute
to 10 seconds), the hardware control register is updated first. The hardware
timer momentarily retains its old value (1) but interprets it under the new
unit (seconds), meaning it evaluates that only 1 second is left.

If the thread is preempted (e.g., by an SMI or a hypervisor) between these two
superio_outb() calls for longer than this intermediate duration, will the
hardware watchdog fire and spuriously reboot the system?

Would it be safer to temporarily disable the watchdog (by writing 0 to the
timeout register) before modifying the control register's mode bit?

>  	superio_exit(data->sio->addr, data->sio->cfg_leave);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013-w83627hf_wdt-improvements-v4-0-b6f6b60a9566@bootlin.com?part=6

Re: [PATCH v4 6/6] watchdog: w83627hf_wdt: Add minute mode counting

[Paul Louvel]

On Tue Oct 13, 2026 at 4:08 PM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] watchdog: spurious hardware reboot during timeout mode transition
> --
>
> commit e9913194bed75434ff662763db21c93683fc01dc
> Author: Paul Louvel <paul.louvel@bootlin.com>
>
> watchdog: w83627hf_wdt: Add minute mode counting
>
> 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.
>
>> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
>> index 0b9195e7e95ef..8f78fe64acde9 100644
>> --- a/drivers/watchdog/w83627hf_wdt.c
>> +++ b/drivers/watchdog/w83627hf_wdt.c
>
> [ ... ]
>
>> @@ -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);
>
> [Severity: High]
> Can this sequence trigger a spurious hardware reboot during a timeout mode
> transition?
>
> In wdt_set_time(), the hardware control register is updated first to change
> between minute and second modes, immediately followed by the timeout register:
>
> 	superio_outb(data->sio->addr, data->reg->control, ctrl);
> 	superio_outb(data->sio->addr, data->reg->timeout, timeout);
>
> When transitioning from minute mode to second mode (for example, from 1 minute
> to 10 seconds), the hardware control register is updated first. The hardware
> timer momentarily retains its old value (1) but interprets it under the new
> unit (seconds), meaning it evaluates that only 1 second is left.
>
> If the thread is preempted (e.g., by an SMI or a hypervisor) between these two
> superio_outb() calls for longer than this intermediate duration, will the
> hardware watchdog fire and spuriously reboot the system?

Yes it could happen if the hardware does not take measures to avoid this. Also,
one second is a long time between those two writes, I doubt this ever happens in
practice.

>
> Would it be safer to temporarily disable the watchdog (by writing 0 to the
> timeout register) before modifying the control register's mode bit?
>

Yes.

>>  	superio_exit(data->sio->addr, data->sio->cfg_leave);
>>

-- 
Paul Louvel, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com