[PATCH v4 0/5] watchdog: f71808e_wdt: migrate to new kernel API

[PATCH v4 0/5] watchdog: f71808e_wdt: migrate to new kernel API

[Ahmad Fatoum]

This series migrates the driver to the new kernel watchdog API and
then to the driver model.

This series migrates the driver to the driver model and then to
the new kernel watchdog API.

I tested it on a f81866.

v3 -> v4:
  - Prepend fix for wrong timeout report (Guenther)
  - Remove WDOG_HW_RUNNING setting in start as the watchdog can
    be stopped (Guenther)
  - Dynamically allocate watchdog driver data (Guenther)

v3 -> RESEND:
  https://lore.kernel.org/linux-watchdog/cover.dedd9f1159389b0a438076ef5e5a46aded186463.1612457906.git-series.a.fatoum@pengutronix.de/#t
  Didn't generate any feedback over 2 months. Resending for exposure.

v2 -> v3:
  https://lore.kernel.org/linux-watchdog/20201020062112.6762-1-a.fatoum@pengutronix.de/
  - factored out identifier renaming again for easier review
  - reordered commits
  - removed refactoring that can go in later. Focusing now on kernel watchdog
    API and platform device/driver migration
  - removed platform_device_id and changed code to match by name

v1 -> v2:
  https://lore.kernel.org/linux-watchdog/20200611191750.28096-1-a.fatoum@pengutronix.de/
  - reworked to platform device/driver pair (Guenther)
  - squashed identifier renaming into the patches that touch
    the respective lines anyway
  - fixed checkpatch.pl nitpicks (Guenther)
  - fixed locally used variable declared without static (0-day)
  - fixed unneded line break due to old line limit (Guenther)
  - renamed struct fintek_wdog_data to struct fintek_wdt

Ahmad Fatoum (5):
  watchdog: f71808e_wdt: fix inaccurate report in WDIOC_GETTIMEOUT
  watchdog: f71808e_wdt: rename variant-independent identifiers appropriately
  watchdog: f71808e_wdt: migrate to new kernel watchdog API
  watchdog: f71808e_wdt: refactor to platform device/driver pair
  watchdog: f71808e_wdt: dynamically allocate watchdog driver data

 drivers/watchdog/Kconfig       |   1 +-
 drivers/watchdog/f71808e_wdt.c | 608 ++++++++++------------------------
 2 files changed, 191 insertions(+), 418 deletions(-)

base-commit: 2734d6c1b1a089fb593ef6a23d4b70903526fe0c
-- 
git-series 0.9.1

[PATCH v4 1/5] watchdog: f71808e_wdt: fix inaccurate report in WDIOC_GETTIMEOUT

[Ahmad Fatoum]

The fintek watchdog timer can configure timeouts of second granularity
only up to 255 seconds. Beyond that, the timeout needs to be configured
with minute granularity. WDIOC_GETTIMEOUT should report the actual
timeout configured, not just echo back the timeout configured by the
user. Do so.

Fixes: 96cb4eb019ce ("watchdog: f71808e_wdt: new watchdog driver for Fintek F71808E and F71882FG")
Suggested-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/f71808e_wdt.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index f60beec1bbae..f7d82d261913 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -228,15 +228,17 @@ static int watchdog_set_timeout(int timeout)
 
 	mutex_lock(&watchdog.lock);
 
-	watchdog.timeout = timeout;
 	if (timeout > 0xff) {
 		watchdog.timer_val = DIV_ROUND_UP(timeout, 60);
 		watchdog.minutes_mode = true;
+		timeout = watchdog.timer_val * 60;
 	} else {
 		watchdog.timer_val = timeout;
 		watchdog.minutes_mode = false;
 	}
 
+	watchdog.timeout = timeout;
+
 	mutex_unlock(&watchdog.lock);
 
 	return 0;
-- 
git-series 0.9.1

Re: [PATCH v4 1/5] watchdog: f71808e_wdt: fix inaccurate report in WDIOC_GETTIMEOUT

[Guenter Roeck]

On Thu, Jul 22, 2021 at 12:14:40PM +0200, Ahmad Fatoum wrote:
> The fintek watchdog timer can configure timeouts of second granularity
> only up to 255 seconds. Beyond that, the timeout needs to be configured
> with minute granularity. WDIOC_GETTIMEOUT should report the actual
> timeout configured, not just echo back the timeout configured by the
> user. Do so.
> 
> Fixes: 96cb4eb019ce ("watchdog: f71808e_wdt: new watchdog driver for Fintek F71808E and F71882FG")
> Suggested-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/f71808e_wdt.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
> index f60beec1bbae..f7d82d261913 100644
> --- a/drivers/watchdog/f71808e_wdt.c
> +++ b/drivers/watchdog/f71808e_wdt.c
> @@ -228,15 +228,17 @@ static int watchdog_set_timeout(int timeout)
>  
>  	mutex_lock(&watchdog.lock);
>  
> -	watchdog.timeout = timeout;
>  	if (timeout > 0xff) {
>  		watchdog.timer_val = DIV_ROUND_UP(timeout, 60);
>  		watchdog.minutes_mode = true;
> +		timeout = watchdog.timer_val * 60;
>  	} else {
>  		watchdog.timer_val = timeout;
>  		watchdog.minutes_mode = false;
>  	}
>  
> +	watchdog.timeout = timeout;
> +
>  	mutex_unlock(&watchdog.lock);
>  
>  	return 0;
> -- 
> git-series 0.9.1

[PATCH v4 2/5] watchdog: f71808e_wdt: rename variant-independent identifiers appropriately

[Ahmad Fatoum]

Code for the common parts of the driver either uses watchdog_ as
prefix for the watchdog API or f71808e_ for everything else.

The driver now supports 9 more variants besides the f71808e,
so let's rename the common parts to start with fintek_wdt_ instead.

This makes code browsing easier, because it's readily apparent
that functions are not variant-specific. Some watchdog_-prefixed
functions remain, but these will be dropped altogether with the move
to the kernel watchdog API in a later commit.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/f71808e_wdt.c | 66 +++++++++++++++++------------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index f7d82d261913..597eaf6905f4 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -113,7 +113,7 @@ MODULE_PARM_DESC(start_withtimeout, "Start watchdog timer on module load with"
 enum chips { f71808fg, f71858fg, f71862fg, f71868, f71869, f71882fg, f71889fg,
 	     f81803, f81865, f81866};
 
-static const char *f71808e_names[] = {
+static const char *fintek_wdt_names[] = {
 	"f71808fg",
 	"f71858fg",
 	"f71862fg",
@@ -136,7 +136,7 @@ static inline int superio_enter(int base);
 static inline void superio_select(int base, int ld);
 static inline void superio_exit(int base);
 
-struct watchdog_data {
+struct fintek_wdt {
 	unsigned short	sioaddr;
 	enum chips	type;
 	unsigned long	opened;
@@ -152,7 +152,7 @@ struct watchdog_data {
 	char		caused_reboot;	/* last reboot was by the watchdog */
 };
 
-static struct watchdog_data watchdog = {
+static struct fintek_wdt watchdog = {
 	.lock = __MUTEX_INITIALIZER(watchdog.lock),
 };
 
@@ -218,7 +218,7 @@ static inline void superio_exit(int base)
 	release_region(base, 2);
 }
 
-static int watchdog_set_timeout(int timeout)
+static int fintek_wdt_set_timeout(int timeout)
 {
 	if (timeout <= 0
 	 || timeout >  max_timeout) {
@@ -244,7 +244,7 @@ static int watchdog_set_timeout(int timeout)
 	return 0;
 }
 
-static int watchdog_set_pulse_width(unsigned int pw)
+static int fintek_wdt_set_pulse_width(unsigned int pw)
 {
 	int err = 0;
 	unsigned int t1 = 25, t2 = 125, t3 = 5000;
@@ -278,7 +278,7 @@ static int watchdog_set_pulse_width(unsigned int pw)
 	return err;
 }
 
-static int watchdog_keepalive(void)
+static int fintek_wdt_keepalive(void)
 {
 	int err = 0;
 
@@ -308,13 +308,13 @@ static int watchdog_keepalive(void)
 	return err;
 }
 
-static int watchdog_start(void)
+static int fintek_wdt_start(void)
 {
 	int err;
 	u8 tmp;
 
 	/* Make sure we don't die as soon as the watchdog is enabled below */
-	err = watchdog_keepalive();
+	err = fintek_wdt_keepalive();
 	if (err)
 		return err;
 
@@ -435,7 +435,7 @@ static int watchdog_start(void)
 	return err;
 }
 
-static int watchdog_stop(void)
+static int fintek_wdt_stop(void)
 {
 	int err = 0;
 
@@ -467,7 +467,7 @@ static int watchdog_get_status(void)
 	return status;
 }
 
-static bool watchdog_is_running(void)
+static bool fintek_wdt_is_running(void)
 {
 	/*
 	 * if we fail to determine the watchdog's status assume it to be
@@ -501,7 +501,7 @@ static int watchdog_open(struct inode *inode, struct file *file)
 	if (test_and_set_bit(0, &watchdog.opened))
 		return -EBUSY;
 
-	err = watchdog_start();
+	err = fintek_wdt_start();
 	if (err) {
 		clear_bit(0, &watchdog.opened);
 		return err;
@@ -519,10 +519,10 @@ static int watchdog_release(struct inode *inode, struct file *file)
 	clear_bit(0, &watchdog.opened);
 
 	if (!watchdog.expect_close) {
-		watchdog_keepalive();
+		fintek_wdt_keepalive();
 		pr_crit("Unexpected close, not stopping watchdog!\n");
 	} else if (!nowayout) {
-		watchdog_stop();
+		fintek_wdt_stop();
 	}
 	return 0;
 }
@@ -563,7 +563,7 @@ static ssize_t watchdog_write(struct file *file, const char __user *buf,
 		}
 
 		/* someone wrote to us, we should restart timer */
-		watchdog_keepalive();
+		fintek_wdt_keepalive();
 	}
 	return count;
 }
@@ -610,24 +610,24 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
 			return -EFAULT;
 
 		if (new_options & WDIOS_DISABLECARD)
-			watchdog_stop();
+			fintek_wdt_stop();
 
 		if (new_options & WDIOS_ENABLECARD)
-			return watchdog_start();
+			return fintek_wdt_start();
 		fallthrough;
 
 	case WDIOC_KEEPALIVE:
-		watchdog_keepalive();
+		fintek_wdt_keepalive();
 		return 0;
 
 	case WDIOC_SETTIMEOUT:
 		if (get_user(new_timeout, uarg.i))
 			return -EFAULT;
 
-		if (watchdog_set_timeout(new_timeout))
+		if (fintek_wdt_set_timeout(new_timeout))
 			return -EINVAL;
 
-		watchdog_keepalive();
+		fintek_wdt_keepalive();
 		fallthrough;
 
 	case WDIOC_GETTIMEOUT:
@@ -643,7 +643,7 @@ static int watchdog_notify_sys(struct notifier_block *this, unsigned long code,
 	void *unused)
 {
 	if (code == SYS_DOWN || code == SYS_HALT)
-		watchdog_stop();
+		fintek_wdt_stop();
 	return NOTIFY_DONE;
 }
 
@@ -681,7 +681,7 @@ static int __init watchdog_init(int sioaddr)
 
 	snprintf(watchdog.ident.identity,
 		sizeof(watchdog.ident.identity), "%s watchdog",
-		f71808e_names[watchdog.type]);
+		fintek_wdt_names[watchdog.type]);
 
 	err = superio_enter(sioaddr);
 	if (err)
@@ -700,10 +700,10 @@ static int __init watchdog_init(int sioaddr)
 
 	superio_exit(sioaddr);
 
-	err = watchdog_set_timeout(timeout);
+	err = fintek_wdt_set_timeout(timeout);
 	if (err)
 		return err;
-	err = watchdog_set_pulse_width(pulse_width);
+	err = fintek_wdt_set_pulse_width(pulse_width);
 	if (err)
 		return err;
 
@@ -726,7 +726,7 @@ static int __init watchdog_init(int sioaddr)
 			goto exit_miscdev;
 		}
 
-		err = watchdog_start();
+		err = fintek_wdt_start();
 		if (err) {
 			pr_err("cannot start watchdog timer\n");
 			goto exit_miscdev;
@@ -774,7 +774,7 @@ static int __init watchdog_init(int sioaddr)
 	return err;
 }
 
-static int __init f71808e_find(int sioaddr)
+static int __init fintek_wdt_find(int sioaddr)
 {
 	u16 devid;
 	int err = superio_enter(sioaddr);
@@ -830,14 +830,14 @@ static int __init f71808e_find(int sioaddr)
 	}
 
 	pr_info("Found %s watchdog chip, revision %d\n",
-		f71808e_names[watchdog.type],
+		fintek_wdt_names[watchdog.type],
 		(int)superio_inb(sioaddr, SIO_REG_DEVREV));
 exit:
 	superio_exit(sioaddr);
 	return err;
 }
 
-static int __init f71808e_init(void)
+static int __init fintek_wdt_init(void)
 {
 	static const unsigned short addrs[] = { 0x2e, 0x4e };
 	int err = -ENODEV;
@@ -849,7 +849,7 @@ static int __init f71808e_init(void)
 	}
 
 	for (i = 0; i < ARRAY_SIZE(addrs); i++) {
-		err = f71808e_find(addrs[i]);
+		err = fintek_wdt_find(addrs[i]);
 		if (err == 0)
 			break;
 	}
@@ -859,11 +859,11 @@ static int __init f71808e_init(void)
 	return watchdog_init(addrs[i]);
 }
 
-static void __exit f71808e_exit(void)
+static void __exit fintek_wdt_exit(void)
 {
-	if (watchdog_is_running()) {
+	if (fintek_wdt_is_running()) {
 		pr_warn("Watchdog timer still running, stopping it\n");
-		watchdog_stop();
+		fintek_wdt_stop();
 	}
 	misc_deregister(&watchdog_miscdev);
 	unregister_reboot_notifier(&watchdog_notifier);
@@ -873,5 +873,5 @@ MODULE_DESCRIPTION("F71808E Watchdog Driver");
 MODULE_AUTHOR("Giel van Schijndel <me@mortis.eu>");
 MODULE_LICENSE("GPL");
 
-module_init(f71808e_init);
-module_exit(f71808e_exit);
+module_init(fintek_wdt_init);
+module_exit(fintek_wdt_exit);
-- 
git-series 0.9.1

Re: [PATCH v4 2/5] watchdog: f71808e_wdt: rename variant-independent identifiers appropriately

[Guenter Roeck]

On Thu, Jul 22, 2021 at 12:14:41PM +0200, Ahmad Fatoum wrote:
> Code for the common parts of the driver either uses watchdog_ as
> prefix for the watchdog API or f71808e_ for everything else.
> 
> The driver now supports 9 more variants besides the f71808e,
> so let's rename the common parts to start with fintek_wdt_ instead.
> 
> This makes code browsing easier, because it's readily apparent
> that functions are not variant-specific. Some watchdog_-prefixed
> functions remain, but these will be dropped altogether with the move
> to the kernel watchdog API in a later commit.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/f71808e_wdt.c | 66 +++++++++++++++++------------------
>  1 file changed, 33 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
> index f7d82d261913..597eaf6905f4 100644
> --- a/drivers/watchdog/f71808e_wdt.c
> +++ b/drivers/watchdog/f71808e_wdt.c
> @@ -113,7 +113,7 @@ MODULE_PARM_DESC(start_withtimeout, "Start watchdog timer on module load with"
>  enum chips { f71808fg, f71858fg, f71862fg, f71868, f71869, f71882fg, f71889fg,
>  	     f81803, f81865, f81866};
>  
> -static const char *f71808e_names[] = {
> +static const char *fintek_wdt_names[] = {
>  	"f71808fg",
>  	"f71858fg",
>  	"f71862fg",
> @@ -136,7 +136,7 @@ static inline int superio_enter(int base);
>  static inline void superio_select(int base, int ld);
>  static inline void superio_exit(int base);
>  
> -struct watchdog_data {
> +struct fintek_wdt {
>  	unsigned short	sioaddr;
>  	enum chips	type;
>  	unsigned long	opened;
> @@ -152,7 +152,7 @@ struct watchdog_data {
>  	char		caused_reboot;	/* last reboot was by the watchdog */
>  };
>  
> -static struct watchdog_data watchdog = {
> +static struct fintek_wdt watchdog = {
>  	.lock = __MUTEX_INITIALIZER(watchdog.lock),
>  };
>  
> @@ -218,7 +218,7 @@ static inline void superio_exit(int base)
>  	release_region(base, 2);
>  }
>  
> -static int watchdog_set_timeout(int timeout)
> +static int fintek_wdt_set_timeout(int timeout)
>  {
>  	if (timeout <= 0
>  	 || timeout >  max_timeout) {
> @@ -244,7 +244,7 @@ static int watchdog_set_timeout(int timeout)
>  	return 0;
>  }
>  
> -static int watchdog_set_pulse_width(unsigned int pw)
> +static int fintek_wdt_set_pulse_width(unsigned int pw)
>  {
>  	int err = 0;
>  	unsigned int t1 = 25, t2 = 125, t3 = 5000;
> @@ -278,7 +278,7 @@ static int watchdog_set_pulse_width(unsigned int pw)
>  	return err;
>  }
>  
> -static int watchdog_keepalive(void)
> +static int fintek_wdt_keepalive(void)
>  {
>  	int err = 0;
>  
> @@ -308,13 +308,13 @@ static int watchdog_keepalive(void)
>  	return err;
>  }
>  
> -static int watchdog_start(void)
> +static int fintek_wdt_start(void)
>  {
>  	int err;
>  	u8 tmp;
>  
>  	/* Make sure we don't die as soon as the watchdog is enabled below */
> -	err = watchdog_keepalive();
> +	err = fintek_wdt_keepalive();
>  	if (err)
>  		return err;
>  
> @@ -435,7 +435,7 @@ static int watchdog_start(void)
>  	return err;
>  }
>  
> -static int watchdog_stop(void)
> +static int fintek_wdt_stop(void)
>  {
>  	int err = 0;
>  
> @@ -467,7 +467,7 @@ static int watchdog_get_status(void)
>  	return status;
>  }
>  
> -static bool watchdog_is_running(void)
> +static bool fintek_wdt_is_running(void)
>  {
>  	/*
>  	 * if we fail to determine the watchdog's status assume it to be
> @@ -501,7 +501,7 @@ static int watchdog_open(struct inode *inode, struct file *file)
>  	if (test_and_set_bit(0, &watchdog.opened))
>  		return -EBUSY;
>  
> -	err = watchdog_start();
> +	err = fintek_wdt_start();
>  	if (err) {
>  		clear_bit(0, &watchdog.opened);
>  		return err;
> @@ -519,10 +519,10 @@ static int watchdog_release(struct inode *inode, struct file *file)
>  	clear_bit(0, &watchdog.opened);
>  
>  	if (!watchdog.expect_close) {
> -		watchdog_keepalive();
> +		fintek_wdt_keepalive();
>  		pr_crit("Unexpected close, not stopping watchdog!\n");
>  	} else if (!nowayout) {
> -		watchdog_stop();
> +		fintek_wdt_stop();
>  	}
>  	return 0;
>  }
> @@ -563,7 +563,7 @@ static ssize_t watchdog_write(struct file *file, const char __user *buf,
>  		}
>  
>  		/* someone wrote to us, we should restart timer */
> -		watchdog_keepalive();
> +		fintek_wdt_keepalive();
>  	}
>  	return count;
>  }
> @@ -610,24 +610,24 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
>  			return -EFAULT;
>  
>  		if (new_options & WDIOS_DISABLECARD)
> -			watchdog_stop();
> +			fintek_wdt_stop();
>  
>  		if (new_options & WDIOS_ENABLECARD)
> -			return watchdog_start();
> +			return fintek_wdt_start();
>  		fallthrough;
>  
>  	case WDIOC_KEEPALIVE:
> -		watchdog_keepalive();
> +		fintek_wdt_keepalive();
>  		return 0;
>  
>  	case WDIOC_SETTIMEOUT:
>  		if (get_user(new_timeout, uarg.i))
>  			return -EFAULT;
>  
> -		if (watchdog_set_timeout(new_timeout))
> +		if (fintek_wdt_set_timeout(new_timeout))
>  			return -EINVAL;
>  
> -		watchdog_keepalive();
> +		fintek_wdt_keepalive();
>  		fallthrough;
>  
>  	case WDIOC_GETTIMEOUT:
> @@ -643,7 +643,7 @@ static int watchdog_notify_sys(struct notifier_block *this, unsigned long code,
>  	void *unused)
>  {
>  	if (code == SYS_DOWN || code == SYS_HALT)
> -		watchdog_stop();
> +		fintek_wdt_stop();
>  	return NOTIFY_DONE;
>  }
>  
> @@ -681,7 +681,7 @@ static int __init watchdog_init(int sioaddr)
>  
>  	snprintf(watchdog.ident.identity,
>  		sizeof(watchdog.ident.identity), "%s watchdog",
> -		f71808e_names[watchdog.type]);
> +		fintek_wdt_names[watchdog.type]);
>  
>  	err = superio_enter(sioaddr);
>  	if (err)
> @@ -700,10 +700,10 @@ static int __init watchdog_init(int sioaddr)
>  
>  	superio_exit(sioaddr);
>  
> -	err = watchdog_set_timeout(timeout);
> +	err = fintek_wdt_set_timeout(timeout);
>  	if (err)
>  		return err;
> -	err = watchdog_set_pulse_width(pulse_width);
> +	err = fintek_wdt_set_pulse_width(pulse_width);
>  	if (err)
>  		return err;
>  
> @@ -726,7 +726,7 @@ static int __init watchdog_init(int sioaddr)
>  			goto exit_miscdev;
>  		}
>  
> -		err = watchdog_start();
> +		err = fintek_wdt_start();
>  		if (err) {
>  			pr_err("cannot start watchdog timer\n");
>  			goto exit_miscdev;
> @@ -774,7 +774,7 @@ static int __init watchdog_init(int sioaddr)
>  	return err;
>  }
>  
> -static int __init f71808e_find(int sioaddr)
> +static int __init fintek_wdt_find(int sioaddr)
>  {
>  	u16 devid;
>  	int err = superio_enter(sioaddr);
> @@ -830,14 +830,14 @@ static int __init f71808e_find(int sioaddr)
>  	}
>  
>  	pr_info("Found %s watchdog chip, revision %d\n",
> -		f71808e_names[watchdog.type],
> +		fintek_wdt_names[watchdog.type],
>  		(int)superio_inb(sioaddr, SIO_REG_DEVREV));
>  exit:
>  	superio_exit(sioaddr);
>  	return err;
>  }
>  
> -static int __init f71808e_init(void)
> +static int __init fintek_wdt_init(void)
>  {
>  	static const unsigned short addrs[] = { 0x2e, 0x4e };
>  	int err = -ENODEV;
> @@ -849,7 +849,7 @@ static int __init f71808e_init(void)
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(addrs); i++) {
> -		err = f71808e_find(addrs[i]);
> +		err = fintek_wdt_find(addrs[i]);
>  		if (err == 0)
>  			break;
>  	}
> @@ -859,11 +859,11 @@ static int __init f71808e_init(void)
>  	return watchdog_init(addrs[i]);
>  }
>  
> -static void __exit f71808e_exit(void)
> +static void __exit fintek_wdt_exit(void)
>  {
> -	if (watchdog_is_running()) {
> +	if (fintek_wdt_is_running()) {
>  		pr_warn("Watchdog timer still running, stopping it\n");
> -		watchdog_stop();
> +		fintek_wdt_stop();
>  	}
>  	misc_deregister(&watchdog_miscdev);
>  	unregister_reboot_notifier(&watchdog_notifier);
> @@ -873,5 +873,5 @@ MODULE_DESCRIPTION("F71808E Watchdog Driver");
>  MODULE_AUTHOR("Giel van Schijndel <me@mortis.eu>");
>  MODULE_LICENSE("GPL");
>  
> -module_init(f71808e_init);
> -module_exit(f71808e_exit);
> +module_init(fintek_wdt_init);
> +module_exit(fintek_wdt_exit);
> -- 
> git-series 0.9.1

Re: [PATCH v4 2/5] watchdog: f71808e_wdt: rename variant-independent identifiers appropriately

[Guenter Roeck]

On Thu, Jul 22, 2021 at 12:14:41PM +0200, Ahmad Fatoum wrote:
> Code for the common parts of the driver either uses watchdog_ as
> prefix for the watchdog API or f71808e_ for everything else.
> 
> The driver now supports 9 more variants besides the f71808e,
> so let's rename the common parts to start with fintek_wdt_ instead.
> 
> This makes code browsing easier, because it's readily apparent
> that functions are not variant-specific. Some watchdog_-prefixed
> functions remain, but these will be dropped altogether with the move
> to the kernel watchdog API in a later commit.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/watchdog/f71808e_wdt.c | 66 +++++++++++++++++------------------
>  1 file changed, 33 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
> index f7d82d261913..597eaf6905f4 100644
> --- a/drivers/watchdog/f71808e_wdt.c
> +++ b/drivers/watchdog/f71808e_wdt.c
> @@ -113,7 +113,7 @@ MODULE_PARM_DESC(start_withtimeout, "Start watchdog timer on module load with"
>  enum chips { f71808fg, f71858fg, f71862fg, f71868, f71869, f71882fg, f71889fg,
>  	     f81803, f81865, f81866};
>  
> -static const char *f71808e_names[] = {
> +static const char *fintek_wdt_names[] = {

Nit: checkpatch rightfully complains that this can be

static const char * const fintek_wdt_names[]

>  	"f71808fg",
>  	"f71858fg",
>  	"f71862fg",
> @@ -136,7 +136,7 @@ static inline int superio_enter(int base);
>  static inline void superio_select(int base, int ld);
>  static inline void superio_exit(int base);
>  
> -struct watchdog_data {
> +struct fintek_wdt {
>  	unsigned short	sioaddr;
>  	enum chips	type;
>  	unsigned long	opened;
> @@ -152,7 +152,7 @@ struct watchdog_data {
>  	char		caused_reboot;	/* last reboot was by the watchdog */
>  };
>  
> -static struct watchdog_data watchdog = {
> +static struct fintek_wdt watchdog = {
>  	.lock = __MUTEX_INITIALIZER(watchdog.lock),
>  };
>  
> @@ -218,7 +218,7 @@ static inline void superio_exit(int base)
>  	release_region(base, 2);
>  }
>  
> -static int watchdog_set_timeout(int timeout)
> +static int fintek_wdt_set_timeout(int timeout)
>  {
>  	if (timeout <= 0
>  	 || timeout >  max_timeout) {
> @@ -244,7 +244,7 @@ static int watchdog_set_timeout(int timeout)
>  	return 0;
>  }
>  
> -static int watchdog_set_pulse_width(unsigned int pw)
> +static int fintek_wdt_set_pulse_width(unsigned int pw)
>  {
>  	int err = 0;
>  	unsigned int t1 = 25, t2 = 125, t3 = 5000;
> @@ -278,7 +278,7 @@ static int watchdog_set_pulse_width(unsigned int pw)
>  	return err;
>  }
>  
> -static int watchdog_keepalive(void)
> +static int fintek_wdt_keepalive(void)
>  {
>  	int err = 0;
>  
> @@ -308,13 +308,13 @@ static int watchdog_keepalive(void)
>  	return err;
>  }
>  
> -static int watchdog_start(void)
> +static int fintek_wdt_start(void)
>  {
>  	int err;
>  	u8 tmp;
>  
>  	/* Make sure we don't die as soon as the watchdog is enabled below */
> -	err = watchdog_keepalive();
> +	err = fintek_wdt_keepalive();
>  	if (err)
>  		return err;
>  
> @@ -435,7 +435,7 @@ static int watchdog_start(void)
>  	return err;
>  }
>  
> -static int watchdog_stop(void)
> +static int fintek_wdt_stop(void)
>  {
>  	int err = 0;
>  
> @@ -467,7 +467,7 @@ static int watchdog_get_status(void)
>  	return status;
>  }
>  
> -static bool watchdog_is_running(void)
> +static bool fintek_wdt_is_running(void)
>  {
>  	/*
>  	 * if we fail to determine the watchdog's status assume it to be
> @@ -501,7 +501,7 @@ static int watchdog_open(struct inode *inode, struct file *file)
>  	if (test_and_set_bit(0, &watchdog.opened))
>  		return -EBUSY;
>  
> -	err = watchdog_start();
> +	err = fintek_wdt_start();
>  	if (err) {
>  		clear_bit(0, &watchdog.opened);
>  		return err;
> @@ -519,10 +519,10 @@ static int watchdog_release(struct inode *inode, struct file *file)
>  	clear_bit(0, &watchdog.opened);
>  
>  	if (!watchdog.expect_close) {
> -		watchdog_keepalive();
> +		fintek_wdt_keepalive();
>  		pr_crit("Unexpected close, not stopping watchdog!\n");
>  	} else if (!nowayout) {
> -		watchdog_stop();
> +		fintek_wdt_stop();
>  	}
>  	return 0;
>  }
> @@ -563,7 +563,7 @@ static ssize_t watchdog_write(struct file *file, const char __user *buf,
>  		}
>  
>  		/* someone wrote to us, we should restart timer */
> -		watchdog_keepalive();
> +		fintek_wdt_keepalive();
>  	}
>  	return count;
>  }
> @@ -610,24 +610,24 @@ static long watchdog_ioctl(struct file *file, unsigned int cmd,
>  			return -EFAULT;
>  
>  		if (new_options & WDIOS_DISABLECARD)
> -			watchdog_stop();
> +			fintek_wdt_stop();
>  
>  		if (new_options & WDIOS_ENABLECARD)
> -			return watchdog_start();
> +			return fintek_wdt_start();
>  		fallthrough;
>  
>  	case WDIOC_KEEPALIVE:
> -		watchdog_keepalive();
> +		fintek_wdt_keepalive();
>  		return 0;
>  
>  	case WDIOC_SETTIMEOUT:
>  		if (get_user(new_timeout, uarg.i))
>  			return -EFAULT;
>  
> -		if (watchdog_set_timeout(new_timeout))
> +		if (fintek_wdt_set_timeout(new_timeout))
>  			return -EINVAL;
>  
> -		watchdog_keepalive();
> +		fintek_wdt_keepalive();
>  		fallthrough;
>  
>  	case WDIOC_GETTIMEOUT:
> @@ -643,7 +643,7 @@ static int watchdog_notify_sys(struct notifier_block *this, unsigned long code,
>  	void *unused)
>  {
>  	if (code == SYS_DOWN || code == SYS_HALT)
> -		watchdog_stop();
> +		fintek_wdt_stop();
>  	return NOTIFY_DONE;
>  }
>  
> @@ -681,7 +681,7 @@ static int __init watchdog_init(int sioaddr)
>  
>  	snprintf(watchdog.ident.identity,
>  		sizeof(watchdog.ident.identity), "%s watchdog",
> -		f71808e_names[watchdog.type]);
> +		fintek_wdt_names[watchdog.type]);
>  
>  	err = superio_enter(sioaddr);
>  	if (err)
> @@ -700,10 +700,10 @@ static int __init watchdog_init(int sioaddr)
>  
>  	superio_exit(sioaddr);
>  
> -	err = watchdog_set_timeout(timeout);
> +	err = fintek_wdt_set_timeout(timeout);
>  	if (err)
>  		return err;
> -	err = watchdog_set_pulse_width(pulse_width);
> +	err = fintek_wdt_set_pulse_width(pulse_width);
>  	if (err)
>  		return err;
>  
> @@ -726,7 +726,7 @@ static int __init watchdog_init(int sioaddr)
>  			goto exit_miscdev;
>  		}
>  
> -		err = watchdog_start();
> +		err = fintek_wdt_start();
>  		if (err) {
>  			pr_err("cannot start watchdog timer\n");
>  			goto exit_miscdev;
> @@ -774,7 +774,7 @@ static int __init watchdog_init(int sioaddr)
>  	return err;
>  }
>  
> -static int __init f71808e_find(int sioaddr)
> +static int __init fintek_wdt_find(int sioaddr)
>  {
>  	u16 devid;
>  	int err = superio_enter(sioaddr);
> @@ -830,14 +830,14 @@ static int __init f71808e_find(int sioaddr)
>  	}
>  
>  	pr_info("Found %s watchdog chip, revision %d\n",
> -		f71808e_names[watchdog.type],
> +		fintek_wdt_names[watchdog.type],
>  		(int)superio_inb(sioaddr, SIO_REG_DEVREV));
>  exit:
>  	superio_exit(sioaddr);
>  	return err;
>  }
>  
> -static int __init f71808e_init(void)
> +static int __init fintek_wdt_init(void)
>  {
>  	static const unsigned short addrs[] = { 0x2e, 0x4e };
>  	int err = -ENODEV;
> @@ -849,7 +849,7 @@ static int __init f71808e_init(void)
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(addrs); i++) {
> -		err = f71808e_find(addrs[i]);
> +		err = fintek_wdt_find(addrs[i]);
>  		if (err == 0)
>  			break;
>  	}
> @@ -859,11 +859,11 @@ static int __init f71808e_init(void)
>  	return watchdog_init(addrs[i]);
>  }
>  
> -static void __exit f71808e_exit(void)
> +static void __exit fintek_wdt_exit(void)
>  {
> -	if (watchdog_is_running()) {
> +	if (fintek_wdt_is_running()) {
>  		pr_warn("Watchdog timer still running, stopping it\n");
> -		watchdog_stop();
> +		fintek_wdt_stop();
>  	}
>  	misc_deregister(&watchdog_miscdev);
>  	unregister_reboot_notifier(&watchdog_notifier);
> @@ -873,5 +873,5 @@ MODULE_DESCRIPTION("F71808E Watchdog Driver");
>  MODULE_AUTHOR("Giel van Schijndel <me@mortis.eu>");
>  MODULE_LICENSE("GPL");
>  
> -module_init(f71808e_init);
> -module_exit(f71808e_exit);
> +module_init(fintek_wdt_init);
> +module_exit(fintek_wdt_exit);
> -- 
> git-series 0.9.1

[PATCH v4 3/5] watchdog: f71808e_wdt: migrate to new kernel watchdog API

[Ahmad Fatoum]

Migrating the driver lets us drop the watchdog misc device boilerplate
and reduces size by 285 lines. It also brings us support for new
functionality like CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED.

This incurs a slight backwards-compatibility break, because the new
kernel watchdog API doesn't support unloading modules for drivers
whose watchdog hardware is reported to be running.

This means following scenario will be no longer supported:
 - BIOS has enabled watchdog
 - Module is loaded and unloaded without opening watchdog
 - module_exit is expected to succeed and disable watchdog HW

Suggested-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/Kconfig       |   1 +-
 drivers/watchdog/f71808e_wdt.c | 388 ++++------------------------------
 2 files changed, 51 insertions(+), 338 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 546dfc1e2349..3cc6aac18006 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1063,6 +1063,7 @@ config EBC_C384_WDT
 config F71808E_WDT
 	tristate "Fintek F718xx, F818xx Super I/O Watchdog"
 	depends on X86
+	select WATCHDOG_CORE
 	help
 	  This is the driver for the hardware watchdog on the Fintek F71808E,
 	  F71862FG, F71868, F71869, F71882FG, F71889FG, F81803, F81865, and
diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index 597eaf6905f4..efc6981d9a9b 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -9,16 +9,10 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/err.h>
-#include <linux/fs.h>
 #include <linux/init.h>
 #include <linux/io.h>
 #include <linux/ioport.h>
-#include <linux/miscdevice.h>
 #include <linux/module.h>
-#include <linux/mutex.h>
-#include <linux/notifier.h>
-#include <linux/reboot.h>
-#include <linux/uaccess.h>
 #include <linux/watchdog.h>
 
 #define DRVNAME "f71808e_wdt"
@@ -137,24 +131,18 @@ static inline void superio_select(int base, int ld);
 static inline void superio_exit(int base);
 
 struct fintek_wdt {
+	struct watchdog_device wdd;
 	unsigned short	sioaddr;
 	enum chips	type;
-	unsigned long	opened;
-	struct mutex	lock;
-	char		expect_close;
 	struct watchdog_info ident;
 
-	unsigned short	timeout;
 	u8		timer_val;	/* content for the wd_time register */
 	char		minutes_mode;
 	u8		pulse_val;	/* pulse width flag */
 	char		pulse_mode;	/* enable pulse output mode? */
-	char		caused_reboot;	/* last reboot was by the watchdog */
 };
 
-static struct fintek_wdt watchdog = {
-	.lock = __MUTEX_INITIALIZER(watchdog.lock),
-};
+static struct fintek_wdt watchdog;
 
 /* Super I/O functions */
 static inline int superio_inb(int base, int reg)
@@ -218,16 +206,8 @@ static inline void superio_exit(int base)
 	release_region(base, 2);
 }
 
-static int fintek_wdt_set_timeout(int timeout)
+static int fintek_wdt_set_timeout(struct watchdog_device *wdd, unsigned int timeout)
 {
-	if (timeout <= 0
-	 || timeout >  max_timeout) {
-		pr_err("watchdog timeout out of range\n");
-		return -EINVAL;
-	}
-
-	mutex_lock(&watchdog.lock);
-
 	if (timeout > 0xff) {
 		watchdog.timer_val = DIV_ROUND_UP(timeout, 60);
 		watchdog.minutes_mode = true;
@@ -237,16 +217,13 @@ static int fintek_wdt_set_timeout(int timeout)
 		watchdog.minutes_mode = false;
 	}
 
-	watchdog.timeout = timeout;
-
-	mutex_unlock(&watchdog.lock);
+	wdd->timeout = timeout;
 
 	return 0;
 }
 
 static int fintek_wdt_set_pulse_width(unsigned int pw)
 {
-	int err = 0;
 	unsigned int t1 = 25, t2 = 125, t3 = 5000;
 
 	if (watchdog.type == f71868) {
@@ -255,8 +232,6 @@ static int fintek_wdt_set_pulse_width(unsigned int pw)
 		t3 = 6000;
 	}
 
-	mutex_lock(&watchdog.lock);
-
 	if        (pw <=  1) {
 		watchdog.pulse_val = 0;
 	} else if (pw <= t1) {
@@ -267,25 +242,21 @@ static int fintek_wdt_set_pulse_width(unsigned int pw)
 		watchdog.pulse_val = 3;
 	} else {
 		pr_err("pulse width out of range\n");
-		err = -EINVAL;
-		goto exit_unlock;
+		return -EINVAL;
 	}
 
 	watchdog.pulse_mode = pw;
 
-exit_unlock:
-	mutex_unlock(&watchdog.lock);
-	return err;
+	return 0;
 }
 
-static int fintek_wdt_keepalive(void)
+static int fintek_wdt_keepalive(struct watchdog_device *wdd)
 {
-	int err = 0;
+	int err;
 
-	mutex_lock(&watchdog.lock);
 	err = superio_enter(watchdog.sioaddr);
 	if (err)
-		goto exit_unlock;
+		return err;
 	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
 
 	if (watchdog.minutes_mode)
@@ -303,25 +274,22 @@ static int fintek_wdt_keepalive(void)
 
 	superio_exit(watchdog.sioaddr);
 
-exit_unlock:
-	mutex_unlock(&watchdog.lock);
-	return err;
+	return 0;
 }
 
-static int fintek_wdt_start(void)
+static int fintek_wdt_start(struct watchdog_device *wdd)
 {
 	int err;
 	u8 tmp;
 
 	/* Make sure we don't die as soon as the watchdog is enabled below */
-	err = fintek_wdt_keepalive();
+	err = fintek_wdt_keepalive(wdd);
 	if (err)
 		return err;
 
-	mutex_lock(&watchdog.lock);
 	err = superio_enter(watchdog.sioaddr);
 	if (err)
-		goto exit_unlock;
+		return err;
 	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
 
 	/* Watchdog pin configuration */
@@ -429,20 +397,17 @@ static int fintek_wdt_start(void)
 
 exit_superio:
 	superio_exit(watchdog.sioaddr);
-exit_unlock:
-	mutex_unlock(&watchdog.lock);
 
 	return err;
 }
 
-static int fintek_wdt_stop(void)
+static int fintek_wdt_stop(struct watchdog_device *wdd)
 {
-	int err = 0;
+	int err;
 
-	mutex_lock(&watchdog.lock);
 	err = superio_enter(watchdog.sioaddr);
 	if (err)
-		goto exit_unlock;
+		return err;
 	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
 
 	superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
@@ -450,232 +415,31 @@ static int fintek_wdt_stop(void)
 
 	superio_exit(watchdog.sioaddr);
 
-exit_unlock:
-	mutex_unlock(&watchdog.lock);
-
-	return err;
-}
-
-static int watchdog_get_status(void)
-{
-	int status = 0;
-
-	mutex_lock(&watchdog.lock);
-	status = (watchdog.caused_reboot) ? WDIOF_CARDRESET : 0;
-	mutex_unlock(&watchdog.lock);
-
-	return status;
-}
-
-static bool fintek_wdt_is_running(void)
-{
-	/*
-	 * if we fail to determine the watchdog's status assume it to be
-	 * running to be on the safe side
-	 */
-	bool is_running = true;
-
-	mutex_lock(&watchdog.lock);
-	if (superio_enter(watchdog.sioaddr))
-		goto exit_unlock;
-	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
-
-	is_running = (superio_inb(watchdog.sioaddr, SIO_REG_ENABLE) & BIT(0))
-		&& (superio_inb(watchdog.sioaddr, F71808FG_REG_WDT_CONF)
-			& BIT(F71808FG_FLAG_WD_EN));
-
-	superio_exit(watchdog.sioaddr);
-
-exit_unlock:
-	mutex_unlock(&watchdog.lock);
-	return is_running;
-}
-
-/* /dev/watchdog api */
-
-static int watchdog_open(struct inode *inode, struct file *file)
-{
-	int err;
-
-	/* If the watchdog is alive we don't need to start it again */
-	if (test_and_set_bit(0, &watchdog.opened))
-		return -EBUSY;
-
-	err = fintek_wdt_start();
-	if (err) {
-		clear_bit(0, &watchdog.opened);
-		return err;
-	}
-
-	if (nowayout)
-		__module_get(THIS_MODULE);
-
-	watchdog.expect_close = 0;
-	return stream_open(inode, file);
-}
-
-static int watchdog_release(struct inode *inode, struct file *file)
-{
-	clear_bit(0, &watchdog.opened);
-
-	if (!watchdog.expect_close) {
-		fintek_wdt_keepalive();
-		pr_crit("Unexpected close, not stopping watchdog!\n");
-	} else if (!nowayout) {
-		fintek_wdt_stop();
-	}
 	return 0;
 }
 
-/*
- *      watchdog_write:
- *      @file: file handle to the watchdog
- *      @buf: buffer to write
- *      @count: count of bytes
- *      @ppos: pointer to the position to write. No seeks allowed
- *
- *      A write to a watchdog device is defined as a keepalive signal. Any
- *      write of data will do, as we we don't define content meaning.
- */
-
-static ssize_t watchdog_write(struct file *file, const char __user *buf,
-			    size_t count, loff_t *ppos)
-{
-	if (count) {
-		if (!nowayout) {
-			size_t i;
-
-			/* In case it was set long ago */
-			bool expect_close = false;
-
-			for (i = 0; i != count; i++) {
-				char c;
-				if (get_user(c, buf + i))
-					return -EFAULT;
-				if (c == 'V')
-					expect_close = true;
-			}
-
-			/* Properly order writes across fork()ed processes */
-			mutex_lock(&watchdog.lock);
-			watchdog.expect_close = expect_close;
-			mutex_unlock(&watchdog.lock);
-		}
-
-		/* someone wrote to us, we should restart timer */
-		fintek_wdt_keepalive();
-	}
-	return count;
-}
-
-/*
- *      watchdog_ioctl:
- *      @inode: inode of the device
- *      @file: file handle to the device
- *      @cmd: watchdog command
- *      @arg: argument pointer
- *
- *      The watchdog API defines a common set of functions for all watchdogs
- *      according to their available features.
- */
-static long watchdog_ioctl(struct file *file, unsigned int cmd,
-	unsigned long arg)
+static bool fintek_wdt_is_running(u8 wdt_conf)
 {
-	int status;
-	int new_options;
-	int new_timeout;
-	union {
-		struct watchdog_info __user *ident;
-		int __user *i;
-	} uarg;
-
-	uarg.i = (int __user *)arg;
-
-	switch (cmd) {
-	case WDIOC_GETSUPPORT:
-		return copy_to_user(uarg.ident, &watchdog.ident,
-			sizeof(watchdog.ident)) ? -EFAULT : 0;
-
-	case WDIOC_GETSTATUS:
-		status = watchdog_get_status();
-		if (status < 0)
-			return status;
-		return put_user(status, uarg.i);
-
-	case WDIOC_GETBOOTSTATUS:
-		return put_user(0, uarg.i);
-
-	case WDIOC_SETOPTIONS:
-		if (get_user(new_options, uarg.i))
-			return -EFAULT;
-
-		if (new_options & WDIOS_DISABLECARD)
-			fintek_wdt_stop();
-
-		if (new_options & WDIOS_ENABLECARD)
-			return fintek_wdt_start();
-		fallthrough;
-
-	case WDIOC_KEEPALIVE:
-		fintek_wdt_keepalive();
-		return 0;
-
-	case WDIOC_SETTIMEOUT:
-		if (get_user(new_timeout, uarg.i))
-			return -EFAULT;
-
-		if (fintek_wdt_set_timeout(new_timeout))
-			return -EINVAL;
-
-		fintek_wdt_keepalive();
-		fallthrough;
-
-	case WDIOC_GETTIMEOUT:
-		return put_user(watchdog.timeout, uarg.i);
-
-	default:
-		return -ENOTTY;
-
-	}
+	return (superio_inb(watchdog.sioaddr, SIO_REG_ENABLE) & BIT(0))
+		&& (wdt_conf & BIT(F71808FG_FLAG_WD_EN));
 }
 
-static int watchdog_notify_sys(struct notifier_block *this, unsigned long code,
-	void *unused)
-{
-	if (code == SYS_DOWN || code == SYS_HALT)
-		fintek_wdt_stop();
-	return NOTIFY_DONE;
-}
-
-static const struct file_operations watchdog_fops = {
-	.owner		= THIS_MODULE,
-	.llseek		= no_llseek,
-	.open		= watchdog_open,
-	.release	= watchdog_release,
-	.write		= watchdog_write,
-	.unlocked_ioctl	= watchdog_ioctl,
-	.compat_ioctl	= compat_ptr_ioctl,
-};
-
-static struct miscdevice watchdog_miscdev = {
-	.minor		= WATCHDOG_MINOR,
-	.name		= "watchdog",
-	.fops		= &watchdog_fops,
-};
-
-static struct notifier_block watchdog_notifier = {
-	.notifier_call = watchdog_notify_sys,
+static const struct watchdog_ops fintek_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = fintek_wdt_start,
+	.stop = fintek_wdt_stop,
+	.ping = fintek_wdt_keepalive,
+	.set_timeout = fintek_wdt_set_timeout,
 };
 
 static int __init watchdog_init(int sioaddr)
 {
+	struct watchdog_device *wdd;
 	int wdt_conf, err = 0;
 
-	/* No need to lock watchdog.lock here because no entry points
-	 * into the module have been registered yet.
-	 */
 	watchdog.sioaddr = sioaddr;
-	watchdog.ident.options = WDIOF_MAGICCLOSE
+	watchdog.ident.options = WDIOF_SETTIMEOUT
+				| WDIOF_MAGICCLOSE
 				| WDIOF_KEEPALIVEPING
 				| WDIOF_CARDRESET;
 
@@ -689,7 +453,6 @@ static int __init watchdog_init(int sioaddr)
 	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
 
 	wdt_conf = superio_inb(sioaddr, F71808FG_REG_WDT_CONF);
-	watchdog.caused_reboot = wdt_conf & BIT(F71808FG_FLAG_WDTMOUT_STS);
 
 	/*
 	 * We don't want WDTMOUT_STS to stick around till regular reboot.
@@ -698,80 +461,34 @@ static int __init watchdog_init(int sioaddr)
 	superio_outb(sioaddr, F71808FG_REG_WDT_CONF,
 		     wdt_conf | BIT(F71808FG_FLAG_WDTMOUT_STS));
 
-	superio_exit(sioaddr);
-
-	err = fintek_wdt_set_timeout(timeout);
-	if (err)
-		return err;
-	err = fintek_wdt_set_pulse_width(pulse_width);
-	if (err)
-		return err;
-
-	err = register_reboot_notifier(&watchdog_notifier);
-	if (err)
-		return err;
+	wdd = &watchdog.wdd;
 
-	err = misc_register(&watchdog_miscdev);
-	if (err) {
-		pr_err("cannot register miscdev on minor=%d\n",
-		       watchdog_miscdev.minor);
-		goto exit_reboot;
-	}
+	if (fintek_wdt_is_running(wdt_conf))
+		set_bit(WDOG_HW_RUNNING, &wdd->status);
 
-	if (start_withtimeout) {
-		if (start_withtimeout <= 0
-		 || start_withtimeout >  max_timeout) {
-			pr_err("starting timeout out of range\n");
-			err = -EINVAL;
-			goto exit_miscdev;
-		}
-
-		err = fintek_wdt_start();
-		if (err) {
-			pr_err("cannot start watchdog timer\n");
-			goto exit_miscdev;
-		}
-
-		mutex_lock(&watchdog.lock);
-		err = superio_enter(sioaddr);
-		if (err)
-			goto exit_unlock;
-		superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
-
-		if (start_withtimeout > 0xff) {
-			/* select minutes for timer units */
-			superio_set_bit(sioaddr, F71808FG_REG_WDT_CONF,
-				F71808FG_FLAG_WD_UNIT);
-			superio_outb(sioaddr, F71808FG_REG_WD_TIME,
-				DIV_ROUND_UP(start_withtimeout, 60));
-		} else {
-			/* select seconds for timer units */
-			superio_clear_bit(sioaddr, F71808FG_REG_WDT_CONF,
-				F71808FG_FLAG_WD_UNIT);
-			superio_outb(sioaddr, F71808FG_REG_WD_TIME,
-				start_withtimeout);
-		}
+	superio_exit(sioaddr);
 
-		superio_exit(sioaddr);
-		mutex_unlock(&watchdog.lock);
+	wdd->info               = &watchdog.ident;
+	wdd->ops                = &fintek_wdt_ops;
+	wdd->min_timeout        = 1;
+	wdd->max_timeout        = max_timeout;
 
-		if (nowayout)
-			__module_get(THIS_MODULE);
+	watchdog_set_nowayout(wdd, nowayout);
+	watchdog_stop_on_unregister(wdd);
+	watchdog_stop_on_reboot(wdd);
+	watchdog_init_timeout(wdd, start_withtimeout, NULL);
 
-		pr_info("watchdog started with initial timeout of %u sec\n",
-			start_withtimeout);
-	}
+	if (wdt_conf & BIT(F71808FG_FLAG_WDTMOUT_STS))
+		wdd->bootstatus = WDIOF_CARDRESET;
 
-	return 0;
-
-exit_unlock:
-	mutex_unlock(&watchdog.lock);
-exit_miscdev:
-	misc_deregister(&watchdog_miscdev);
-exit_reboot:
-	unregister_reboot_notifier(&watchdog_notifier);
+	/*
+	 * WATCHDOG_HANDLE_BOOT_ENABLED can result in keepalive being directly
+	 * called without a set_timeout before, so it needs to be done here once
+	 */
+	fintek_wdt_set_timeout(wdd, timeout);
+	fintek_wdt_set_pulse_width(pulse_width);
 
-	return err;
+	return watchdog_register_device(wdd);
 }
 
 static int __init fintek_wdt_find(int sioaddr)
@@ -861,12 +578,7 @@ static int __init fintek_wdt_init(void)
 
 static void __exit fintek_wdt_exit(void)
 {
-	if (fintek_wdt_is_running()) {
-		pr_warn("Watchdog timer still running, stopping it\n");
-		fintek_wdt_stop();
-	}
-	misc_deregister(&watchdog_miscdev);
-	unregister_reboot_notifier(&watchdog_notifier);
+	watchdog_unregister_device(&watchdog.wdd);
 }
 
 MODULE_DESCRIPTION("F71808E Watchdog Driver");
-- 
git-series 0.9.1

Re: [PATCH v4 3/5] watchdog: f71808e_wdt: migrate to new kernel watchdog API

[Guenter Roeck]

On Thu, Jul 22, 2021 at 12:14:42PM +0200, Ahmad Fatoum wrote:
> Migrating the driver lets us drop the watchdog misc device boilerplate
> and reduces size by 285 lines. It also brings us support for new
> functionality like CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED.
> 
> This incurs a slight backwards-compatibility break, because the new
> kernel watchdog API doesn't support unloading modules for drivers
> whose watchdog hardware is reported to be running.
> 
> This means following scenario will be no longer supported:
>  - BIOS has enabled watchdog
>  - Module is loaded and unloaded without opening watchdog
>  - module_exit is expected to succeed and disable watchdog HW
> 
> Suggested-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/watchdog/Kconfig       |   1 +-
>  drivers/watchdog/f71808e_wdt.c | 388 ++++------------------------------
>  2 files changed, 51 insertions(+), 338 deletions(-)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 546dfc1e2349..3cc6aac18006 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1063,6 +1063,7 @@ config EBC_C384_WDT
>  config F71808E_WDT
>  	tristate "Fintek F718xx, F818xx Super I/O Watchdog"
>  	depends on X86
> +	select WATCHDOG_CORE
>  	help
>  	  This is the driver for the hardware watchdog on the Fintek F71808E,
>  	  F71862FG, F71868, F71869, F71882FG, F71889FG, F81803, F81865, and
> diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
> index 597eaf6905f4..efc6981d9a9b 100644
> --- a/drivers/watchdog/f71808e_wdt.c
> +++ b/drivers/watchdog/f71808e_wdt.c
> @@ -9,16 +9,10 @@
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>  
>  #include <linux/err.h>
> -#include <linux/fs.h>
>  #include <linux/init.h>
>  #include <linux/io.h>
>  #include <linux/ioport.h>
> -#include <linux/miscdevice.h>
>  #include <linux/module.h>
> -#include <linux/mutex.h>
> -#include <linux/notifier.h>
> -#include <linux/reboot.h>
> -#include <linux/uaccess.h>
>  #include <linux/watchdog.h>
>  
>  #define DRVNAME "f71808e_wdt"
> @@ -137,24 +131,18 @@ static inline void superio_select(int base, int ld);
>  static inline void superio_exit(int base);
>  
>  struct fintek_wdt {
> +	struct watchdog_device wdd;
>  	unsigned short	sioaddr;
>  	enum chips	type;
> -	unsigned long	opened;
> -	struct mutex	lock;
> -	char		expect_close;
>  	struct watchdog_info ident;
>  
> -	unsigned short	timeout;
>  	u8		timer_val;	/* content for the wd_time register */
>  	char		minutes_mode;
>  	u8		pulse_val;	/* pulse width flag */
>  	char		pulse_mode;	/* enable pulse output mode? */
> -	char		caused_reboot;	/* last reboot was by the watchdog */
>  };
>  
> -static struct fintek_wdt watchdog = {
> -	.lock = __MUTEX_INITIALIZER(watchdog.lock),
> -};
> +static struct fintek_wdt watchdog;
>  
>  /* Super I/O functions */
>  static inline int superio_inb(int base, int reg)
> @@ -218,16 +206,8 @@ static inline void superio_exit(int base)
>  	release_region(base, 2);
>  }
>  
> -static int fintek_wdt_set_timeout(int timeout)
> +static int fintek_wdt_set_timeout(struct watchdog_device *wdd, unsigned int timeout)
>  {
> -	if (timeout <= 0
> -	 || timeout >  max_timeout) {
> -		pr_err("watchdog timeout out of range\n");
> -		return -EINVAL;
> -	}
> -
> -	mutex_lock(&watchdog.lock);
> -
>  	if (timeout > 0xff) {
>  		watchdog.timer_val = DIV_ROUND_UP(timeout, 60);
>  		watchdog.minutes_mode = true;
> @@ -237,16 +217,13 @@ static int fintek_wdt_set_timeout(int timeout)
>  		watchdog.minutes_mode = false;
>  	}
>  
> -	watchdog.timeout = timeout;
> -
> -	mutex_unlock(&watchdog.lock);
> +	wdd->timeout = timeout;
>  
>  	return 0;
>  }
>  
>  static int fintek_wdt_set_pulse_width(unsigned int pw)
>  {
> -	int err = 0;
>  	unsigned int t1 = 25, t2 = 125, t3 = 5000;
>  
>  	if (watchdog.type == f71868) {
> @@ -255,8 +232,6 @@ static int fintek_wdt_set_pulse_width(unsigned int pw)
>  		t3 = 6000;
>  	}
>  
> -	mutex_lock(&watchdog.lock);
> -
>  	if        (pw <=  1) {
>  		watchdog.pulse_val = 0;
>  	} else if (pw <= t1) {
> @@ -267,25 +242,21 @@ static int fintek_wdt_set_pulse_width(unsigned int pw)
>  		watchdog.pulse_val = 3;
>  	} else {
>  		pr_err("pulse width out of range\n");
> -		err = -EINVAL;
> -		goto exit_unlock;
> +		return -EINVAL;
>  	}
>  
>  	watchdog.pulse_mode = pw;
>  
> -exit_unlock:
> -	mutex_unlock(&watchdog.lock);
> -	return err;
> +	return 0;
>  }
>  
> -static int fintek_wdt_keepalive(void)
> +static int fintek_wdt_keepalive(struct watchdog_device *wdd)
>  {
> -	int err = 0;
> +	int err;
>  
> -	mutex_lock(&watchdog.lock);
>  	err = superio_enter(watchdog.sioaddr);
>  	if (err)
> -		goto exit_unlock;
> +		return err;
>  	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
>  
>  	if (watchdog.minutes_mode)
> @@ -303,25 +274,22 @@ static int fintek_wdt_keepalive(void)
>  
>  	superio_exit(watchdog.sioaddr);
>  
> -exit_unlock:
> -	mutex_unlock(&watchdog.lock);
> -	return err;
> +	return 0;
>  }
>  
> -static int fintek_wdt_start(void)
> +static int fintek_wdt_start(struct watchdog_device *wdd)
>  {
>  	int err;
>  	u8 tmp;
>  
>  	/* Make sure we don't die as soon as the watchdog is enabled below */
> -	err = fintek_wdt_keepalive();
> +	err = fintek_wdt_keepalive(wdd);
>  	if (err)
>  		return err;
>  
> -	mutex_lock(&watchdog.lock);
>  	err = superio_enter(watchdog.sioaddr);
>  	if (err)
> -		goto exit_unlock;
> +		return err;
>  	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
>  
>  	/* Watchdog pin configuration */
> @@ -429,20 +397,17 @@ static int fintek_wdt_start(void)
>  
>  exit_superio:
>  	superio_exit(watchdog.sioaddr);
> -exit_unlock:
> -	mutex_unlock(&watchdog.lock);
>  
>  	return err;
>  }
>  
> -static int fintek_wdt_stop(void)
> +static int fintek_wdt_stop(struct watchdog_device *wdd)
>  {
> -	int err = 0;
> +	int err;
>  
> -	mutex_lock(&watchdog.lock);
>  	err = superio_enter(watchdog.sioaddr);
>  	if (err)
> -		goto exit_unlock;
> +		return err;
>  	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
>  
>  	superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
> @@ -450,232 +415,31 @@ static int fintek_wdt_stop(void)
>  
>  	superio_exit(watchdog.sioaddr);
>  
> -exit_unlock:
> -	mutex_unlock(&watchdog.lock);
> -
> -	return err;
> -}
> -
> -static int watchdog_get_status(void)
> -{
> -	int status = 0;
> -
> -	mutex_lock(&watchdog.lock);
> -	status = (watchdog.caused_reboot) ? WDIOF_CARDRESET : 0;
> -	mutex_unlock(&watchdog.lock);
> -
> -	return status;
> -}
> -
> -static bool fintek_wdt_is_running(void)
> -{
> -	/*
> -	 * if we fail to determine the watchdog's status assume it to be
> -	 * running to be on the safe side
> -	 */
> -	bool is_running = true;
> -
> -	mutex_lock(&watchdog.lock);
> -	if (superio_enter(watchdog.sioaddr))
> -		goto exit_unlock;
> -	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
> -
> -	is_running = (superio_inb(watchdog.sioaddr, SIO_REG_ENABLE) & BIT(0))
> -		&& (superio_inb(watchdog.sioaddr, F71808FG_REG_WDT_CONF)
> -			& BIT(F71808FG_FLAG_WD_EN));
> -
> -	superio_exit(watchdog.sioaddr);
> -
> -exit_unlock:
> -	mutex_unlock(&watchdog.lock);
> -	return is_running;
> -}
> -
> -/* /dev/watchdog api */
> -
> -static int watchdog_open(struct inode *inode, struct file *file)
> -{
> -	int err;
> -
> -	/* If the watchdog is alive we don't need to start it again */
> -	if (test_and_set_bit(0, &watchdog.opened))
> -		return -EBUSY;
> -
> -	err = fintek_wdt_start();
> -	if (err) {
> -		clear_bit(0, &watchdog.opened);
> -		return err;
> -	}
> -
> -	if (nowayout)
> -		__module_get(THIS_MODULE);
> -
> -	watchdog.expect_close = 0;
> -	return stream_open(inode, file);
> -}
> -
> -static int watchdog_release(struct inode *inode, struct file *file)
> -{
> -	clear_bit(0, &watchdog.opened);
> -
> -	if (!watchdog.expect_close) {
> -		fintek_wdt_keepalive();
> -		pr_crit("Unexpected close, not stopping watchdog!\n");
> -	} else if (!nowayout) {
> -		fintek_wdt_stop();
> -	}
>  	return 0;
>  }
>  
> -/*
> - *      watchdog_write:
> - *      @file: file handle to the watchdog
> - *      @buf: buffer to write
> - *      @count: count of bytes
> - *      @ppos: pointer to the position to write. No seeks allowed
> - *
> - *      A write to a watchdog device is defined as a keepalive signal. Any
> - *      write of data will do, as we we don't define content meaning.
> - */
> -
> -static ssize_t watchdog_write(struct file *file, const char __user *buf,
> -			    size_t count, loff_t *ppos)
> -{
> -	if (count) {
> -		if (!nowayout) {
> -			size_t i;
> -
> -			/* In case it was set long ago */
> -			bool expect_close = false;
> -
> -			for (i = 0; i != count; i++) {
> -				char c;
> -				if (get_user(c, buf + i))
> -					return -EFAULT;
> -				if (c == 'V')
> -					expect_close = true;
> -			}
> -
> -			/* Properly order writes across fork()ed processes */
> -			mutex_lock(&watchdog.lock);
> -			watchdog.expect_close = expect_close;
> -			mutex_unlock(&watchdog.lock);
> -		}
> -
> -		/* someone wrote to us, we should restart timer */
> -		fintek_wdt_keepalive();
> -	}
> -	return count;
> -}
> -
> -/*
> - *      watchdog_ioctl:
> - *      @inode: inode of the device
> - *      @file: file handle to the device
> - *      @cmd: watchdog command
> - *      @arg: argument pointer
> - *
> - *      The watchdog API defines a common set of functions for all watchdogs
> - *      according to their available features.
> - */
> -static long watchdog_ioctl(struct file *file, unsigned int cmd,
> -	unsigned long arg)
> +static bool fintek_wdt_is_running(u8 wdt_conf)
>  {
> -	int status;
> -	int new_options;
> -	int new_timeout;
> -	union {
> -		struct watchdog_info __user *ident;
> -		int __user *i;
> -	} uarg;
> -
> -	uarg.i = (int __user *)arg;
> -
> -	switch (cmd) {
> -	case WDIOC_GETSUPPORT:
> -		return copy_to_user(uarg.ident, &watchdog.ident,
> -			sizeof(watchdog.ident)) ? -EFAULT : 0;
> -
> -	case WDIOC_GETSTATUS:
> -		status = watchdog_get_status();
> -		if (status < 0)
> -			return status;
> -		return put_user(status, uarg.i);
> -
> -	case WDIOC_GETBOOTSTATUS:
> -		return put_user(0, uarg.i);
> -
> -	case WDIOC_SETOPTIONS:
> -		if (get_user(new_options, uarg.i))
> -			return -EFAULT;
> -
> -		if (new_options & WDIOS_DISABLECARD)
> -			fintek_wdt_stop();
> -
> -		if (new_options & WDIOS_ENABLECARD)
> -			return fintek_wdt_start();
> -		fallthrough;
> -
> -	case WDIOC_KEEPALIVE:
> -		fintek_wdt_keepalive();
> -		return 0;
> -
> -	case WDIOC_SETTIMEOUT:
> -		if (get_user(new_timeout, uarg.i))
> -			return -EFAULT;
> -
> -		if (fintek_wdt_set_timeout(new_timeout))
> -			return -EINVAL;
> -
> -		fintek_wdt_keepalive();
> -		fallthrough;
> -
> -	case WDIOC_GETTIMEOUT:
> -		return put_user(watchdog.timeout, uarg.i);
> -
> -	default:
> -		return -ENOTTY;
> -
> -	}
> +	return (superio_inb(watchdog.sioaddr, SIO_REG_ENABLE) & BIT(0))
> +		&& (wdt_conf & BIT(F71808FG_FLAG_WD_EN));
>  }
>  
> -static int watchdog_notify_sys(struct notifier_block *this, unsigned long code,
> -	void *unused)
> -{
> -	if (code == SYS_DOWN || code == SYS_HALT)
> -		fintek_wdt_stop();
> -	return NOTIFY_DONE;
> -}
> -
> -static const struct file_operations watchdog_fops = {
> -	.owner		= THIS_MODULE,
> -	.llseek		= no_llseek,
> -	.open		= watchdog_open,
> -	.release	= watchdog_release,
> -	.write		= watchdog_write,
> -	.unlocked_ioctl	= watchdog_ioctl,
> -	.compat_ioctl	= compat_ptr_ioctl,
> -};
> -
> -static struct miscdevice watchdog_miscdev = {
> -	.minor		= WATCHDOG_MINOR,
> -	.name		= "watchdog",
> -	.fops		= &watchdog_fops,
> -};
> -
> -static struct notifier_block watchdog_notifier = {
> -	.notifier_call = watchdog_notify_sys,
> +static const struct watchdog_ops fintek_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = fintek_wdt_start,
> +	.stop = fintek_wdt_stop,
> +	.ping = fintek_wdt_keepalive,
> +	.set_timeout = fintek_wdt_set_timeout,
>  };
>  
>  static int __init watchdog_init(int sioaddr)
>  {
> +	struct watchdog_device *wdd;
>  	int wdt_conf, err = 0;
>  
> -	/* No need to lock watchdog.lock here because no entry points
> -	 * into the module have been registered yet.
> -	 */
>  	watchdog.sioaddr = sioaddr;
> -	watchdog.ident.options = WDIOF_MAGICCLOSE
> +	watchdog.ident.options = WDIOF_SETTIMEOUT
> +				| WDIOF_MAGICCLOSE
>  				| WDIOF_KEEPALIVEPING
>  				| WDIOF_CARDRESET;
>  
> @@ -689,7 +453,6 @@ static int __init watchdog_init(int sioaddr)
>  	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
>  
>  	wdt_conf = superio_inb(sioaddr, F71808FG_REG_WDT_CONF);
> -	watchdog.caused_reboot = wdt_conf & BIT(F71808FG_FLAG_WDTMOUT_STS);
>  
>  	/*
>  	 * We don't want WDTMOUT_STS to stick around till regular reboot.
> @@ -698,80 +461,34 @@ static int __init watchdog_init(int sioaddr)
>  	superio_outb(sioaddr, F71808FG_REG_WDT_CONF,
>  		     wdt_conf | BIT(F71808FG_FLAG_WDTMOUT_STS));
>  
> -	superio_exit(sioaddr);
> -
> -	err = fintek_wdt_set_timeout(timeout);
> -	if (err)
> -		return err;
> -	err = fintek_wdt_set_pulse_width(pulse_width);
> -	if (err)
> -		return err;
> -
> -	err = register_reboot_notifier(&watchdog_notifier);
> -	if (err)
> -		return err;
> +	wdd = &watchdog.wdd;
>  
> -	err = misc_register(&watchdog_miscdev);
> -	if (err) {
> -		pr_err("cannot register miscdev on minor=%d\n",
> -		       watchdog_miscdev.minor);
> -		goto exit_reboot;
> -	}
> +	if (fintek_wdt_is_running(wdt_conf))
> +		set_bit(WDOG_HW_RUNNING, &wdd->status);
>  
> -	if (start_withtimeout) {
> -		if (start_withtimeout <= 0
> -		 || start_withtimeout >  max_timeout) {
> -			pr_err("starting timeout out of range\n");
> -			err = -EINVAL;
> -			goto exit_miscdev;
> -		}
> -
> -		err = fintek_wdt_start();
> -		if (err) {
> -			pr_err("cannot start watchdog timer\n");
> -			goto exit_miscdev;
> -		}
> -
> -		mutex_lock(&watchdog.lock);
> -		err = superio_enter(sioaddr);
> -		if (err)
> -			goto exit_unlock;
> -		superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
> -
> -		if (start_withtimeout > 0xff) {
> -			/* select minutes for timer units */
> -			superio_set_bit(sioaddr, F71808FG_REG_WDT_CONF,
> -				F71808FG_FLAG_WD_UNIT);
> -			superio_outb(sioaddr, F71808FG_REG_WD_TIME,
> -				DIV_ROUND_UP(start_withtimeout, 60));
> -		} else {
> -			/* select seconds for timer units */
> -			superio_clear_bit(sioaddr, F71808FG_REG_WDT_CONF,
> -				F71808FG_FLAG_WD_UNIT);
> -			superio_outb(sioaddr, F71808FG_REG_WD_TIME,
> -				start_withtimeout);
> -		}
> +	superio_exit(sioaddr);
>  
> -		superio_exit(sioaddr);
> -		mutex_unlock(&watchdog.lock);
> +	wdd->info               = &watchdog.ident;
> +	wdd->ops                = &fintek_wdt_ops;
> +	wdd->min_timeout        = 1;
> +	wdd->max_timeout        = max_timeout;
>  
> -		if (nowayout)
> -			__module_get(THIS_MODULE);
> +	watchdog_set_nowayout(wdd, nowayout);
> +	watchdog_stop_on_unregister(wdd);
> +	watchdog_stop_on_reboot(wdd);
> +	watchdog_init_timeout(wdd, start_withtimeout, NULL);
>  
> -		pr_info("watchdog started with initial timeout of %u sec\n",
> -			start_withtimeout);
> -	}
> +	if (wdt_conf & BIT(F71808FG_FLAG_WDTMOUT_STS))
> +		wdd->bootstatus = WDIOF_CARDRESET;
>  
> -	return 0;
> -
> -exit_unlock:
> -	mutex_unlock(&watchdog.lock);
> -exit_miscdev:
> -	misc_deregister(&watchdog_miscdev);
> -exit_reboot:
> -	unregister_reboot_notifier(&watchdog_notifier);
> +	/*
> +	 * WATCHDOG_HANDLE_BOOT_ENABLED can result in keepalive being directly
> +	 * called without a set_timeout before, so it needs to be done here once
> +	 */
> +	fintek_wdt_set_timeout(wdd, timeout);

The use of start_withtimeout and timeout variables is confusing,
as is the use of the max_timeout variable.

Starting with max_timeout,

static const int max_timeout = WATCHDOG_MAX_TIMEOUT;

	wdd->max_timeout        = max_timeout;

is its only use. That has zero value.

start_withtimeout is supposed to be the initial timeout, after module
load (similar to the open_timeout watchdog module parameter). Its value
is written into the 'timeout' field of struct watchdog_device, only to
be almost immediately overwritten with the value of the 'timeout'
variable and module parameter. That has zero value. It is also different
to the original code, which actually starts the watchdog with a timeout
of start_withtimeout (and presumably resets the system if the watchdog
device isn't opened by the time it expires).

> +	fintek_wdt_set_pulse_width(pulse_width);
>  
> -	return err;
> +	return watchdog_register_device(wdd);
>  }
>  
>  static int __init fintek_wdt_find(int sioaddr)
> @@ -861,12 +578,7 @@ static int __init fintek_wdt_init(void)
>  
>  static void __exit fintek_wdt_exit(void)
>  {
> -	if (fintek_wdt_is_running()) {
> -		pr_warn("Watchdog timer still running, stopping it\n");
> -		fintek_wdt_stop();
> -	}
> -	misc_deregister(&watchdog_miscdev);
> -	unregister_reboot_notifier(&watchdog_notifier);
> +	watchdog_unregister_device(&watchdog.wdd);
>  }
>  
>  MODULE_DESCRIPTION("F71808E Watchdog Driver");
> -- 
> git-series 0.9.1

[PATCH v4 4/5] watchdog: f71808e_wdt: refactor to platform device/driver pair

[Ahmad Fatoum]

Driver so far wasn't ported to the driver model and registered
the watchdog device out of the init after probing the I/O ports
for a watchdog with correct vendor and device revision.

Keep the device detection part at init time, but move watchdog
registration to a platform driver probe function.

Suggested-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/f71808e_wdt.c | 42 +++++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 4 deletions(-)

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index efc6981d9a9b..d4cae73da002 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -13,6 +13,7 @@
 #include <linux/io.h>
 #include <linux/ioport.h>
 #include <linux/module.h>
+#include <linux/platform_device.h>
 #include <linux/watchdog.h>
 
 #define DRVNAME "f71808e_wdt"
@@ -432,10 +433,18 @@ static const struct watchdog_ops fintek_wdt_ops = {
 	.set_timeout = fintek_wdt_set_timeout,
 };
 
-static int __init watchdog_init(int sioaddr)
+static int fintek_wdt_probe(struct platform_device *pdev)
 {
 	struct watchdog_device *wdd;
 	int wdt_conf, err = 0;
+	struct resource *res;
+	int sioaddr;
+
+	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+	if (!res)
+		return -ENXIO;
+
+	sioaddr = res->start;
 
 	watchdog.sioaddr = sioaddr;
 	watchdog.ident.options = WDIOF_SETTIMEOUT
@@ -468,6 +477,7 @@ static int __init watchdog_init(int sioaddr)
 
 	superio_exit(sioaddr);
 
+	wdd->parent		= &pdev->dev;
 	wdd->info               = &watchdog.ident;
 	wdd->ops                = &fintek_wdt_ops;
 	wdd->min_timeout        = 1;
@@ -488,7 +498,7 @@ static int __init watchdog_init(int sioaddr)
 	fintek_wdt_set_timeout(wdd, timeout);
 	fintek_wdt_set_pulse_width(pulse_width);
 
-	return watchdog_register_device(wdd);
+	return devm_watchdog_register_device(&pdev->dev, wdd);
 }
 
 static int __init fintek_wdt_find(int sioaddr)
@@ -554,9 +564,19 @@ static int __init fintek_wdt_find(int sioaddr)
 	return err;
 }
 
+static struct platform_driver fintek_wdt_driver = {
+	.probe          = fintek_wdt_probe,
+	.driver         = {
+		.name   = DRVNAME,
+	},
+};
+
+static struct platform_device *fintek_wdt_pdev;
+
 static int __init fintek_wdt_init(void)
 {
 	static const unsigned short addrs[] = { 0x2e, 0x4e };
+	struct resource wdt_res = {};
 	int err = -ENODEV;
 	int i;
 
@@ -573,12 +593,26 @@ static int __init fintek_wdt_init(void)
 	if (i == ARRAY_SIZE(addrs))
 		return err;
 
-	return watchdog_init(addrs[i]);
+	platform_driver_register(&fintek_wdt_driver);
+
+	wdt_res.name = "superio port";
+	wdt_res.flags = IORESOURCE_IO;
+	wdt_res.start = addrs[i];
+	wdt_res.end   = addrs[i] + 1;
+
+	fintek_wdt_pdev = platform_device_register_simple(DRVNAME, -1, &wdt_res, 1);
+	if (IS_ERR(fintek_wdt_pdev)) {
+		platform_driver_unregister(&fintek_wdt_driver);
+		return PTR_ERR(fintek_wdt_pdev);
+	}
+
+	return 0;
 }
 
 static void __exit fintek_wdt_exit(void)
 {
-	watchdog_unregister_device(&watchdog.wdd);
+	platform_device_unregister(fintek_wdt_pdev);
+	platform_driver_unregister(&fintek_wdt_driver);
 }
 
 MODULE_DESCRIPTION("F71808E Watchdog Driver");
-- 
git-series 0.9.1

Re: [PATCH v4 4/5] watchdog: f71808e_wdt: refactor to platform device/driver pair

[Guenter Roeck]

On Thu, Jul 22, 2021 at 12:14:43PM +0200, Ahmad Fatoum wrote:
> Driver so far wasn't ported to the driver model and registered
> the watchdog device out of the init after probing the I/O ports
> for a watchdog with correct vendor and device revision.
> 
> Keep the device detection part at init time, but move watchdog
> registration to a platform driver probe function.
> 
> Suggested-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/f71808e_wdt.c | 42 +++++++++++++++++++++++++++++++----
>  1 file changed, 38 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
> index efc6981d9a9b..d4cae73da002 100644
> --- a/drivers/watchdog/f71808e_wdt.c
> +++ b/drivers/watchdog/f71808e_wdt.c
> @@ -13,6 +13,7 @@
>  #include <linux/io.h>
>  #include <linux/ioport.h>
>  #include <linux/module.h>
> +#include <linux/platform_device.h>
>  #include <linux/watchdog.h>
>  
>  #define DRVNAME "f71808e_wdt"
> @@ -432,10 +433,18 @@ static const struct watchdog_ops fintek_wdt_ops = {
>  	.set_timeout = fintek_wdt_set_timeout,
>  };
>  
> -static int __init watchdog_init(int sioaddr)
> +static int fintek_wdt_probe(struct platform_device *pdev)
>  {
>  	struct watchdog_device *wdd;
>  	int wdt_conf, err = 0;
> +	struct resource *res;
> +	int sioaddr;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_IO, 0);
> +	if (!res)
> +		return -ENXIO;
> +
> +	sioaddr = res->start;
>  
>  	watchdog.sioaddr = sioaddr;
>  	watchdog.ident.options = WDIOF_SETTIMEOUT
> @@ -468,6 +477,7 @@ static int __init watchdog_init(int sioaddr)
>  
>  	superio_exit(sioaddr);
>  
> +	wdd->parent		= &pdev->dev;
>  	wdd->info               = &watchdog.ident;
>  	wdd->ops                = &fintek_wdt_ops;
>  	wdd->min_timeout        = 1;
> @@ -488,7 +498,7 @@ static int __init watchdog_init(int sioaddr)
>  	fintek_wdt_set_timeout(wdd, timeout);
>  	fintek_wdt_set_pulse_width(pulse_width);
>  
> -	return watchdog_register_device(wdd);
> +	return devm_watchdog_register_device(&pdev->dev, wdd);
>  }
>  
>  static int __init fintek_wdt_find(int sioaddr)
> @@ -554,9 +564,19 @@ static int __init fintek_wdt_find(int sioaddr)
>  	return err;
>  }
>  
> +static struct platform_driver fintek_wdt_driver = {
> +	.probe          = fintek_wdt_probe,
> +	.driver         = {
> +		.name   = DRVNAME,
> +	},
> +};
> +
> +static struct platform_device *fintek_wdt_pdev;
> +
>  static int __init fintek_wdt_init(void)
>  {
>  	static const unsigned short addrs[] = { 0x2e, 0x4e };
> +	struct resource wdt_res = {};
>  	int err = -ENODEV;
>  	int i;
>  
> @@ -573,12 +593,26 @@ static int __init fintek_wdt_init(void)
>  	if (i == ARRAY_SIZE(addrs))
>  		return err;
>  
> -	return watchdog_init(addrs[i]);
> +	platform_driver_register(&fintek_wdt_driver);
> +
> +	wdt_res.name = "superio port";
> +	wdt_res.flags = IORESOURCE_IO;
> +	wdt_res.start = addrs[i];
> +	wdt_res.end   = addrs[i] + 1;
> +
> +	fintek_wdt_pdev = platform_device_register_simple(DRVNAME, -1, &wdt_res, 1);
> +	if (IS_ERR(fintek_wdt_pdev)) {
> +		platform_driver_unregister(&fintek_wdt_driver);
> +		return PTR_ERR(fintek_wdt_pdev);
> +	}
> +
> +	return 0;
>  }
>  
>  static void __exit fintek_wdt_exit(void)
>  {
> -	watchdog_unregister_device(&watchdog.wdd);
> +	platform_device_unregister(fintek_wdt_pdev);
> +	platform_driver_unregister(&fintek_wdt_driver);
>  }
>  
>  MODULE_DESCRIPTION("F71808E Watchdog Driver");
> -- 
> git-series 0.9.1

[PATCH v4 5/5] watchdog: f71808e_wdt: dynamically allocate watchdog driver data

[Ahmad Fatoum]

While the driver will only match against a single device, convention is
to dynamically allocate the driver data.

Suggested-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/f71808e_wdt.c | 198 +++++++++++++++++++---------------
 1 file changed, 111 insertions(+), 87 deletions(-)

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index d4cae73da002..f495bf6fb0ab 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -143,7 +143,9 @@ struct fintek_wdt {
 	char		pulse_mode;	/* enable pulse output mode? */
 };
 
-static struct fintek_wdt watchdog;
+struct fintek_wdt_pdata {
+	enum chips	type;
+};
 
 /* Super I/O functions */
 static inline int superio_inb(int base, int reg)
@@ -209,13 +211,15 @@ static inline void superio_exit(int base)
 
 static int fintek_wdt_set_timeout(struct watchdog_device *wdd, unsigned int timeout)
 {
+	struct fintek_wdt *wd = watchdog_get_drvdata(wdd);
+
 	if (timeout > 0xff) {
-		watchdog.timer_val = DIV_ROUND_UP(timeout, 60);
-		watchdog.minutes_mode = true;
-		timeout = watchdog.timer_val * 60;
+		wd->timer_val = DIV_ROUND_UP(timeout, 60);
+		wd->minutes_mode = true;
+		timeout = wd->timer_val * 60;
 	} else {
-		watchdog.timer_val = timeout;
-		watchdog.minutes_mode = false;
+		wd->timer_val = timeout;
+		wd->minutes_mode = false;
 	}
 
 	wdd->timeout = timeout;
@@ -223,63 +227,65 @@ static int fintek_wdt_set_timeout(struct watchdog_device *wdd, unsigned int time
 	return 0;
 }
 
-static int fintek_wdt_set_pulse_width(unsigned int pw)
+static int fintek_wdt_set_pulse_width(struct fintek_wdt *wd, unsigned int pw)
 {
 	unsigned int t1 = 25, t2 = 125, t3 = 5000;
 
-	if (watchdog.type == f71868) {
+	if (wd->type == f71868) {
 		t1 = 30;
 		t2 = 150;
 		t3 = 6000;
 	}
 
 	if        (pw <=  1) {
-		watchdog.pulse_val = 0;
+		wd->pulse_val = 0;
 	} else if (pw <= t1) {
-		watchdog.pulse_val = 1;
+		wd->pulse_val = 1;
 	} else if (pw <= t2) {
-		watchdog.pulse_val = 2;
+		wd->pulse_val = 2;
 	} else if (pw <= t3) {
-		watchdog.pulse_val = 3;
+		wd->pulse_val = 3;
 	} else {
 		pr_err("pulse width out of range\n");
 		return -EINVAL;
 	}
 
-	watchdog.pulse_mode = pw;
+	wd->pulse_mode = pw;
 
 	return 0;
 }
 
 static int fintek_wdt_keepalive(struct watchdog_device *wdd)
 {
+	struct fintek_wdt *wd = watchdog_get_drvdata(wdd);
 	int err;
 
-	err = superio_enter(watchdog.sioaddr);
+	err = superio_enter(wd->sioaddr);
 	if (err)
 		return err;
-	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
+	superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT);
 
-	if (watchdog.minutes_mode)
+	if (wd->minutes_mode)
 		/* select minutes for timer units */
-		superio_set_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
+		superio_set_bit(wd->sioaddr, F71808FG_REG_WDT_CONF,
 				F71808FG_FLAG_WD_UNIT);
 	else
 		/* select seconds for timer units */
-		superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
+		superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF,
 				F71808FG_FLAG_WD_UNIT);
 
 	/* Set timer value */
-	superio_outb(watchdog.sioaddr, F71808FG_REG_WD_TIME,
-			   watchdog.timer_val);
+	superio_outb(wd->sioaddr, F71808FG_REG_WD_TIME,
+			   wd->timer_val);
 
-	superio_exit(watchdog.sioaddr);
+	superio_exit(wd->sioaddr);
 
 	return 0;
 }
 
 static int fintek_wdt_start(struct watchdog_device *wdd)
 {
+	struct fintek_wdt *wd = watchdog_get_drvdata(wdd);
 	int err;
 	u8 tmp;
 
@@ -288,57 +294,57 @@ static int fintek_wdt_start(struct watchdog_device *wdd)
 	if (err)
 		return err;
 
-	err = superio_enter(watchdog.sioaddr);
+	err = superio_enter(wd->sioaddr);
 	if (err)
 		return err;
-	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
+	superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT);
 
 	/* Watchdog pin configuration */
-	switch (watchdog.type) {
+	switch (wd->type) {
 	case f71808fg:
 		/* Set pin 21 to GPIO23/WDTRST#, then to WDTRST# */
-		superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT2, 3);
-		superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT3, 3);
+		superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT2, 3);
+		superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 3);
 		break;
 
 	case f71862fg:
 		if (f71862fg_pin == 63) {
 			/* SPI must be disabled first to use this pin! */
-			superio_clear_bit(watchdog.sioaddr, SIO_REG_ROM_ADDR_SEL, 6);
-			superio_set_bit(watchdog.sioaddr, SIO_REG_MFUNCT3, 4);
+			superio_clear_bit(wd->sioaddr, SIO_REG_ROM_ADDR_SEL, 6);
+			superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT3, 4);
 		} else if (f71862fg_pin == 56) {
-			superio_set_bit(watchdog.sioaddr, SIO_REG_MFUNCT1, 1);
+			superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT1, 1);
 		}
 		break;
 
 	case f71868:
 	case f71869:
 		/* GPIO14 --> WDTRST# */
-		superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT1, 4);
+		superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT1, 4);
 		break;
 
 	case f71882fg:
 		/* Set pin 56 to WDTRST# */
-		superio_set_bit(watchdog.sioaddr, SIO_REG_MFUNCT1, 1);
+		superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT1, 1);
 		break;
 
 	case f71889fg:
 		/* set pin 40 to WDTRST# */
-		superio_outb(watchdog.sioaddr, SIO_REG_MFUNCT3,
-			superio_inb(watchdog.sioaddr, SIO_REG_MFUNCT3) & 0xcf);
+		superio_outb(wd->sioaddr, SIO_REG_MFUNCT3,
+			superio_inb(wd->sioaddr, SIO_REG_MFUNCT3) & 0xcf);
 		break;
 
 	case f81803:
 		/* Enable TSI Level register bank */
-		superio_clear_bit(watchdog.sioaddr, SIO_REG_CLOCK_SEL, 3);
+		superio_clear_bit(wd->sioaddr, SIO_REG_CLOCK_SEL, 3);
 		/* Set pin 27 to WDTRST# */
-		superio_outb(watchdog.sioaddr, SIO_REG_TSI_LEVEL_SEL, 0x5f &
-			superio_inb(watchdog.sioaddr, SIO_REG_TSI_LEVEL_SEL));
+		superio_outb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL, 0x5f &
+			superio_inb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL));
 		break;
 
 	case f81865:
 		/* Set pin 70 to WDTRST# */
-		superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT3, 5);
+		superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 5);
 		break;
 
 	case f81866:
@@ -348,12 +354,12 @@ static int fintek_wdt_start(struct watchdog_device *wdd)
 		 *     BIT5: 0 -> WDTRST#
 		 *           1 -> GPIO15
 		 */
-		tmp = superio_inb(watchdog.sioaddr, SIO_F81866_REG_PORT_SEL);
+		tmp = superio_inb(wd->sioaddr, SIO_F81866_REG_PORT_SEL);
 		tmp &= ~(BIT(3) | BIT(0));
 		tmp |= BIT(2);
-		superio_outb(watchdog.sioaddr, SIO_F81866_REG_PORT_SEL, tmp);
+		superio_outb(wd->sioaddr, SIO_F81866_REG_PORT_SEL, tmp);
 
-		superio_clear_bit(watchdog.sioaddr, SIO_F81866_REG_GPIO1, 5);
+		superio_clear_bit(wd->sioaddr, SIO_F81866_REG_GPIO1, 5);
 		break;
 
 	default:
@@ -365,63 +371,64 @@ static int fintek_wdt_start(struct watchdog_device *wdd)
 		goto exit_superio;
 	}
 
-	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
-	superio_set_bit(watchdog.sioaddr, SIO_REG_ENABLE, 0);
+	superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT);
+	superio_set_bit(wd->sioaddr, SIO_REG_ENABLE, 0);
 
-	if (watchdog.type == f81865 || watchdog.type == f81866)
-		superio_set_bit(watchdog.sioaddr, F81865_REG_WDO_CONF,
+	if (wd->type == f81865 || wd->type == f81866)
+		superio_set_bit(wd->sioaddr, F81865_REG_WDO_CONF,
 				F81865_FLAG_WDOUT_EN);
 	else
-		superio_set_bit(watchdog.sioaddr, F71808FG_REG_WDO_CONF,
+		superio_set_bit(wd->sioaddr, F71808FG_REG_WDO_CONF,
 				F71808FG_FLAG_WDOUT_EN);
 
-	superio_set_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
+	superio_set_bit(wd->sioaddr, F71808FG_REG_WDT_CONF,
 			F71808FG_FLAG_WD_EN);
 
-	if (watchdog.pulse_mode) {
+	if (wd->pulse_mode) {
 		/* Select "pulse" output mode with given duration */
-		u8 wdt_conf = superio_inb(watchdog.sioaddr,
+		u8 wdt_conf = superio_inb(wd->sioaddr,
 				F71808FG_REG_WDT_CONF);
 
 		/* Set WD_PSWIDTH bits (1:0) */
-		wdt_conf = (wdt_conf & 0xfc) | (watchdog.pulse_val & 0x03);
+		wdt_conf = (wdt_conf & 0xfc) | (wd->pulse_val & 0x03);
 		/* Set WD_PULSE to "pulse" mode */
 		wdt_conf |= BIT(F71808FG_FLAG_WD_PULSE);
 
-		superio_outb(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
+		superio_outb(wd->sioaddr, F71808FG_REG_WDT_CONF,
 				wdt_conf);
 	} else {
 		/* Select "level" output mode */
-		superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
+		superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF,
 				F71808FG_FLAG_WD_PULSE);
 	}
 
 exit_superio:
-	superio_exit(watchdog.sioaddr);
+	superio_exit(wd->sioaddr);
 
 	return err;
 }
 
 static int fintek_wdt_stop(struct watchdog_device *wdd)
 {
+	struct fintek_wdt *wd = watchdog_get_drvdata(wdd);
 	int err;
 
-	err = superio_enter(watchdog.sioaddr);
+	err = superio_enter(wd->sioaddr);
 	if (err)
 		return err;
-	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
+	superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT);
 
-	superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
+	superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF,
 			F71808FG_FLAG_WD_EN);
 
-	superio_exit(watchdog.sioaddr);
+	superio_exit(wd->sioaddr);
 
 	return 0;
 }
 
-static bool fintek_wdt_is_running(u8 wdt_conf)
+static bool fintek_wdt_is_running(struct fintek_wdt *wd, u8 wdt_conf)
 {
-	return (superio_inb(watchdog.sioaddr, SIO_REG_ENABLE) & BIT(0))
+	return (superio_inb(wd->sioaddr, SIO_REG_ENABLE) & BIT(0))
 		&& (wdt_conf & BIT(F71808FG_FLAG_WD_EN));
 }
 
@@ -435,7 +442,9 @@ static const struct watchdog_ops fintek_wdt_ops = {
 
 static int fintek_wdt_probe(struct platform_device *pdev)
 {
+	struct fintek_wdt_pdata *pdata;
 	struct watchdog_device *wdd;
+	struct fintek_wdt *wd;
 	int wdt_conf, err = 0;
 	struct resource *res;
 	int sioaddr;
@@ -446,20 +455,27 @@ static int fintek_wdt_probe(struct platform_device *pdev)
 
 	sioaddr = res->start;
 
-	watchdog.sioaddr = sioaddr;
-	watchdog.ident.options = WDIOF_SETTIMEOUT
-				| WDIOF_MAGICCLOSE
-				| WDIOF_KEEPALIVEPING
-				| WDIOF_CARDRESET;
+	wd = devm_kzalloc(&pdev->dev, sizeof(*wd), GFP_KERNEL);
+	if (!wd)
+		return -ENOMEM;
+
+	pdata = pdev->dev.platform_data;
+
+	wd->type = pdata->type;
+	wd->sioaddr = sioaddr;
+	wd->ident.options = WDIOF_SETTIMEOUT
+			| WDIOF_MAGICCLOSE
+			| WDIOF_KEEPALIVEPING
+			| WDIOF_CARDRESET;
 
-	snprintf(watchdog.ident.identity,
-		sizeof(watchdog.ident.identity), "%s watchdog",
-		fintek_wdt_names[watchdog.type]);
+	snprintf(wd->ident.identity,
+		sizeof(wd->ident.identity), "%s watchdog",
+		fintek_wdt_names[wd->type]);
 
 	err = superio_enter(sioaddr);
 	if (err)
 		return err;
-	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
+	superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT);
 
 	wdt_conf = superio_inb(sioaddr, F71808FG_REG_WDT_CONF);
 
@@ -470,19 +486,20 @@ static int fintek_wdt_probe(struct platform_device *pdev)
 	superio_outb(sioaddr, F71808FG_REG_WDT_CONF,
 		     wdt_conf | BIT(F71808FG_FLAG_WDTMOUT_STS));
 
-	wdd = &watchdog.wdd;
+	wdd = &wd->wdd;
 
-	if (fintek_wdt_is_running(wdt_conf))
+	if (fintek_wdt_is_running(wd, wdt_conf))
 		set_bit(WDOG_HW_RUNNING, &wdd->status);
 
 	superio_exit(sioaddr);
 
 	wdd->parent		= &pdev->dev;
-	wdd->info               = &watchdog.ident;
+	wdd->info               = &wd->ident;
 	wdd->ops                = &fintek_wdt_ops;
 	wdd->min_timeout        = 1;
 	wdd->max_timeout        = max_timeout;
 
+	watchdog_set_drvdata(wdd, wd);
 	watchdog_set_nowayout(wdd, nowayout);
 	watchdog_stop_on_unregister(wdd);
 	watchdog_stop_on_reboot(wdd);
@@ -496,13 +513,14 @@ static int fintek_wdt_probe(struct platform_device *pdev)
 	 * called without a set_timeout before, so it needs to be done here once
 	 */
 	fintek_wdt_set_timeout(wdd, timeout);
-	fintek_wdt_set_pulse_width(pulse_width);
+	fintek_wdt_set_pulse_width(wd, pulse_width);
 
 	return devm_watchdog_register_device(&pdev->dev, wdd);
 }
 
 static int __init fintek_wdt_find(int sioaddr)
 {
+	enum chips type;
 	u16 devid;
 	int err = superio_enter(sioaddr);
 	if (err)
@@ -518,36 +536,36 @@ static int __init fintek_wdt_find(int sioaddr)
 	devid = force_id ? force_id : superio_inw(sioaddr, SIO_REG_DEVID);
 	switch (devid) {
 	case SIO_F71808_ID:
-		watchdog.type = f71808fg;
+		type = f71808fg;
 		break;
 	case SIO_F71862_ID:
-		watchdog.type = f71862fg;
+		type = f71862fg;
 		break;
 	case SIO_F71868_ID:
-		watchdog.type = f71868;
+		type = f71868;
 		break;
 	case SIO_F71869_ID:
 	case SIO_F71869A_ID:
-		watchdog.type = f71869;
+		type = f71869;
 		break;
 	case SIO_F71882_ID:
-		watchdog.type = f71882fg;
+		type = f71882fg;
 		break;
 	case SIO_F71889_ID:
-		watchdog.type = f71889fg;
+		type = f71889fg;
 		break;
 	case SIO_F71858_ID:
 		/* Confirmed (by datasheet) not to have a watchdog. */
 		err = -ENODEV;
 		goto exit;
 	case SIO_F81803_ID:
-		watchdog.type = f81803;
+		type = f81803;
 		break;
 	case SIO_F81865_ID:
-		watchdog.type = f81865;
+		type = f81865;
 		break;
 	case SIO_F81866_ID:
-		watchdog.type = f81866;
+		type = f81866;
 		break;
 	default:
 		pr_info("Unrecognized Fintek device: %04x\n",
@@ -557,11 +575,12 @@ static int __init fintek_wdt_find(int sioaddr)
 	}
 
 	pr_info("Found %s watchdog chip, revision %d\n",
-		fintek_wdt_names[watchdog.type],
+		fintek_wdt_names[type],
 		(int)superio_inb(sioaddr, SIO_REG_DEVREV));
+
 exit:
 	superio_exit(sioaddr);
-	return err;
+	return err ? err : type;
 }
 
 static struct platform_driver fintek_wdt_driver = {
@@ -576,8 +595,9 @@ static struct platform_device *fintek_wdt_pdev;
 static int __init fintek_wdt_init(void)
 {
 	static const unsigned short addrs[] = { 0x2e, 0x4e };
+	struct fintek_wdt_pdata pdata;
 	struct resource wdt_res = {};
-	int err = -ENODEV;
+	int ret;
 	int i;
 
 	if (f71862fg_pin != 63 && f71862fg_pin != 56) {
@@ -586,12 +606,14 @@ static int __init fintek_wdt_init(void)
 	}
 
 	for (i = 0; i < ARRAY_SIZE(addrs); i++) {
-		err = fintek_wdt_find(addrs[i]);
-		if (err == 0)
+		ret = fintek_wdt_find(addrs[i]);
+		if (ret >= 0)
 			break;
 	}
 	if (i == ARRAY_SIZE(addrs))
-		return err;
+		return ret;
+
+	pdata.type = ret;
 
 	platform_driver_register(&fintek_wdt_driver);
 
@@ -600,7 +622,9 @@ static int __init fintek_wdt_init(void)
 	wdt_res.start = addrs[i];
 	wdt_res.end   = addrs[i] + 1;
 
-	fintek_wdt_pdev = platform_device_register_simple(DRVNAME, -1, &wdt_res, 1);
+	fintek_wdt_pdev = platform_device_register_resndata(NULL, DRVNAME, -1,
+							    &wdt_res, 1,
+							    &pdata, sizeof(pdata));
 	if (IS_ERR(fintek_wdt_pdev)) {
 		platform_driver_unregister(&fintek_wdt_driver);
 		return PTR_ERR(fintek_wdt_pdev);
-- 
git-series 0.9.1

Re: [PATCH v4 5/5] watchdog: f71808e_wdt: dynamically allocate watchdog driver data

[Guenter Roeck]

On Thu, Jul 22, 2021 at 12:14:44PM +0200, Ahmad Fatoum wrote:
> While the driver will only match against a single device, convention is
> to dynamically allocate the driver data.
> 
> Suggested-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/f71808e_wdt.c | 198 +++++++++++++++++++---------------
>  1 file changed, 111 insertions(+), 87 deletions(-)
> 
> diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
> index d4cae73da002..f495bf6fb0ab 100644
> --- a/drivers/watchdog/f71808e_wdt.c
> +++ b/drivers/watchdog/f71808e_wdt.c
> @@ -143,7 +143,9 @@ struct fintek_wdt {
>  	char		pulse_mode;	/* enable pulse output mode? */
>  };
>  
> -static struct fintek_wdt watchdog;
> +struct fintek_wdt_pdata {
> +	enum chips	type;
> +};
>  
>  /* Super I/O functions */
>  static inline int superio_inb(int base, int reg)
> @@ -209,13 +211,15 @@ static inline void superio_exit(int base)
>  
>  static int fintek_wdt_set_timeout(struct watchdog_device *wdd, unsigned int timeout)
>  {
> +	struct fintek_wdt *wd = watchdog_get_drvdata(wdd);
> +
>  	if (timeout > 0xff) {
> -		watchdog.timer_val = DIV_ROUND_UP(timeout, 60);
> -		watchdog.minutes_mode = true;
> -		timeout = watchdog.timer_val * 60;
> +		wd->timer_val = DIV_ROUND_UP(timeout, 60);
> +		wd->minutes_mode = true;
> +		timeout = wd->timer_val * 60;
>  	} else {
> -		watchdog.timer_val = timeout;
> -		watchdog.minutes_mode = false;
> +		wd->timer_val = timeout;
> +		wd->minutes_mode = false;
>  	}
>  
>  	wdd->timeout = timeout;
> @@ -223,63 +227,65 @@ static int fintek_wdt_set_timeout(struct watchdog_device *wdd, unsigned int time
>  	return 0;
>  }
>  
> -static int fintek_wdt_set_pulse_width(unsigned int pw)
> +static int fintek_wdt_set_pulse_width(struct fintek_wdt *wd, unsigned int pw)
>  {
>  	unsigned int t1 = 25, t2 = 125, t3 = 5000;
>  
> -	if (watchdog.type == f71868) {
> +	if (wd->type == f71868) {
>  		t1 = 30;
>  		t2 = 150;
>  		t3 = 6000;
>  	}
>  
>  	if        (pw <=  1) {
> -		watchdog.pulse_val = 0;
> +		wd->pulse_val = 0;
>  	} else if (pw <= t1) {
> -		watchdog.pulse_val = 1;
> +		wd->pulse_val = 1;
>  	} else if (pw <= t2) {
> -		watchdog.pulse_val = 2;
> +		wd->pulse_val = 2;
>  	} else if (pw <= t3) {
> -		watchdog.pulse_val = 3;
> +		wd->pulse_val = 3;
>  	} else {
>  		pr_err("pulse width out of range\n");
>  		return -EINVAL;
>  	}
>  
> -	watchdog.pulse_mode = pw;
> +	wd->pulse_mode = pw;
>  
>  	return 0;
>  }
>  
>  static int fintek_wdt_keepalive(struct watchdog_device *wdd)
>  {
> +	struct fintek_wdt *wd = watchdog_get_drvdata(wdd);
>  	int err;
>  
> -	err = superio_enter(watchdog.sioaddr);
> +	err = superio_enter(wd->sioaddr);
>  	if (err)
>  		return err;
> -	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
> +	superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT);
>  
> -	if (watchdog.minutes_mode)
> +	if (wd->minutes_mode)
>  		/* select minutes for timer units */
> -		superio_set_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
> +		superio_set_bit(wd->sioaddr, F71808FG_REG_WDT_CONF,
>  				F71808FG_FLAG_WD_UNIT);
>  	else
>  		/* select seconds for timer units */
> -		superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
> +		superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF,
>  				F71808FG_FLAG_WD_UNIT);
>  
>  	/* Set timer value */
> -	superio_outb(watchdog.sioaddr, F71808FG_REG_WD_TIME,
> -			   watchdog.timer_val);
> +	superio_outb(wd->sioaddr, F71808FG_REG_WD_TIME,
> +			   wd->timer_val);
>  
> -	superio_exit(watchdog.sioaddr);
> +	superio_exit(wd->sioaddr);
>  
>  	return 0;
>  }
>  
>  static int fintek_wdt_start(struct watchdog_device *wdd)
>  {
> +	struct fintek_wdt *wd = watchdog_get_drvdata(wdd);
>  	int err;
>  	u8 tmp;
>  
> @@ -288,57 +294,57 @@ static int fintek_wdt_start(struct watchdog_device *wdd)
>  	if (err)
>  		return err;
>  
> -	err = superio_enter(watchdog.sioaddr);
> +	err = superio_enter(wd->sioaddr);
>  	if (err)
>  		return err;
> -	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
> +	superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT);
>  
>  	/* Watchdog pin configuration */
> -	switch (watchdog.type) {
> +	switch (wd->type) {
>  	case f71808fg:
>  		/* Set pin 21 to GPIO23/WDTRST#, then to WDTRST# */
> -		superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT2, 3);
> -		superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT3, 3);
> +		superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT2, 3);
> +		superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 3);
>  		break;
>  
>  	case f71862fg:
>  		if (f71862fg_pin == 63) {
>  			/* SPI must be disabled first to use this pin! */
> -			superio_clear_bit(watchdog.sioaddr, SIO_REG_ROM_ADDR_SEL, 6);
> -			superio_set_bit(watchdog.sioaddr, SIO_REG_MFUNCT3, 4);
> +			superio_clear_bit(wd->sioaddr, SIO_REG_ROM_ADDR_SEL, 6);
> +			superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT3, 4);
>  		} else if (f71862fg_pin == 56) {
> -			superio_set_bit(watchdog.sioaddr, SIO_REG_MFUNCT1, 1);
> +			superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT1, 1);
>  		}
>  		break;
>  
>  	case f71868:
>  	case f71869:
>  		/* GPIO14 --> WDTRST# */
> -		superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT1, 4);
> +		superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT1, 4);
>  		break;
>  
>  	case f71882fg:
>  		/* Set pin 56 to WDTRST# */
> -		superio_set_bit(watchdog.sioaddr, SIO_REG_MFUNCT1, 1);
> +		superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT1, 1);
>  		break;
>  
>  	case f71889fg:
>  		/* set pin 40 to WDTRST# */
> -		superio_outb(watchdog.sioaddr, SIO_REG_MFUNCT3,
> -			superio_inb(watchdog.sioaddr, SIO_REG_MFUNCT3) & 0xcf);
> +		superio_outb(wd->sioaddr, SIO_REG_MFUNCT3,
> +			superio_inb(wd->sioaddr, SIO_REG_MFUNCT3) & 0xcf);
>  		break;
>  
>  	case f81803:
>  		/* Enable TSI Level register bank */
> -		superio_clear_bit(watchdog.sioaddr, SIO_REG_CLOCK_SEL, 3);
> +		superio_clear_bit(wd->sioaddr, SIO_REG_CLOCK_SEL, 3);
>  		/* Set pin 27 to WDTRST# */
> -		superio_outb(watchdog.sioaddr, SIO_REG_TSI_LEVEL_SEL, 0x5f &
> -			superio_inb(watchdog.sioaddr, SIO_REG_TSI_LEVEL_SEL));
> +		superio_outb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL, 0x5f &
> +			superio_inb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL));
>  		break;
>  
>  	case f81865:
>  		/* Set pin 70 to WDTRST# */
> -		superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT3, 5);
> +		superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 5);
>  		break;
>  
>  	case f81866:
> @@ -348,12 +354,12 @@ static int fintek_wdt_start(struct watchdog_device *wdd)
>  		 *     BIT5: 0 -> WDTRST#
>  		 *           1 -> GPIO15
>  		 */
> -		tmp = superio_inb(watchdog.sioaddr, SIO_F81866_REG_PORT_SEL);
> +		tmp = superio_inb(wd->sioaddr, SIO_F81866_REG_PORT_SEL);
>  		tmp &= ~(BIT(3) | BIT(0));
>  		tmp |= BIT(2);
> -		superio_outb(watchdog.sioaddr, SIO_F81866_REG_PORT_SEL, tmp);
> +		superio_outb(wd->sioaddr, SIO_F81866_REG_PORT_SEL, tmp);
>  
> -		superio_clear_bit(watchdog.sioaddr, SIO_F81866_REG_GPIO1, 5);
> +		superio_clear_bit(wd->sioaddr, SIO_F81866_REG_GPIO1, 5);
>  		break;
>  
>  	default:
> @@ -365,63 +371,64 @@ static int fintek_wdt_start(struct watchdog_device *wdd)
>  		goto exit_superio;
>  	}
>  
> -	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
> -	superio_set_bit(watchdog.sioaddr, SIO_REG_ENABLE, 0);
> +	superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT);
> +	superio_set_bit(wd->sioaddr, SIO_REG_ENABLE, 0);
>  
> -	if (watchdog.type == f81865 || watchdog.type == f81866)
> -		superio_set_bit(watchdog.sioaddr, F81865_REG_WDO_CONF,
> +	if (wd->type == f81865 || wd->type == f81866)
> +		superio_set_bit(wd->sioaddr, F81865_REG_WDO_CONF,
>  				F81865_FLAG_WDOUT_EN);
>  	else
> -		superio_set_bit(watchdog.sioaddr, F71808FG_REG_WDO_CONF,
> +		superio_set_bit(wd->sioaddr, F71808FG_REG_WDO_CONF,
>  				F71808FG_FLAG_WDOUT_EN);
>  
> -	superio_set_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
> +	superio_set_bit(wd->sioaddr, F71808FG_REG_WDT_CONF,
>  			F71808FG_FLAG_WD_EN);
>  
> -	if (watchdog.pulse_mode) {
> +	if (wd->pulse_mode) {
>  		/* Select "pulse" output mode with given duration */
> -		u8 wdt_conf = superio_inb(watchdog.sioaddr,
> +		u8 wdt_conf = superio_inb(wd->sioaddr,
>  				F71808FG_REG_WDT_CONF);
>  
>  		/* Set WD_PSWIDTH bits (1:0) */
> -		wdt_conf = (wdt_conf & 0xfc) | (watchdog.pulse_val & 0x03);
> +		wdt_conf = (wdt_conf & 0xfc) | (wd->pulse_val & 0x03);
>  		/* Set WD_PULSE to "pulse" mode */
>  		wdt_conf |= BIT(F71808FG_FLAG_WD_PULSE);
>  
> -		superio_outb(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
> +		superio_outb(wd->sioaddr, F71808FG_REG_WDT_CONF,
>  				wdt_conf);
>  	} else {
>  		/* Select "level" output mode */
> -		superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
> +		superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF,
>  				F71808FG_FLAG_WD_PULSE);
>  	}
>  
>  exit_superio:
> -	superio_exit(watchdog.sioaddr);
> +	superio_exit(wd->sioaddr);
>  
>  	return err;
>  }
>  
>  static int fintek_wdt_stop(struct watchdog_device *wdd)
>  {
> +	struct fintek_wdt *wd = watchdog_get_drvdata(wdd);
>  	int err;
>  
> -	err = superio_enter(watchdog.sioaddr);
> +	err = superio_enter(wd->sioaddr);
>  	if (err)
>  		return err;
> -	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
> +	superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT);
>  
> -	superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
> +	superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF,
>  			F71808FG_FLAG_WD_EN);
>  
> -	superio_exit(watchdog.sioaddr);
> +	superio_exit(wd->sioaddr);
>  
>  	return 0;
>  }
>  
> -static bool fintek_wdt_is_running(u8 wdt_conf)
> +static bool fintek_wdt_is_running(struct fintek_wdt *wd, u8 wdt_conf)
>  {
> -	return (superio_inb(watchdog.sioaddr, SIO_REG_ENABLE) & BIT(0))
> +	return (superio_inb(wd->sioaddr, SIO_REG_ENABLE) & BIT(0))
>  		&& (wdt_conf & BIT(F71808FG_FLAG_WD_EN));
>  }
>  
> @@ -435,7 +442,9 @@ static const struct watchdog_ops fintek_wdt_ops = {
>  
>  static int fintek_wdt_probe(struct platform_device *pdev)
>  {
> +	struct fintek_wdt_pdata *pdata;
>  	struct watchdog_device *wdd;
> +	struct fintek_wdt *wd;
>  	int wdt_conf, err = 0;
>  	struct resource *res;
>  	int sioaddr;
> @@ -446,20 +455,27 @@ static int fintek_wdt_probe(struct platform_device *pdev)
>  
>  	sioaddr = res->start;
>  
> -	watchdog.sioaddr = sioaddr;
> -	watchdog.ident.options = WDIOF_SETTIMEOUT
> -				| WDIOF_MAGICCLOSE
> -				| WDIOF_KEEPALIVEPING
> -				| WDIOF_CARDRESET;
> +	wd = devm_kzalloc(&pdev->dev, sizeof(*wd), GFP_KERNEL);
> +	if (!wd)
> +		return -ENOMEM;
> +
> +	pdata = pdev->dev.platform_data;
> +
> +	wd->type = pdata->type;
> +	wd->sioaddr = sioaddr;
> +	wd->ident.options = WDIOF_SETTIMEOUT
> +			| WDIOF_MAGICCLOSE
> +			| WDIOF_KEEPALIVEPING
> +			| WDIOF_CARDRESET;
>  
> -	snprintf(watchdog.ident.identity,
> -		sizeof(watchdog.ident.identity), "%s watchdog",
> -		fintek_wdt_names[watchdog.type]);
> +	snprintf(wd->ident.identity,
> +		sizeof(wd->ident.identity), "%s watchdog",
> +		fintek_wdt_names[wd->type]);
>  
>  	err = superio_enter(sioaddr);
>  	if (err)
>  		return err;
> -	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
> +	superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT);
>  
>  	wdt_conf = superio_inb(sioaddr, F71808FG_REG_WDT_CONF);
>  
> @@ -470,19 +486,20 @@ static int fintek_wdt_probe(struct platform_device *pdev)
>  	superio_outb(sioaddr, F71808FG_REG_WDT_CONF,
>  		     wdt_conf | BIT(F71808FG_FLAG_WDTMOUT_STS));
>  
> -	wdd = &watchdog.wdd;
> +	wdd = &wd->wdd;
>  
> -	if (fintek_wdt_is_running(wdt_conf))
> +	if (fintek_wdt_is_running(wd, wdt_conf))
>  		set_bit(WDOG_HW_RUNNING, &wdd->status);
>  
>  	superio_exit(sioaddr);
>  
>  	wdd->parent		= &pdev->dev;
> -	wdd->info               = &watchdog.ident;
> +	wdd->info               = &wd->ident;
>  	wdd->ops                = &fintek_wdt_ops;
>  	wdd->min_timeout        = 1;
>  	wdd->max_timeout        = max_timeout;
>  
> +	watchdog_set_drvdata(wdd, wd);
>  	watchdog_set_nowayout(wdd, nowayout);
>  	watchdog_stop_on_unregister(wdd);
>  	watchdog_stop_on_reboot(wdd);
> @@ -496,13 +513,14 @@ static int fintek_wdt_probe(struct platform_device *pdev)
>  	 * called without a set_timeout before, so it needs to be done here once
>  	 */
>  	fintek_wdt_set_timeout(wdd, timeout);
> -	fintek_wdt_set_pulse_width(pulse_width);
> +	fintek_wdt_set_pulse_width(wd, pulse_width);
>  
>  	return devm_watchdog_register_device(&pdev->dev, wdd);
>  }
>  
>  static int __init fintek_wdt_find(int sioaddr)
>  {
> +	enum chips type;
>  	u16 devid;
>  	int err = superio_enter(sioaddr);
>  	if (err)
> @@ -518,36 +536,36 @@ static int __init fintek_wdt_find(int sioaddr)
>  	devid = force_id ? force_id : superio_inw(sioaddr, SIO_REG_DEVID);
>  	switch (devid) {
>  	case SIO_F71808_ID:
> -		watchdog.type = f71808fg;
> +		type = f71808fg;
>  		break;
>  	case SIO_F71862_ID:
> -		watchdog.type = f71862fg;
> +		type = f71862fg;
>  		break;
>  	case SIO_F71868_ID:
> -		watchdog.type = f71868;
> +		type = f71868;
>  		break;
>  	case SIO_F71869_ID:
>  	case SIO_F71869A_ID:
> -		watchdog.type = f71869;
> +		type = f71869;
>  		break;
>  	case SIO_F71882_ID:
> -		watchdog.type = f71882fg;
> +		type = f71882fg;
>  		break;
>  	case SIO_F71889_ID:
> -		watchdog.type = f71889fg;
> +		type = f71889fg;
>  		break;
>  	case SIO_F71858_ID:
>  		/* Confirmed (by datasheet) not to have a watchdog. */
>  		err = -ENODEV;
>  		goto exit;
>  	case SIO_F81803_ID:
> -		watchdog.type = f81803;
> +		type = f81803;
>  		break;
>  	case SIO_F81865_ID:
> -		watchdog.type = f81865;
> +		type = f81865;
>  		break;
>  	case SIO_F81866_ID:
> -		watchdog.type = f81866;
> +		type = f81866;
>  		break;
>  	default:
>  		pr_info("Unrecognized Fintek device: %04x\n",
> @@ -557,11 +575,12 @@ static int __init fintek_wdt_find(int sioaddr)
>  	}
>  
>  	pr_info("Found %s watchdog chip, revision %d\n",
> -		fintek_wdt_names[watchdog.type],
> +		fintek_wdt_names[type],
>  		(int)superio_inb(sioaddr, SIO_REG_DEVREV));
> +
>  exit:
>  	superio_exit(sioaddr);
> -	return err;
> +	return err ? err : type;
>  }
>  
>  static struct platform_driver fintek_wdt_driver = {
> @@ -576,8 +595,9 @@ static struct platform_device *fintek_wdt_pdev;
>  static int __init fintek_wdt_init(void)
>  {
>  	static const unsigned short addrs[] = { 0x2e, 0x4e };
> +	struct fintek_wdt_pdata pdata;
>  	struct resource wdt_res = {};
> -	int err = -ENODEV;
> +	int ret;
>  	int i;
>  
>  	if (f71862fg_pin != 63 && f71862fg_pin != 56) {
> @@ -586,12 +606,14 @@ static int __init fintek_wdt_init(void)
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(addrs); i++) {
> -		err = fintek_wdt_find(addrs[i]);
> -		if (err == 0)
> +		ret = fintek_wdt_find(addrs[i]);
> +		if (ret >= 0)
>  			break;
>  	}
>  	if (i == ARRAY_SIZE(addrs))
> -		return err;
> +		return ret;
> +
> +	pdata.type = ret;
>  
>  	platform_driver_register(&fintek_wdt_driver);
>  
> @@ -600,7 +622,9 @@ static int __init fintek_wdt_init(void)
>  	wdt_res.start = addrs[i];
>  	wdt_res.end   = addrs[i] + 1;
>  
> -	fintek_wdt_pdev = platform_device_register_simple(DRVNAME, -1, &wdt_res, 1);
> +	fintek_wdt_pdev = platform_device_register_resndata(NULL, DRVNAME, -1,
> +							    &wdt_res, 1,
> +							    &pdata, sizeof(pdata));
>  	if (IS_ERR(fintek_wdt_pdev)) {
>  		platform_driver_unregister(&fintek_wdt_driver);
>  		return PTR_ERR(fintek_wdt_pdev);
> -- 
> git-series 0.9.1