[PATCH V2 1/2] watchdog: f71808e_wdt: separate declaration and assignment

[PATCH V2 1/2] watchdog: f71808e_wdt: separate declaration and assignment

[Ji-Ze Hong (Peter Hong)]

Separate declaration and assignment in watchdog_start()

Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
---
 drivers/watchdog/f71808e_wdt.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index 9a1c761258ce..bd2ced9f39f4 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -338,8 +338,10 @@ static int f71862fg_pin_configure(unsigned short ioaddr)
 
 static int watchdog_start(void)
 {
+	int err;
+
 	/* Make sure we don't die as soon as the watchdog is enabled below */
-	int err = watchdog_keepalive();
+	err = watchdog_keepalive();
 	if (err)
 		return err;
 
-- 
2.7.4

[PATCH V2 2/2] watchdog: f71808e_wdt: fix F81866 bit operation

[Ji-Ze Hong (Peter Hong)]

Fix error bit operation in watchdog_start()

Fixes: 14b24a88a3660 ("watchdog: f71808e_wdt: Add F81866 support")
Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>
---
 drivers/watchdog/f71808e_wdt.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
index bd2ced9f39f4..afd1446241b3 100644
--- a/drivers/watchdog/f71808e_wdt.c
+++ b/drivers/watchdog/f71808e_wdt.c
@@ -339,6 +339,7 @@ static int f71862fg_pin_configure(unsigned short ioaddr)
 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();
@@ -388,19 +389,18 @@ static int watchdog_start(void)
 		break;
 
 	case f81866:
-		/* Set pin 70 to WDTRST# */
-		superio_clear_bit(watchdog.sioaddr, SIO_F81866_REG_PORT_SEL,
-				  BIT(3) | BIT(0));
-		superio_set_bit(watchdog.sioaddr, SIO_F81866_REG_PORT_SEL,
-				BIT(2));
 		/*
 		 * GPIO1 Control Register when 27h BIT3:2 = 01 & BIT0 = 0.
 		 * The PIN 70(GPIO15/WDTRST) is controlled by 2Ch:
 		 *     BIT5: 0 -> WDTRST#
 		 *           1 -> GPIO15
 		 */
-		superio_clear_bit(watchdog.sioaddr, SIO_F81866_REG_GPIO1,
-				  BIT(5));
+		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:
-- 
2.7.4

Re: [PATCH V2 1/2] watchdog: f71808e_wdt: separate declaration and assignment

[Guenter Roeck]

On Wed, Mar 27, 2019 at 02:42:50PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Separate declaration and assignment in watchdog_start()
> 
> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>

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

Note though that it would be much more valuable if you could consider
converting the driver to use the watchdog subsystem instead of trying
to clean up the current code.

Thanks,
Guenter

> ---
>  drivers/watchdog/f71808e_wdt.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
> index 9a1c761258ce..bd2ced9f39f4 100644
> --- a/drivers/watchdog/f71808e_wdt.c
> +++ b/drivers/watchdog/f71808e_wdt.c
> @@ -338,8 +338,10 @@ static int f71862fg_pin_configure(unsigned short ioaddr)
>  
>  static int watchdog_start(void)
>  {
> +	int err;
> +
>  	/* Make sure we don't die as soon as the watchdog is enabled below */
> -	int err = watchdog_keepalive();
> +	err = watchdog_keepalive();
>  	if (err)
>  		return err;
>  
> -- 
> 2.7.4
> 

Re: [PATCH V2 2/2] watchdog: f71808e_wdt: fix F81866 bit operation

[Guenter Roeck]

On Wed, Mar 27, 2019 at 02:42:51PM +0800, Ji-Ze Hong (Peter Hong) wrote:
> Fix error bit operation in watchdog_start()
> 
> Fixes: 14b24a88a3660 ("watchdog: f71808e_wdt: Add F81866 support")
> Signed-off-by: Ji-Ze Hong (Peter Hong) <hpeter+linux_kernel@gmail.com>

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

> ---
>  drivers/watchdog/f71808e_wdt.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/watchdog/f71808e_wdt.c b/drivers/watchdog/f71808e_wdt.c
> index bd2ced9f39f4..afd1446241b3 100644
> --- a/drivers/watchdog/f71808e_wdt.c
> +++ b/drivers/watchdog/f71808e_wdt.c
> @@ -339,6 +339,7 @@ static int f71862fg_pin_configure(unsigned short ioaddr)
>  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();
> @@ -388,19 +389,18 @@ static int watchdog_start(void)
>  		break;
>  
>  	case f81866:
> -		/* Set pin 70 to WDTRST# */
> -		superio_clear_bit(watchdog.sioaddr, SIO_F81866_REG_PORT_SEL,
> -				  BIT(3) | BIT(0));
> -		superio_set_bit(watchdog.sioaddr, SIO_F81866_REG_PORT_SEL,
> -				BIT(2));
>  		/*
>  		 * GPIO1 Control Register when 27h BIT3:2 = 01 & BIT0 = 0.
>  		 * The PIN 70(GPIO15/WDTRST) is controlled by 2Ch:
>  		 *     BIT5: 0 -> WDTRST#
>  		 *           1 -> GPIO15
>  		 */
> -		superio_clear_bit(watchdog.sioaddr, SIO_F81866_REG_GPIO1,
> -				  BIT(5));
> +		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:
> -- 
> 2.7.4
>