[PATCH v1 0/8] watchdog: f71808e_wdt: migrate to kernel

[PATCH v1 0/8] watchdog: f71808e_wdt: migrate to kernel

[Ahmad Fatoum]

Hello,

this series fixes some issues I ran into with the status UAPI of the driver and
then migrates it over to the kernel watchdog API.

I tested it on a f81866.

Cheers,
Ahmad Fatoum (8):
  docs: watchdog: codify ident.options as superset of possible status
    flags
  watchdog: f71808e_wdt: indicate WDIOF_CARDRESET support in
    watchdog_info.options
  watchdog: f71808e_wdt: remove use of wrong watchdog_info option
  watchdog: f71808e_wdt: clear watchdog timeout occurred flag
  watchdog: f71808e_wdt: do stricter parameter validation
  watchdog: f71808e_wdt: consolidate variant handling into single array
  watchdog: f71808e_wdt: migrate to new kernel watchdog API
  watchdog: f71808e_wdt: rename variant-independent identifiers
    appropriately

 Documentation/watchdog/watchdog-api.rst |   2 +-
 drivers/watchdog/Kconfig                |   1 +
 drivers/watchdog/f71808e_wdt.c          | 784 ++++++++----------------
 3 files changed, 242 insertions(+), 545 deletions(-)

-- 
2.27.0

[PATCH v1 1/8] docs: watchdog: codify ident.options as superset of possible status flags

[Ahmad Fatoum]

The FIXME comment has been in-tree since the very first git commit.
The described behavior has been since relied on by some userspace, e.g.
the util-linux wdctl command and has been ignored by some kernelspace,
like the f71808e_wdt driver.

The functionality is useful to have to be able to differentiate between a
driver that doesn't support WDIOF_CARDRESET and one that does, but hasn't
had a watchdog reset, thus drop the FIXME to encourage drivers adopting
this convention.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 Documentation/watchdog/watchdog-api.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/watchdog/watchdog-api.rst b/Documentation/watchdog/watchdog-api.rst
index c6c1e9fa9f73..800dcd7586f2 100644
--- a/Documentation/watchdog/watchdog-api.rst
+++ b/Documentation/watchdog/watchdog-api.rst
@@ -168,7 +168,7 @@ the fields returned in the ident struct are:
 
 the options field can have the following bits set, and describes what
 kind of information that the GET_STATUS and GET_BOOT_STATUS ioctls can
-return.   [FIXME -- Is this correct?]
+return.
 
 	================	=========================
 	WDIOF_OVERHEAT		Reset due to CPU overheat
-- 
2.27.0

Re: [PATCH v1 1/8] docs: watchdog: codify ident.options as superset of possible status flags

[Guenter Roeck]

On Thu, Jun 11, 2020 at 09:17:42PM +0200, Ahmad Fatoum wrote:
> The FIXME comment has been in-tree since the very first git commit.
> The described behavior has been since relied on by some userspace, e.g.
> the util-linux wdctl command and has been ignored by some kernelspace,
> like the f71808e_wdt driver.
> 
> The functionality is useful to have to be able to differentiate between a
> driver that doesn't support WDIOF_CARDRESET and one that does, but hasn't
> had a watchdog reset, thus drop the FIXME to encourage drivers adopting
> this convention.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

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

> ---
>  Documentation/watchdog/watchdog-api.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/watchdog/watchdog-api.rst b/Documentation/watchdog/watchdog-api.rst
> index c6c1e9fa9f73..800dcd7586f2 100644
> --- a/Documentation/watchdog/watchdog-api.rst
> +++ b/Documentation/watchdog/watchdog-api.rst
> @@ -168,7 +168,7 @@ the fields returned in the ident struct are:
>  
>  the options field can have the following bits set, and describes what
>  kind of information that the GET_STATUS and GET_BOOT_STATUS ioctls can
> -return.   [FIXME -- Is this correct?]
> +return.
>  
>  	================	=========================
>  	WDIOF_OVERHEAT		Reset due to CPU overheat

[PATCH v1 2/8] watchdog: f71808e_wdt: indicate WDIOF_CARDRESET support in watchdog_info.options

[Ahmad Fatoum]

The driver supports populating bootstatus with WDIOF_CARDRESET, but so
far userspace couldn't portably determine whether absence of this flag
meant no watchdog reset or no driver support. Or-in the bit to fix this.

Fixes: b97cb21a4634 ("watchdog: f71808e_wdt: Fix WDTMOUT_STS register read")
Cc: stable@vger.kernel.org
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/f71808e_wdt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index a3c44d75d80e..c8ce80c13403 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -692,7 +692,8 @@ static int __init watchdog_init(int sioaddr)
 	watchdog.sioaddr = sioaddr;
 	watchdog.ident.options = WDIOC_SETTIMEOUT
 				| WDIOF_MAGICCLOSE
-				| WDIOF_KEEPALIVEPING;
+				| WDIOF_KEEPALIVEPING
+				| WDIOF_CARDRESET;
 
 	snprintf(watchdog.ident.identity,
 		sizeof(watchdog.ident.identity), "%s watchdog",
-- 
2.27.0

Re: [PATCH v1 2/8] watchdog: f71808e_wdt: indicate WDIOF_CARDRESET support in watchdog_info.options

[Guenter Roeck]

On Thu, Jun 11, 2020 at 09:17:43PM +0200, Ahmad Fatoum wrote:
> The driver supports populating bootstatus with WDIOF_CARDRESET, but so
> far userspace couldn't portably determine whether absence of this flag
> meant no watchdog reset or no driver support. Or-in the bit to fix this.
> 
> Fixes: b97cb21a4634 ("watchdog: f71808e_wdt: Fix WDTMOUT_STS register read")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

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

> ---
>  drivers/watchdog/f71808e_wdt.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
> index a3c44d75d80e..c8ce80c13403 100644
> --- a/drivers/watchdog/f71808e_wdt.c
> +++ b/drivers/watchdog/f71808e_wdt.c
> @@ -692,7 +692,8 @@ static int __init watchdog_init(int sioaddr)
>  	watchdog.sioaddr = sioaddr;
>  	watchdog.ident.options = WDIOC_SETTIMEOUT
>  				| WDIOF_MAGICCLOSE
> -				| WDIOF_KEEPALIVEPING;
> +				| WDIOF_KEEPALIVEPING
> +				| WDIOF_CARDRESET;
>  
>  	snprintf(watchdog.ident.identity,
>  		sizeof(watchdog.ident.identity), "%s watchdog",

[PATCH v1 3/8] watchdog: f71808e_wdt: remove use of wrong watchdog_info option

[Ahmad Fatoum]

The flags that should be or-ed into the watchdog_info.options by drivers
all start with WDIOF_, e.g. WDIOF_SETTIMEOUT, which indicates that the
driver's watchdog_ops has a usable set_timeout.

WDIOC_SETTIMEOUT was used instead, which expands to 0xc0045706, which
equals:

   WDIOF_FANFAULT | WDIOF_EXTERN1 | WDIOF_PRETIMEOUT | WDIOF_ALARMONLY |
   WDIOF_MAGICCLOSE | 0xc0045000

These were so far indicated to userspace on WDIOC_GETSUPPORT.
As the driver has not yet been migrated to the new watchdog kernel API,
the constant can just be dropped without substitute.

Fixes: 96cb4eb019ce ("watchdog: f71808e_wdt: new watchdog driver for
       Fintek F71808E and F71882FG")
Cc: stable@vger.kernel.org
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/f71808e_wdt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index c8ce80c13403..8e5584c54423 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -690,8 +690,7 @@ static int __init watchdog_init(int sioaddr)
 	 * into the module have been registered yet.
 	 */
 	watchdog.sioaddr = sioaddr;
-	watchdog.ident.options = WDIOC_SETTIMEOUT
-				| WDIOF_MAGICCLOSE
+	watchdog.ident.options = WDIOF_MAGICCLOSE
 				| WDIOF_KEEPALIVEPING
 				| WDIOF_CARDRESET;
 
-- 
2.27.0

Re: [PATCH v1 3/8] watchdog: f71808e_wdt: remove use of wrong watchdog_info option

[Guenter Roeck]

On Thu, Jun 11, 2020 at 09:17:44PM +0200, Ahmad Fatoum wrote:
> The flags that should be or-ed into the watchdog_info.options by drivers
> all start with WDIOF_, e.g. WDIOF_SETTIMEOUT, which indicates that the
> driver's watchdog_ops has a usable set_timeout.
> 
> WDIOC_SETTIMEOUT was used instead, which expands to 0xc0045706, which
> equals:
> 
>    WDIOF_FANFAULT | WDIOF_EXTERN1 | WDIOF_PRETIMEOUT | WDIOF_ALARMONLY |
>    WDIOF_MAGICCLOSE | 0xc0045000
> 
> These were so far indicated to userspace on WDIOC_GETSUPPORT.
> As the driver has not yet been migrated to the new watchdog kernel API,
> the constant can just be dropped without substitute.
> 
> Fixes: 96cb4eb019ce ("watchdog: f71808e_wdt: new watchdog driver for
>        Fintek F71808E and F71882FG")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

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

> ---
>  drivers/watchdog/f71808e_wdt.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
> index c8ce80c13403..8e5584c54423 100644
> --- a/drivers/watchdog/f71808e_wdt.c
> +++ b/drivers/watchdog/f71808e_wdt.c
> @@ -690,8 +690,7 @@ static int __init watchdog_init(int sioaddr)
>  	 * into the module have been registered yet.
>  	 */
>  	watchdog.sioaddr = sioaddr;
> -	watchdog.ident.options = WDIOC_SETTIMEOUT
> -				| WDIOF_MAGICCLOSE
> +	watchdog.ident.options = WDIOF_MAGICCLOSE
>  				| WDIOF_KEEPALIVEPING
>  				| WDIOF_CARDRESET;
>  

[PATCH v1 4/8] watchdog: f71808e_wdt: clear watchdog timeout occurred flag

[Ahmad Fatoum]

The flag indicating a watchdog timeout having occurred normally persists
till Power-On Reset of the Fintek Super I/O chip. The user can clear it
by writing a `1' to the bit.

The driver doesn't offer a restart method, so regular system reboot
might not reset the Super I/O and if the watchdog isn't enabled, we
won't touch the register containing the bit on the next boot.
In this case all subsequent regular reboots will be wrongly flagged
by the driver as being caused by the watchdog.

Fix this by having the flag cleared after read. This is also done by
other drivers like those for the i6300esb and mpc8xxx_wdt.

Fixes: b97cb21a4634 ("watchdog: f71808e_wdt: Fix WDTMOUT_STS register read")
Cc: stable@vger.kernel.org
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/f71808e_wdt.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index 8e5584c54423..26bf366aebc2 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -706,6 +706,13 @@ static int __init watchdog_init(int sioaddr)
 	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.
+	 * Write 1 to the bit to clear it to zero.
+	 */
+	superio_outb(sioaddr, F71808FG_REG_WDT_CONF,
+		     wdt_conf | BIT(F71808FG_FLAG_WDTMOUT_STS));
+
 	superio_exit(sioaddr);
 
 	err = watchdog_set_timeout(timeout);
-- 
2.27.0

Re: [PATCH v1 4/8] watchdog: f71808e_wdt: clear watchdog timeout occurred flag

[Guenter Roeck]

On Thu, Jun 11, 2020 at 09:17:45PM +0200, Ahmad Fatoum wrote:
> The flag indicating a watchdog timeout having occurred normally persists
> till Power-On Reset of the Fintek Super I/O chip. The user can clear it
> by writing a `1' to the bit.
> 
> The driver doesn't offer a restart method, so regular system reboot
> might not reset the Super I/O and if the watchdog isn't enabled, we
> won't touch the register containing the bit on the next boot.
> In this case all subsequent regular reboots will be wrongly flagged
> by the driver as being caused by the watchdog.
> 
> Fix this by having the flag cleared after read. This is also done by
> other drivers like those for the i6300esb and mpc8xxx_wdt.
> 
> Fixes: b97cb21a4634 ("watchdog: f71808e_wdt: Fix WDTMOUT_STS register read")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

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

> ---
>  drivers/watchdog/f71808e_wdt.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
> index 8e5584c54423..26bf366aebc2 100644
> --- a/drivers/watchdog/f71808e_wdt.c
> +++ b/drivers/watchdog/f71808e_wdt.c
> @@ -706,6 +706,13 @@ static int __init watchdog_init(int sioaddr)
>  	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.
> +	 * Write 1 to the bit to clear it to zero.
> +	 */
> +	superio_outb(sioaddr, F71808FG_REG_WDT_CONF,
> +		     wdt_conf | BIT(F71808FG_FLAG_WDTMOUT_STS));
> +
>  	superio_exit(sioaddr);
>  
>  	err = watchdog_set_timeout(timeout);

[PATCH v1 5/8] watchdog: f71808e_wdt: do stricter parameter validation

[Ahmad Fatoum]

We check the f71862fg_pin module parameter every time a watchdog device
for the f71862fg is opened, but the parameter can't change at runtime.

If we move the check to the start of init:

  - We catch userspace passing invalid, but unused, values
  - We check the condition only once
  - We simplify the code

Do so.

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

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index 26bf366aebc2..9f7823819ed1 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -306,27 +306,6 @@ static int watchdog_keepalive(void)
 	return err;
 }
 
-static int f71862fg_pin_configure(unsigned short ioaddr)
-{
-	/* When ioaddr is non-zero the calling function has to take care of
-	   mutex handling and superio preparation! */
-
-	if (f71862fg_pin == 63) {
-		if (ioaddr) {
-			/* SPI must be disabled first to use this pin! */
-			superio_clear_bit(ioaddr, SIO_REG_ROM_ADDR_SEL, 6);
-			superio_set_bit(ioaddr, SIO_REG_MFUNCT3, 4);
-		}
-	} else if (f71862fg_pin == 56) {
-		if (ioaddr)
-			superio_set_bit(ioaddr, SIO_REG_MFUNCT1, 1);
-	} else {
-		pr_err("Invalid argument f71862fg_pin=%d\n", f71862fg_pin);
-		return -EINVAL;
-	}
-	return 0;
-}
-
 static int watchdog_start(void)
 {
 	int err;
@@ -352,9 +331,13 @@ static int watchdog_start(void)
 		break;
 
 	case f71862fg:
-		err = f71862fg_pin_configure(watchdog.sioaddr);
-		if (err)
-			goto exit_superio;
+		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);
+		} else if (f71862fg_pin == 56) {
+			superio_set_bit(watchdog.sioaddr, SIO_REG_MFUNCT1, 1);
+		}
 		break;
 
 	case f71868:
@@ -810,7 +793,6 @@ static int __init f71808e_find(int sioaddr)
 		break;
 	case SIO_F71862_ID:
 		watchdog.type = f71862fg;
-		err = f71862fg_pin_configure(0); /* validate module parameter */
 		break;
 	case SIO_F71868_ID:
 		watchdog.type = f71868;
@@ -859,6 +841,11 @@ static int __init f71808e_init(void)
 	int err = -ENODEV;
 	int i;
 
+	if (f71862fg_pin != 63 && f71862fg_pin != 56) {
+		pr_err("Invalid argument f71862fg_pin=%d\n", f71862fg_pin);
+		return -EINVAL;
+	}
+
 	for (i = 0; i < ARRAY_SIZE(addrs); i++) {
 		err = f71808e_find(addrs[i]);
 		if (err == 0)
-- 
2.27.0

Re: [PATCH v1 5/8] watchdog: f71808e_wdt: do stricter parameter validation

[Guenter Roeck]

On Thu, Jun 11, 2020 at 09:17:46PM +0200, Ahmad Fatoum wrote:
> We check the f71862fg_pin module parameter every time a watchdog device
> for the f71862fg is opened, but the parameter can't change at runtime.
> 
> If we move the check to the start of init:
> 
>   - We catch userspace passing invalid, but unused, values
>   - We check the condition only once
>   - We simplify the code
> 
> Do so.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

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

> ---
>  drivers/watchdog/f71808e_wdt.c | 37 +++++++++++-----------------------
>  1 file changed, 12 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
> index 26bf366aebc2..9f7823819ed1 100644
> --- a/drivers/watchdog/f71808e_wdt.c
> +++ b/drivers/watchdog/f71808e_wdt.c
> @@ -306,27 +306,6 @@ static int watchdog_keepalive(void)
>  	return err;
>  }
>  
> -static int f71862fg_pin_configure(unsigned short ioaddr)
> -{
> -	/* When ioaddr is non-zero the calling function has to take care of
> -	   mutex handling and superio preparation! */
> -
> -	if (f71862fg_pin == 63) {
> -		if (ioaddr) {
> -			/* SPI must be disabled first to use this pin! */
> -			superio_clear_bit(ioaddr, SIO_REG_ROM_ADDR_SEL, 6);
> -			superio_set_bit(ioaddr, SIO_REG_MFUNCT3, 4);
> -		}
> -	} else if (f71862fg_pin == 56) {
> -		if (ioaddr)
> -			superio_set_bit(ioaddr, SIO_REG_MFUNCT1, 1);
> -	} else {
> -		pr_err("Invalid argument f71862fg_pin=%d\n", f71862fg_pin);
> -		return -EINVAL;
> -	}
> -	return 0;
> -}
> -
>  static int watchdog_start(void)
>  {
>  	int err;
> @@ -352,9 +331,13 @@ static int watchdog_start(void)
>  		break;
>  
>  	case f71862fg:
> -		err = f71862fg_pin_configure(watchdog.sioaddr);
> -		if (err)
> -			goto exit_superio;
> +		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);
> +		} else if (f71862fg_pin == 56) {
> +			superio_set_bit(watchdog.sioaddr, SIO_REG_MFUNCT1, 1);
> +		}
>  		break;
>  
>  	case f71868:
> @@ -810,7 +793,6 @@ static int __init f71808e_find(int sioaddr)
>  		break;
>  	case SIO_F71862_ID:
>  		watchdog.type = f71862fg;
> -		err = f71862fg_pin_configure(0); /* validate module parameter */
>  		break;
>  	case SIO_F71868_ID:
>  		watchdog.type = f71868;
> @@ -859,6 +841,11 @@ static int __init f71808e_init(void)
>  	int err = -ENODEV;
>  	int i;
>  
> +	if (f71862fg_pin != 63 && f71862fg_pin != 56) {
> +		pr_err("Invalid argument f71862fg_pin=%d\n", f71862fg_pin);
> +		return -EINVAL;
> +	}
> +
>  	for (i = 0; i < ARRAY_SIZE(addrs); i++) {
>  		err = f71808e_find(addrs[i]);
>  		if (err == 0)

[PATCH v1 6/8] watchdog: f71808e_wdt: consolidate variant handling into single array

[Ahmad Fatoum]

Driver has two big switches: one to map hardware ID to an enum constant
one more to map the enum constant to the variant's pin configuration
routine.

Drop the enum and give every variant an array entry identifying it.
This arguably simplifies the code by making it a little more compact.

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

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index 9f7823819ed1..9299ad4d4a52 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -110,22 +110,6 @@ module_param(start_withtimeout, uint, 0);
 MODULE_PARM_DESC(start_withtimeout, "Start watchdog timer on module load with"
 	" given initial timeout. Zero (default) disables this feature.");
 
-enum chips { f71808fg, f71858fg, f71862fg, f71868, f71869, f71882fg, f71889fg,
-	     f81803, f81865, f81866};
-
-static const char *f71808e_names[] = {
-	"f71808fg",
-	"f71858fg",
-	"f71862fg",
-	"f71868",
-	"f71869",
-	"f71882fg",
-	"f71889fg",
-	"f81803",
-	"f81865",
-	"f81866",
-};
-
 /* Super-I/O Function prototypes */
 static inline int superio_inb(int base, int reg);
 static inline int superio_inw(int base, int reg);
@@ -136,9 +120,17 @@ 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 f71808e_variant {
+	u16 id;
+	const char *wdt_name; /* NULL if chip lacks watchdog timer */
+	void (*pinconf)(struct watchdog_data *wd);
+};
+
 struct watchdog_data {
 	unsigned short	sioaddr;
-	enum chips	type;
+	const struct f71808e_variant *variant;
 	unsigned long	opened;
 	struct mutex	lock;
 	char		expect_close;
@@ -156,6 +148,12 @@ static struct watchdog_data watchdog = {
 	.lock = __MUTEX_INITIALIZER(watchdog.lock),
 };
 
+static inline bool has_f81865_wdo_conf(struct watchdog_data *wd)
+{
+	return wd->variant->id == SIO_F81865_ID
+		|| wd->variant->id == SIO_F81866_ID;
+}
+
 /* Super I/O functions */
 static inline int superio_inb(int base, int reg)
 {
@@ -247,7 +245,7 @@ static int watchdog_set_pulse_width(unsigned int pw)
 	int err = 0;
 	unsigned int t1 = 25, t2 = 125, t3 = 5000;
 
-	if (watchdog.type == f71868) {
+	if (watchdog.variant->id == SIO_F71868_ID) {
 		t1 = 30;
 		t2 = 150;
 		t3 = 6000;
@@ -309,7 +307,6 @@ static int watchdog_keepalive(void)
 static int watchdog_start(void)
 {
 	int err;
-	u8 tmp;
 
 	/* Make sure we don't die as soon as the watchdog is enabled below */
 	err = watchdog_keepalive();
@@ -323,81 +320,12 @@ static int watchdog_start(void)
 	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
 
 	/* Watchdog pin configuration */
-	switch (watchdog.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);
-		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);
-		} else if (f71862fg_pin == 56) {
-			superio_set_bit(watchdog.sioaddr, SIO_REG_MFUNCT1, 1);
-		}
-		break;
-
-	case f71868:
-	case f71869:
-		/* GPIO14 --> WDTRST# */
-		superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT1, 4);
-		break;
-
-	case f71882fg:
-		/* Set pin 56 to WDTRST# */
-		superio_set_bit(watchdog.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);
-		break;
-
-	case f81803:
-		/* Enable TSI Level register bank */
-		superio_clear_bit(watchdog.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));
-		break;
-
-	case f81865:
-		/* Set pin 70 to WDTRST# */
-		superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT3, 5);
-		break;
-
-	case f81866:
-		/*
-		 * GPIO1 Control Register when 27h BIT3:2 = 01 & BIT0 = 0.
-		 * The PIN 70(GPIO15/WDTRST) is controlled by 2Ch:
-		 *     BIT5: 0 -> WDTRST#
-		 *           1 -> GPIO15
-		 */
-		tmp = superio_inb(watchdog.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_clear_bit(watchdog.sioaddr, SIO_F81866_REG_GPIO1, 5);
-		break;
-
-	default:
-		/*
-		 * 'default' label to shut up the compiler and catch
-		 * programmer errors
-		 */
-		err = -ENODEV;
-		goto exit_superio;
-	}
+	watchdog.variant->pinconf(&watchdog);
 
 	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
 	superio_set_bit(watchdog.sioaddr, SIO_REG_ENABLE, 0);
 
-	if (watchdog.type == f81865 || watchdog.type == f81866)
+	if (has_f81865_wdo_conf(&watchdog))
 		superio_set_bit(watchdog.sioaddr, F81865_REG_WDO_CONF,
 				F81865_FLAG_WDOUT_EN);
 	else
@@ -425,7 +353,6 @@ static int watchdog_start(void)
 				F71808FG_FLAG_WD_PULSE);
 	}
 
-exit_superio:
 	superio_exit(watchdog.sioaddr);
 exit_unlock:
 	mutex_unlock(&watchdog.lock);
@@ -679,7 +606,7 @@ static int __init watchdog_init(int sioaddr)
 
 	snprintf(watchdog.ident.identity,
 		sizeof(watchdog.ident.identity), "%s watchdog",
-		f71808e_names[watchdog.type]);
+		watchdog.variant->wdt_name);
 
 	err = superio_enter(sioaddr);
 	if (err)
@@ -772,73 +699,123 @@ static int __init watchdog_init(int sioaddr)
 	return err;
 }
 
-static int __init f71808e_find(int sioaddr)
+static void f71808fg_pinconf(struct watchdog_data *wd)
+{
+	/* Set pin 21 to GPIO23/WDTRST#, then to WDTRST# */
+	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT2, 3);
+	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 3);
+}
+static void f71862fg_pinconf(struct watchdog_data *wd)
+{
+	if (f71862fg_pin == 63) {
+		/* SPI must be disabled first to use this pin! */
+		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(wd->sioaddr, SIO_REG_MFUNCT1, 1);
+	}
+}
+static void f71868_pinconf(struct watchdog_data *wd)
+{
+	/* GPIO14 --> WDTRST# */
+	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT1, 4);
+}
+static void f71882fg_pinconf(struct watchdog_data *wd)
+{
+	/* Set pin 56 to WDTRST# */
+	superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT1, 1);
+}
+static void f71889fg_pinconf(struct watchdog_data *wd)
+{
+	/* set pin 40 to WDTRST# */
+	superio_outb(wd->sioaddr, SIO_REG_MFUNCT3,
+		     superio_inb(wd->sioaddr, SIO_REG_MFUNCT3) & 0xcf);
+}
+static void f81803_pinconf(struct watchdog_data *wd)
+{
+	/* Enable TSI Level register bank */
+	superio_clear_bit(wd->sioaddr, SIO_REG_CLOCK_SEL, 3);
+	/* Set pin 27 to WDTRST# */
+	superio_outb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL, 0x5f &
+		     superio_inb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL));
+}
+static void f81865_pinconf(struct watchdog_data *wd)
+{
+	/* Set pin 70 to WDTRST# */
+	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 5);
+}
+static void f81866_pinconf(struct watchdog_data *wd)
+{
+	/*
+	 * GPIO1 Control Register when 27h BIT3:2 = 01 & BIT0 = 0.
+	 * The PIN 70(GPIO15/WDTRST) is controlled by 2Ch:
+	 *     BIT5: 0 -> WDTRST#
+	 *           1 -> GPIO15
+	 */
+	u8 tmp = superio_inb(wd->sioaddr, SIO_F81866_REG_PORT_SEL);
+	tmp &= ~(BIT(3) | BIT(0));
+	tmp |= BIT(2);
+	superio_outb(wd->sioaddr, SIO_F81866_REG_PORT_SEL, tmp);
+
+	superio_clear_bit(wd->sioaddr, SIO_F81866_REG_GPIO1, 5);
+}
+
+struct f71808e_variant f71808e_variants[] = {
+	{ SIO_F71808_ID,  "f71808fg", f71808fg_pinconf },
+	{ SIO_F71862_ID,  "f71862fg", f71862fg_pinconf },
+	{ SIO_F71868_ID,  "f71868",   f71868_pinconf },
+	{ SIO_F71869_ID,  "f71869",   f71868_pinconf },
+	{ SIO_F71869A_ID, "f71869",   f71868_pinconf },
+	{ SIO_F71882_ID,  "f71882fg", f71882fg_pinconf },
+	{ SIO_F71889_ID,  "f71889fg", f71889fg_pinconf },
+	{ SIO_F81803_ID,  "f81803",   f81803_pinconf },
+	{ SIO_F81865_ID,  "f81865",   f81865_pinconf },
+	{ SIO_F81866_ID,  "f81866",   f81866_pinconf },
+	/* Confirmed (by datasheet) not to have a watchdog. */
+	{ SIO_F71858_ID,  NULL,       NULL },
+	{ /* sentinel */ }
+};
+
+static struct f71808e_variant __init *f71808e_find(int sioaddr)
 {
+	struct f71808e_variant *variant;
 	u16 devid;
 	int err = superio_enter(sioaddr);
 	if (err)
-		return err;
+		return ERR_PTR(err);
 
 	devid = superio_inw(sioaddr, SIO_REG_MANID);
 	if (devid != SIO_FINTEK_ID) {
 		pr_debug("Not a Fintek device\n");
-		err = -ENODEV;
-		goto exit;
+		superio_exit(sioaddr);
+		return ERR_PTR(-ENODEV);
 	}
 
 	devid = force_id ? force_id : superio_inw(sioaddr, SIO_REG_DEVID);
-	switch (devid) {
-	case SIO_F71808_ID:
-		watchdog.type = f71808fg;
-		break;
-	case SIO_F71862_ID:
-		watchdog.type = f71862fg;
-		break;
-	case SIO_F71868_ID:
-		watchdog.type = f71868;
-		break;
-	case SIO_F71869_ID:
-	case SIO_F71869A_ID:
-		watchdog.type = f71869;
-		break;
-	case SIO_F71882_ID:
-		watchdog.type = f71882fg;
-		break;
-	case SIO_F71889_ID:
-		watchdog.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;
-		break;
-	case SIO_F81865_ID:
-		watchdog.type = f81865;
-		break;
-	case SIO_F81866_ID:
-		watchdog.type = f81866;
-		break;
-	default:
-		pr_info("Unrecognized Fintek device: %04x\n",
-			(unsigned int)devid);
-		err = -ENODEV;
-		goto exit;
+	for (variant = f71808e_variants; variant->id; variant++) {
+		if (variant->id == devid)
+			break;
+	}
+
+	if (!variant->wdt_name) {
+		if (!variant->id)
+			pr_info("Unrecognized Fintek device: %04x\n", devid);
+		superio_exit(sioaddr);
+		return ERR_PTR(-ENODEV);
 	}
 
 	pr_info("Found %s watchdog chip, revision %d\n",
-		f71808e_names[watchdog.type],
-		(int)superio_inb(sioaddr, SIO_REG_DEVREV));
-exit:
+		variant->wdt_name,
+		superio_inb(sioaddr, SIO_REG_DEVREV));
+
 	superio_exit(sioaddr);
-	return err;
+	return variant;
 }
 
 static int __init f71808e_init(void)
 {
 	static const unsigned short addrs[] = { 0x2e, 0x4e };
-	int err = -ENODEV;
+	struct f71808e_variant *variant;
 	int i;
 
 	if (f71862fg_pin != 63 && f71862fg_pin != 56) {
@@ -847,12 +824,14 @@ static int __init f71808e_init(void)
 	}
 
 	for (i = 0; i < ARRAY_SIZE(addrs); i++) {
-		err = f71808e_find(addrs[i]);
-		if (err == 0)
+		variant = f71808e_find(addrs[i]);
+		if (!IS_ERR(variant))
 			break;
 	}
 	if (i == ARRAY_SIZE(addrs))
-		return err;
+		return PTR_ERR(variant);
+
+	watchdog.variant = variant;
 
 	return watchdog_init(addrs[i]);
 }
-- 
2.27.0

Re: [PATCH v1 6/8] watchdog: f71808e_wdt: consolidate variant handling into single array

[Guenter Roeck]

On Thu, Jun 11, 2020 at 09:17:47PM +0200, Ahmad Fatoum wrote:
> Driver has two big switches: one to map hardware ID to an enum constant
> one more to map the enum constant to the variant's pin configuration
> routine.
> 
> Drop the enum and give every variant an array entry identifying it.
> This arguably simplifies the code by making it a little more compact.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Please run checkpatch --strict and fix the issues it reports.

> ---
>  drivers/watchdog/f71808e_wdt.c | 263 +++++++++++++++------------------
>  1 file changed, 121 insertions(+), 142 deletions(-)
> 
> diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
> index 9f7823819ed1..9299ad4d4a52 100644
> --- a/drivers/watchdog/f71808e_wdt.c
> +++ b/drivers/watchdog/f71808e_wdt.c
> @@ -110,22 +110,6 @@ module_param(start_withtimeout, uint, 0);
>  MODULE_PARM_DESC(start_withtimeout, "Start watchdog timer on module load with"
>  	" given initial timeout. Zero (default) disables this feature.");
>  
> -enum chips { f71808fg, f71858fg, f71862fg, f71868, f71869, f71882fg, f71889fg,
> -	     f81803, f81865, f81866};
> -
> -static const char *f71808e_names[] = {
> -	"f71808fg",
> -	"f71858fg",
> -	"f71862fg",
> -	"f71868",
> -	"f71869",
> -	"f71882fg",
> -	"f71889fg",
> -	"f81803",
> -	"f81865",
> -	"f81866",
> -};
> -
>  /* Super-I/O Function prototypes */
>  static inline int superio_inb(int base, int reg);
>  static inline int superio_inw(int base, int reg);
> @@ -136,9 +120,17 @@ 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 f71808e_variant {
> +	u16 id;
> +	const char *wdt_name; /* NULL if chip lacks watchdog timer */
> +	void (*pinconf)(struct watchdog_data *wd);
> +};
> +
>  struct watchdog_data {
>  	unsigned short	sioaddr;
> -	enum chips	type;
> +	const struct f71808e_variant *variant;
>  	unsigned long	opened;
>  	struct mutex	lock;
>  	char		expect_close;
> @@ -156,6 +148,12 @@ static struct watchdog_data watchdog = {
>  	.lock = __MUTEX_INITIALIZER(watchdog.lock),
>  };
>  
> +static inline bool has_f81865_wdo_conf(struct watchdog_data *wd)
> +{
> +	return wd->variant->id == SIO_F81865_ID
> +		|| wd->variant->id == SIO_F81866_ID;

Nit: unnecessary continuation line (limit is now 100).

> +}
> +
>  /* Super I/O functions */
>  static inline int superio_inb(int base, int reg)
>  {
> @@ -247,7 +245,7 @@ static int watchdog_set_pulse_width(unsigned int pw)
>  	int err = 0;
>  	unsigned int t1 = 25, t2 = 125, t3 = 5000;
>  
> -	if (watchdog.type == f71868) {
> +	if (watchdog.variant->id == SIO_F71868_ID) {
>  		t1 = 30;
>  		t2 = 150;
>  		t3 = 6000;
> @@ -309,7 +307,6 @@ static int watchdog_keepalive(void)
>  static int watchdog_start(void)
>  {
>  	int err;
> -	u8 tmp;
>  
>  	/* Make sure we don't die as soon as the watchdog is enabled below */
>  	err = watchdog_keepalive();
> @@ -323,81 +320,12 @@ static int watchdog_start(void)
>  	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
>  
>  	/* Watchdog pin configuration */
> -	switch (watchdog.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);
> -		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);
> -		} else if (f71862fg_pin == 56) {
> -			superio_set_bit(watchdog.sioaddr, SIO_REG_MFUNCT1, 1);
> -		}
> -		break;
> -
> -	case f71868:
> -	case f71869:
> -		/* GPIO14 --> WDTRST# */
> -		superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT1, 4);
> -		break;
> -
> -	case f71882fg:
> -		/* Set pin 56 to WDTRST# */
> -		superio_set_bit(watchdog.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);
> -		break;
> -
> -	case f81803:
> -		/* Enable TSI Level register bank */
> -		superio_clear_bit(watchdog.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));
> -		break;
> -
> -	case f81865:
> -		/* Set pin 70 to WDTRST# */
> -		superio_clear_bit(watchdog.sioaddr, SIO_REG_MFUNCT3, 5);
> -		break;
> -
> -	case f81866:
> -		/*
> -		 * GPIO1 Control Register when 27h BIT3:2 = 01 & BIT0 = 0.
> -		 * The PIN 70(GPIO15/WDTRST) is controlled by 2Ch:
> -		 *     BIT5: 0 -> WDTRST#
> -		 *           1 -> GPIO15
> -		 */
> -		tmp = superio_inb(watchdog.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_clear_bit(watchdog.sioaddr, SIO_F81866_REG_GPIO1, 5);
> -		break;
> -
> -	default:
> -		/*
> -		 * 'default' label to shut up the compiler and catch
> -		 * programmer errors
> -		 */
> -		err = -ENODEV;
> -		goto exit_superio;
> -	}
> +	watchdog.variant->pinconf(&watchdog);
>  
>  	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
>  	superio_set_bit(watchdog.sioaddr, SIO_REG_ENABLE, 0);
>  
> -	if (watchdog.type == f81865 || watchdog.type == f81866)
> +	if (has_f81865_wdo_conf(&watchdog))
>  		superio_set_bit(watchdog.sioaddr, F81865_REG_WDO_CONF,
>  				F81865_FLAG_WDOUT_EN);
>  	else
> @@ -425,7 +353,6 @@ static int watchdog_start(void)
>  				F71808FG_FLAG_WD_PULSE);
>  	}
>  
> -exit_superio:
>  	superio_exit(watchdog.sioaddr);
>  exit_unlock:
>  	mutex_unlock(&watchdog.lock);
> @@ -679,7 +606,7 @@ static int __init watchdog_init(int sioaddr)
>  
>  	snprintf(watchdog.ident.identity,
>  		sizeof(watchdog.ident.identity), "%s watchdog",
> -		f71808e_names[watchdog.type]);
> +		watchdog.variant->wdt_name);
>  
>  	err = superio_enter(sioaddr);
>  	if (err)
> @@ -772,73 +699,123 @@ static int __init watchdog_init(int sioaddr)
>  	return err;
>  }
>  
> -static int __init f71808e_find(int sioaddr)
> +static void f71808fg_pinconf(struct watchdog_data *wd)
> +{
> +	/* Set pin 21 to GPIO23/WDTRST#, then to WDTRST# */
> +	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT2, 3);
> +	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 3);
> +}
> +static void f71862fg_pinconf(struct watchdog_data *wd)
> +{
> +	if (f71862fg_pin == 63) {
> +		/* SPI must be disabled first to use this pin! */
> +		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(wd->sioaddr, SIO_REG_MFUNCT1, 1);
> +	}
> +}
> +static void f71868_pinconf(struct watchdog_data *wd)
> +{
> +	/* GPIO14 --> WDTRST# */
> +	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT1, 4);
> +}
> +static void f71882fg_pinconf(struct watchdog_data *wd)
> +{
> +	/* Set pin 56 to WDTRST# */
> +	superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT1, 1);
> +}
> +static void f71889fg_pinconf(struct watchdog_data *wd)
> +{
> +	/* set pin 40 to WDTRST# */
> +	superio_outb(wd->sioaddr, SIO_REG_MFUNCT3,
> +		     superio_inb(wd->sioaddr, SIO_REG_MFUNCT3) & 0xcf);
> +}
> +static void f81803_pinconf(struct watchdog_data *wd)
> +{
> +	/* Enable TSI Level register bank */
> +	superio_clear_bit(wd->sioaddr, SIO_REG_CLOCK_SEL, 3);
> +	/* Set pin 27 to WDTRST# */
> +	superio_outb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL, 0x5f &
> +		     superio_inb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL));
> +}
> +static void f81865_pinconf(struct watchdog_data *wd)
> +{
> +	/* Set pin 70 to WDTRST# */
> +	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 5);
> +}
> +static void f81866_pinconf(struct watchdog_data *wd)
> +{
> +	/*
> +	 * GPIO1 Control Register when 27h BIT3:2 = 01 & BIT0 = 0.
> +	 * The PIN 70(GPIO15/WDTRST) is controlled by 2Ch:
> +	 *     BIT5: 0 -> WDTRST#
> +	 *           1 -> GPIO15
> +	 */
> +	u8 tmp = superio_inb(wd->sioaddr, SIO_F81866_REG_PORT_SEL);
> +	tmp &= ~(BIT(3) | BIT(0));
> +	tmp |= BIT(2);
> +	superio_outb(wd->sioaddr, SIO_F81866_REG_PORT_SEL, tmp);
> +
> +	superio_clear_bit(wd->sioaddr, SIO_F81866_REG_GPIO1, 5);
> +}
> +
> +struct f71808e_variant f71808e_variants[] = {
> +	{ SIO_F71808_ID,  "f71808fg", f71808fg_pinconf },
> +	{ SIO_F71862_ID,  "f71862fg", f71862fg_pinconf },
> +	{ SIO_F71868_ID,  "f71868",   f71868_pinconf },
> +	{ SIO_F71869_ID,  "f71869",   f71868_pinconf },
> +	{ SIO_F71869A_ID, "f71869",   f71868_pinconf },
> +	{ SIO_F71882_ID,  "f71882fg", f71882fg_pinconf },
> +	{ SIO_F71889_ID,  "f71889fg", f71889fg_pinconf },
> +	{ SIO_F81803_ID,  "f81803",   f81803_pinconf },
> +	{ SIO_F81865_ID,  "f81865",   f81865_pinconf },
> +	{ SIO_F81866_ID,  "f81866",   f81866_pinconf },
> +	/* Confirmed (by datasheet) not to have a watchdog. */
> +	{ SIO_F71858_ID,  NULL,       NULL },
> +	{ /* sentinel */ }
> +};
> +
> +static struct f71808e_variant __init *f71808e_find(int sioaddr)
>  {
> +	struct f71808e_variant *variant;
>  	u16 devid;
>  	int err = superio_enter(sioaddr);
>  	if (err)
> -		return err;
> +		return ERR_PTR(err);
>  
>  	devid = superio_inw(sioaddr, SIO_REG_MANID);
>  	if (devid != SIO_FINTEK_ID) {
>  		pr_debug("Not a Fintek device\n");
> -		err = -ENODEV;
> -		goto exit;
> +		superio_exit(sioaddr);
> +		return ERR_PTR(-ENODEV);
>  	}
>  
>  	devid = force_id ? force_id : superio_inw(sioaddr, SIO_REG_DEVID);
> -	switch (devid) {
> -	case SIO_F71808_ID:
> -		watchdog.type = f71808fg;
> -		break;
> -	case SIO_F71862_ID:
> -		watchdog.type = f71862fg;
> -		break;
> -	case SIO_F71868_ID:
> -		watchdog.type = f71868;
> -		break;
> -	case SIO_F71869_ID:
> -	case SIO_F71869A_ID:
> -		watchdog.type = f71869;
> -		break;
> -	case SIO_F71882_ID:
> -		watchdog.type = f71882fg;
> -		break;
> -	case SIO_F71889_ID:
> -		watchdog.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;
> -		break;
> -	case SIO_F81865_ID:
> -		watchdog.type = f81865;
> -		break;
> -	case SIO_F81866_ID:
> -		watchdog.type = f81866;
> -		break;
> -	default:
> -		pr_info("Unrecognized Fintek device: %04x\n",
> -			(unsigned int)devid);
> -		err = -ENODEV;
> -		goto exit;
> +	for (variant = f71808e_variants; variant->id; variant++) {
> +		if (variant->id == devid)
> +			break;
> +	}
> +
> +	if (!variant->wdt_name) {
> +		if (!variant->id)
> +			pr_info("Unrecognized Fintek device: %04x\n", devid);
> +		superio_exit(sioaddr);
> +		return ERR_PTR(-ENODEV);
>  	}
>  
>  	pr_info("Found %s watchdog chip, revision %d\n",
> -		f71808e_names[watchdog.type],
> -		(int)superio_inb(sioaddr, SIO_REG_DEVREV));
> -exit:
> +		variant->wdt_name,
> +		superio_inb(sioaddr, SIO_REG_DEVREV));
> +
>  	superio_exit(sioaddr);
> -	return err;
> +	return variant;
>  }
>  
>  static int __init f71808e_init(void)
>  {
>  	static const unsigned short addrs[] = { 0x2e, 0x4e };
> -	int err = -ENODEV;
> +	struct f71808e_variant *variant;
>  	int i;
>  
>  	if (f71862fg_pin != 63 && f71862fg_pin != 56) {
> @@ -847,12 +824,14 @@ static int __init f71808e_init(void)
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(addrs); i++) {
> -		err = f71808e_find(addrs[i]);
> -		if (err == 0)
> +		variant = f71808e_find(addrs[i]);
> +		if (!IS_ERR(variant))
>  			break;
>  	}
>  	if (i == ARRAY_SIZE(addrs))
> -		return err;
> +		return PTR_ERR(variant);
> +
> +	watchdog.variant = variant;
>  
>  	return watchdog_init(addrs[i]);
>  }

[PATCH v1 7/8] watchdog: f71808e_wdt: migrate to new kernel watchdog API

[Ahmad Fatoum]

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

This changes behavior on module unloading: Whereas before, the watchdog
was automatically stopped on module unload, now explicit stopping of
the watchdog before unload is required.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/watchdog/Kconfig       |   1 +
 drivers/watchdog/f71808e_wdt.c | 497 ++++++++-------------------------
 2 files changed, 111 insertions(+), 387 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 0663c604bd64..c312dd5fc0ca 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -1052,6 +1052,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 9299ad4d4a52..7c42cbf9912e 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"
@@ -129,23 +123,15 @@ struct f71808e_variant {
 };
 
 struct watchdog_data {
+	struct watchdog_device wdd;
 	unsigned short	sioaddr;
 	const struct f71808e_variant *variant;
-	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 watchdog_data watchdog = {
-	.lock = __MUTEX_INITIALIZER(watchdog.lock),
 };
 
 static inline bool has_f81865_wdo_conf(struct watchdog_data *wd)
@@ -216,487 +202,225 @@ static inline void superio_exit(int base)
 	release_region(base, 2);
 }
 
-static int watchdog_set_timeout(int timeout)
+static int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int new_timeout)
 {
-	if (timeout <= 0
-	 || timeout >  max_timeout) {
-		pr_err("watchdog timeout out of range\n");
-		return -EINVAL;
-	}
+	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
 
-	mutex_lock(&watchdog.lock);
-
-	watchdog.timeout = timeout;
-	if (timeout > 0xff) {
-		watchdog.timer_val = DIV_ROUND_UP(timeout, 60);
-		watchdog.minutes_mode = true;
+	wdd->timeout = new_timeout;
+	if (new_timeout > 0xff) {
+		wd->timer_val = DIV_ROUND_UP(new_timeout, 60);
+		wd->minutes_mode = true;
 	} else {
-		watchdog.timer_val = timeout;
-		watchdog.minutes_mode = false;
+		wd->timer_val = new_timeout;
+		wd->minutes_mode = false;
 	}
 
-	mutex_unlock(&watchdog.lock);
-
 	return 0;
 }
 
-static int watchdog_set_pulse_width(unsigned int pw)
+static int watchdog_set_pulse_width(struct watchdog_data *wd, unsigned int pw)
 {
-	int err = 0;
 	unsigned int t1 = 25, t2 = 125, t3 = 5000;
 
-	if (watchdog.variant->id == SIO_F71868_ID) {
+	if (wd->variant->id == SIO_F71868_ID) {
 		t1 = 30;
 		t2 = 150;
 		t3 = 6000;
 	}
 
-	mutex_lock(&watchdog.lock);
-
 	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");
-		err = -EINVAL;
-		goto exit_unlock;
+		return -EINVAL;
 	}
 
-	watchdog.pulse_mode = pw;
+	wd->pulse_mode = pw;
 
-exit_unlock:
-	mutex_unlock(&watchdog.lock);
-	return err;
+	return 0;
 }
 
-static int watchdog_keepalive(void)
+static int watchdog_keepalive(struct watchdog_device *wdd)
 {
-	int err = 0;
+	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
+	int err;
 
-	mutex_lock(&watchdog.lock);
-	err = superio_enter(watchdog.sioaddr);
+	err = superio_enter(wd->sioaddr);
 	if (err)
-		goto exit_unlock;
-	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
+		return err;
+	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);
 
-exit_unlock:
-	mutex_unlock(&watchdog.lock);
-	return err;
+	return 0;
 }
 
-static int watchdog_start(void)
+static int watchdog_start(struct watchdog_device *wdd)
 {
+	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
 	int err;
 
 	/* Make sure we don't die as soon as the watchdog is enabled below */
-	err = watchdog_keepalive();
+	err = watchdog_keepalive(wdd);
 	if (err)
 		return err;
 
-	mutex_lock(&watchdog.lock);
-	err = superio_enter(watchdog.sioaddr);
+	err = superio_enter(wd->sioaddr);
 	if (err)
-		goto exit_unlock;
-	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
+		return err;
+	superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT);
 
 	/* Watchdog pin configuration */
-	watchdog.variant->pinconf(&watchdog);
+	wd->variant->pinconf(wd);
 
-	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 (has_f81865_wdo_conf(&watchdog))
-		superio_set_bit(watchdog.sioaddr, F81865_REG_WDO_CONF,
+	if (has_f81865_wdo_conf(wd))
+		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) {
+	set_bit(WDOG_HW_RUNNING, &wdd->status);
+
+	if (wd->pulse_mode) {
 		/* Select "pulse" output mode with given duration */
-		u8 wdt_conf = superio_inb(watchdog.sioaddr,
-				F71808FG_REG_WDT_CONF);
+		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,
-				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,
-				F71808FG_FLAG_WD_PULSE);
+		superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF,
+				  F71808FG_FLAG_WD_PULSE);
 	}
 
-	superio_exit(watchdog.sioaddr);
-exit_unlock:
-	mutex_unlock(&watchdog.lock);
+	superio_exit(wd->sioaddr);
 
 	return err;
 }
 
-static int watchdog_stop(void)
-{
-	int err = 0;
-
-	mutex_lock(&watchdog.lock);
-	err = superio_enter(watchdog.sioaddr);
-	if (err)
-		goto exit_unlock;
-	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
-
-	superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
-			F71808FG_FLAG_WD_EN);
-
-	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 watchdog_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)
+static int watchdog_stop(struct watchdog_device *wdd)
 {
+	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
 	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 = watchdog_start();
-	if (err) {
-		clear_bit(0, &watchdog.opened);
+	err = superio_enter(wd->sioaddr);
+	if (err)
 		return err;
-	}
+	superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT);
 
-	if (nowayout)
-		__module_get(THIS_MODULE);
+	superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF,
+			  F71808FG_FLAG_WD_EN);
 
-	watchdog.expect_close = 0;
-	return stream_open(inode, file);
-}
-
-static int watchdog_release(struct inode *inode, struct file *file)
-{
-	clear_bit(0, &watchdog.opened);
+	superio_exit(wd->sioaddr);
 
-	if (!watchdog.expect_close) {
-		watchdog_keepalive();
-		pr_crit("Unexpected close, not stopping watchdog!\n");
-	} else if (!nowayout) {
-		watchdog_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 */
-		watchdog_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)
-{
-	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)
-			watchdog_stop();
-
-		if (new_options & WDIOS_ENABLECARD)
-			return watchdog_start();
-		/* fall through */
-
-	case WDIOC_KEEPALIVE:
-		watchdog_keepalive();
-		return 0;
-
-	case WDIOC_SETTIMEOUT:
-		if (get_user(new_timeout, uarg.i))
-			return -EFAULT;
-
-		if (watchdog_set_timeout(new_timeout))
-			return -EINVAL;
-
-		watchdog_keepalive();
-		/* fall through */
-
-	case WDIOC_GETTIMEOUT:
-		return put_user(watchdog.timeout, uarg.i);
-
-	default:
-		return -ENOTTY;
-
-	}
-}
+static const struct watchdog_ops f71808e_wdog_ops = {
+	.owner = THIS_MODULE,
+	.start = watchdog_start,
+	.stop = watchdog_stop,
+	.ping = watchdog_keepalive,
+	.set_timeout = watchdog_set_timeout,
+};
 
-static int watchdog_notify_sys(struct notifier_block *this, unsigned long code,
-	void *unused)
+static bool watchdog_is_running(struct watchdog_data *wd, u8 wdt_conf)
 {
-	if (code == SYS_DOWN || code == SYS_HALT)
-		watchdog_stop();
-	return NOTIFY_DONE;
+	return (superio_inb(wd->sioaddr, SIO_REG_ENABLE) & BIT(0))
+		&& (wdt_conf & BIT(F71808FG_FLAG_WD_EN));
 }
 
-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 int __init watchdog_init(int sioaddr)
+static int __init watchdog_init(struct watchdog_data *wd)
 {
+	struct watchdog_device *wdd = &wd->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
+	wd->ident.options = WDIOF_SETTIMEOUT
+				| WDIOF_MAGICCLOSE
 				| WDIOF_KEEPALIVEPING
 				| WDIOF_CARDRESET;
 
-	snprintf(watchdog.ident.identity,
-		sizeof(watchdog.ident.identity), "%s watchdog",
-		watchdog.variant->wdt_name);
+	snprintf(wd->ident.identity, sizeof(wd->ident.identity),
+		 "%s watchdog", wd->variant->wdt_name);
 
-	err = superio_enter(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);
 
-	wdt_conf = superio_inb(sioaddr, F71808FG_REG_WDT_CONF);
-	watchdog.caused_reboot = wdt_conf & BIT(F71808FG_FLAG_WDTMOUT_STS);
+	wdt_conf = superio_inb(wd->sioaddr, F71808FG_REG_WDT_CONF);
 
 	/*
 	 * We don't want WDTMOUT_STS to stick around till regular reboot.
 	 * Write 1 to the bit to clear it to zero.
 	 */
-	superio_outb(sioaddr, F71808FG_REG_WDT_CONF,
+	superio_outb(wd->sioaddr, F71808FG_REG_WDT_CONF,
 		     wdt_conf | BIT(F71808FG_FLAG_WDTMOUT_STS));
 
-	superio_exit(sioaddr);
+	if (watchdog_is_running(wd, wdt_conf))
+		set_bit(WDOG_HW_RUNNING, &wdd->status);
 
-	err = watchdog_set_timeout(timeout);
-	if (err)
-		return err;
-	err = watchdog_set_pulse_width(pulse_width);
-	if (err)
-		return err;
+	superio_exit(wd->sioaddr);
 
-	err = register_reboot_notifier(&watchdog_notifier);
+	err = watchdog_set_pulse_width(wd, pulse_width);
 	if (err)
 		return err;
 
-	err = misc_register(&watchdog_miscdev);
-	if (err) {
-		pr_err("cannot register miscdev on minor=%d\n",
-		       watchdog_miscdev.minor);
-		goto exit_reboot;
-	}
-
-	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 = watchdog_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);
-		mutex_unlock(&watchdog.lock);
-
-		if (nowayout)
-			__module_get(THIS_MODULE);
+	wdd->info		= &wd->ident;
+	wdd->ops		= &f71808e_wdog_ops;
+	wdd->min_timeout	= 1;
+	wdd->max_timeout	= max_timeout;
+	wdd->timeout		= timeout;
 
-		pr_info("watchdog started with initial timeout of %u sec\n",
-			start_withtimeout);
-	}
+	watchdog_set_nowayout(wdd, nowayout);
+	watchdog_stop_on_unregister(wdd);
+	watchdog_stop_on_reboot(wdd);
+	watchdog_init_timeout(wdd, start_withtimeout, NULL);
+	watchdog_set_drvdata(wdd, wd);
 
-	return 0;
+	if (wdt_conf & BIT(F71808FG_FLAG_WDTMOUT_STS))
+		wdd->bootstatus	= WDIOF_CARDRESET;
 
-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
+	 */
+	watchdog_set_timeout(wdd, wdd->timeout);
 
-	return err;
+	return watchdog_register_device(wdd);
 }
 
 static void f71808fg_pinconf(struct watchdog_data *wd)
@@ -812,8 +536,11 @@ static struct f71808e_variant __init *f71808e_find(int sioaddr)
 	return variant;
 }
 
+static struct watchdog_data watchdog;
+
 static int __init f71808e_init(void)
 {
+	struct watchdog_data *wd = &watchdog;
 	static const unsigned short addrs[] = { 0x2e, 0x4e };
 	struct f71808e_variant *variant;
 	int i;
@@ -831,19 +558,15 @@ static int __init f71808e_init(void)
 	if (i == ARRAY_SIZE(addrs))
 		return PTR_ERR(variant);
 
-	watchdog.variant = variant;
+	wd->variant = variant;
+	wd->sioaddr = addrs[i];
 
-	return watchdog_init(addrs[i]);
+	return watchdog_init(wd);
 }
 
 static void __exit f71808e_exit(void)
 {
-	if (watchdog_is_running()) {
-		pr_warn("Watchdog timer still running, stopping it\n");
-		watchdog_stop();
-	}
-	misc_deregister(&watchdog_miscdev);
-	unregister_reboot_notifier(&watchdog_notifier);
+	watchdog_unregister_device(&watchdog.wdd);
 }
 
 MODULE_DESCRIPTION("F71808E Watchdog Driver");
-- 
2.27.0

Re: [PATCH v1 7/8] watchdog: f71808e_wdt: migrate to new kernel watchdog API

[Guenter Roeck]

On Thu, Jun 11, 2020 at 09:17:48PM +0200, Ahmad Fatoum wrote:
> Migrating the driver lets us drop the watchdog misc device boilerplate
> and reduce size by 270~ lines. It also brings us support for new
> functionality like CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED.
> 
> This changes behavior on module unloading: Whereas before, the watchdog
> was automatically stopped on module unload, now explicit stopping of
> the watchdog before unload is required.
> 

Any chance you can convert the driver to a platform device ?

Thanks,
Guenter

> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/watchdog/Kconfig       |   1 +
>  drivers/watchdog/f71808e_wdt.c | 497 ++++++++-------------------------
>  2 files changed, 111 insertions(+), 387 deletions(-)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 0663c604bd64..c312dd5fc0ca 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1052,6 +1052,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 9299ad4d4a52..7c42cbf9912e 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"
> @@ -129,23 +123,15 @@ struct f71808e_variant {
>  };
>  
>  struct watchdog_data {
> +	struct watchdog_device wdd;
>  	unsigned short	sioaddr;
>  	const struct f71808e_variant *variant;
> -	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 watchdog_data watchdog = {
> -	.lock = __MUTEX_INITIALIZER(watchdog.lock),
>  };
>  
>  static inline bool has_f81865_wdo_conf(struct watchdog_data *wd)
> @@ -216,487 +202,225 @@ static inline void superio_exit(int base)
>  	release_region(base, 2);
>  }
>  
> -static int watchdog_set_timeout(int timeout)
> +static int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int new_timeout)
>  {
> -	if (timeout <= 0
> -	 || timeout >  max_timeout) {
> -		pr_err("watchdog timeout out of range\n");
> -		return -EINVAL;
> -	}
> +	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
>  
> -	mutex_lock(&watchdog.lock);
> -
> -	watchdog.timeout = timeout;
> -	if (timeout > 0xff) {
> -		watchdog.timer_val = DIV_ROUND_UP(timeout, 60);
> -		watchdog.minutes_mode = true;
> +	wdd->timeout = new_timeout;
> +	if (new_timeout > 0xff) {
> +		wd->timer_val = DIV_ROUND_UP(new_timeout, 60);
> +		wd->minutes_mode = true;
>  	} else {
> -		watchdog.timer_val = timeout;
> -		watchdog.minutes_mode = false;
> +		wd->timer_val = new_timeout;
> +		wd->minutes_mode = false;
>  	}
>  
> -	mutex_unlock(&watchdog.lock);
> -
>  	return 0;
>  }
>  
> -static int watchdog_set_pulse_width(unsigned int pw)
> +static int watchdog_set_pulse_width(struct watchdog_data *wd, unsigned int pw)
>  {
> -	int err = 0;
>  	unsigned int t1 = 25, t2 = 125, t3 = 5000;
>  
> -	if (watchdog.variant->id == SIO_F71868_ID) {
> +	if (wd->variant->id == SIO_F71868_ID) {
>  		t1 = 30;
>  		t2 = 150;
>  		t3 = 6000;
>  	}
>  
> -	mutex_lock(&watchdog.lock);
> -
>  	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");
> -		err = -EINVAL;
> -		goto exit_unlock;
> +		return -EINVAL;
>  	}
>  
> -	watchdog.pulse_mode = pw;
> +	wd->pulse_mode = pw;
>  
> -exit_unlock:
> -	mutex_unlock(&watchdog.lock);
> -	return err;
> +	return 0;
>  }
>  
> -static int watchdog_keepalive(void)
> +static int watchdog_keepalive(struct watchdog_device *wdd)
>  {
> -	int err = 0;
> +	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
> +	int err;
>  
> -	mutex_lock(&watchdog.lock);
> -	err = superio_enter(watchdog.sioaddr);
> +	err = superio_enter(wd->sioaddr);
>  	if (err)
> -		goto exit_unlock;
> -	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
> +		return err;
> +	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);
>  
> -exit_unlock:
> -	mutex_unlock(&watchdog.lock);
> -	return err;
> +	return 0;
>  }
>  
> -static int watchdog_start(void)
> +static int watchdog_start(struct watchdog_device *wdd)
>  {
> +	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
>  	int err;
>  
>  	/* Make sure we don't die as soon as the watchdog is enabled below */
> -	err = watchdog_keepalive();
> +	err = watchdog_keepalive(wdd);
>  	if (err)
>  		return err;
>  
> -	mutex_lock(&watchdog.lock);
> -	err = superio_enter(watchdog.sioaddr);
> +	err = superio_enter(wd->sioaddr);
>  	if (err)
> -		goto exit_unlock;
> -	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
> +		return err;
> +	superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT);
>  
>  	/* Watchdog pin configuration */
> -	watchdog.variant->pinconf(&watchdog);
> +	wd->variant->pinconf(wd);
>  
> -	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 (has_f81865_wdo_conf(&watchdog))
> -		superio_set_bit(watchdog.sioaddr, F81865_REG_WDO_CONF,
> +	if (has_f81865_wdo_conf(wd))
> +		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) {
> +	set_bit(WDOG_HW_RUNNING, &wdd->status);
> +
> +	if (wd->pulse_mode) {
>  		/* Select "pulse" output mode with given duration */
> -		u8 wdt_conf = superio_inb(watchdog.sioaddr,
> -				F71808FG_REG_WDT_CONF);
> +		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,
> -				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,
> -				F71808FG_FLAG_WD_PULSE);
> +		superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF,
> +				  F71808FG_FLAG_WD_PULSE);
>  	}
>  
> -	superio_exit(watchdog.sioaddr);
> -exit_unlock:
> -	mutex_unlock(&watchdog.lock);
> +	superio_exit(wd->sioaddr);
>  
>  	return err;
>  }
>  
> -static int watchdog_stop(void)
> -{
> -	int err = 0;
> -
> -	mutex_lock(&watchdog.lock);
> -	err = superio_enter(watchdog.sioaddr);
> -	if (err)
> -		goto exit_unlock;
> -	superio_select(watchdog.sioaddr, SIO_F71808FG_LD_WDT);
> -
> -	superio_clear_bit(watchdog.sioaddr, F71808FG_REG_WDT_CONF,
> -			F71808FG_FLAG_WD_EN);
> -
> -	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 watchdog_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)
> +static int watchdog_stop(struct watchdog_device *wdd)
>  {
> +	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
>  	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 = watchdog_start();
> -	if (err) {
> -		clear_bit(0, &watchdog.opened);
> +	err = superio_enter(wd->sioaddr);
> +	if (err)
>  		return err;
> -	}
> +	superio_select(wd->sioaddr, SIO_F71808FG_LD_WDT);
>  
> -	if (nowayout)
> -		__module_get(THIS_MODULE);
> +	superio_clear_bit(wd->sioaddr, F71808FG_REG_WDT_CONF,
> +			  F71808FG_FLAG_WD_EN);
>  
> -	watchdog.expect_close = 0;
> -	return stream_open(inode, file);
> -}
> -
> -static int watchdog_release(struct inode *inode, struct file *file)
> -{
> -	clear_bit(0, &watchdog.opened);
> +	superio_exit(wd->sioaddr);
>  
> -	if (!watchdog.expect_close) {
> -		watchdog_keepalive();
> -		pr_crit("Unexpected close, not stopping watchdog!\n");
> -	} else if (!nowayout) {
> -		watchdog_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 */
> -		watchdog_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)
> -{
> -	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)
> -			watchdog_stop();
> -
> -		if (new_options & WDIOS_ENABLECARD)
> -			return watchdog_start();
> -		/* fall through */
> -
> -	case WDIOC_KEEPALIVE:
> -		watchdog_keepalive();
> -		return 0;
> -
> -	case WDIOC_SETTIMEOUT:
> -		if (get_user(new_timeout, uarg.i))
> -			return -EFAULT;
> -
> -		if (watchdog_set_timeout(new_timeout))
> -			return -EINVAL;
> -
> -		watchdog_keepalive();
> -		/* fall through */
> -
> -	case WDIOC_GETTIMEOUT:
> -		return put_user(watchdog.timeout, uarg.i);
> -
> -	default:
> -		return -ENOTTY;
> -
> -	}
> -}
> +static const struct watchdog_ops f71808e_wdog_ops = {
> +	.owner = THIS_MODULE,
> +	.start = watchdog_start,
> +	.stop = watchdog_stop,
> +	.ping = watchdog_keepalive,
> +	.set_timeout = watchdog_set_timeout,
> +};
>  
> -static int watchdog_notify_sys(struct notifier_block *this, unsigned long code,
> -	void *unused)
> +static bool watchdog_is_running(struct watchdog_data *wd, u8 wdt_conf)
>  {
> -	if (code == SYS_DOWN || code == SYS_HALT)
> -		watchdog_stop();
> -	return NOTIFY_DONE;
> +	return (superio_inb(wd->sioaddr, SIO_REG_ENABLE) & BIT(0))
> +		&& (wdt_conf & BIT(F71808FG_FLAG_WD_EN));
>  }
>  
> -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 int __init watchdog_init(int sioaddr)
> +static int __init watchdog_init(struct watchdog_data *wd)
>  {
> +	struct watchdog_device *wdd = &wd->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
> +	wd->ident.options = WDIOF_SETTIMEOUT
> +				| WDIOF_MAGICCLOSE
>  				| WDIOF_KEEPALIVEPING
>  				| WDIOF_CARDRESET;
>  
> -	snprintf(watchdog.ident.identity,
> -		sizeof(watchdog.ident.identity), "%s watchdog",
> -		watchdog.variant->wdt_name);
> +	snprintf(wd->ident.identity, sizeof(wd->ident.identity),
> +		 "%s watchdog", wd->variant->wdt_name);
>  
> -	err = superio_enter(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);
>  
> -	wdt_conf = superio_inb(sioaddr, F71808FG_REG_WDT_CONF);
> -	watchdog.caused_reboot = wdt_conf & BIT(F71808FG_FLAG_WDTMOUT_STS);
> +	wdt_conf = superio_inb(wd->sioaddr, F71808FG_REG_WDT_CONF);
>  
>  	/*
>  	 * We don't want WDTMOUT_STS to stick around till regular reboot.
>  	 * Write 1 to the bit to clear it to zero.
>  	 */
> -	superio_outb(sioaddr, F71808FG_REG_WDT_CONF,
> +	superio_outb(wd->sioaddr, F71808FG_REG_WDT_CONF,
>  		     wdt_conf | BIT(F71808FG_FLAG_WDTMOUT_STS));
>  
> -	superio_exit(sioaddr);
> +	if (watchdog_is_running(wd, wdt_conf))
> +		set_bit(WDOG_HW_RUNNING, &wdd->status);
>  
> -	err = watchdog_set_timeout(timeout);
> -	if (err)
> -		return err;
> -	err = watchdog_set_pulse_width(pulse_width);
> -	if (err)
> -		return err;
> +	superio_exit(wd->sioaddr);
>  
> -	err = register_reboot_notifier(&watchdog_notifier);
> +	err = watchdog_set_pulse_width(wd, pulse_width);
>  	if (err)
>  		return err;
>  
> -	err = misc_register(&watchdog_miscdev);
> -	if (err) {
> -		pr_err("cannot register miscdev on minor=%d\n",
> -		       watchdog_miscdev.minor);
> -		goto exit_reboot;
> -	}
> -
> -	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 = watchdog_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);
> -		mutex_unlock(&watchdog.lock);
> -
> -		if (nowayout)
> -			__module_get(THIS_MODULE);
> +	wdd->info		= &wd->ident;
> +	wdd->ops		= &f71808e_wdog_ops;
> +	wdd->min_timeout	= 1;
> +	wdd->max_timeout	= max_timeout;
> +	wdd->timeout		= timeout;
>  
> -		pr_info("watchdog started with initial timeout of %u sec\n",
> -			start_withtimeout);
> -	}
> +	watchdog_set_nowayout(wdd, nowayout);
> +	watchdog_stop_on_unregister(wdd);
> +	watchdog_stop_on_reboot(wdd);
> +	watchdog_init_timeout(wdd, start_withtimeout, NULL);
> +	watchdog_set_drvdata(wdd, wd);
>  
> -	return 0;
> +	if (wdt_conf & BIT(F71808FG_FLAG_WDTMOUT_STS))
> +		wdd->bootstatus	= WDIOF_CARDRESET;
>  
> -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
> +	 */
> +	watchdog_set_timeout(wdd, wdd->timeout);
>  
> -	return err;
> +	return watchdog_register_device(wdd);
>  }
>  
>  static void f71808fg_pinconf(struct watchdog_data *wd)
> @@ -812,8 +536,11 @@ static struct f71808e_variant __init *f71808e_find(int sioaddr)
>  	return variant;
>  }
>  
> +static struct watchdog_data watchdog;
> +
>  static int __init f71808e_init(void)
>  {
> +	struct watchdog_data *wd = &watchdog;
>  	static const unsigned short addrs[] = { 0x2e, 0x4e };
>  	struct f71808e_variant *variant;
>  	int i;
> @@ -831,19 +558,15 @@ static int __init f71808e_init(void)
>  	if (i == ARRAY_SIZE(addrs))
>  		return PTR_ERR(variant);
>  
> -	watchdog.variant = variant;
> +	wd->variant = variant;
> +	wd->sioaddr = addrs[i];
>  
> -	return watchdog_init(addrs[i]);
> +	return watchdog_init(wd);
>  }
>  
>  static void __exit f71808e_exit(void)
>  {
> -	if (watchdog_is_running()) {
> -		pr_warn("Watchdog timer still running, stopping it\n");
> -		watchdog_stop();
> -	}
> -	misc_deregister(&watchdog_miscdev);
> -	unregister_reboot_notifier(&watchdog_notifier);
> +	watchdog_unregister_device(&watchdog.wdd);
>  }
>  
>  MODULE_DESCRIPTION("F71808E Watchdog Driver");

[PATCH v1 8/8] 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_ instead.

This makes code browsing easier, because it's readily apparent whether a
function is variant-specific or not. Also the watchdog_ namespace isn't
used anymore for the driver-internal functions.

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

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index 7c42cbf9912e..c866d05e8788 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -114,18 +114,18 @@ 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_wdog_data;
 
-struct f71808e_variant {
+struct fintek_variant {
 	u16 id;
 	const char *wdt_name; /* NULL if chip lacks watchdog timer */
-	void (*pinconf)(struct watchdog_data *wd);
+	void (*pinconf)(struct fintek_wdog_data *wd);
 };
 
-struct watchdog_data {
+struct fintek_wdog_data {
 	struct watchdog_device wdd;
 	unsigned short	sioaddr;
-	const struct f71808e_variant *variant;
+	const struct fintek_variant *variant;
 	struct watchdog_info ident;
 
 	u8		timer_val;	/* content for the wd_time register */
@@ -134,7 +134,7 @@ struct watchdog_data {
 	char		pulse_mode;	/* enable pulse output mode? */
 };
 
-static inline bool has_f81865_wdo_conf(struct watchdog_data *wd)
+static inline bool has_f81865_wdo_conf(struct fintek_wdog_data *wd)
 {
 	return wd->variant->id == SIO_F81865_ID
 		|| wd->variant->id == SIO_F81866_ID;
@@ -202,9 +202,9 @@ static inline void superio_exit(int base)
 	release_region(base, 2);
 }
 
-static int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int new_timeout)
+static int fintek_wdog_set_timeout(struct watchdog_device *wdd, unsigned int new_timeout)
 {
-	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
+	struct fintek_wdog_data *wd = watchdog_get_drvdata(wdd);
 
 	wdd->timeout = new_timeout;
 	if (new_timeout > 0xff) {
@@ -218,7 +218,7 @@ static int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int new_ti
 	return 0;
 }
 
-static int watchdog_set_pulse_width(struct watchdog_data *wd, unsigned int pw)
+static int fintek_wdog_set_pulse_width(struct fintek_wdog_data *wd, unsigned int pw)
 {
 	unsigned int t1 = 25, t2 = 125, t3 = 5000;
 
@@ -246,9 +246,9 @@ static int watchdog_set_pulse_width(struct watchdog_data *wd, unsigned int pw)
 	return 0;
 }
 
-static int watchdog_keepalive(struct watchdog_device *wdd)
+static int fintek_wdog_keepalive(struct watchdog_device *wdd)
 {
-	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
+	struct fintek_wdog_data *wd = watchdog_get_drvdata(wdd);
 	int err;
 
 	err = superio_enter(wd->sioaddr);
@@ -274,13 +274,13 @@ static int watchdog_keepalive(struct watchdog_device *wdd)
 	return 0;
 }
 
-static int watchdog_start(struct watchdog_device *wdd)
+static int fintek_wdog_start(struct watchdog_device *wdd)
 {
-	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
+	struct fintek_wdog_data *wd = watchdog_get_drvdata(wdd);
 	int err;
 
 	/* Make sure we don't die as soon as the watchdog is enabled below */
-	err = watchdog_keepalive(wdd);
+	err = fintek_wdog_keepalive(wdd);
 	if (err)
 		return err;
 
@@ -328,9 +328,9 @@ static int watchdog_start(struct watchdog_device *wdd)
 	return err;
 }
 
-static int watchdog_stop(struct watchdog_device *wdd)
+static int fintek_wdog_stop(struct watchdog_device *wdd)
 {
-	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
+	struct fintek_wdog_data *wd = watchdog_get_drvdata(wdd);
 	int err;
 
 	err = superio_enter(wd->sioaddr);
@@ -346,21 +346,21 @@ static int watchdog_stop(struct watchdog_device *wdd)
 	return 0;
 }
 
-static const struct watchdog_ops f71808e_wdog_ops = {
+static const struct watchdog_ops fintek_wdog_ops = {
 	.owner = THIS_MODULE,
-	.start = watchdog_start,
-	.stop = watchdog_stop,
-	.ping = watchdog_keepalive,
-	.set_timeout = watchdog_set_timeout,
+	.start = fintek_wdog_start,
+	.stop = fintek_wdog_stop,
+	.ping = fintek_wdog_keepalive,
+	.set_timeout = fintek_wdog_set_timeout,
 };
 
-static bool watchdog_is_running(struct watchdog_data *wd, u8 wdt_conf)
+static bool fintek_wdog_is_running(struct fintek_wdog_data *wd, u8 wdt_conf)
 {
 	return (superio_inb(wd->sioaddr, SIO_REG_ENABLE) & BIT(0))
 		&& (wdt_conf & BIT(F71808FG_FLAG_WD_EN));
 }
 
-static int __init watchdog_init(struct watchdog_data *wd)
+static int __init fintek_wdog_init(struct fintek_wdog_data *wd)
 {
 	struct watchdog_device *wdd = &wd->wdd;
 	int wdt_conf, err = 0;
@@ -390,17 +390,17 @@ static int __init watchdog_init(struct watchdog_data *wd)
 	superio_outb(wd->sioaddr, F71808FG_REG_WDT_CONF,
 		     wdt_conf | BIT(F71808FG_FLAG_WDTMOUT_STS));
 
-	if (watchdog_is_running(wd, wdt_conf))
+	if (fintek_wdog_is_running(wd, wdt_conf))
 		set_bit(WDOG_HW_RUNNING, &wdd->status);
 
 	superio_exit(wd->sioaddr);
 
-	err = watchdog_set_pulse_width(wd, pulse_width);
+	err = fintek_wdog_set_pulse_width(wd, pulse_width);
 	if (err)
 		return err;
 
 	wdd->info		= &wd->ident;
-	wdd->ops		= &f71808e_wdog_ops;
+	wdd->ops		= &fintek_wdog_ops;
 	wdd->min_timeout	= 1;
 	wdd->max_timeout	= max_timeout;
 	wdd->timeout		= timeout;
@@ -418,18 +418,18 @@ static int __init watchdog_init(struct watchdog_data *wd)
 	 * WATCHDOG_HANDLE_BOOT_ENABLED can result in keepalive being directly
 	 * called without a set_timeout before, so it needs to be done here once
 	 */
-	watchdog_set_timeout(wdd, wdd->timeout);
+	fintek_wdog_set_timeout(wdd, wdd->timeout);
 
 	return watchdog_register_device(wdd);
 }
 
-static void f71808fg_pinconf(struct watchdog_data *wd)
+static void f71808fg_pinconf(struct fintek_wdog_data *wd)
 {
 	/* Set pin 21 to GPIO23/WDTRST#, then to WDTRST# */
 	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT2, 3);
 	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 3);
 }
-static void f71862fg_pinconf(struct watchdog_data *wd)
+static void f71862fg_pinconf(struct fintek_wdog_data *wd)
 {
 	if (f71862fg_pin == 63) {
 		/* SPI must be disabled first to use this pin! */
@@ -439,23 +439,23 @@ static void f71862fg_pinconf(struct watchdog_data *wd)
 		superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT1, 1);
 	}
 }
-static void f71868_pinconf(struct watchdog_data *wd)
+static void f71868_pinconf(struct fintek_wdog_data *wd)
 {
 	/* GPIO14 --> WDTRST# */
 	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT1, 4);
 }
-static void f71882fg_pinconf(struct watchdog_data *wd)
+static void f71882fg_pinconf(struct fintek_wdog_data *wd)
 {
 	/* Set pin 56 to WDTRST# */
 	superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT1, 1);
 }
-static void f71889fg_pinconf(struct watchdog_data *wd)
+static void f71889fg_pinconf(struct fintek_wdog_data *wd)
 {
 	/* set pin 40 to WDTRST# */
 	superio_outb(wd->sioaddr, SIO_REG_MFUNCT3,
 		     superio_inb(wd->sioaddr, SIO_REG_MFUNCT3) & 0xcf);
 }
-static void f81803_pinconf(struct watchdog_data *wd)
+static void f81803_pinconf(struct fintek_wdog_data *wd)
 {
 	/* Enable TSI Level register bank */
 	superio_clear_bit(wd->sioaddr, SIO_REG_CLOCK_SEL, 3);
@@ -463,12 +463,12 @@ static void f81803_pinconf(struct watchdog_data *wd)
 	superio_outb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL, 0x5f &
 		     superio_inb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL));
 }
-static void f81865_pinconf(struct watchdog_data *wd)
+static void f81865_pinconf(struct fintek_wdog_data *wd)
 {
 	/* Set pin 70 to WDTRST# */
 	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 5);
 }
-static void f81866_pinconf(struct watchdog_data *wd)
+static void f81866_pinconf(struct fintek_wdog_data *wd)
 {
 	/*
 	 * GPIO1 Control Register when 27h BIT3:2 = 01 & BIT0 = 0.
@@ -484,7 +484,7 @@ static void f81866_pinconf(struct watchdog_data *wd)
 	superio_clear_bit(wd->sioaddr, SIO_F81866_REG_GPIO1, 5);
 }
 
-struct f71808e_variant f71808e_variants[] = {
+struct fintek_variant fintek_variants[] = {
 	{ SIO_F71808_ID,  "f71808fg", f71808fg_pinconf },
 	{ SIO_F71862_ID,  "f71862fg", f71862fg_pinconf },
 	{ SIO_F71868_ID,  "f71868",   f71868_pinconf },
@@ -500,9 +500,9 @@ struct f71808e_variant f71808e_variants[] = {
 	{ /* sentinel */ }
 };
 
-static struct f71808e_variant __init *f71808e_find(int sioaddr)
+static struct fintek_variant __init *fintek_wdog_find(int sioaddr)
 {
-	struct f71808e_variant *variant;
+	struct fintek_variant *variant;
 	u16 devid;
 	int err = superio_enter(sioaddr);
 	if (err)
@@ -516,7 +516,7 @@ static struct f71808e_variant __init *f71808e_find(int sioaddr)
 	}
 
 	devid = force_id ? force_id : superio_inw(sioaddr, SIO_REG_DEVID);
-	for (variant = f71808e_variants; variant->id; variant++) {
+	for (variant = fintek_variants; variant->id; variant++) {
 		if (variant->id == devid)
 			break;
 	}
@@ -536,13 +536,13 @@ static struct f71808e_variant __init *f71808e_find(int sioaddr)
 	return variant;
 }
 
-static struct watchdog_data watchdog;
+static struct fintek_wdog_data watchdog;
 
-static int __init f71808e_init(void)
+static int __init fintek_wdog_probe(void)
 {
-	struct watchdog_data *wd = &watchdog;
+	struct fintek_wdog_data *wd = &watchdog;
 	static const unsigned short addrs[] = { 0x2e, 0x4e };
-	struct f71808e_variant *variant;
+	struct fintek_variant *variant;
 	int i;
 
 	if (f71862fg_pin != 63 && f71862fg_pin != 56) {
@@ -551,7 +551,7 @@ static int __init f71808e_init(void)
 	}
 
 	for (i = 0; i < ARRAY_SIZE(addrs); i++) {
-		variant = f71808e_find(addrs[i]);
+		variant = fintek_wdog_find(addrs[i]);
 		if (!IS_ERR(variant))
 			break;
 	}
@@ -561,17 +561,17 @@ static int __init f71808e_init(void)
 	wd->variant = variant;
 	wd->sioaddr = addrs[i];
 
-	return watchdog_init(wd);
+	return fintek_wdog_init(wd);
 }
 
-static void __exit f71808e_exit(void)
+static void __exit fintek_wdog_exit(void)
 {
 	watchdog_unregister_device(&watchdog.wdd);
 }
 
-MODULE_DESCRIPTION("F71808E Watchdog Driver");
+MODULE_DESCRIPTION("Fintek 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_wdog_probe);
+module_exit(fintek_wdog_exit);
-- 
2.27.0

Re: [PATCH v1 8/8] watchdog: f71808e_wdt: rename variant-independent identifiers appropriately

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 1408 bytes --]

Hi Ahmad,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on linus/master linux/master v5.7 next-20200611]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Ahmad-Fatoum/watchdog-f71808e_wdt-migrate-to-kernel/20200612-093801
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: i386-randconfig-s001-20200612 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-250-g42323db3-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 ARCH=i386 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> drivers/watchdog/f71808e_wdt.c:487:23: sparse: sparse: symbol 'fintek_variants' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 26451 bytes --]

[RFC PATCH] watchdog: f71808e_wdt: fintek_variants[] can be static

[kernel test robot]

Signed-off-by: kernel test robot <lkp@intel.com>
---
 f71808e_wdt.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index c866d05e8788b..849620041c0ef 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -484,7 +484,7 @@ static void f81866_pinconf(struct fintek_wdog_data *wd)
 	superio_clear_bit(wd->sioaddr, SIO_F81866_REG_GPIO1, 5);
 }
 
-struct fintek_variant fintek_variants[] = {
+static struct fintek_variant fintek_variants[] = {
 	{ SIO_F71808_ID,  "f71808fg", f71808fg_pinconf },
 	{ SIO_F71862_ID,  "f71862fg", f71862fg_pinconf },
 	{ SIO_F71868_ID,  "f71868",   f71868_pinconf },

Re: [PATCH v1 8/8] watchdog: f71808e_wdt: rename variant-independent identifiers appropriately

[Guenter Roeck]

On Thu, Jun 11, 2020 at 09:17:49PM +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_ instead.
> 
> This makes code browsing easier, because it's readily apparent whether a
> function is variant-specific or not. Also the watchdog_ namespace isn't
> used anymore for the driver-internal functions.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Reported-by: kernel test robot <lkp@intel.com>
> ---
>  drivers/watchdog/f71808e_wdt.c | 98 +++++++++++++++++-----------------
>  1 file changed, 49 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
> index 7c42cbf9912e..c866d05e8788 100644
> --- a/drivers/watchdog/f71808e_wdt.c
> +++ b/drivers/watchdog/f71808e_wdt.c
> @@ -114,18 +114,18 @@ 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_wdog_data;
>  
> -struct f71808e_variant {
> +struct fintek_variant {
>  	u16 id;
>  	const char *wdt_name; /* NULL if chip lacks watchdog timer */
> -	void (*pinconf)(struct watchdog_data *wd);
> +	void (*pinconf)(struct fintek_wdog_data *wd);
>  };
>  
> -struct watchdog_data {
> +struct fintek_wdog_data {
>  	struct watchdog_device wdd;
>  	unsigned short	sioaddr;
> -	const struct f71808e_variant *variant;
> +	const struct fintek_variant *variant;
>  	struct watchdog_info ident;
>  
>  	u8		timer_val;	/* content for the wd_time register */
> @@ -134,7 +134,7 @@ struct watchdog_data {
>  	char		pulse_mode;	/* enable pulse output mode? */
>  };
>  
> -static inline bool has_f81865_wdo_conf(struct watchdog_data *wd)
> +static inline bool has_f81865_wdo_conf(struct fintek_wdog_data *wd)
>  {
>  	return wd->variant->id == SIO_F81865_ID
>  		|| wd->variant->id == SIO_F81866_ID;
> @@ -202,9 +202,9 @@ static inline void superio_exit(int base)
>  	release_region(base, 2);
>  }
>  
> -static int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int new_timeout)
> +static int fintek_wdog_set_timeout(struct watchdog_device *wdd, unsigned int new_timeout)
>  {
> -	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
> +	struct fintek_wdog_data *wd = watchdog_get_drvdata(wdd);
>  
>  	wdd->timeout = new_timeout;
>  	if (new_timeout > 0xff) {
> @@ -218,7 +218,7 @@ static int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int new_ti
>  	return 0;
>  }
>  
> -static int watchdog_set_pulse_width(struct watchdog_data *wd, unsigned int pw)
> +static int fintek_wdog_set_pulse_width(struct fintek_wdog_data *wd, unsigned int pw)
>  {
>  	unsigned int t1 = 25, t2 = 125, t3 = 5000;
>  
> @@ -246,9 +246,9 @@ static int watchdog_set_pulse_width(struct watchdog_data *wd, unsigned int pw)
>  	return 0;
>  }
>  
> -static int watchdog_keepalive(struct watchdog_device *wdd)
> +static int fintek_wdog_keepalive(struct watchdog_device *wdd)
>  {
> -	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
> +	struct fintek_wdog_data *wd = watchdog_get_drvdata(wdd);
>  	int err;
>  
>  	err = superio_enter(wd->sioaddr);
> @@ -274,13 +274,13 @@ static int watchdog_keepalive(struct watchdog_device *wdd)
>  	return 0;
>  }
>  
> -static int watchdog_start(struct watchdog_device *wdd)
> +static int fintek_wdog_start(struct watchdog_device *wdd)
>  {
> -	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
> +	struct fintek_wdog_data *wd = watchdog_get_drvdata(wdd);
>  	int err;
>  
>  	/* Make sure we don't die as soon as the watchdog is enabled below */
> -	err = watchdog_keepalive(wdd);
> +	err = fintek_wdog_keepalive(wdd);
>  	if (err)
>  		return err;
>  
> @@ -328,9 +328,9 @@ static int watchdog_start(struct watchdog_device *wdd)
>  	return err;
>  }
>  
> -static int watchdog_stop(struct watchdog_device *wdd)
> +static int fintek_wdog_stop(struct watchdog_device *wdd)
>  {
> -	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
> +	struct fintek_wdog_data *wd = watchdog_get_drvdata(wdd);
>  	int err;
>  
>  	err = superio_enter(wd->sioaddr);
> @@ -346,21 +346,21 @@ static int watchdog_stop(struct watchdog_device *wdd)
>  	return 0;
>  }
>  
> -static const struct watchdog_ops f71808e_wdog_ops = {
> +static const struct watchdog_ops fintek_wdog_ops = {
>  	.owner = THIS_MODULE,
> -	.start = watchdog_start,
> -	.stop = watchdog_stop,
> -	.ping = watchdog_keepalive,
> -	.set_timeout = watchdog_set_timeout,
> +	.start = fintek_wdog_start,
> +	.stop = fintek_wdog_stop,
> +	.ping = fintek_wdog_keepalive,
> +	.set_timeout = fintek_wdog_set_timeout,
>  };
>  
> -static bool watchdog_is_running(struct watchdog_data *wd, u8 wdt_conf)
> +static bool fintek_wdog_is_running(struct fintek_wdog_data *wd, u8 wdt_conf)
>  {
>  	return (superio_inb(wd->sioaddr, SIO_REG_ENABLE) & BIT(0))
>  		&& (wdt_conf & BIT(F71808FG_FLAG_WD_EN));
>  }
>  
> -static int __init watchdog_init(struct watchdog_data *wd)
> +static int __init fintek_wdog_init(struct fintek_wdog_data *wd)
>  {
>  	struct watchdog_device *wdd = &wd->wdd;
>  	int wdt_conf, err = 0;
> @@ -390,17 +390,17 @@ static int __init watchdog_init(struct watchdog_data *wd)
>  	superio_outb(wd->sioaddr, F71808FG_REG_WDT_CONF,
>  		     wdt_conf | BIT(F71808FG_FLAG_WDTMOUT_STS));
>  
> -	if (watchdog_is_running(wd, wdt_conf))
> +	if (fintek_wdog_is_running(wd, wdt_conf))
>  		set_bit(WDOG_HW_RUNNING, &wdd->status);
>  
>  	superio_exit(wd->sioaddr);
>  
> -	err = watchdog_set_pulse_width(wd, pulse_width);
> +	err = fintek_wdog_set_pulse_width(wd, pulse_width);
>  	if (err)
>  		return err;
>  
>  	wdd->info		= &wd->ident;
> -	wdd->ops		= &f71808e_wdog_ops;
> +	wdd->ops		= &fintek_wdog_ops;
>  	wdd->min_timeout	= 1;
>  	wdd->max_timeout	= max_timeout;
>  	wdd->timeout		= timeout;
> @@ -418,18 +418,18 @@ static int __init watchdog_init(struct watchdog_data *wd)
>  	 * WATCHDOG_HANDLE_BOOT_ENABLED can result in keepalive being directly
>  	 * called without a set_timeout before, so it needs to be done here once
>  	 */
> -	watchdog_set_timeout(wdd, wdd->timeout);
> +	fintek_wdog_set_timeout(wdd, wdd->timeout);
>  
>  	return watchdog_register_device(wdd);
>  }
>  
> -static void f71808fg_pinconf(struct watchdog_data *wd)
> +static void f71808fg_pinconf(struct fintek_wdog_data *wd)
>  {
>  	/* Set pin 21 to GPIO23/WDTRST#, then to WDTRST# */
>  	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT2, 3);
>  	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 3);
>  }
> -static void f71862fg_pinconf(struct watchdog_data *wd)
> +static void f71862fg_pinconf(struct fintek_wdog_data *wd)
>  {
>  	if (f71862fg_pin == 63) {
>  		/* SPI must be disabled first to use this pin! */
> @@ -439,23 +439,23 @@ static void f71862fg_pinconf(struct watchdog_data *wd)
>  		superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT1, 1);
>  	}
>  }
> -static void f71868_pinconf(struct watchdog_data *wd)
> +static void f71868_pinconf(struct fintek_wdog_data *wd)
>  {
>  	/* GPIO14 --> WDTRST# */
>  	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT1, 4);
>  }
> -static void f71882fg_pinconf(struct watchdog_data *wd)
> +static void f71882fg_pinconf(struct fintek_wdog_data *wd)
>  {
>  	/* Set pin 56 to WDTRST# */
>  	superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT1, 1);
>  }
> -static void f71889fg_pinconf(struct watchdog_data *wd)
> +static void f71889fg_pinconf(struct fintek_wdog_data *wd)
>  {
>  	/* set pin 40 to WDTRST# */
>  	superio_outb(wd->sioaddr, SIO_REG_MFUNCT3,
>  		     superio_inb(wd->sioaddr, SIO_REG_MFUNCT3) & 0xcf);
>  }
> -static void f81803_pinconf(struct watchdog_data *wd)
> +static void f81803_pinconf(struct fintek_wdog_data *wd)
>  {
>  	/* Enable TSI Level register bank */
>  	superio_clear_bit(wd->sioaddr, SIO_REG_CLOCK_SEL, 3);
> @@ -463,12 +463,12 @@ static void f81803_pinconf(struct watchdog_data *wd)
>  	superio_outb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL, 0x5f &
>  		     superio_inb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL));
>  }
> -static void f81865_pinconf(struct watchdog_data *wd)
> +static void f81865_pinconf(struct fintek_wdog_data *wd)
>  {
>  	/* Set pin 70 to WDTRST# */
>  	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 5);
>  }
> -static void f81866_pinconf(struct watchdog_data *wd)
> +static void f81866_pinconf(struct fintek_wdog_data *wd)
>  {
>  	/*
>  	 * GPIO1 Control Register when 27h BIT3:2 = 01 & BIT0 = 0.
> @@ -484,7 +484,7 @@ static void f81866_pinconf(struct watchdog_data *wd)
>  	superio_clear_bit(wd->sioaddr, SIO_F81866_REG_GPIO1, 5);
>  }
>  
> -struct f71808e_variant f71808e_variants[] = {
> +struct fintek_variant fintek_variants[] = {

0-day has a point here. Granted, that is inherited, but still ...

>  	{ SIO_F71808_ID,  "f71808fg", f71808fg_pinconf },
>  	{ SIO_F71862_ID,  "f71862fg", f71862fg_pinconf },
>  	{ SIO_F71868_ID,  "f71868",   f71868_pinconf },
> @@ -500,9 +500,9 @@ struct f71808e_variant f71808e_variants[] = {
>  	{ /* sentinel */ }
>  };
>  
> -static struct f71808e_variant __init *f71808e_find(int sioaddr)
> +static struct fintek_variant __init *fintek_wdog_find(int sioaddr)
>  {
> -	struct f71808e_variant *variant;
> +	struct fintek_variant *variant;
>  	u16 devid;
>  	int err = superio_enter(sioaddr);
>  	if (err)
> @@ -516,7 +516,7 @@ static struct f71808e_variant __init *f71808e_find(int sioaddr)
>  	}
>  
>  	devid = force_id ? force_id : superio_inw(sioaddr, SIO_REG_DEVID);
> -	for (variant = f71808e_variants; variant->id; variant++) {
> +	for (variant = fintek_variants; variant->id; variant++) {
>  		if (variant->id == devid)
>  			break;
>  	}
> @@ -536,13 +536,13 @@ static struct f71808e_variant __init *f71808e_find(int sioaddr)
>  	return variant;
>  }
>  
> -static struct watchdog_data watchdog;
> +static struct fintek_wdog_data watchdog;
>  
> -static int __init f71808e_init(void)
> +static int __init fintek_wdog_probe(void)
>  {
> -	struct watchdog_data *wd = &watchdog;
> +	struct fintek_wdog_data *wd = &watchdog;
>  	static const unsigned short addrs[] = { 0x2e, 0x4e };
> -	struct f71808e_variant *variant;
> +	struct fintek_variant *variant;
>  	int i;
>  
>  	if (f71862fg_pin != 63 && f71862fg_pin != 56) {
> @@ -551,7 +551,7 @@ static int __init f71808e_init(void)
>  	}
>  
>  	for (i = 0; i < ARRAY_SIZE(addrs); i++) {
> -		variant = f71808e_find(addrs[i]);
> +		variant = fintek_wdog_find(addrs[i]);
>  		if (!IS_ERR(variant))
>  			break;
>  	}
> @@ -561,17 +561,17 @@ static int __init f71808e_init(void)
>  	wd->variant = variant;
>  	wd->sioaddr = addrs[i];
>  
> -	return watchdog_init(wd);
> +	return fintek_wdog_init(wd);
>  }
>  
> -static void __exit f71808e_exit(void)
> +static void __exit fintek_wdog_exit(void)
>  {
>  	watchdog_unregister_device(&watchdog.wdd);
>  }
>  
> -MODULE_DESCRIPTION("F71808E Watchdog Driver");
> +MODULE_DESCRIPTION("Fintek 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_wdog_probe);
> +module_exit(fintek_wdog_exit);

Re: [PATCH v1 8/8] watchdog: f71808e_wdt: rename variant-independent identifiers appropriately

[Ahmad Fatoum]

On 6/30/20 11:29 PM, Guenter Roeck wrote:
> On Thu, Jun 11, 2020 at 09:17:49PM +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_ instead.
>>
>> This makes code browsing easier, because it's readily apparent whether a
>> function is variant-specific or not. Also the watchdog_ namespace isn't
>> used anymore for the driver-internal functions.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> Reported-by: kernel test robot <lkp@intel.com>
>> ---
>>  drivers/watchdog/f71808e_wdt.c | 98 +++++++++++++++++-----------------
>>  1 file changed, 49 insertions(+), 49 deletions(-)
>>
>> diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
>> index 7c42cbf9912e..c866d05e8788 100644
>> --- a/drivers/watchdog/f71808e_wdt.c
>> +++ b/drivers/watchdog/f71808e_wdt.c
>> @@ -114,18 +114,18 @@ 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_wdog_data;
>>  
>> -struct f71808e_variant {
>> +struct fintek_variant {
>>  	u16 id;
>>  	const char *wdt_name; /* NULL if chip lacks watchdog timer */
>> -	void (*pinconf)(struct watchdog_data *wd);
>> +	void (*pinconf)(struct fintek_wdog_data *wd);
>>  };
>>  
>> -struct watchdog_data {
>> +struct fintek_wdog_data {
>>  	struct watchdog_device wdd;
>>  	unsigned short	sioaddr;
>> -	const struct f71808e_variant *variant;
>> +	const struct fintek_variant *variant;
>>  	struct watchdog_info ident;
>>  
>>  	u8		timer_val;	/* content for the wd_time register */
>> @@ -134,7 +134,7 @@ struct watchdog_data {
>>  	char		pulse_mode;	/* enable pulse output mode? */
>>  };
>>  
>> -static inline bool has_f81865_wdo_conf(struct watchdog_data *wd)
>> +static inline bool has_f81865_wdo_conf(struct fintek_wdog_data *wd)
>>  {
>>  	return wd->variant->id == SIO_F81865_ID
>>  		|| wd->variant->id == SIO_F81866_ID;
>> @@ -202,9 +202,9 @@ static inline void superio_exit(int base)
>>  	release_region(base, 2);
>>  }
>>  
>> -static int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int new_timeout)
>> +static int fintek_wdog_set_timeout(struct watchdog_device *wdd, unsigned int new_timeout)
>>  {
>> -	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
>> +	struct fintek_wdog_data *wd = watchdog_get_drvdata(wdd);
>>  
>>  	wdd->timeout = new_timeout;
>>  	if (new_timeout > 0xff) {
>> @@ -218,7 +218,7 @@ static int watchdog_set_timeout(struct watchdog_device *wdd, unsigned int new_ti
>>  	return 0;
>>  }
>>  
>> -static int watchdog_set_pulse_width(struct watchdog_data *wd, unsigned int pw)
>> +static int fintek_wdog_set_pulse_width(struct fintek_wdog_data *wd, unsigned int pw)
>>  {
>>  	unsigned int t1 = 25, t2 = 125, t3 = 5000;
>>  
>> @@ -246,9 +246,9 @@ static int watchdog_set_pulse_width(struct watchdog_data *wd, unsigned int pw)
>>  	return 0;
>>  }
>>  
>> -static int watchdog_keepalive(struct watchdog_device *wdd)
>> +static int fintek_wdog_keepalive(struct watchdog_device *wdd)
>>  {
>> -	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
>> +	struct fintek_wdog_data *wd = watchdog_get_drvdata(wdd);
>>  	int err;
>>  
>>  	err = superio_enter(wd->sioaddr);
>> @@ -274,13 +274,13 @@ static int watchdog_keepalive(struct watchdog_device *wdd)
>>  	return 0;
>>  }
>>  
>> -static int watchdog_start(struct watchdog_device *wdd)
>> +static int fintek_wdog_start(struct watchdog_device *wdd)
>>  {
>> -	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
>> +	struct fintek_wdog_data *wd = watchdog_get_drvdata(wdd);
>>  	int err;
>>  
>>  	/* Make sure we don't die as soon as the watchdog is enabled below */
>> -	err = watchdog_keepalive(wdd);
>> +	err = fintek_wdog_keepalive(wdd);
>>  	if (err)
>>  		return err;
>>  
>> @@ -328,9 +328,9 @@ static int watchdog_start(struct watchdog_device *wdd)
>>  	return err;
>>  }
>>  
>> -static int watchdog_stop(struct watchdog_device *wdd)
>> +static int fintek_wdog_stop(struct watchdog_device *wdd)
>>  {
>> -	struct watchdog_data *wd = watchdog_get_drvdata(wdd);
>> +	struct fintek_wdog_data *wd = watchdog_get_drvdata(wdd);
>>  	int err;
>>  
>>  	err = superio_enter(wd->sioaddr);
>> @@ -346,21 +346,21 @@ static int watchdog_stop(struct watchdog_device *wdd)
>>  	return 0;
>>  }
>>  
>> -static const struct watchdog_ops f71808e_wdog_ops = {
>> +static const struct watchdog_ops fintek_wdog_ops = {
>>  	.owner = THIS_MODULE,
>> -	.start = watchdog_start,
>> -	.stop = watchdog_stop,
>> -	.ping = watchdog_keepalive,
>> -	.set_timeout = watchdog_set_timeout,
>> +	.start = fintek_wdog_start,
>> +	.stop = fintek_wdog_stop,
>> +	.ping = fintek_wdog_keepalive,
>> +	.set_timeout = fintek_wdog_set_timeout,
>>  };
>>  
>> -static bool watchdog_is_running(struct watchdog_data *wd, u8 wdt_conf)
>> +static bool fintek_wdog_is_running(struct fintek_wdog_data *wd, u8 wdt_conf)
>>  {
>>  	return (superio_inb(wd->sioaddr, SIO_REG_ENABLE) & BIT(0))
>>  		&& (wdt_conf & BIT(F71808FG_FLAG_WD_EN));
>>  }
>>  
>> -static int __init watchdog_init(struct watchdog_data *wd)
>> +static int __init fintek_wdog_init(struct fintek_wdog_data *wd)
>>  {
>>  	struct watchdog_device *wdd = &wd->wdd;
>>  	int wdt_conf, err = 0;
>> @@ -390,17 +390,17 @@ static int __init watchdog_init(struct watchdog_data *wd)
>>  	superio_outb(wd->sioaddr, F71808FG_REG_WDT_CONF,
>>  		     wdt_conf | BIT(F71808FG_FLAG_WDTMOUT_STS));
>>  
>> -	if (watchdog_is_running(wd, wdt_conf))
>> +	if (fintek_wdog_is_running(wd, wdt_conf))
>>  		set_bit(WDOG_HW_RUNNING, &wdd->status);
>>  
>>  	superio_exit(wd->sioaddr);
>>  
>> -	err = watchdog_set_pulse_width(wd, pulse_width);
>> +	err = fintek_wdog_set_pulse_width(wd, pulse_width);
>>  	if (err)
>>  		return err;
>>  
>>  	wdd->info		= &wd->ident;
>> -	wdd->ops		= &f71808e_wdog_ops;
>> +	wdd->ops		= &fintek_wdog_ops;
>>  	wdd->min_timeout	= 1;
>>  	wdd->max_timeout	= max_timeout;
>>  	wdd->timeout		= timeout;
>> @@ -418,18 +418,18 @@ static int __init watchdog_init(struct watchdog_data *wd)
>>  	 * WATCHDOG_HANDLE_BOOT_ENABLED can result in keepalive being directly
>>  	 * called without a set_timeout before, so it needs to be done here once
>>  	 */
>> -	watchdog_set_timeout(wdd, wdd->timeout);
>> +	fintek_wdog_set_timeout(wdd, wdd->timeout);
>>  
>>  	return watchdog_register_device(wdd);
>>  }
>>  
>> -static void f71808fg_pinconf(struct watchdog_data *wd)
>> +static void f71808fg_pinconf(struct fintek_wdog_data *wd)
>>  {
>>  	/* Set pin 21 to GPIO23/WDTRST#, then to WDTRST# */
>>  	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT2, 3);
>>  	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 3);
>>  }
>> -static void f71862fg_pinconf(struct watchdog_data *wd)
>> +static void f71862fg_pinconf(struct fintek_wdog_data *wd)
>>  {
>>  	if (f71862fg_pin == 63) {
>>  		/* SPI must be disabled first to use this pin! */
>> @@ -439,23 +439,23 @@ static void f71862fg_pinconf(struct watchdog_data *wd)
>>  		superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT1, 1);
>>  	}
>>  }
>> -static void f71868_pinconf(struct watchdog_data *wd)
>> +static void f71868_pinconf(struct fintek_wdog_data *wd)
>>  {
>>  	/* GPIO14 --> WDTRST# */
>>  	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT1, 4);
>>  }
>> -static void f71882fg_pinconf(struct watchdog_data *wd)
>> +static void f71882fg_pinconf(struct fintek_wdog_data *wd)
>>  {
>>  	/* Set pin 56 to WDTRST# */
>>  	superio_set_bit(wd->sioaddr, SIO_REG_MFUNCT1, 1);
>>  }
>> -static void f71889fg_pinconf(struct watchdog_data *wd)
>> +static void f71889fg_pinconf(struct fintek_wdog_data *wd)
>>  {
>>  	/* set pin 40 to WDTRST# */
>>  	superio_outb(wd->sioaddr, SIO_REG_MFUNCT3,
>>  		     superio_inb(wd->sioaddr, SIO_REG_MFUNCT3) & 0xcf);
>>  }
>> -static void f81803_pinconf(struct watchdog_data *wd)
>> +static void f81803_pinconf(struct fintek_wdog_data *wd)
>>  {
>>  	/* Enable TSI Level register bank */
>>  	superio_clear_bit(wd->sioaddr, SIO_REG_CLOCK_SEL, 3);
>> @@ -463,12 +463,12 @@ static void f81803_pinconf(struct watchdog_data *wd)
>>  	superio_outb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL, 0x5f &
>>  		     superio_inb(wd->sioaddr, SIO_REG_TSI_LEVEL_SEL));
>>  }
>> -static void f81865_pinconf(struct watchdog_data *wd)
>> +static void f81865_pinconf(struct fintek_wdog_data *wd)
>>  {
>>  	/* Set pin 70 to WDTRST# */
>>  	superio_clear_bit(wd->sioaddr, SIO_REG_MFUNCT3, 5);
>>  }
>> -static void f81866_pinconf(struct watchdog_data *wd)
>> +static void f81866_pinconf(struct fintek_wdog_data *wd)
>>  {
>>  	/*
>>  	 * GPIO1 Control Register when 27h BIT3:2 = 01 & BIT0 = 0.
>> @@ -484,7 +484,7 @@ static void f81866_pinconf(struct watchdog_data *wd)
>>  	superio_clear_bit(wd->sioaddr, SIO_F81866_REG_GPIO1, 5);
>>  }
>>  
>> -struct f71808e_variant f71808e_variants[] = {
>> +struct fintek_variant fintek_variants[] = {
> 
> 0-day has a point here. Granted, that is inherited, but still ...

Ye, will fix in v2. Was just waiting for more review.

Thanks,
Ahmad

> 
>>  	{ SIO_F71808_ID,  "f71808fg", f71808fg_pinconf },
>>  	{ SIO_F71862_ID,  "f71862fg", f71862fg_pinconf },
>>  	{ SIO_F71868_ID,  "f71868",   f71868_pinconf },
>> @@ -500,9 +500,9 @@ struct f71808e_variant f71808e_variants[] = {
>>  	{ /* sentinel */ }
>>  };
>>  
>> -static struct f71808e_variant __init *f71808e_find(int sioaddr)
>> +static struct fintek_variant __init *fintek_wdog_find(int sioaddr)
>>  {
>> -	struct f71808e_variant *variant;
>> +	struct fintek_variant *variant;
>>  	u16 devid;
>>  	int err = superio_enter(sioaddr);
>>  	if (err)
>> @@ -516,7 +516,7 @@ static struct f71808e_variant __init *f71808e_find(int sioaddr)
>>  	}
>>  
>>  	devid = force_id ? force_id : superio_inw(sioaddr, SIO_REG_DEVID);
>> -	for (variant = f71808e_variants; variant->id; variant++) {
>> +	for (variant = fintek_variants; variant->id; variant++) {
>>  		if (variant->id == devid)
>>  			break;
>>  	}
>> @@ -536,13 +536,13 @@ static struct f71808e_variant __init *f71808e_find(int sioaddr)
>>  	return variant;
>>  }
>>  
>> -static struct watchdog_data watchdog;
>> +static struct fintek_wdog_data watchdog;
>>  
>> -static int __init f71808e_init(void)
>> +static int __init fintek_wdog_probe(void)
>>  {
>> -	struct watchdog_data *wd = &watchdog;
>> +	struct fintek_wdog_data *wd = &watchdog;
>>  	static const unsigned short addrs[] = { 0x2e, 0x4e };
>> -	struct f71808e_variant *variant;
>> +	struct fintek_variant *variant;
>>  	int i;
>>  
>>  	if (f71862fg_pin != 63 && f71862fg_pin != 56) {
>> @@ -551,7 +551,7 @@ static int __init f71808e_init(void)
>>  	}
>>  
>>  	for (i = 0; i < ARRAY_SIZE(addrs); i++) {
>> -		variant = f71808e_find(addrs[i]);
>> +		variant = fintek_wdog_find(addrs[i]);
>>  		if (!IS_ERR(variant))
>>  			break;
>>  	}
>> @@ -561,17 +561,17 @@ static int __init f71808e_init(void)
>>  	wd->variant = variant;
>>  	wd->sioaddr = addrs[i];
>>  
>> -	return watchdog_init(wd);
>> +	return fintek_wdog_init(wd);
>>  }
>>  
>> -static void __exit f71808e_exit(void)
>> +static void __exit fintek_wdog_exit(void)
>>  {
>>  	watchdog_unregister_device(&watchdog.wdd);
>>  }
>>  
>> -MODULE_DESCRIPTION("F71808E Watchdog Driver");
>> +MODULE_DESCRIPTION("Fintek 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_wdog_probe);
>> +module_exit(fintek_wdog_exit);
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |