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

[PATCH 0/9] 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 #1: 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.

This series depends on:

  https://lore.kernel.org/all/20260707-w83627hf_wdt-nct6126d-v4-0-829e5f63d548@bootlin.com/

Thanks.

Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
Changes in v2:
- EDITME: describe what is new in this series revision.
- EDITME: use bulletpoints and terse descriptions.
- Link to v1: https://patch.msgid.link/20260724-w83627hf_wdt-improvements-v1-0-71be4f59ef3f@bootlin.com

To: Wim Van Sebroeck <wim@linux-watchdog.org>
To: Guenter Roeck <linux@roeck-us.net>
Cc: linux-watchdog@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Paul Louvel (9):
      watchdog: w83627hf_wdt: Replace magic numbers with descriptive macros
      watchdog: w83627hf_wdt: Convert to platform driver model
      watchdog: w83627hf_wdt: Use private driver data structure
      watchdog: w83627hf_wdt: Move register offsets into driver data
      watchdog: w83627hf_wdt: Add Super I/O configuration port as a platform device resource
      watchdog: w83627hf_wdt: Store Super I/O unlocking sequence in platform data
      watchdog: w83627hf_wdt: Add minute mode counting
      watchdog: w83627hf_wdt: Report all initialization failures in probe
      watchdog: w83627hf_wdt: Use dev_* logging instead of pr_*

 drivers/watchdog/w83627hf_wdt.c | 457 +++++++++++++++++++++++++---------------
 1 file changed, 290 insertions(+), 167 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 1/9] 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.

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

Re: [PATCH 1/9] watchdog: w83627hf_wdt: Replace magic numbers with descriptive macros

[Guenter Roeck]

On 7/25/26 03:29, Paul Louvel wrote:
> Raw hex values for Super I/O register addresses are difficult to understand
> without constantly consulting the datasheet. Use named macros instead.
> 

This is not the only change. The patch also introduces the use of BIT().

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

Address 0x30 is always the enable register for a logical device (here: the watchdog
logical device plus for some of the chips some GPIO information). Bit 0 enables the
watchdog logical device.

Guenter

[PATCH 2/9] watchdog: w83627hf_wdt: Convert to platform driver model

[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 | 131 ++++++++++++++++++++++++++--------------
 1 file changed, 84 insertions(+), 47 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 1529a4e16820..993e65ce4447 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,8 @@ 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 {};
+
 static void superio_outb(int reg, int val)
 {
 	outb(reg, WDT_EFER);
@@ -464,6 +467,37 @@ 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);
+	enum chips chip = id->driver_data;
+	int ret;
+
+	pr_info("WDT driver for %s Super I/O chip initialising\n", id->name);
+
+	snprintf(wdt_info.identity, sizeof(wdt_info.identity), "%s Watchdog",
+		 id->name);
+
+	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 = devm_watchdog_register_device(&pdev->dev, &wdt_dev);
+	if (ret)
+		return ret;
+
+	pr_info("initialized. timeout=%d sec (nowayout=%d)\n", wdt_dev.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 +524,49 @@ 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)
 {
+	struct wdt_pdata pdata;
 	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 +580,25 @@ 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, &pdata,
+						 sizeof(pdata));
+	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

Re: [PATCH 2/9] watchdog: w83627hf_wdt: Convert to platform driver model

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Potential NULL pointer dereference in `wdt_probe()` when matching via `driver_override`.
- [High] State contamination across unbind/bind cycles due to the use of shared global device structures.
- [Medium] The watchdog device's parent is not set, resulting in an incomplete sysfs topology and preventing property parsing.
- [Low] An uninitialized empty structure `struct wdt_pdata` is passed to `platform_device_register_data()`.
--

--- Patch [2]: [PATCH 2/9] watchdog: w83627hf_wdt: Convert to platform driver model ---
commit dfce32dc3eb2e74bc7730f263ff7f45b7b16d876
Author: Paul Louvel <paul.louvel@bootlin.com>

watchdog: w83627hf_wdt: Convert to platform driver model

This commit converts the w83627hf_wdt driver to use the Linux driver model
by registering it as a platform driver. It replaces direct device 
initialization and registration with a probe function and standard
platform device structures.

> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> index 1529a4e168209..993e65ce4447c 100644
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c

[ ... ]

> @@ -464,6 +467,37 @@ 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);
> +	enum chips chip = id->driver_data;

[Severity: High]
Could platform_get_device_id() return a NULL pointer here if a privileged
user manually forces the device to bind to this driver using the
driver_override sysfs interface?

If so, does id->driver_data trigger a NULL pointer dereference since the
platform bus matching logic bypasses the id_table?

> +	int ret;
> +
> +	pr_info("WDT driver for %s Super I/O chip initialising\n", id->name);
> +
> +	snprintf(wdt_info.identity, sizeof(wdt_info.identity), "%s Watchdog",
> +		 id->name);
> +
> +	watchdog_init_timeout(&wdt_dev, timeout, NULL);

[Severity: High]
Does modifying statically allocated global structures like wdt_info and
wdt_dev instead of using dynamically allocated per-device state cause state
contamination across unbind/bind cycles?

If the driver is unbound and rebound via sysfs, the wdt_dev.status bitmask
retains accumulated state flags like WDOG_HW_RUNNING. If this flag is falsely
retained, wouldn't the watchdog core spawn a worker thread to automatically
ping the watchdog on behalf of the kernel, defeating the watchdog's purpose
if userspace hangs?

> +	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 = devm_watchdog_register_device(&pdev->dev, &wdt_dev);

[Severity: Medium]
Is the sysfs topology broken here because the global wdt_dev struct lacks
a .parent = &pdev->dev assignment before registration?

Since the parent is not set and watchdog_init_timeout() earlier is passed a
NULL device pointer, won't the watchdog character device be created at the
root of the sysfs hierarchy instead of as a child of the platform device,
preventing standard properties like timeout-sec from being parsed?

> +	if (ret)
> +		return ret;
> +
> +	pr_info("initialized. timeout=%d sec (nowayout=%d)\n", wdt_dev.timeout,
> +		nowayout);
> +
> +	return ret;
> +}

[ ... ]

> @@ -533,35 +580,25 @@ 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, &pdata,
> +						 sizeof(pdata));

[Severity: Low]
Does passing the uninitialized stack variable pdata serve a purpose here?
Although sizeof(pdata) evaluating to 0 makes it currently safe, would it
be clearer to pass NULL and 0 for the size instead?

> +	if (IS_ERR(wdt_pdev)) {
> +		platform_driver_unregister(&wdt_driver);
> +		return PTR_ERR(wdt_pdev);
> +	}
>  
> -	return ret;
> +	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725-w83627hf_wdt-improvements-v1-0-4e9a1b4e8297@bootlin.com?part=2

Re: [PATCH 2/9] watchdog: w83627hf_wdt: Convert to platform driver model

[Guenter Roeck]

On 7/25/26 03:29, Paul Louvel wrote:
> 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 | 131 ++++++++++++++++++++++++++--------------
>   1 file changed, 84 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> index 1529a4e16820..993e65ce4447 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,8 @@ 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 {};
> +
>   static void superio_outb(int reg, int val)
>   {
>   	outb(reg, WDT_EFER);
> @@ -464,6 +467,37 @@ 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);
> +	enum chips chip = id->driver_data;
> +	int ret;
> +
> +	pr_info("WDT driver for %s Super I/O chip initialising\n", id->name);
> +

Please drop as noise.

> +	snprintf(wdt_info.identity, sizeof(wdt_info.identity), "%s Watchdog",
> +		 id->name);
> +
> +	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;

		return dev_err_probe(...);

> +	}
> +
> +	ret = devm_watchdog_register_device(&pdev->dev, &wdt_dev);
> +	if (ret)
> +		return ret;
> +
> +	pr_info("initialized. timeout=%d sec (nowayout=%d)\n", wdt_dev.timeout,
> +		nowayout);

	dev_info()

> +
> +	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 +524,49 @@ 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)
>   {
> +	struct wdt_pdata pdata;
>   	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 +580,25 @@ 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, &pdata,
> +						 sizeof(pdata));
> +	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);
> 

[PATCH 3/9] watchdog: w83627hf_wdt: Use private driver data structure

[Paul Louvel]

Allocating a per-device data structure is a common practice in the Linux
driver model. Add one that will be used to store chip-specific register
offsets, and configuration.

Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
 drivers/watchdog/w83627hf_wdt.c | 49 ++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 993e65ce4447..72e167f37885 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -132,6 +132,11 @@ MODULE_PARM_DESC(early_disable, "Disable watchdog at boot time (default=0)");
 
 struct wdt_pdata {};
 
+struct w83627hf_data {
+	struct watchdog_device wdd;
+	struct watchdog_info info;
+};
+
 static void superio_outb(int reg, int val)
 {
 	outb(reg, WDT_EFER);
@@ -331,10 +336,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,
@@ -343,14 +344,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.
@@ -471,28 +464,44 @@ static int wdt_probe(struct platform_device *pdev)
 {
 	const struct platform_device_id *id = platform_get_device_id(pdev);
 	enum chips chip = id->driver_data;
+	struct watchdog_device *wdd;
+	struct w83627hf_data *data;
 	int ret;
 
 	pr_info("WDT driver for %s Super I/O chip initialising\n", id->name);
 
-	snprintf(wdt_info.identity, sizeof(wdt_info.identity), "%s Watchdog",
-		 id->name);
+	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->info = &data->info;
+	wdd->ops = &wdt_ops;
+	wdd->timeout = WATCHDOG_TIMEOUT;
+	wdd->min_timeout = 1;
+	wdd->max_timeout = 255;
 
-	watchdog_init_timeout(&wdt_dev, timeout, NULL);
-	watchdog_set_nowayout(&wdt_dev, nowayout);
-	watchdog_stop_on_reboot(&wdt_dev);
+	watchdog_set_drvdata(wdd, data);
+	watchdog_init_timeout(wdd, timeout, NULL);
+	watchdog_set_nowayout(wdd, nowayout);
+	watchdog_stop_on_reboot(wdd);
 
-	ret = w83627hf_init(&wdt_dev, chip);
+	ret = w83627hf_init(wdd, chip);
 	if (ret) {
 		pr_err("failed to initialize watchdog (err=%d)\n", ret);
 		return ret;
 	}
 
-	ret = devm_watchdog_register_device(&pdev->dev, &wdt_dev);
+	ret = devm_watchdog_register_device(&pdev->dev, wdd);
 	if (ret)
 		return ret;
 
-	pr_info("initialized. timeout=%d sec (nowayout=%d)\n", wdt_dev.timeout,
+	pr_info("initialized. timeout=%d sec (nowayout=%d)\n", wdd->timeout,
 		nowayout);
 
 	return ret;

-- 
2.55.0

Re: [PATCH 3/9] watchdog: w83627hf_wdt: Use private driver data structure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Possible NULL pointer dereference of `id` in `wdt_probe` if device is matched via `driver_override`.
--

--- Patch [3]: [PATCH 3/9] watchdog: w83627hf_wdt: Use private driver data structure ---
commit a08865ae24d92d72840774e0d257ddcdf06b2ed4
Author: Paul Louvel <paul.louvel@bootlin.com>

watchdog: w83627hf_wdt: Use private driver data structure
    
Allocating a per-device data structure is a common practice in the Linux
driver model. Add one that will be used to store chip-specific register
offsets, and configuration.

> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> index 993e65ce4447c..72e167f37885f 100644
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c
[ ... ]
> @@ -471,28 +464,44 @@ static int wdt_probe(struct platform_device *pdev)
>  {
>  	const struct platform_device_id *id = platform_get_device_id(pdev);
>  	enum chips chip = id->driver_data;
                          ^^^^^^^^^^^^^^^

[Severity: High]
Can platform_get_device_id() return NULL here?

If a user with root privileges writes the name of this driver ("w83627hf_wdt")
to the driver_override sysfs attribute of an arbitrary platform device and
forces it to bind, the device might not match any entry in the driver's
wdt_ids table.

In this case, platform_get_device_id() would return NULL, causing a NULL
pointer dereference when accessing id->driver_data.

Note that this issue appears to have been introduced when the driver was
converted to the platform driver model in patch 2 of this series.

[ ... ]
> +	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);
                                ^^^^^^^^

[Severity: High]
Does this also cause a NULL pointer dereference if id is NULL under the same
driver_override condition?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725-w83627hf_wdt-improvements-v1-0-4e9a1b4e8297@bootlin.com?part=3

[PATCH 4/9] 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 | 73 ++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 34 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 72e167f37885..f5333c432198 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 */
 
@@ -135,6 +132,11 @@ struct wdt_pdata {};
 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)
@@ -173,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;
 
@@ -212,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;
@@ -244,48 +247,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();
@@ -293,7 +297,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;
@@ -301,12 +305,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)
@@ -318,6 +322,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;
 
@@ -326,7 +331,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;
@@ -354,10 +359,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;
@@ -372,13 +373,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;
@@ -433,9 +430,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);
@@ -443,10 +437,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;
@@ -486,6 +476,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

[PATCH 5/9] watchdog: w83627hf_wdt: Add Super I/O configuration port as a platform device resource

[Paul Louvel]

The Super I/O configuration port address is a global variable.
Instead, use a platform device resource.

Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
 drivers/watchdog/w83627hf_wdt.c | 146 +++++++++++++++++++++-------------------
 1 file changed, 77 insertions(+), 69 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index f5333c432198..6363d571182f 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -41,7 +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 */
 
@@ -76,11 +75,6 @@ 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_EFDR (WDT_EFIR+1) /* Extended Function Data Register */
-
 #define W83627HF_LD_WDT		0x08
 
 #define W83627HF_ID		0x52
@@ -137,40 +131,41 @@ struct w83627hf_data {
 		int timeout;
 		int csr;
 	} reg;
+	int sioaddr;
 };
 
-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);
+	outb(reg, base);
+	outb(val, base + 1);
 }
 
-static inline int superio_inb(int reg)
+static inline int superio_inb(int base, int reg)
 {
-	outb(reg, WDT_EFER);
-	return inb(WDT_EFDR);
+	outb(reg, base);
+	return inb(base + 1);
 }
 
-static int superio_enter(void)
+static int superio_enter(int base)
 {
-	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(wdt_cfg_enter, base); /* Enter extended function mode */
+	outb_p(wdt_cfg_enter, base); /* 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)
 {
-	outb_p(wdt_cfg_leave, WDT_EFER); /* Leave extended function mode */
-	release_region(wdt_io, 2);
+	outb_p(wdt_cfg_leave, base); /* Leave extended function mode */
+	release_region(base, 2);
 }
 
 static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
@@ -179,52 +174,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);
 	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:
@@ -247,42 +242,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);
 
 	return 0;
 }
@@ -292,13 +287,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);
 	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);
 
 	return 0;
 }
@@ -326,13 +321,13 @@ static unsigned int wdt_get_time(struct watchdog_device *wdog)
 	unsigned int timeleft;
 	int ret;
 
-	ret = superio_enter();
+	ret = superio_enter(data->sioaddr);
 	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);
 
 	return timeleft;
 }
@@ -359,11 +354,11 @@ static int wdt_find(int addr)
 	u8 val;
 	int ret;
 
-	ret = superio_enter();
+	ret = superio_enter(addr);
 	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;
@@ -432,7 +427,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
@@ -446,7 +441,7 @@ static int wdt_find(int addr)
 		pr_err("Unsupported chip ID: 0x%02x\n", val);
 		break;
 	}
-	superio_exit();
+	superio_exit(addr);
 	return ret;
 }
 
@@ -456,10 +451,15 @@ static int wdt_probe(struct platform_device *pdev)
 	enum chips chip = id->driver_data;
 	struct watchdog_device *wdd;
 	struct w83627hf_data *data;
+	struct resource *res;
 	int ret;
 
 	pr_info("WDT driver for %s Super I/O chip initialising\n", id->name);
 
+	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+	if (!res)
+		return -ENXIO;
+
 	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
@@ -476,6 +476,7 @@ static int wdt_probe(struct platform_device *pdev)
 	wdd->min_timeout = 1;
 	wdd->max_timeout = 255;
 
+	data->sioaddr = res->start;
 	data->reg.timeout = W83627HF_WDT_TIMEOUT;
 	data->reg.control = W83627HF_WDT_CONTROL;
 	data->reg.csr = W836X7HF_WDT_CSR;
@@ -579,17 +580,19 @@ static struct platform_device *wdt_pdev;
 static int __init wdt_init(void)
 {
 	struct wdt_pdata pdata;
+	struct resource res;
+	int sioaddr;
 	int ret;
 	int chip;
 
 	/* 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);
 	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);
 		if (chip < 0)
 			return chip;
 	}
@@ -598,9 +601,14 @@ static int __init wdt_init(void)
 	if (ret)
 		return ret;
 
-	wdt_pdev = platform_device_register_data(NULL, wdt_ids[chip].name,
-						 PLATFORM_DEVID_NONE, &pdata,
-						 sizeof(pdata));
+	res.name = "Super I/O port";
+	res.flags = IORESOURCE_IO;
+	res.start = sioaddr;
+	res.end = sioaddr + 1;
+
+	wdt_pdev = platform_device_register_resndata(NULL, wdt_ids[chip].name,
+						     PLATFORM_DEVID_NONE, &res,
+						     1, &pdata, sizeof(pdata));
 	if (IS_ERR(wdt_pdev)) {
 		platform_driver_unregister(&wdt_driver);
 		return PTR_ERR(wdt_pdev);

-- 
2.55.0

Re: [PATCH 5/9] watchdog: w83627hf_wdt: Add Super I/O configuration port as a platform device resource

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Uninitialized stack memory in `struct resource res` causes a kernel panic during device registration.
- [High] Modeling the globally shared Super I/O configuration port as an exclusive platform device resource breaks mutual exclusion (muxing) for other drivers.
--

--- Patch [5]: [PATCH 5/9] watchdog: w83627hf_wdt: Add Super I/O configuration port as a platform device resource ---
commit da82fc7c463fccfdf1ba03b6c7b6e7de29a800a0
Author: Paul Louvel <paul.louvel@bootlin.com>

watchdog: w83627hf_wdt: Add Super I/O configuration port as a platform device resource

This patch modifies the w83627hf watchdog driver to pass the Super I/O
configuration port as a platform device resource instead of using a global
variable. It updates the driver initialization to retrieve the base address
from the platform resource during probe.

> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> index f5333c4321984..6363d571182fa 100644
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c
[ ... ]
> @@ -598,9 +601,14 @@ static int __init wdt_init(void)
>  	if (ret)
>  		return ret;
>  
> -	wdt_pdev = platform_device_register_data(NULL, wdt_ids[chip].name,
> -						 PLATFORM_DEVID_NONE, &pdata,
> -						 sizeof(pdata));
> +	res.name = "Super I/O port";
> +	res.flags = IORESOURCE_IO;
> +	res.start = sioaddr;
> +	res.end = sioaddr + 1;

[Severity: High]
Is this struct resource fully initialized before being passed to
platform_device_register_resndata()?

Because it is allocated on the stack in wdt_init(), fields like parent,
sibling, and child will contain uninitialized stack data. When the resource
is copied during platform device registration, these uninitialized pointers
are preserved.

Later, when platform_device_add() is called, it checks the parent pointer.
If the stack garbage is non-zero, it skips the fallback to ioport_resource
and passes the garbage pointer to insert_resource(), which will cause a
kernel panic when traversing the resource tree.

Could this be fixed by zero-initializing the struct when it is declared?

> +
> +	wdt_pdev = platform_device_register_resndata(NULL, wdt_ids[chip].name,
> +						     PLATFORM_DEVID_NONE, &res,
> +						     1, &pdata, sizeof(pdata));

[Severity: High]
Will registering the Super I/O configuration port as a static platform device
resource break the mutual exclusion for other drivers sharing this port?

The Super I/O configuration port is a globally shared resource, and drivers
like hwmon dynamically access it using request_muxed_region(). Registering
it statically here inserts it into the global I/O resource tree as belonging
to the watchdog device.

If another driver holds the muxed region and the watchdog module is
unloaded, release_resource() will forcefully remove the watchdog's resource
and orphan any children. This breaks the IORESOURCE_MUXED wait queue logic,
allowing a third driver to concurrently claim the region, potentially leading
to hardware data corruption or a deadlock when the original driver fails to
release the region.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725-w83627hf_wdt-improvements-v1-0-4e9a1b4e8297@bootlin.com?part=5

[PATCH 6/9] watchdog: w83627hf_wdt: Store Super I/O unlocking sequence in platform data

[Paul Louvel]

Instead of using two global variable, store the unlocking sequence in the
platform data.

Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
 drivers/watchdog/w83627hf_wdt.c | 58 +++++++++++++++++++++++++----------------
 1 file changed, 35 insertions(+), 23 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 6363d571182f..fed05fcf82bb 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -41,9 +41,6 @@
 #define WATCHDOG_NAME "w83627hf/thf/hg/dhg WDT"
 #define WATCHDOG_TIMEOUT 60		/* 60 sec default timeout */
 
-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,
@@ -121,7 +118,10 @@ 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 {};
+struct wdt_pdata {
+	int siocfg_enter;
+	int siocfg_leave;
+};
 
 struct w83627hf_data {
 	struct watchdog_device wdd;
@@ -132,6 +132,8 @@ struct w83627hf_data {
 		int csr;
 	} reg;
 	int sioaddr;
+	int siocfg_enter;
+	int siocfg_leave;
 };
 
 static void superio_outb(int base, int reg, int val)
@@ -146,13 +148,13 @@ static inline int superio_inb(int base, int reg)
 	return inb(base + 1);
 }
 
-static int superio_enter(int base)
+static int superio_enter(int base, int enter)
 {
 	if (!request_muxed_region(base, 2, WATCHDOG_NAME))
 		return -EBUSY;
 
-	outb_p(wdt_cfg_enter, base); /* Enter extended function mode */
-	outb_p(wdt_cfg_enter, base); /* Again according to manual */
+	outb_p(enter, base); /* Enter extended function mode */
+	outb_p(enter, base); /* Again according to manual */
 
 	return 0;
 }
@@ -162,9 +164,9 @@ static void superio_select(int base, int ld)
 	superio_outb(base, SIO_REG_LDSEL, ld);
 }
 
-static void superio_exit(int base)
+static void superio_exit(int base, int leave)
 {
-	outb_p(wdt_cfg_leave, base); /* Leave extended function mode */
+	outb_p(leave, base); /* Leave extended function mode */
 	release_region(base, 2);
 }
 
@@ -174,7 +176,7 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
 	int ret;
 	unsigned char t;
 
-	ret = superio_enter(data->sioaddr);
+	ret = superio_enter(data->sioaddr, data->siocfg_enter);
 	if (ret)
 		return ret;
 
@@ -277,7 +279,7 @@ static int w83627hf_init(struct watchdog_device *wdog, enum chips chip)
 	t &= ~(WDT_CSR_STATUS | WDT_CSR_KBD_INT_RESET | WDT_CSR_MOUSE_INT_RESET);
 	superio_outb(data->sioaddr, data->reg.csr, t);
 
-	superio_exit(data->sioaddr);
+	superio_exit(data->sioaddr, data->siocfg_leave);
 
 	return 0;
 }
@@ -287,13 +289,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(data->sioaddr);
+	ret = superio_enter(data->sioaddr, data->siocfg_enter);
 	if (ret)
 		return ret;
 
 	superio_select(data->sioaddr, W83627HF_LD_WDT);
 	superio_outb(data->sioaddr, data->reg.timeout, timeout);
-	superio_exit(data->sioaddr);
+	superio_exit(data->sioaddr, data->siocfg_leave);
 
 	return 0;
 }
@@ -321,13 +323,13 @@ static unsigned int wdt_get_time(struct watchdog_device *wdog)
 	unsigned int timeleft;
 	int ret;
 
-	ret = superio_enter(data->sioaddr);
+	ret = superio_enter(data->sioaddr, data->siocfg_enter);
 	if (ret)
 		return 0;
 
 	superio_select(data->sioaddr, W83627HF_LD_WDT);
 	timeleft = superio_inb(data->sioaddr, data->reg.timeout);
-	superio_exit(data->sioaddr);
+	superio_exit(data->sioaddr, data->siocfg_leave);
 
 	return timeleft;
 }
@@ -349,12 +351,12 @@ 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(addr);
+	ret = superio_enter(addr, enter);
 	if (ret)
 		return ret;
 	superio_select(addr, W83627HF_LD_WDT);
@@ -441,13 +443,14 @@ static int wdt_find(int addr)
 		pr_err("Unsupported chip ID: 0x%02x\n", val);
 		break;
 	}
-	superio_exit(addr);
+	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;
 	enum chips chip = id->driver_data;
 	struct watchdog_device *wdd;
 	struct w83627hf_data *data;
@@ -477,6 +480,8 @@ static int wdt_probe(struct platform_device *pdev)
 	wdd->max_timeout = 255;
 
 	data->sioaddr = res->start;
+	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;
@@ -520,12 +525,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 = {
@@ -535,6 +544,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,
 	},
 	{}
 };
@@ -579,20 +589,22 @@ static struct platform_device *wdt_pdev;
 
 static int __init wdt_init(void)
 {
-	struct wdt_pdata pdata;
 	struct resource res;
 	int sioaddr;
 	int ret;
 	int chip;
 
+	pdata.siocfg_enter = 0x87;
+	pdata.siocfg_leave = 0xAA;
+
 	/* Apply system-specific quirks */
 	dmi_check_system(wdt_dmi_table);
 
 	sioaddr = SIO_REG_CONF_ADDR0;
-	chip = wdt_find(sioaddr);
+	chip = wdt_find(sioaddr, pdata.siocfg_enter, pdata.siocfg_leave);
 	if (chip < 0) {
 		sioaddr = SIO_REG_CONF_ADDR1;
-		chip = wdt_find(sioaddr);
+		chip = wdt_find(sioaddr, pdata.siocfg_enter, pdata.siocfg_leave);
 		if (chip < 0)
 			return chip;
 	}

-- 
2.55.0

Re: [PATCH 6/9] watchdog: w83627hf_wdt: Store Super I/O unlocking sequence in platform data

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The local stack variable `struct resource res` is partially uninitialized before being passed to `platform_device_register_resndata()`, leaking stack garbage and risking kernel panics.
- [High] Missing NULL check for `pdata` (`pdev->dev.platform_data`) in `wdt_probe` leads to an unconditional NULL pointer dereference.
- [High] Missing NULL check for `id` (`platform_get_device_id(pdev)`) in `wdt_probe` leads to a kernel panic.
--

--- Patch [6]: [PATCH 6/9] watchdog: w83627hf_wdt: Store Super I/O unlocking sequence in platform data ---
commit a8ddfa0e2fa7007bc7d8b51099f9722c0113c5c5
Author: Paul Louvel <paul.louvel@bootlin.com>

watchdog: w83627hf_wdt: Store Super I/O unlocking sequence in platform data

This patch stores the Super I/O unlocking sequence in the platform data
instead of using global variables.

> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> index 6363d571182fa..fed05fcf82bbf 100644
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c
[ ... ]
> @@ -441,13 +443,14 @@ static int wdt_find(int addr)
>  		pr_err("Unsupported chip ID: 0x%02x\n", val);
>  		break;
>  	}
> -	superio_exit(addr);
> +	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;
>  	enum chips chip = id->driver_data;

[Severity: High]
Is it possible for id to be NULL here? If this driver is bound to an
arbitrary device via the driver_override sysfs interface (which skips setting
pdev->id_entry), platform_get_device_id(pdev) will return NULL. Will this
cause a kernel panic when dereferencing id->driver_data?

>  	struct watchdog_device *wdd;
>  	struct w83627hf_data *data;
[ ... ]
> @@ -477,6 +480,8 @@ static int wdt_probe(struct platform_device *pdev)
>  	wdd->max_timeout = 255;
>  
>  	data->sioaddr = res->start;
> +	data->siocfg_enter = pdata->siocfg_enter;
> +	data->siocfg_leave = pdata->siocfg_leave;

[Severity: High]
Could pdata be NULL at this point? If a local privileged user manually
triggers driver binding via sysfs on a dummy platform device that lacks
platform_data, pdev->dev.platform_data will be NULL. Dereferencing pdata
here would result in a NULL pointer dereference.

>  	data->reg.timeout = W83627HF_WDT_TIMEOUT;
>  	data->reg.control = W83627HF_WDT_CONTROL;
>  	data->reg.csr = W836X7HF_WDT_CSR;
[ ... ]
> @@ -579,20 +589,22 @@ static struct platform_device *wdt_pdev;
>  
>  static int __init wdt_init(void)
>  {
> -	struct wdt_pdata pdata;
>  	struct resource res;

[Severity: High]
Does this code leave internal fields of the stack variable res uninitialized?
Since res is declared on the stack without being zero-initialized, fields
like parent, sibling, and child may contain stack garbage.

Later, when res is passed to platform_device_register_resndata(), it uses
kmemdup() to copy this struct. During platform_device_add(), the kernel
iterates over resources and checks if (!r->parent). Since res.parent
contains non-NULL stack garbage, it bypasses the default assignment and
passes the garbage pointer to insert_resource(), which could result in a
page fault and kernel panic during module load.

>  	int sioaddr;
>  	int ret;
>  	int chip;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725-w83627hf_wdt-improvements-v1-0-4e9a1b4e8297@bootlin.com?part=6

[PATCH 7/9] 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 | 70 +++++++++++++++++++++++++++++++----------
 1 file changed, 53 insertions(+), 17 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index fed05fcf82bb..90d41f21aa5e 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;
@@ -134,6 +136,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)
@@ -253,22 +258,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);
@@ -287,6 +281,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);
@@ -294,6 +289,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);
 
@@ -302,7 +306,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)
@@ -312,6 +318,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;
@@ -329,6 +346,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;
@@ -477,7 +496,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 = res->start;
 	data->siocfg_enter = pdata->siocfg_enter;
@@ -502,12 +521,29 @@ 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) {
 		pr_err("failed to initialize watchdog (err=%d)\n", ret);
 		return ret;
 	}
 
+	if (data->early_timer_val) {
+		if (early_disable) {
+			pr_warn("Stopping previously enabled watchdog until userland kicks in\n");
+			ret = wdt_stop(wdd);
+		} else {
+			pr_info("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(&pdev->dev, wdd);
 	if (ret)
 		return ret;

-- 
2.55.0

[PATCH 8/9] watchdog: w83627hf_wdt: Report all initialization failures in probe

[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 | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index 90d41f21aa5e..cf1a3fb3a6fd 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -479,12 +479,16 @@ static int wdt_probe(struct platform_device *pdev)
 	pr_info("WDT driver for %s Super I/O chip initialising\n", id->name);
 
 	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
-	if (!res)
-		return -ENXIO;
+	if (!res) {
+		ret = -ENXIO;
+		goto fail;
+	}
 
 	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
-	if (!data)
-		return -ENOMEM;
+	if (!data) {
+		ret = -ENOMEM;
+		goto fail;
+	}
 
 	data->info.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE;
 	snprintf(data->info.identity, sizeof(data->info.identity),
@@ -524,10 +528,8 @@ static int wdt_probe(struct platform_device *pdev)
 	wdt_set_timeout(wdd, wdd->timeout);
 
 	ret = w83627hf_init(wdd, chip);
-	if (ret) {
-		pr_err("failed to initialize watchdog (err=%d)\n", ret);
-		return ret;
-	}
+	if (ret)
+		goto fail;
 
 	if (data->early_timer_val) {
 		if (early_disable) {
@@ -541,16 +543,19 @@ static int wdt_probe(struct platform_device *pdev)
 		}
 
 		if (ret)
-			return ret;
+			goto fail;
 	}
 
 	ret = devm_watchdog_register_device(&pdev->dev, wdd);
 	if (ret)
-		return ret;
+		goto fail;
 
 	pr_info("initialized. timeout=%d sec (nowayout=%d)\n", wdd->timeout,
 		nowayout);
 
+	return 0;
+fail:
+	pr_err("failed to initialize watchdog (err=%d)\n", ret);
 	return ret;
 }
 

-- 
2.55.0

Re: [PATCH 8/9] watchdog: w83627hf_wdt: Report all initialization failures in probe

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Redundant and discouraged custom error logging on memory allocation failure.
--

--- Patch [8]: [PATCH 8/9] watchdog: w83627hf_wdt: Report all initialization failures in probe ---
commit 2d2f57b98d7747ee2d5b243ffbdaf86d9f565235
Author: Paul Louvel <paul.louvel@bootlin.com>

watchdog: w83627hf_wdt: Report all initialization failures in probe

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.

> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> index 90d41f21aa5e2..cf1a3fb3a6fd3 100644
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c
> @@ -479,12 +479,16 @@ static int wdt_probe(struct platform_device *pdev)
[ ... ]
>  	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> -	if (!data)
> -		return -ENOMEM;
> +	if (!data) {
> +		ret = -ENOMEM;
> +		goto fail;
> +	}

[Severity: Low]
Does this introduce redundant logging for memory allocation failures?

When devm_kzalloc() fails, the kernel's memory management subsystem already
dumps a highly visible warning and stack trace. Routing this failure path
to the fail label adds a custom pr_err() for the OOM condition.

Could we return -ENOMEM directly here instead of jumping to the fail label
to avoid redundant log spam?

[ ... ]
> +fail:
> +	pr_err("failed to initialize watchdog (err=%d)\n", ret);
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260725-w83627hf_wdt-improvements-v1-0-4e9a1b4e8297@bootlin.com?part=8

Re: [PATCH 8/9] watchdog: w83627hf_wdt: Report all initialization failures in probe

[Guenter Roeck]

On 7/25/26 03:29, Paul Louvel wrote:
> 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 | 25 +++++++++++++++----------
>   1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> index 90d41f21aa5e..cf1a3fb3a6fd 100644
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c
> @@ -479,12 +479,16 @@ static int wdt_probe(struct platform_device *pdev)
>   	pr_info("WDT driver for %s Super I/O chip initialising\n", id->name);
>   
>   	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> -	if (!res)
> -		return -ENXIO;
> +	if (!res) {
> +		ret = -ENXIO;
> +		goto fail;

No way. Use "return dev_err_probe(...);" instead.> +	}
>   
>   	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> -	if (!data)
> -		return -ENOMEM;
> +	if (!data) {
> +		ret = -ENOMEM;
> +		goto fail;
> +	}

Not for this one, as Sashiko points out.

Guenter

[PATCH 9/9] watchdog: w83627hf_wdt: Use dev_* logging instead of pr_*

[Paul Louvel]

The pr_* logging functions do not identify which device generated a
message. Use dev_* logging functions instead.
Since the device name is printed with this family of function, remove
the duplicate name in the initialization message.

Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>
---
 drivers/watchdog/w83627hf_wdt.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
index cf1a3fb3a6fd..83c5d8b52b13 100644
--- a/drivers/watchdog/w83627hf_wdt.c
+++ b/drivers/watchdog/w83627hf_wdt.c
@@ -471,12 +471,13 @@ 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;
 	enum chips chip = id->driver_data;
+	struct device *dev = &pdev->dev;
 	struct watchdog_device *wdd;
 	struct w83627hf_data *data;
 	struct resource *res;
 	int ret;
 
-	pr_info("WDT driver for %s Super I/O chip initialising\n", id->name);
+	dev_info(dev, "WDT driver initialising\n");
 
 	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
 	if (!res) {
@@ -533,11 +534,11 @@ static int wdt_probe(struct platform_device *pdev)
 
 	if (data->early_timer_val) {
 		if (early_disable) {
-			pr_warn("Stopping previously enabled watchdog until userland kicks in\n");
+			dev_warn(dev, "Stopping previously enabled watchdog until userland kicks in\n");
 			ret = wdt_stop(wdd);
 		} else {
-			pr_info("Watchdog already running. Resetting timeout to %d sec\n",
-				wdd->timeout);
+			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);
 		}
@@ -550,13 +551,12 @@ static int wdt_probe(struct platform_device *pdev)
 	if (ret)
 		goto fail;
 
-	pr_info("initialized. timeout=%d sec (nowayout=%d)\n", wdd->timeout,
-		nowayout);
+	dev_info(dev, "initialized. timeout=%d sec (nowayout=%d)\n",
+		 wdd->timeout, nowayout);
 
 	return 0;
 fail:
-	pr_err("failed to initialize watchdog (err=%d)\n", ret);
-	return ret;
+	return dev_err_probe(dev, ret, "failed to initialize watchdog\n");
 }
 
 /*

-- 
2.55.0

Re: [PATCH 9/9] watchdog: w83627hf_wdt: Use dev_* logging instead of pr_*

[Guenter Roeck]

On 7/25/26 03:29, Paul Louvel wrote:
> The pr_* logging functions do not identify which device generated a
> message. Use dev_* logging functions instead.
> Since the device name is printed with this family of function, remove
> the duplicate name in the initialization message.
> 
> Signed-off-by: Paul Louvel <paul.louvel@bootlin.com>

Please combine this patch with the patch introducing the probe function,
and, as mentioned, do not use goto() and do not generate a message for
memory allocation failures.

Thanks,
Guenter
> ---
>   drivers/watchdog/w83627hf_wdt.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_wdt.c
> index cf1a3fb3a6fd..83c5d8b52b13 100644
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c
> @@ -471,12 +471,13 @@ 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;
>   	enum chips chip = id->driver_data;
> +	struct device *dev = &pdev->dev;
>   	struct watchdog_device *wdd;
>   	struct w83627hf_data *data;
>   	struct resource *res;
>   	int ret;
>   
> -	pr_info("WDT driver for %s Super I/O chip initialising\n", id->name);
> +	dev_info(dev, "WDT driver initialising\n");
>   
>   	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
>   	if (!res) {
> @@ -533,11 +534,11 @@ static int wdt_probe(struct platform_device *pdev)
>   
>   	if (data->early_timer_val) {
>   		if (early_disable) {
> -			pr_warn("Stopping previously enabled watchdog until userland kicks in\n");
> +			dev_warn(dev, "Stopping previously enabled watchdog until userland kicks in\n");
>   			ret = wdt_stop(wdd);
>   		} else {
> -			pr_info("Watchdog already running. Resetting timeout to %d sec\n",
> -				wdd->timeout);
> +			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);
>   		}
> @@ -550,13 +551,12 @@ static int wdt_probe(struct platform_device *pdev)
>   	if (ret)
>   		goto fail;
>   
> -	pr_info("initialized. timeout=%d sec (nowayout=%d)\n", wdd->timeout,
> -		nowayout);
> +	dev_info(dev, "initialized. timeout=%d sec (nowayout=%d)\n",
> +		 wdd->timeout, nowayout);
>   
>   	return 0;
>   fail:
> -	pr_err("failed to initialize watchdog (err=%d)\n", ret);
> -	return ret;
> +	return dev_err_probe(dev, ret, "failed to initialize watchdog\n");
>   }
>   
>   /*
>